[PATCH] D48810: [llvm-objdump] Add --file-headers (-f) option

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In https://reviews.llvm.org/D48810#1150661, @paulsemel wrote:

> About the other informations: I think they are more "format specific" and thus I wanted to add an other patch for it. Do you want me to do it in this patch ?


No, I don't think it's that important, and I'm happy for that to be added later, when someone wants to add it.



================
Comment at: lib/Object/COFFObjectFile.cpp:916
+    return PE32Header->AddressOfEntryPoint;
+  return errorCodeToError(object_error::parse_failed);
+}
----------------
paulsemel wrote:
> jhenderson wrote:
> > It looks like GNU objdump prints 0 rather than failing in this case.
> Yes, I figured it out this night.. I really don't know why. Do you think we might follow the behavior ?
I think it makes sense to. The other information in the file-headers dump is potentially useful, so preventing it being printed is bad (though if there's a clean way of not printing the address at all for COFF object files, I'm all for it not being printed in this case).


================
Comment at: test/tools/llvm-objdump/file-headers.test:1
+# RUN: llvm-objdump -f %p/Inputs/common-symbol-elf | FileCheck %s
+
----------------
paulsemel wrote:
> jhenderson wrote:
> > I must admit that I'm surprised we're not using yaml2obj to test here. I am not familiar with llvm-objdump, and I see that using pre-canned binaries is normal for its tests, but how do you feel about using yaml2obj instead?
> > 
> > Assuming you'd prefer to use pre-canned binaries, is it normal in the tests to use files that aren't specifically for that test?
> > 
> > There should also probably be separate elf and coff test cases.
> Yes, I anyway definitely need to add a COFF test. I can use yaml2obj here, this is not a problem for me, but as a lot of tests in this tool are doing this way, I just didn't want to break the move :)
I think when we can do better, there's no reason to stick to the same pattern. Somebody needs to break the mould at some point!


Repository:
  rL LLVM

https://reviews.llvm.org/D48810