[PATCH] D62054: [llvm-objdump] Make --disassemble-functions imply -d

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/X86/disassemble-implied-by-disassemble-functions.test:6
+FileHeader:      
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
----------------
Nit: I'm trying to fight against unnecessary whitespace in these blobs. Reduce the space between the tag and value to the minimum needed for the values to line up within the block i.e:
```
  Class:   ELFCLASS64
  Data:    ELFDATA2LSB
  Type:    ET_REL
​  Machine: EM_X86_64
```


================
Comment at: llvm/test/tools/llvm-objdump/X86/disassemble-implied-by-disassemble-functions.test:14
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000010
+    Content:         8D4701C3662E0F1F84000000000066908D4701C3
----------------
Get rid of this, as it's not necessary for the test.


================
Comment at: llvm/test/tools/llvm-objdump/X86/disassemble-implied-by-disassemble-functions.test:15
+    AddressAlign:    0x0000000000000010
+    Content:         8D4701C3662E0F1F84000000000066908D4701C3
+Symbols:         
----------------
Let's reduce this right down to just a single nop instruction (0x90) or ret (0xc3).


================
Comment at: llvm/test/tools/llvm-objdump/X86/disassemble-implied-by-disassemble-functions.test:20-24
+  - Name:            add1
+    Type:            STT_FUNC
+    Section:         .text
+    Binding:         STB_GLOBAL
+    Size:            0x0000000000000004
----------------
You probably only need one symbol and section for this test case. The rest of the disassembly/disassemble-functions functionality (such as --disassemble-functions dumps only the specified symbols) should be tested separately (I actually am currently working on those tests, and hope to have a patch up ready by the end of the day).


================
Comment at: llvm/test/tools/llvm-objdump/X86/disassemble-implied-by-disassemble-functions.test:33
+
+# CHECK: <stdin>:    file format ELF64-x86-64
+
----------------
I don't think you need to check this first bit.


================
Comment at: llvm/test/tools/llvm-objdump/X86/disassemble-implied-by-disassemble-functions.test:38-42
+# CHECK: 0000000000000000 add1:
+# CHECK:        0: 8d 47 01                      leal    1(%rdi), %eax
+# CHECK:        3: c3                            retq
+# CHECK:        4: 66 2e 0f 1f 84 00 00 00 00 00 nopw    %cs:(%rax,%rax)
+# CHECK:        e: 66 90                         nop
----------------
You can probably afford to get away with just checking that the `add1:` and `main:` lines.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62054