[PATCH] D131309: [ELF] Add ability to get a symbol by name from the hash table section

Thu Aug 25 18:52:53 PDT 2022

MaskRay added a comment.

In D131309#3749907 <https://reviews.llvm.org/D131309#3749907>, @jhuber6 wrote:

> In D131309#3748194 <https://reviews.llvm.org/D131309#3748194>, @MaskRay wrote:
>
>> In D131309#3734346 <https://reviews.llvm.org/D131309#3734346>, @jhenderson wrote:
>>
>>> Mostly just nits remaining from me, plus a couple of "normal behaviour" test cases I missed earlier.
>>>
>>> FWIW, I think implementing SHT_HASH support first, as this patch does, should definitely happen. This is because the SHT_HASH is the thing that is in the gABI, and not SHT_GNU_HASH, however widely the GNU version is supported. Last I checked my company's system didn't use GNU hash tables yet, for example. I wouldn't be opposed to follow-up work for the GNU hash table though.
>>
>> OK, I did not know you have internal users with DT_HASH. https://maskray.me/blog/2022-08-21-glibc-and-dt-gnu-hash#what-is-dt_gnu_hash My investigation suggests that multiple major Linux distributions have switched to --hash-style=gnu before 2010...
>> Nowadays DT_HASH is unlikely to be used on non-mips Linux.
>> My mild objection is because such a feature would nearly assuredly benefit nobody, but I think I'll not hold a strong objection.
>> In case related, FreeBSD rtld has supported DT_GNU_HASH since 2012.
>> As of today, their clang driver does not pass `--hash-style=` to ld, so the default `--hash-style=both` is used, but this does not mean that `DT_HASH` is mandatory.
>>
>> Note that the bloom filter in a GNU hash table is optional. @jhuber6 can just change the hash function, then the existing cost is almost a DT_GNU_HASH implementation.
>>
>>> Presumably LLD has the GNU hash alogrithm in its source code, since it can generate such tables? Could that code be moved to libObject if needed?
>
> Are you okay with me landing this patch now and adding the GNU hash support later?

I wish that we don't add unneeded functionality...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131309