This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objdump] --private-headers: change errors to warnings for dynamic section dumping
ClosedPublic

Authored by MaskRay on Mar 25 2022, 1:48 PM.

Details

Summary

Fix #54456: objcopy --only-keep-debug produces a linked image with invalid
empty dynamic section. llvm-objdump -p currently reports an error which seems
excessive.

% llvm-readelf -l a.out
llvm-readelf: warning: 'a.out': no valid dynamic table was found
...

Follow the spirit of llvm-readelf -l (D64472) and report a warning instead.
This allows later files to be dumped despite warnings for an input file.
This improves objdump compatibility in that the exit code is now 0 instead of 1.

% llvm-objdump -p a.out  # new behavior
...
Program Header:
llvm-objdump: warning: 'a.out': invalid empty dynamic section
% objdump -p a.out
...
Dynamic Section:

Diff Detail

Event Timeline

MaskRay created this revision.Mar 25 2022, 1:48 PM
MaskRay requested review of this revision.Mar 25 2022, 1:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 25 2022, 1:48 PM
MaskRay edited the summary of this revision. (Show Details)Mar 25 2022, 1:52 PM
raj.khem accepted this revision.Mar 25 2022, 2:33 PM

I built this on top if 14.x branch and it fixes the problems, I was seeing and the warning is still emitted as expected.

This revision is now accepted and ready to land.Mar 25 2022, 2:33 PM

Mostly looks good. Some small improvements to the testing suggested.

llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test
2–4

Generally, I think dumper tools should avoid emitting errors for individual files, because it prevents later files from being dumped. @grimar and I both in the past made significant improvements to llvm-readobj and other tools (in my case llvm-dwarfdump at least) to achieve this. I don't think the same attention has been applied to llvm-objdump.

I've therefore suggested a rewording of the comment.

llvm/test/tools/llvm-objdump/ELF/invalid-phdr.test
8–9

Might be worth a CHECK-EMPTY after this line now, to show nothing is actually printed.

llvm/test/tools/llvm-objdump/ELF/program-headers.test
1

Ditto.

1

Ditto.

MaskRay updated this revision to Diff 418505.Mar 28 2022, 12:38 AM
MaskRay marked 3 inline comments as done.

Address comments

MaskRay marked an inline comment as done.Mar 28 2022, 12:38 AM
MaskRay edited the summary of this revision. (Show Details)Mar 28 2022, 12:54 AM
This revision was landed with ongoing or failed builds.Mar 28 2022, 1:01 AM
This revision was automatically updated to reflect the committed changes.