This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objdump] Emit warning if --start-address/--stop-address specify range outside file's address range.
ClosedPublic

Authored by ychen on Jul 15 2019, 3:57 PM.

Details

Summary

NB: the warning is about the input file itself regardless of the options used
such as -r, -s etc..

Only sections with the SHF_ALLOC flag are considered.

https://bugs.llvm.org/show_bug.cgi?id=41911

Diff Detail

Repository
rL LLVM

Event Timeline

ychen created this revision.Jul 15 2019, 3:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2019, 3:57 PM
MaskRay added inline comments.Jul 15 2019, 6:53 PM
llvm/tools/llvm-objdump/llvm-objdump.cpp
2016 ↗(On Diff #209984)

Why don't ET_CORE and ET_REL need the warning?

2043 ↗(On Diff #209984)

covers

Start < BaseAddr + Size) && Stop > BaseAddr checks whether two ranges overlap.

2061 ↗(On Diff #209984)

I suggest prefixing the function with maybeWarn (or check) because it doesn't always warn.

ychen edited the summary of this revision. (Show Details)Jul 15 2019, 7:03 PM
ychen updated this revision to Diff 210013.Jul 15 2019, 7:18 PM
  • update
ychen marked 3 inline comments as done.Jul 15 2019, 7:22 PM
ychen added inline comments.
llvm/tools/llvm-objdump/llvm-objdump.cpp
2016 ↗(On Diff #209984)

Section addresses in ET_REL files are not VMA, probably may not be very useful. ET_CORE could be here, I'm not sure its potential usefulness either. What's your opinion?

MaskRay added inline comments.Jul 15 2019, 7:39 PM
llvm/tools/llvm-objdump/llvm-objdump.cpp
2016 ↗(On Diff #209984)

Ah, I agree that --start-address should not on ET_REL. ET_CORE doesn't have a section header table (it is not covered by a PT_LOAD).

I think either the current condition or return Elf->getEType() != ELF::ET_REL; is fine.

I have a slight preference for !=ET_REL because it (1) makes code a bit simpler (2) there is no fundamental problem with ET_CORE (3) will work if someone attaches the section header to the core (though we don't need to test ET_CORE for now).

ychen updated this revision to Diff 211125.EditedJul 22 2019, 9:22 AM
  • Use !=ET_REL
ychen updated this revision to Diff 211131.Jul 22 2019, 9:32 AM
ychen marked an inline comment as done.
  • update
MaskRay accepted this revision.Jul 23 2019, 8:01 PM

LGMT but please wait for other opinions

llvm/tools/llvm-objdump/llvm-objdump.cpp
2016 ↗(On Diff #209984)

You can write this as return Elf->getEType() != ELF::ET_REL; and delete the surrounding {}.

This revision is now accepted and ready to land.Jul 23 2019, 8:01 PM
This revision was automatically updated to reflect the committed changes.