[PATCH] D69997: [llvm-objdump] Print relocation addends in hexadecimal

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Thanks for the update. I've got one major request and a number of cosmetic requests in the test, but otherwise looks good.



================
Comment at: llvm/test/tools/llvm-objdump/relocations-elf.test:1
 # RUN: yaml2obj --docnum=1 %s > %t
 # RUN: llvm-objdump --reloc %t > %t1
----------------
This test needs moving into the X86 directory, as the disassembler is target dependent. Alternatively, split the inline relocations part into a separate test in that directory.


================
Comment at: llvm/test/tools/llvm-objdump/relocations-elf.test:94
+
+## Check ranges of addends being displayed in a dump of relocations and mixed with diassembly
+# RUN: yaml2obj --docnum=3 %s > %t3
----------------
Nit: missing full stop.


================
Comment at: llvm/test/tools/llvm-objdump/relocations-elf.test:108
+--- !ELF
+FileHeader: !FileHeader
+  Class: ELFCLASS64
----------------
Delete "!FileHeader"


================
Comment at: llvm/test/tools/llvm-objdump/relocations-elf.test:109-112
+  Class: ELFCLASS64
+  Data: ELFDATA2LSB
+  Type: ET_REL
+  Machine: EM_X86_64
----------------
Please line up the values so that they are aligned vertically:

```
  Class:   ELFCLASS64
  Data:    ELFDATA2LSB
  Type:    ET_REL
  Machine: EM_X86_64
```


================
Comment at: llvm/test/tools/llvm-objdump/relocations-elf.test:117
+  Type: SHT_PROGBITS
+  Content: "0000000000000000"
+  AddressAlign: 16
----------------
Size: 8 instead of Content would be slightly more readable, I guess.


================
Comment at: llvm/test/tools/llvm-objdump/relocations-elf.test:118
+  Content: "0000000000000000"
+  AddressAlign: 16
+  Flags: [SHF_EXECINSTR,SHF_ALLOC]
----------------
This line can be deleted.


================
Comment at: llvm/test/tools/llvm-objdump/relocations-elf.test:120
+  Flags: [SHF_EXECINSTR,SHF_ALLOC]
+
+- Name: .rela.text
----------------
This line should be deleted.


================
Comment at: llvm/test/tools/llvm-objdump/relocations-elf.test:124
+  Info: .text
+  AddressAlign: 4
+  Relocations:
----------------
This line can be deleted.


================
Comment at: llvm/test/tools/llvm-objdump/relocations-elf.test:129
+      Symbol: glob
+      Type: R_X86_64_64
+    - Offset: 0x1
----------------
Here and below, add some spaces after "Type:" to make R_X86... line up with the other values.


================
Comment at: llvm/test/tools/llvm-objdump/relocations-elf.test:146-147
+      Addend: 0
+
+
+Symbols:
----------------
Get rid of the extra whitespace here.


================
Comment at: llvm/test/tools/llvm-objdump/relocations-elf.test:149
+Symbols:
+  - Name: glob
+    Section: .text
----------------
Add some padding before "glob" to line this up with the rest of the values in the following lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69997