This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj] - Improve error reporting for hash tables.
ClosedPublic

Authored by grimar on May 21 2020, 4:06 AM.

Details

Summary

This improves the next points for broken hash tables:

  1. Use reportUniqueWarning to prevent duplication when --hash-table and --elf-hash-histogram are used together.
  1. Dump nbuckets and nchain fields. It is often possible to dump them even when the table itself goes past the EOF etc.

Diff Detail

Event Timeline

grimar created this revision.May 21 2020, 4:06 AM
Herald added a project: Restricted Project. · View Herald Transcript
jhenderson added inline comments.May 26 2020, 12:56 AM
llvm/test/tools/llvm-readobj/ELF/hash-table.test
233

Nit: I'd indent this line one more space (i.e. to be two spaces more indented than the line above).

llvm/tools/llvm-readobj/ELFDumper.cpp
2640

Up to you, but you could avoid needing to pass in the Obj as well, by using Dumper.getElfObject()->getELFFile(), I believe.

2667–2668

What happens if the HashTable ends before the nbucket/nchain fields can be read?

grimar planned changes to this revision.May 26 2020, 6:27 AM
grimar marked an inline comment as done.
grimar added inline comments.
llvm/tools/llvm-readobj/ELFDumper.cpp
2667–2668

Seems nothing good.. We have a test (--docnum=3), but I think it just a luck that it "worked" for me.
The problem is probably not about where HashTable end, but where file buffer ends: it would be probably OK to
read past the end of the section, but not past the end of the file. That what I missed here and in D80215.
I think this change is better to be just reverted.

grimar updated this revision to Diff 267198.May 29 2020, 5:33 AM
grimar marked 4 inline comments as done.
  • Updated implementation.
  • Addressed review comments.
llvm/tools/llvm-readobj/ELFDumper.cpp
2640

Thanks for this hint, I've used it somewhere else. The Dumper argument is gone.

2667–2668

I've updated the implementation to be similar to one for the GNU hash table.

jhenderson accepted this revision.Jun 1 2020, 12:37 AM

LGTM, with nit.

llvm/tools/llvm-readobj/ELFDumper.cpp
2650

Nit: I'd rename this MakeError, because I read BuildError and think there's something wrong with the build, rather than that this is for building an error.

This revision is now accepted and ready to land.Jun 1 2020, 12:37 AM
This revision was automatically updated to reflect the committed changes.