[PATCH] D80373: [llvm-readobj] - Improve error reporting for hash tables.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 29 05:56:44 PDT 2020


grimar added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2640
+static bool checkHashTable(const ELFDumper<ELFT> &Dumper,
+                           const ELFFile<ELFT> *Obj,
+                           const typename ELFT::Hash *H) {
----------------
jhenderson wrote:
> Up to you, but you could avoid needing to pass in the `Obj` as well, by using `Dumper.getElfObject()->getELFFile()`, I believe.
Thanks for this hint, I've used it somewhere else. The `Dumper` argument is gone.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2667-2668
     return;
   W.printNumber("Num Buckets", HashTable->nbucket);
   W.printNumber("Num Chains", HashTable->nchain);
+  if (!checkHashTable(*this, ObjF->getELFFile(), HashTable))
----------------
grimar wrote:
> jhenderson wrote:
> > What happens if the HashTable ends before the nbucket/nchain fields can be read?
> 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.
I've updated the implementation to be similar to one for the GNU hash table.


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

https://reviews.llvm.org/D80373





More information about the llvm-commits mailing list