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

[Rahul Chaudhry via Phabricator via llvm-commits]

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


================
Comment at: lib/Object/ELF.cpp:229
+  Elf_Rela Rela;
+  Rela.r_info = 0;
+  Rela.r_addend = 0;
----------------
jakehehrlich wrote:
> Is there a more sensible info we can set this to? It would have to be processor specific which might be super annoying. If there isn't a reasonable way to set the relocation type then I think it should be dropped llvm-readobj output in the llvm case.
The type is set appropriately now (2 lines below).


================
Comment at: lib/Object/ELF.cpp:234
+
+  typename ELFT::uint Base = 0;
+  for (const Elf_Relr &R : relrs) {
----------------
jakehehrlich wrote:
> nit: I think I'd prefer this use Elf_Addr since it represents an offset for a dynamic relocation (e.g. and address)
Defined a 'Word' type locally, and using it for Base/Entry/Offset now.
Elf_Addr is a packed version of this uint type for endianness conversion.
At least Entry can't be Elf_Addr, since we need to do bit-shifts on it (operator >>= is undefined for packed types).
And if Entry is Word, then it makes sense to keep Base/Offset as Word as well, otherwise we end up with several unnecessary conversions below when doing arithmetic and assignments.


================
Comment at: test/tools/llvm-readobj/elf-relr-relocs.test:6
+# LLVM1:      Section (1) .relr.dyn {
+# LLVM1-NEXT:   0x10D60 R_X86_64_NONE - 0x0
+# LLVM1-NEXT:   0x10D68 R_X86_64_NONE - 0x0
----------------
jakehehrlich wrote:
> R_X86_64_NONE is not a valid relocation type. See my r_info comment above about this issue.
When raw contents of '.relr.dyn' section are dumped, I'm printing a dummy 'R_RELR_RELATIVE' relocation type.
With -decode-relr, the correct relative relocation type is printed.


Repository:
  rL LLVM

https://reviews.llvm.org/D47919