[PATCH] D48904: [llvm-objdump] Add --archive-headers (-a) option

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In https://reviews.llvm.org/D48904#1152494, @paulsemel wrote:

> Alright :)
>  The fact is that this error is often occurring on the first entry when we create the archive file with GNU ar (I don't know why, I have to admit it). So, if we throw an error when we encounter this, we are simply not printing any entry of those files at all... (but the other entries are ok !)
>  That's actually what GNU objdump is doing. It is just reporting the file as not parsable in the archive, and going to the next file.
>  I think this is a good behavior. What do you think ?


Okay, that makes sense to me. Does GNU objdump print it on stdout, along with the rest of the information, or stderr? We should do whatever it does. We should also test it - I think a good way to do so would be to add a non-object (e.g. a text file) to the archive used for the main test, and show that the "ill-formed" message is printed, but then the rest of the check continues. What do you think?



================
Comment at: test/tools/llvm-objdump/archive-headers-disas.test:3
+
+# CHECK: rw-r--r-- 204299/200   1416 Tue Oct 30 15:33:29 2012 1.o
+# CHECK: Disassembly of section .text:
----------------
I'd add before each CHECK group a check showing the file name/format line (e.g. "test.a(test.o): file format ELF64-x86-64).


================
Comment at: test/tools/llvm-objdump/archive-headers-disas.test:6-11
+# CHECK:        0:	55 	pushq	%rbp
+# CHECK:        1:	48 89 e5 	movq	%rsp, %rbp
+# CHECK:        4:	b8 00 00 00 00 	movl	$0, %eax
+# CHECK:        9:	e8 00 00 00 00 	callq	0 <main+0xe>
+# CHECK:        e:	5d 	popq	%rbp
+# CHECK:        f:	c3 	retq
----------------
I'd probably omit the actual disassembly part of the CHECK to make it less fragile to minor changes in how it is printed. It probably makes sense to just keep the first three CHECKs for each member (i.e. the --archive-headers line, the "Disassembly of..." line and the symbol name.


================
Comment at: test/tools/llvm-objdump/archive-headers.test:4
+
+# CHECK: rw-r--r-- 204299/200  1416 Tue Oct 30 15:33:29 2012 1.o
+# CHECK: rw-r--r-- 204299/200  1224 Tue Oct 30 15:33:29 2012 2.o
----------------
Similar to the other test, I think it's worth checking the file name line that precedes each dump.


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:212
+    llvm::ArchiveHeaders("archive-headers",
+                         cl::desc("Print archive headers for Mach-O archives "));
+
----------------
This help text needs updating to remove reference to Mach-O.


Repository:
  rL LLVM

https://reviews.llvm.org/D48904