This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objdump] - Change the output for --all-headers.
ClosedPublic

Authored by grimar on Jan 11 2019, 4:15 AM.

Details

Summary

This is for https://bugs.llvm.org/show_bug.cgi?id=40008,

it starts printing the file headers when --all-headers is given and
do a few cosmetic changes.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Jan 11 2019, 4:15 AM
jhenderson added inline comments.Jan 11 2019, 4:53 AM
tools/llvm-objdump/llvm-objdump.cpp
2304 ↗(On Diff #181242)

Won't the extra '\n' result in a weird extra blank line when printing just the file headers?

I also noticed that there was a lot of inconsistency between the different outputs in this area. What I'd like to see is each different dump to append a blank line, only if there is more to come.

I don't feel too strongly about it though, as long as we are consistent.

grimar marked an inline comment as done.Jan 11 2019, 5:06 AM
grimar added inline comments.
tools/llvm-objdump/llvm-objdump.cpp
2304 ↗(On Diff #181242)

With that change it becomes consistent with what is done for COFF:
https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-objdump/llvm-objdump.cpp#L2437

It prints an additional empty line after "start address" but does not seem it look too bad perhaps:
(I mean the last extra line, the first one looks a bit wierd to me, but it was already there).

umb@umb-virtual-machine:~/LLVM$ /home/umb/LLVM/build/bin/llvm-objdump ooo --file-headers

ooo: file format ELF64-x86-64

architecture: x86_64
start address: 0x0000000000201000

umb@umb-virtual-machine:~/LLVM$ /home/umb/LLVM/build/bin/llvm-objdump ooo --section-headers

I am also noticed a few inconsistencies (and also code style issues), but I think it is better to fix/refactor in a separate patch(es),
all at once perhaps.

jhenderson accepted this revision.Jan 11 2019, 5:29 AM

LGTM.

tools/llvm-objdump/llvm-objdump.cpp
2304 ↗(On Diff #181242)

Okay, sounds reasonable.

This revision is now accepted and ready to land.Jan 11 2019, 5:29 AM
This revision was automatically updated to reflect the committed changes.