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

Thu Jun 18 02:41:56 PDT 2020

grimar marked an inline comment as done.
grimar added a comment.

In D82010#2100106 <https://reviews.llvm.org/D82010#2100106>, @jhenderson wrote:

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

Right. It it what I mentioned in the description:
"Also with this change we start to report a warning when the histogram is requested for
the GNU hash table, but the dynamic symbols table is empty (size == 0)."

GNU readelf doesn't warn about that. But isn't it a bug? When we have a zero sized dynamic table it is already incorrect, because it should have a zero symbol at least.
And it is wierd to have a hash table in this case too.

================
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
----------------
jhenderson wrote:
> It might also locate it by name, but I hope it wouldn't :-)
> 
> Perhaps this should say "The code locates the..."
There is also an option currently where it uses the `DT_SYMTAB` tag.
In this test cases we have no such tag, so I think your suggestion is ok, I'll apply it.

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

https://reviews.llvm.org/D82010