[PATCH] D47919: llvm-readobj: add experimental support for SHT_RELR sections.

Rahul Chaudhry via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 26 12:39:55 PDT 2018


rahulchaudhry marked 8 inline comments as done.
rahulchaudhry added inline comments.


================
Comment at: test/tools/llvm-readobj/elf-relr-relocs.test:12
+# LLVM1-NEXT:   0xF0501         R_RELR_RELATIVE - 0x0
+# LLVM1-NEXT:   0xA700550400009 R_RELR_RELATIVE - 0x0
+# LLVM1-NEXT: }
----------------
jakehehrlich wrote:
> I think you can drop the R_RELR_RELATIVE and the addend in this case.
See below.


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:3418
+        Rela.r_addend = 0;
+        printDynamicRelocation(Obj, Rela, ELF::SHT_RELR);
+      }
----------------
jakehehrlich wrote:
> As mentioned above, I think in this case you don't have to print out the relocation type and addend. You might just inline something here that prints out the relr entry instead.
The addend gets skipped in GNUStyle::printDynamicRelocation (it's only printed for RELA).
This GNUStyle dump is purely speculative, as there is no implementation for this in GNU binutils yet.
We'll need to come back and adjust the format here anyway once gnu-readelf supports printing SHT_RELR sections.

Also, see below.



================
Comment at: tools/llvm-readobj/ELFDumper.cpp:4106
+        Rela.r_addend = 0;
+        printRelocation(Obj, Rela, SymTab, Sec->sh_type);
+      }
----------------
jakehehrlich wrote:
> Same thing here.
See below.


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:4326
+        Rela.r_addend = 0;
+        printDynamicRelocation(Obj, Rela, ELF::SHT_RELR);
+      }
----------------
jakehehrlich wrote:
> same thing here.
I'd argue that both type and addend should be printed here for consistency.

- The case for addend is easy. LLVMStyle prints it for all relocations, even SHT_REL, which don't have addends. The case for SHT_RELR is similar. A 0x0 is printed as addend in both cases.

- For printing a type (even a dummy one, like R_RELR_RELATIVE), the rationale is wrt. two readobj flags: '-dyn-relocations' and '-expand-relocs'.

  -dyn-relocations prints all dynamic relocations in a single Dynamic Relocations { } block. It does not separate them by sections as -relocations does. It would be disorienting to have some relocations from .rela.dyn in the block (with full type/symbol/addend info), followed by a bunch of hex numbers (with no type or description), followed by some more relocations from .rela.plt section (with full type/symbol/addend info).

  LLVMStyle::printDynamicRelocation also handles -expand-relocs flag and prints each entry on multiple lines. This is another reason to use this function to print relocations here instead of inlining a special case.



Repository:
  rL LLVM

https://reviews.llvm.org/D47919





More information about the llvm-commits mailing list