
Date: Wed, 08 Aug 2012 15:57:15 +0200
From: musl <b.brezillon.musl@...il.com>
To: musl@...ts.openwall.com
CC: Szabolcs Nagy <nsz@...t70.net>
Subject: Re: ldso : dladdr support
Same as before except I use bit mask to support multiple hash algorithms.
On 08/08/2012 13:52, Szabolcs Nagy wrote:
> * musl <b.brezillon.musl@...il.com> [20120808 11:55:45 +0200]:
>> Here is the new patch for dladdr + gnu hash support.
>>
>> On 08/08/2012 01:09, Rich Felker wrote:
>>> don't remember which) where I described some optimizations that should
>>> be present if gnu hash is to be supported. Basically, the dynamic
>>> linker should keep track of whether there's one of the two hashes
>>> that's supported by ALL loaded libs, and in that case, only ever use
>>> that hash, to avoid computing two different hashes.
>> The hash for given algo is only computed once (if needed).
>> That's the reason for the computed table.
>> If all libs uses the same hash algo, the other will never be computed.
> a lib can support both hash tables
> (so it's nontrivial to use the one that all libs support,
> eg instead of a single alg index a bit mask should be used)
>
See patch changes.
>> +#define SYSV_HASH_ALG_IDX 0
>> +#define GNU_HASH_ALG_IDX 1
>> +#define HASH_ALG_CNT 2
> if we go with this approach then use shorter names
> (thes are only used locally)
>
> eg. SYSV_HASH, GNU_HASH, NHASH
Corrected.
>> + for (h1 &= (uint32_t)2;; sym++) {
>> + h2 = *hashval++;
>> + if ((h1 == (h2 & ~1)) && !strcmp(s, strings + sym>st_name))
> these is still a ~1
>
> looking at it now probably writing out & 0xfffffffe
> is the cleanest
>
>> + static uint32_t precomp[HASH_ALG_CNT][3] = {
>> + {0x6b366be, 0x6b3afd, 0x595a4cc},
>> + {0xf9040207, 0xf4dc4ae, 0x1f4039c9},
>> + };
>> + uint32_t h[HASH_ALG_CNT];
>> + uint8_t computed[HASH_ALG_CNT] = {0, 0};
>> + uint8_t alg = dso>hashalg;
>> + if (alg == SYSV_HASH_ALG_IDX)
>> + h[alg] = sysv_hash(s);
>> + else
>> + h[alg] = gnu_hash(s);
>> +
>> + computed[alg] = 1;
>> +
>> + if (h[alg] == precomp[alg][0] && !strcmp(s, "dlopen")) rtld_used = 1;
>> + if (h[alg] == precomp[alg][1] && !strcmp(s, "dlsym")) rtld_used = 1;
>> + if (h[alg] == precomp[alg][2] && !strcmp(s, "__stack_chk_fail")) ssp_used = 1;
>> +
>> for (; dso; dso=dso>next) {
>> Sym *sym;
>> +
>> if (!dso>global) continue;
>>  sym = lookup(s, h, dso);
>> +
>> + alg = dso>hashalg;
>> + if (!computed[alg]) {
>> + if (alg == SYSV_HASH_ALG_IDX) {
>> + h[alg] = sysv_hash(s);
>> + sym = sysv_lookup(s, h[alg], dso);
>> + }
>> + else {
>> + h[alg] = gnu_hash(s);
>> + sym = gnu_lookup(s, h[alg], dso);
>> + }
>> + computed[alg] = 1;
>> + } else {
>> + if (alg == SYSV_HASH_ALG_IDX)
>> + sym = sysv_lookup(s, h[alg], dso);
>> + else
>> + sym = gnu_lookup(s, h[alg], dso);
>> + }
> instead of arrays i'd write
>
> alg = dso>hashalg;
> if (alg == SYSV_HASH) {
> if (sysv_ok) {
> ...
> sysv_ok = 1;
> }
> sym = sysv_lookup(s, sysv_h, dso);
> } else {
> }
Haven't changed it yet.
> since there are many ifs anyway
>
> the table approach is nicer when all data and functions
> are in a table:
>
> if (!ok[alg]) {
> h[alg] = hash[alg](s);
> ok[alg] = 1;
> }
> sym = lookup[alg](s, h[alg], dso);
>
>> + p>hashalg = SYSV_HASH_ALG_IDX;
>> p>strings = (void *)(p>base + dyn[DT_STRTAB]);
>> + for (; v[0]; v+=2) if (v[0] == DT_GNU_HASH) {
>> + p>hashtab = (void *)(p>base + v[1]);
>> + p>hashalg = GNU_HASH_ALG_IDX;
>> + }
> so it seems gnu hash is used whenever it's present
> i'm not sure if that's the right default..
>
> another possibility is to have a plain find_sym function
> which is simple and only supports sysv hash
> and whenever it encounters a lib that has no sysv hash in
> it a find_sym_gnu is called that does the hard work
> (so using gnu only libs is penalized, i don't know how
> common that case is though)
View attachment "ldsoadddladdrandgnuhashsupport.patch" of type "text/xpatch" (10516 bytes)
Powered by blists  more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.