This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objdump] Fix llvm-objdump --all-headers output order
ClosedPublic

Authored by justice_adams on Sep 9 2019, 8:38 AM.

Details

Summary

Made llvm-objdump --all-headers output match the order of GNU objdump for compatibility reasons.

Old order of the headers output:

  • file header
  • section header table
  • symbol table
  • program header table
  • dynamic section

New order of the headers output (GNU compatible):

  • file header information
  • program header table
  • dynamic section
  • section header table
  • symbol table

I've ensured all lit tests pass on Windows and Linux with the following changes

Diff Detail

Repository
rL LLVM

Event Timeline

justice_adams created this revision.Sep 9 2019, 8:38 AM
ychen added inline comments.Sep 9 2019, 11:48 AM
llvm/test/tools/llvm-objdump/all-headers.test
11 ↗(On Diff #219362)

This probably still needs to be checked.

I would expect:

# CHECK-EMPTY:
# CHECK-NEXT: Program Header:
# CHECK: Dynamic Section:
# CHECK: Sections:
# CHECK: SYMBOL TABLE:

Restored check-empty check as suggested by @ychen

Please reupload the patch with context so that reviewers could see lines around your changes (https://llvm.org/docs/Phabricator.html#id4).

llvm/test/tools/llvm-objdump/all-headers.test
14 ↗(On Diff #219619)

nit: excessive space.

Looks reasonable. Verified that check-lld (a heavy llvm-objdump user) also passed.

jhenderson added inline comments.Sep 11 2019, 2:20 AM
llvm/test/tools/llvm-objdump/all-headers.test
12 ↗(On Diff #219619)

I think this still needs to be CHECK-NEXT to show that there is exactly one line between start address: and Program Header:

@grimar my apologies. I reuploaded the patch with more context according to the documentation you linked! Also, made the changes suggested via the comments above

This revision is now accepted and ready to land.Sep 12 2019, 12:52 AM
grimar accepted this revision.Sep 12 2019, 12:53 AM

LGTM. Thanks!

MaskRay accepted this revision.Sep 12 2019, 1:13 AM

Thanks for the review all. I don't have commit access, @grimar could you commit this for me?

This revision was automatically updated to reflect the committed changes.