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

[Paul Semel via Phabricator via llvm-commits]

paulsemel added inline comments.


================
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
----------------
jhenderson wrote:
> paulsemel wrote:
> > jhenderson wrote:
> > > jhenderson wrote:
> > > > 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.
> > > Sorry, maybe I confused you - I meant each member should have a disassembly check, so the later members should have it just like the earlier ones, but only the first part, like you've done here. Otherwise, it looks like the first few cases are different and special.
> > If there is not disassembly part, that means that there is nothing to disassemble :)
> > I think just putting this line for the disassembly part is ok, as this is not what we are checking here (but this still ensures that both options are working correctly together).
> Sorry, just to be clear, do the other members have no text section then?
Yes that there is not code sections (you can test with objdump, the output is the same :) )


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:2230-2231
+    outs() << ":\tfile format " << o->getFileFormatName() << "\n";
+    if (!ArchiveHeaders || MachOOpt)
+      outs() << "\n";
   }
----------------
jhenderson wrote:
> paulsemel wrote:
> > jhenderson wrote:
> > > Maybe a silly question, but why is this case special? Why not always print the double new line?
> > When combined with other options, GNU objdump is not putting a new line, and effectively I find it nicer without it. What do you think ?
> Okay, that does sound sensible, but would you mind showing what you mean a bit more, with some output from both versions? Why is ArchiveHeaders special, and not, say, Disassemble?
Ok, I double checked, and it seems that GNU objdump is putting the two newlines too.. let's stick with what was here before then :)


Repository:
  rL LLVM

https://reviews.llvm.org/D48904