[PATCH] D82010: [llvm-readobj] - Add a validation of the GNU hash table to printGnuHashHistogram().

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

If I'm following things right, the behaviour for an empty dynsym is changing with this patch. Is the new behaviour compatible with GNU?



================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnuhash.test:130
 # SYMNDX-NEXT:   Buckets: [2]
-# SYMNDX-NEXT: warning: '[[FILE]]': the first hashed symbol index (2) is larger than the number of dynamic symbols (2)
+# SYMNDX-NEXT: warning: '[[FILE]]': unable to dump 'Values' for the SHT_GNU_HASH section: the first hashed symbol index (2) is larger than the number of dynamic symbols (2)
 # SYMNDX-NEXT: }
----------------
Hmm... This message is clearly not quite right even before your change: 2 is obviously not larger than 2...

Anyway, that's a separate issue.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/hash-histogram.test:335
+
+## Case A: when the buckets array is not empty and has a non-null value we report a warning.
+# RUN: yaml2obj --docnum=7 -DVAL=0x1 %s -o %t9
----------------
Perhaps this should be non-zero rather than non-null? Similar null -> zero below.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/hash-histogram.test:342
+
+## Case B: we do not report a warning, when the buckets array contains only null values.
+# RUN: yaml2obj --docnum=7 -DVAL=0x0 %s -o %t10
----------------
No need for the comma here or in Case C.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/hash-histogram.test:386
+
+## The code might locate the dynamic symbol table by the section type. Use SHT_PROGBITS to hide it.
+# RUN: yaml2obj --docnum=8 -DTYPE=SHT_PROGBITS %s -o %t12
----------------
It might also locate it by name, but I hope it wouldn't :-)

Perhaps this should say "The code locates the..."


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

https://reviews.llvm.org/D82010