Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[llvm-readobj] Remove --raw-relr #89426

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Apr 19, 2024

https://reviews.llvm.org/D47919 dumped RELR relocations as
R_*_RELATIVE and added --raw-relr (not in GNU) for testing purposes
(more readable than llvm-readelf -x .relr.dyn). The option is obsolete
after llvm-readelf -r output gets improved (#89162).

Since --raw-relr never seems to get more adoption. Let's remove it to
avoid some complexity.

Created using spr 1.3.5-bogner
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 19, 2024

@llvm/pr-subscribers-llvm-binary-utilities

Author: Fangrui Song (MaskRay)

Changes

https://reviews.llvm.org/D47919 dumped RELR relocations as
R_*_RELATIVE and added --raw-relr (not in GNU) for testing purposes
(more readable than llvm-readelf -x .relr.dyn). The option is obsolete
after llvm-readelf -r output gets improved (#89162).

Since --raw-relr never seems to get more adoption. Let's remove it to
avoid some complexity.


Full diff: https://github.com/llvm/llvm-project/pull/89426.diff

8 Files Affected:

  • (modified) llvm/docs/CommandGuide/llvm-readelf.rst (-4)
  • (modified) llvm/docs/CommandGuide/llvm-readobj.rst (-4)
  • (modified) llvm/docs/ReleaseNotes.rst (+4)
  • (modified) llvm/test/tools/llvm-readobj/ELF/relr-relocs.test (+2-47)
  • (modified) llvm/tools/llvm-readobj/ELFDumper.cpp (+15-53)
  • (modified) llvm/tools/llvm-readobj/Opts.td (-1)
  • (modified) llvm/tools/llvm-readobj/llvm-readobj.cpp (-2)
  • (modified) llvm/tools/llvm-readobj/llvm-readobj.h (-1)
diff --git a/llvm/docs/CommandGuide/llvm-readelf.rst b/llvm/docs/CommandGuide/llvm-readelf.rst
index 675628fdda45ec..284c3aa470a6f8 100644
--- a/llvm/docs/CommandGuide/llvm-readelf.rst
+++ b/llvm/docs/CommandGuide/llvm-readelf.rst
@@ -152,10 +152,6 @@ OPTIONS
 
  Display the program headers.
 
-.. option:: --raw-relr
-
- Do not decode relocations in RELR relocation sections when displaying them.
-
 .. option:: --relocations, --relocs, -r
 
  Display the relocation entries in the file.
diff --git a/llvm/docs/CommandGuide/llvm-readobj.rst b/llvm/docs/CommandGuide/llvm-readobj.rst
index ca7fb253f00a0b..8bd29eafbbfcf5 100644
--- a/llvm/docs/CommandGuide/llvm-readobj.rst
+++ b/llvm/docs/CommandGuide/llvm-readobj.rst
@@ -255,10 +255,6 @@ The following options are implemented only for the ELF file format.
 
  Display the program headers.
 
-.. option:: --raw-relr
-
- Do not decode relocations in RELR relocation sections when displaying them.
-
 .. option:: --section-mapping
 
  Display the section to segment mapping.
diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index 76ef6ceb940780..580dc512d9690c 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -198,6 +198,10 @@ Changes to the LLVM tools
   documentation for SPGO
   <https://clang.llvm.org/docs/UsersManual.html#using-sampling-profilers>`_.
 
+* llvm-readelf's ``-r`` output for RELR has been improved.
+  (`#89162 <https://github.com/llvm/llvm-project/pull/89162>`_)
+  ``--raw-relr`` has been removed.
+
 Changes to LLDB
 ---------------------------------
 
diff --git a/llvm/test/tools/llvm-readobj/ELF/relr-relocs.test b/llvm/test/tools/llvm-readobj/ELF/relr-relocs.test
index 9b59f991c051e2..c22239900bcf26 100644
--- a/llvm/test/tools/llvm-readobj/ELF/relr-relocs.test
+++ b/llvm/test/tools/llvm-readobj/ELF/relr-relocs.test
@@ -1,16 +1,6 @@
 ## This is a test to test how SHT_RELR sections are dumped.
 
 # RUN: yaml2obj --docnum=1 %s -o %t1
-# RUN: llvm-readobj --relocations --raw-relr %t1 \
-# RUN:   | FileCheck --check-prefix=RAW-LLVM1 %s
-# RAW-LLVM1:      Section (1) .relr.dyn {
-# RAW-LLVM1-NEXT:   0x10D60
-# RAW-LLVM1-NEXT:   0x103
-# RAW-LLVM1-NEXT:   0x20000
-# RAW-LLVM1-NEXT:   0xF0501
-# RAW-LLVM1-NEXT:   0xA700550400009
-# RAW-LLVM1-NEXT: }
-
 # RUN: llvm-readobj --relocations %t1 | \
 # RUN:   FileCheck --match-full-lines --check-prefix=LLVM1 %s
 
@@ -38,15 +28,6 @@
 # LLVM1-NEXT:   0x20390 R_X86_64_RELATIVE -
 # LLVM1-NEXT: }
 
-# RUN: llvm-readelf --relocations --raw-relr %t1 \
-# RUN:   | FileCheck --check-prefix=RAW-GNU1 %s
-# RAW-GNU1:      Relocation section '.relr.dyn' at offset 0x40 contains 5 entries:
-# RAW-GNU1:      0000000000010d60
-# RAW-GNU1-NEXT: 0000000000000103
-# RAW-GNU1-NEXT: 0000000000020000
-# RAW-GNU1-NEXT: 00000000000f0501
-# RAW-GNU1-NEXT: 000a700550400009
-
 # RUN: llvm-readelf --relocations %t1 | FileCheck --check-prefix=GNU1 --match-full-lines --strict-whitespace %s
 #      GNU1:Relocation section '.relr.dyn' at offset 0x40 contains 21 entries:
 # GNU1-NEXT:Index: Entry            Address           Symbolic Address
@@ -107,16 +88,6 @@ Symbols:
     Value:   0x20210
 
 # RUN: yaml2obj --docnum=2 %s -o %t2
-# RUN: llvm-readobj --relocations --raw-relr %t2 | \
-# RUN:   FileCheck --check-prefix=RAW-LLVM2 %s
-# RAW-LLVM2:      Section (1) .relr.dyn {
-# RAW-LLVM2-NEXT:   0x10D60
-# RAW-LLVM2-NEXT:   0x103
-# RAW-LLVM2-NEXT:   0x20000
-# RAW-LLVM2-NEXT:   0xF0501
-# RAW-LLVM2-NEXT:   0x50400009
-# RAW-LLVM2-NEXT: }
-
 # RUN: llvm-readobj --relocations %t2 | \
 # RUN:   FileCheck --match-full-lines --check-prefix=LLVM2 %s
 
@@ -137,15 +108,6 @@ Symbols:
 # LLVM2-NEXT:   0x200F4 R_386_RELATIVE -
 # LLVM2-NEXT: }
 
-# RUN: llvm-readelf --relocations --raw-relr %t2 | \
-# RUN:   FileCheck --check-prefix=RAW-GNU2 %s
-# RAW-GNU2:      Relocation section '.relr.dyn' at offset 0x34 contains 5 entries:
-# RAW-GNU2:      00010d60
-# RAW-GNU2-NEXT: 00000103
-# RAW-GNU2-NEXT: 00020000
-# RAW-GNU2-NEXT: 000f0501
-# RAW-GNU2-NEXT: 50400009
-
 # RUN: llvm-readelf --relocations %t2 | FileCheck --check-prefix=GNU2 --match-full-lines --strict-whitespace %s
 #      GNU2:Relocation section '.relr.dyn' at offset 0x34 contains 14 entries:
 # GNU2-NEXT:Index: Entry    Address   Symbolic Address
@@ -232,21 +194,14 @@ Symbols:
 ## only relative relocations and do not have an associated symbol table, like other
 ## relocation sections.
 
-## Case A: check we do not report warnings when the sh_link field is set to an arbitrary value
-##         and the --relocations option is requested.
+## Check we do not report warnings when the sh_link field is set to an arbitrary value
+## and the --relocations option is requested.
 # RUN: yaml2obj --docnum=2 -DLINK=0xff %s -o %t2.has.link
 # RUN: llvm-readobj --relocations %t2.has.link 2>&1 | \
 # RUN:   FileCheck -DFILE=%t2.has.link --check-prefix=LLVM2 %s --implicit-check-not=warning:
 # RUN: llvm-readelf --relocations %t2.has.link 2>&1 | \
 # RUN:   FileCheck -DFILE=%t2.has.link --check-prefix=GNU2 %s --implicit-check-not=warning:
 
-## Case B: check we do not report warnings when the sh_link field is set to an arbitrary value
-##         and --relocations and --raw-relr options are requested.
-# RUN: llvm-readobj --relocations --raw-relr %t2.has.link | \
-# RUN:   FileCheck -DFILE=%t2.has.link --check-prefix=RAW-LLVM2 %s
-# RUN: llvm-readelf --relocations --raw-relr %t2.has.link 2>&1 | \
-# RUN:   FileCheck -DFILE=%t2.has.link --check-prefix=RAW-GNU2 %s
-
 ## .symtab is invalid. Check we report a warning and print entries without symbolization.
 # RUN: yaml2obj --docnum=3 -DENTSIZE=1 %s -o %t3.err1
 # RUN: llvm-readelf -r %t3.err1 2>&1 | FileCheck -DFILE=%t3.err1 --check-prefixes=GNU3,GNU3-ERR1 --match-full-lines %s
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index f145653ac743c5..a752cc4015293f 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -285,7 +285,6 @@ template <typename ELFT> class ELFDumper : public ObjDumper {
 
   virtual void printRelRelaReloc(const Relocation<ELFT> &R,
                                  const RelSymbol<ELFT> &RelSym) = 0;
-  virtual void printRelrReloc(const Elf_Relr &R) = 0;
   virtual void printDynamicRelocHeader(unsigned Type, StringRef Name,
                                        const DynRegionInfo &Reg) {}
   void printReloc(const Relocation<ELFT> &R, unsigned RelIndex,
@@ -294,11 +293,10 @@ template <typename ELFT> class ELFDumper : public ObjDumper {
   void printDynamicRelocationsHelper();
   void printRelocationsHelper(const Elf_Shdr &Sec);
   void forEachRelocationDo(
-      const Elf_Shdr &Sec, bool RawRelr,
+      const Elf_Shdr &Sec,
       llvm::function_ref<void(const Relocation<ELFT> &, unsigned,
                               const Elf_Shdr &, const Elf_Shdr *)>
-          RelRelaFn,
-      llvm::function_ref<void(const Elf_Relr &)> RelrFn);
+          RelRelaFn);
 
   virtual void printSymtabMessage(const Elf_Shdr *Symtab, size_t Offset,
                                   bool NonVisibilityBitsUsed,
@@ -669,7 +667,6 @@ template <typename ELFT> class GNUELFDumper : public ELFDumper<ELFT> {
                          DataRegion<Elf_Word> ShndxTable, StringRef StrTable,
                          uint32_t Bucket);
   void printRelr(const Elf_Shdr &Sec);
-  void printRelrReloc(const Elf_Relr &R) override;
   void printRelRelaReloc(const Relocation<ELFT> &R,
                          const RelSymbol<ELFT> &RelSym) override;
   void printSymbol(const Elf_Sym &Symbol, unsigned SymIndex,
@@ -734,7 +731,6 @@ template <typename ELFT> class LLVMELFDumper : public ELFDumper<ELFT> {
                                bool IsGnu) const override;
 
 private:
-  void printRelrReloc(const Elf_Relr &R) override;
   void printRelRelaReloc(const Relocation<ELFT> &R,
                          const RelSymbol<ELFT> &RelSym) override;
 
@@ -3800,11 +3796,6 @@ template <class ELFT> void GNUELFDumper<ELFT>::printGroupSections() {
     OS << "There are no section groups in this file.\n";
 }
 
-template <class ELFT>
-void GNUELFDumper<ELFT>::printRelrReloc(const Elf_Relr &R) {
-  OS << to_string(format_hex_no_prefix(R, ELFT::Is64Bits ? 16 : 8)) << "\n";
-}
-
 template <class ELFT>
 void GNUELFDumper<ELFT>::printRelRelaReloc(const Relocation<ELFT> &R,
                                            const RelSymbol<ELFT> &RelSym) {
@@ -3851,22 +3842,11 @@ template <class ELFT>
 static void printRelocHeaderFields(formatted_raw_ostream &OS, unsigned SType,
                                    const typename ELFT::Ehdr &EHeader) {
   bool IsRela = SType == ELF::SHT_RELA || SType == ELF::SHT_ANDROID_RELA;
-  bool IsRelr =
-      SType == ELF::SHT_RELR || SType == ELF::SHT_ANDROID_RELR ||
-      (EHeader.e_machine == EM_AARCH64 && SType == ELF::SHT_AARCH64_AUTH_RELR);
-  if (ELFT::Is64Bits)
-    OS << "    ";
-  else
-    OS << " ";
-  if (IsRelr && opts::RawRelr)
-    OS << "Data  ";
-  else
-    OS << "Offset";
   if (ELFT::Is64Bits)
-    OS << "             Info             Type"
-       << "               Symbol's Value  Symbol's Name";
+    OS << "    Offset             Info             Type               Symbol's "
+          "Value  Symbol's Name";
   else
-    OS << "     Info    Type                Sym. Value  Symbol's Name";
+    OS << " Offset     Info    Type                Sym. Value  Symbol's Name";
   if (IsRela)
     OS << " + Addend";
   OS << "\n";
@@ -3894,10 +3874,10 @@ static bool isRelocationSec(const typename ELFT::Shdr &Sec,
 
 template <class ELFT> void GNUELFDumper<ELFT>::printRelocations() {
   auto PrintAsRelr = [&](const Elf_Shdr &Sec) {
-    return !opts::RawRelr && (Sec.sh_type == ELF::SHT_RELR ||
-                              Sec.sh_type == ELF::SHT_ANDROID_RELR ||
-                              (this->Obj.getHeader().e_machine == EM_AARCH64 &&
-                               Sec.sh_type == ELF::SHT_AARCH64_AUTH_RELR));
+    return Sec.sh_type == ELF::SHT_RELR ||
+           Sec.sh_type == ELF::SHT_ANDROID_RELR ||
+           (this->Obj.getHeader().e_machine == EM_AARCH64 &&
+            Sec.sh_type == ELF::SHT_AARCH64_AUTH_RELR);
   };
   auto GetEntriesNum = [&](const Elf_Shdr &Sec) -> Expected<size_t> {
     // Android's packed relocation section needs to be unpacked first
@@ -4902,10 +4882,8 @@ void ELFDumper<ELFT>::printDynamicReloc(const Relocation<ELFT> &R) {
 template <class ELFT>
 void ELFDumper<ELFT>::printRelocationsHelper(const Elf_Shdr &Sec) {
   this->forEachRelocationDo(
-      Sec, opts::RawRelr,
-      [&](const Relocation<ELFT> &R, unsigned Ndx, const Elf_Shdr &Sec,
-          const Elf_Shdr *SymTab) { printReloc(R, Ndx, Sec, SymTab); },
-      [&](const Elf_Relr &R) { printRelrReloc(R); });
+      Sec, [&](const Relocation<ELFT> &R, unsigned Ndx, const Elf_Shdr &Sec,
+               const Elf_Shdr *SymTab) { printReloc(R, Ndx, Sec, SymTab); });
 }
 
 template <class ELFT> void ELFDumper<ELFT>::printDynamicRelocationsHelper() {
@@ -6371,11 +6349,10 @@ void ELFDumper<ELFT>::printDependentLibsHelper(
 
 template <class ELFT>
 void ELFDumper<ELFT>::forEachRelocationDo(
-    const Elf_Shdr &Sec, bool RawRelr,
+    const Elf_Shdr &Sec,
     llvm::function_ref<void(const Relocation<ELFT> &, unsigned,
                             const Elf_Shdr &, const Elf_Shdr *)>
-        RelRelaFn,
-    llvm::function_ref<void(const Elf_Relr &)> RelrFn) {
+        RelRelaFn) {
   auto Warn = [&](Error &&E,
                   const Twine &Prefix = "unable to read relocations from") {
     this->reportUniqueWarning(Prefix + " " + describe(Sec) + ": " +
@@ -6427,11 +6404,6 @@ void ELFDumper<ELFT>::forEachRelocationDo(
       Warn(RangeOrErr.takeError());
       break;
     }
-    if (RawRelr) {
-      for (const Elf_Relr &R : *RangeOrErr)
-        RelrFn(R);
-      break;
-    }
 
     for (const Elf_Rel &R : Obj.decode_relrs(*RangeOrErr))
       RelRelaFn(Relocation<ELFT>(R, IsMips64EL), RelNdx++, Sec,
@@ -6741,9 +6713,8 @@ void ELFDumper<ELFT>::printRelocatableStackSizes(
     DataExtractor Data(Contents, Obj.isLE(), sizeof(Elf_Addr));
 
     forEachRelocationDo(
-        *RelocSec, /*RawRelr=*/false,
-        [&](const Relocation<ELFT> &R, unsigned Ndx, const Elf_Shdr &Sec,
-            const Elf_Shdr *SymTab) {
+        *RelocSec, [&](const Relocation<ELFT> &R, unsigned Ndx,
+                       const Elf_Shdr &Sec, const Elf_Shdr *SymTab) {
           if (!IsSupportedFn || !IsSupportedFn(R.Type)) {
             reportUniqueWarning(
                 describe(*RelocSec) +
@@ -6754,10 +6725,6 @@ void ELFDumper<ELFT>::printRelocatableStackSizes(
 
           this->printStackSize(R, *RelocSec, Ndx, SymTab, FunctionSec,
                                *StackSizesELFSec, Resolver, Data);
-        },
-        [](const Elf_Relr &) {
-          llvm_unreachable("can't get here, because we only support "
-                           "SHT_REL/SHT_RELA sections");
         });
   }
 }
@@ -7147,11 +7114,6 @@ template <class ELFT> void LLVMELFDumper<ELFT>::printRelocations() {
   }
 }
 
-template <class ELFT>
-void LLVMELFDumper<ELFT>::printRelrReloc(const Elf_Relr &R) {
-  W.startLine() << W.hex(R) << "\n";
-}
-
 template <class ELFT>
 void LLVMELFDumper<ELFT>::printExpandedRelRelaReloc(const Relocation<ELFT> &R,
                                                     StringRef SymbolName,
diff --git a/llvm/tools/llvm-readobj/Opts.td b/llvm/tools/llvm-readobj/Opts.td
index 1e9cde6b2e87c6..7d574d875d22ea 100644
--- a/llvm/tools/llvm-readobj/Opts.td
+++ b/llvm/tools/llvm-readobj/Opts.td
@@ -62,7 +62,6 @@ def memtag : FF<"memtag", "Display memory tagging metadata (modes, Android notes
 def needed_libs : FF<"needed-libs", "Display the needed libraries">, Group<grp_elf>;
 def notes : FF<"notes", "Display notes">, Group<grp_elf>;
 def program_headers : FF<"program-headers", "Display program headers">, Group<grp_elf>;
-def raw_relr : FF<"raw-relr", "Do not decode relocations in SHT_RELR section, display raw contents">, Group<grp_elf>;
 def version_info : FF<"version-info", "Display version sections">, Group<grp_elf>;
 
 // Mach-O specific options.
diff --git a/llvm/tools/llvm-readobj/llvm-readobj.cpp b/llvm/tools/llvm-readobj/llvm-readobj.cpp
index a0b57656601659..9ac324cc672f05 100644
--- a/llvm/tools/llvm-readobj/llvm-readobj.cpp
+++ b/llvm/tools/llvm-readobj/llvm-readobj.cpp
@@ -134,7 +134,6 @@ static bool Memtag;
 static bool NeededLibraries;
 static bool Notes;
 static bool ProgramHeaders;
-bool RawRelr;
 static bool SectionGroups;
 static bool VersionInfo;
 
@@ -273,7 +272,6 @@ static void parseOptions(const opt::InputArgList &Args) {
   opts::Notes = Args.hasArg(OPT_notes);
   opts::PrettyPrint = Args.hasArg(OPT_pretty_print);
   opts::ProgramHeaders = Args.hasArg(OPT_program_headers);
-  opts::RawRelr = Args.hasArg(OPT_raw_relr);
   opts::SectionGroups = Args.hasArg(OPT_section_groups);
   if (Arg *A = Args.getLastArg(OPT_sort_symbols_EQ)) {
     std::string SortKeysString = A->getValue();
diff --git a/llvm/tools/llvm-readobj/llvm-readobj.h b/llvm/tools/llvm-readobj/llvm-readobj.h
index 532e43d4e16b0d..e4ee2c1396b28d 100644
--- a/llvm/tools/llvm-readobj/llvm-readobj.h
+++ b/llvm/tools/llvm-readobj/llvm-readobj.h
@@ -38,7 +38,6 @@ extern bool SectionRelocations;
 extern bool SectionSymbols;
 extern bool SectionData;
 extern bool ExpandRelocs;
-extern bool RawRelr;
 extern bool CodeViewSubsectionBytes;
 extern bool Demangle;
 enum OutputStyleTy { LLVM, GNU, JSON, UNKNOWN };

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM too.

@MaskRay MaskRay merged commit 89c95ef into main Apr 22, 2024
6 of 7 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/llvm-readobj-remove-raw-relr branch April 22, 2024 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants