[PATCH] D103974: [llvm-objdump] Add testing for --print-imm-hex, --headers, --section-headers and --private-headers

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/ELF/private-headers.test:1
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objdump --private-headers %t | FileCheck %s
----------------
jhenderson wrote:
> Add a comment to the start of the test briefly outlining the test purpose.
> 
> A quick skim suggests this is essentially the only testing for dumping this information for llvm-objdump for ELF. I'm guessing the code path is completely unrelated to the Mach-O equivalent behaviour, so it looks like it needs far more thorough testing, showing that all the various properties can be dumped correctly (e.g. all the different flags). Take a look at the equivalent llvm-readobj testing for an idea of what would be best.
> 
> Assuming more widespread testing is added, I'd split this up into separate testing for each of the different things (program header/version info/dynamic section) and then have a very minimal test that shows that --private-headers dumps all three.
`{,llvm-|objdump -p / --private-headers dumps` program headers, the dynamic section, and GNU symbol versioning sections.

`dynamic-section.test` tests `-p` but there is no test in this directory testing `--private-headers`.


================
Comment at: llvm/test/tools/llvm-objdump/ELF/private-headers.test:11
+# CHECK-NEXT:  NEEDED bar
+# CHECK:  Version definitions:
+# CHECK-NEXT:1 0x01 0x075bcd15 foo
----------------
For versions. `llvm-readobj/ELF/` has some `llvm-readelf -V` tests which may be used as a reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103974