[PATCH] D68303: [llvm-objdump] Also suppress --dyn-relocs when --disassemble is provided

Wed Oct 2 01:30:41 PDT 2019

jhenderson added inline comments.

================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-disassemble-dynamic-relocs.test:13
+# CHECK: Disassembly of section .text:
+# CHECK: e8 55 00 00 00                callq   85 <bar at plt>
+
----------------
Does objdump (GNU or LLVM) print dynamic relocations inline like it does with static ones? Your comment suggests it does, but you don't attempt to test it anywhere.

================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-disassemble-dynamic-relocs.test:17
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
----------------
Here and throughout, get rid of excessive space.

================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-disassemble-dynamic-relocs.test:36
+    Address:         0x0000000000001520
+    Content:         488D3DF1220000488D05EA2200004839F87415488B05BE1200004885C07409FFE00F1F8000000000C30F1F8000000000488D3DC1220000488D35BA2200004829FE48C1FE034889F048C1E83F4801C648D1FE7414488B05851200004885C07408FFE0660F1F440000C30F1F8000000000803DA922000000752F5548833D66120000004889E5740C488B3D62220000E87D000000E868FFFFFFC60581220000015DC30F1F8000000000C30F1F8000000000E97BFFFFFFCCCCCCCCCCCCCCCCCCCCCC554889E5B000E855000000C3
+  - Name:            .plt
----------------
This seems like an awful lot of content unnecessarily... can you not just have a single call instruction to a given symbol? Do you even need that? Isn't it sufficient to show that "Disassembly of .text" is printed?

(If relocations are printed inline, you might want a call instruction, but could use a RELATIVE relocation to remove the need for a dynamic symbol).

================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-disassemble-dynamic-relocs.test:37-41
+  - Name:            .plt
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x0000000000001620
+    Content:         FF35FA210000FF25FC2100000F1F4000FF25FA2100006800000000E9E0FFFFFFFF25F22100006801000000E9D0FFFFFF
----------------
I don't think you need a .plt. Just a relocation in .rela.dyn (or .rela.plt) is sufficient.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68303