[PATCH] D88817: [llvm-readobj/elf] - Ignore the hash table when on EM_S390/EM_ALPHA platforms.

Tue Oct 6 01:30:26 PDT 2020

grimar added a comment.

In D88817#2313661 <https://reviews.llvm.org/D88817#2313661>, @uweigand wrote:

> In D88817#2313657 <https://reviews.llvm.org/D88817#2313657>, @grimar wrote:
>
>> In D88817#2311572 <https://reviews.llvm.org/D88817#2311572>, @uweigand wrote:
>>
>>> It looks like this will trigger a warning being emitted on every use of llvm-readobj on a EM_S390 object file, which is probably not what we want.   (They all also contain .gnu.hash, so the fact that .hash is ignored is not anything that the user should be concerned about. --  This warning may just cause unnecessary confusion, or parse error with automated tools etc.)
>>
>> I'd like to wait for opinions of @jhenderson/@MaskRay to confirm them are happy to silently ignore the `.hash` section on `EM_S390`/`EM_ALPHA` platforms. It might probably
>> be confusing to see a `.hash` section in an object and no output with `--hash-table`.
>
> If you use an option that specifically refers to the hash table like `--hash-table`, I'd be fine with a warning in that case.  But the warning as currently implemented seems to trigger with **any** invocation of llvm-readobj, even though most users will never care about the hash table (or even know about it in the first place).

Yes, I see. The current logic also always tries to infer the size of the dynamic symbol table from the DT_HASH hash table, if present. It will be inconsistent that we do it in one case (on all other platforms),
but silently don't do in another case (on `EM_S390`/`EM_ALPHA`). But since the reason of the inconsistency is that the `.hash` table on these platforms violates ELF specification, personally I think we can ignore it.
Because the only another real option I see is to change the code to support 8 bytes entries, what looks excessive, given that the `.hash` table is a depricated section that was just broken from the begining on s390/alpha.

So I just want to hear from others that they are happy with doing it, before I start adjusting the patch.

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

https://reviews.llvm.org/D88817