[PATCH] D73269: [llvm-readobj] - Add a few warnings for --gnu-hash-table.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2525
+    // vector. We should not report a warning in this case.
+    bool IsEmptyHashTable =
+        GnuHashTable->symndx == NumSyms && BloomFilter.size() == 1 &&
----------------
grimar wrote:
> MaskRay wrote:
> > `GnuHashTable->symndx == 1` is probably sufficient.
> > 
> > The other conditions are redundant.
> > GnuHashTable->symndx == 1 is probably sufficient.
> 
> It is not, because it can be > 1. For example when we have an object with an
> undefined dynamic symbol and use lld:
> 
> 
> **1.s:**
> .global bar
> 
> ```
> as 1.s -o 1.o
> lld 1.o -o 1.so -shared --hash-style=gnu
> llvm-readelf --gnu-hash-table 1.so
> ```
> 
> ```
> GnuHashTable {
>   Num Buckets: 1
>   First Hashed Symbol Index: 2
>   Num Mask Words: 1
>   Shift Count: 26
>   Bloom Filter: [0x0]
>   Buckets: [0]
>   Values: []
> }
> ```
> 
> For GNU ld the output is different, btw (but it does not seem to be critical to fix LLD here, I guess):
> 
> ```
> ld.bfd 1.o -o 1.so -shared --hash-style=gnu
> llvm-readelf --gnu-hash-table 1.so
> ```
> 
> ```
> GnuHashTable {
>   Num Buckets: 1
>   First Hashed Symbol Index: 1
>   Num Mask Words: 1
>   Shift Count: 0
>   Bloom Filter: [0x0]
>   Buckets: [0]
>   Values: [0x0]
> }
> ```
> 
OK, `!Buckets.empty() && Buckets[0] == 0` is sufficient.

The number of buckets does not matter. The size of the bloom filter does not matter (it can preclude some symbols. Since there is no defined dynamic symbol, it shouldn't matter if the bloom filter precludes anything).


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

https://reviews.llvm.org/D73269