[PATCH] D60250: [llvm-objdump] Allow -dynamic-reloc on ET_EXEC files

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-objdump/dynamic-reloc-dyn.test:21
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x00000000000002D8
+    AddressAlign:    0x0000000000000008
----------------
jhenderson wrote:
> This is a very arbitrary-looking address and alignment, which might also be a bit fragile. You might find it better to set AddressAlign of 0x400 or 0x1000 and the Address to the same value.
Here and below, you don't need to pad the fields with lots of zeroes. just do 0x2D8 or similar. It makes it more readable in my experience.


================
Comment at: test/tools/llvm-objdump/dynamic-reloc-dyn.test:21-22
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x00000000000002D8
+    AddressAlign:    0x0000000000000008
+    EntSize:         0x0000000000000018
----------------
This is a very arbitrary-looking address and alignment, which might also be a bit fragile. You might find it better to set AddressAlign of 0x400 or 0x1000 and the Address to the same value.


================
Comment at: test/tools/llvm-objdump/dynamic-reloc-dyn.test:23
+    AddressAlign:    0x0000000000000008
+    EntSize:         0x0000000000000018
+    Relocations:
----------------
I don't believe you need this field.


================
Comment at: test/tools/llvm-objdump/dynamic-reloc-dyn.test:25
+    Relocations:
+      - Offset:          0x00000000000020D0
+        Symbol:          x
----------------
I'd just leave this Offset as 0.


================
Comment at: test/tools/llvm-objdump/dynamic-reloc-dyn.test:34
+    AddressAlign:    0x0000000000000008
+    EntSize:         0x0000000000000010
+    Entries:
----------------
You don't need to specify this field.


================
Comment at: test/tools/llvm-objdump/dynamic-reloc-dyn.test:43-44
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    Address:         0x00000000000020D0
+    AddressAlign:    0x0000000000000008
+    Content:         '0000000000000000'
----------------
Similar to above, this Address feels a bit arbitrary. You might want to force the alignment to a round number by e.g. setting the AddressAlign to 0x1000.


================
Comment at: test/tools/llvm-objdump/dynamic-reloc-static.test:1
+# RUN: yaml2obj %s > %t
+# RUN: not llvm-objdump -R %t 2>&1 | FileCheck %s
----------------
Please add a comment to this test about what makes this a static object.


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

https://reviews.llvm.org/D60250