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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LGMT but please wait for other opinions



================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:2016
+  if (const auto *Elf = dyn_cast<ELFObjectFileBase>(Obj)) {
+    if (Elf->getEType() == ELF::ET_EXEC ||
+        Elf->getEType() == ELF::ET_DYN)
----------------
MaskRay wrote:
> ychen wrote:
> > MaskRay wrote:
> > > Why don't ET_CORE and ET_REL need the warning?
> > 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?
> 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).
You can write this as `return Elf->getEType() != ELF::ET_REL;` and delete the surrounding `{}`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64779/new/

https://reviews.llvm.org/D64779