[PATCH] D80215: [llvm-readelf] - --elf-hash-histogram: do not crash when the .gnu.hash goes past the EOF.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/hash-histogram.test:277
+# RUN: llvm-readelf --elf-hash-histogram %t7 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t7 -DMASKWORDS=4294967295 -DNBUCKETS=3 --check-prefix=ERR5
+
----------------
This and below includes reference to 2 variables (MASKWORDS and NBUCKETS) that don't appear to be used?

Should you also add something that shows that the hash histogram isn't printed in these cases?


================
Comment at: llvm/test/tools/llvm-readobj/ELF/hash-histogram.test:299
+      Shift2: 0x0
+## The number of words in the Bloom filter. The value of 2 is no-op.
+      MaskWords: [[MASKWORDS]]
----------------
Perhaps move this comment and the similar one below, to where they are actually used.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2677
+      Obj->getBufSize()) {
+    reportWarning(createError("unable to dump the SHT_GNU_HASH "
+                              "section at 0x" +
----------------
Again, probably in a follow-up, perhaps this should be changed to `reportUniqueWarning` (and similar in `checkHashTable`). That way, if a user requested both the hash histogram and hash tables (which seems like a not unreasonable request), they'd only get the warning once.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2703-2706
   W.printNumber("Num Buckets", GnuHashTable->nbuckets);
   W.printNumber("First Hashed Symbol Index", GnuHashTable->symndx);
   W.printNumber("Num Mask Words", GnuHashTable->maskwords);
   W.printNumber("Shift Count", GnuHashTable->shift2);
----------------
I note that `printHashTable` doesn't print any information at all if the hash table is bad, but this prints the header information. Perhaps (in a follow-up), `printHashTable` could be changed to print the header information like here?


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

https://reviews.llvm.org/D80215