[PATCH] D75671: [llvm-readobj][llvm-readelf][test] - Add a test to check how we dump relocation addends.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 18 05:24:35 PDT 2020


grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/reloc-addends.test:24-25
+
+## FIXME: We either should not dump an addend or should read it from a
+##        destination location for a SHT_REL case.
+
----------------
jhenderson wrote:
> Unless you have a patch to imminently fix this, I'd recommend filing a bug. Same goes for the other FIXMEs below (and indeed all FIXMEs in general).
I've added a link to the existent bug here and reported a new bug and added a link for a FIXME below.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/reloc-addends.test:82
+        Type:   R_X86_64_NONE
+## Addend == an arbitrary negative number.
+      - Addend: 0xFFFFFFFFFFFFCFC7 ## -12345
----------------
jhenderson wrote:
> Any particular reason you have an arbitrary negative number, but not an arbitrary positive one?
Nope. I just do not like the negative numbers representation I think, my brain assumes them are "special".
I.e added the arbitrary positive number case.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/reloc-addends.test:86
+
+## Starting from here we check ELFCLASS32 objects.
+
----------------
jhenderson wrote:
> Is there anything actually stopping us using R_X86_64 for both 64 and 32 bit cases, and therefore allowing us to use the same YAML throughout?
32 and 64 bit cases not only have different relocations, but also the different size of `r_addend`.
I`d probably prefer not to mix them in this test case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75671/new/

https://reviews.llvm.org/D75671





More information about the llvm-commits mailing list