[PATCH] D49016: [llvm-objdump] Add dynamic section printing to private-headers option

[Paul Semel via Phabricator via llvm-commits]

paulsemel added inline comments.


================
Comment at: include/llvm/Object/ELF.h:513
+template <class ELFT>
+Expected<typename ELFT::DynRange> ELFFile<ELFT>::dynamicEntries() const {
+  ArrayRef<Elf_Dyn> Dyn;
----------------
jhenderson wrote:
> jhenderson wrote:
> > Again, this should be moved to the source file.
> You've marked this as done but `dynamicEntries` is still in the header file...
Oops.. 


================
Comment at: test/tools/llvm-objdump/private-headers-dynamic-section.test:1
+# RUN: llvm-objdump -p %p/Inputs/private-headers-x86_64.elf | FileCheck %s
+
----------------
jhenderson wrote:
> jhenderson wrote:
> > One more test suggestion: use llvm-objcopy to remove the section headers for this test input and then run the same dump. It should produce the same output.
> Sorry, I guess I wasn't clear. I was expecting these to be in the same test file. So the run commands would look something like:
> 
> 
> ```
> # RUN: llvm-objdump -p %p/Inputs/private-headers-x86_64.elf | FileCheck %s
> # RUN: llvm-objcopy <insert appropriate arguments here> %p/Inputs/private-headers-x86_64.elf %t-stripped.elf
> # RUN: llvm-objdump -p %t-stripped.elf | FileCheck %s
> ```
> 
> This means that you can show that the output is identical with and without the section headers.
You can't just strip the section header entry and not the whole section with llvm-objcopy (which make sense). I had to craft the binary by hand.. So this is not really possible


Repository:
  rL LLVM

https://reviews.llvm.org/D49016