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

[George Rimar via Phabricator via llvm-commits]

grimar 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
+
----------------
jhenderson wrote:
> 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?
Fixed.


================
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]]
----------------
jhenderson wrote:
> Perhaps move this comment and the similar one below, to where they are actually used.
Moved right parts.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2677
+      Obj->getBufSize()) {
+    reportWarning(createError("unable to dump the SHT_GNU_HASH "
+                              "section at 0x" +
----------------
jhenderson wrote:
> 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.
Fixed in this patch, because I had to change the implementation of this method to support the case when the header can be read, but the rest of the table is broken.


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

https://reviews.llvm.org/D80215