Page MenuHomePhabricator

[llvm-readelf/obj] - Improve the error reporting in printStackSize().
ClosedPublic

Authored by grimar on Nov 17 2020, 5:25 AM.

Details

Summary

This stops using RelocationRef API in the printStackSize method
and starts using the "regular" API that is used in almost all other places
in ELFDumper.cpp.

This is not only makes the code to be more consistent, but helps to diagnose
issues better, because the ELFObjectFile API, which is used
currently to implement stack sized dumping sometimes has a behavior
that just doesn't work well for broken inputs.

E.g see how it gets the symbol_end iterator. It will just not work
well for a case when the sh_size is broken.

template <class ELFT>
basic_symbol_iterator ELFObjectFile<ELFT>::symbol_end() const {
...
  DataRefImpl Sym = toDRI(SymTab, SymTab->sh_size / sizeof(Elf_Sym));
  return basic_symbol_iterator(SymbolRef(Sym, this));
}

Diff Detail

Event Timeline

grimar created this revision.Nov 17 2020, 5:25 AM
Herald added a project: Restricted Project. · View Herald Transcript
grimar requested review of this revision.Nov 17 2020, 5:25 AM
jhenderson added inline comments.Nov 18 2020, 1:35 AM
llvm/test/tools/llvm-readobj/ELF/stack-sizes.test
397–410

Would --implicit-check-not=warning: be worthwhile?

llvm/tools/llvm-readobj/ELFDumper.cpp
5905–5906

I think you can delete this consumeError since you're calling takeError above, right?

grimar updated this revision to Diff 306627.Nov 20 2020, 2:03 AM
grimar marked 2 inline comments as done.
grimar edited the summary of this revision. (Show Details)
  • Addressed review comments.
llvm/test/tools/llvm-readobj/ELF/stack-sizes.test
397–410

Done.

llvm/tools/llvm-readobj/ELFDumper.cpp
5905–5906

Right. Thanks!

jhenderson accepted this revision.Nov 20 2020, 2:33 AM

Looks good. Might be worth giving @MaskRay a chance to comment.

This revision is now accepted and ready to land.Nov 20 2020, 2:33 AM