[PATCH glibc 3/9] Implement __libc_early_init

[Carlos O'Donell]

On 4/27/20 8:57 AM, Adhemerval Zanella wrote:
> 
> 
> On 24/04/2020 17:18, Carlos O'Donell via Libc-alpha wrote:
>> On 4/24/20 2:22 PM, Florian Weimer wrote:
>>> * Carlos O'Donell via Libc-alpha:
>>>
>>>> Please clarify or remove the comment related to libc_already_loaded.
>>>
>>> I have removed the initialization.  It is no longer relevant in the
>>> current patch.  It may have been obsoleted by other dl_open_worker
>>> changes earlier.
>>
>> Thanks!
> [...]
>>> diff --git a/elf/dl-lookup-direct.c b/elf/dl-lookup-direct.c
>>> new file mode 100644
>>> index 0000000000..5637ae89de
>>> --- /dev/null
>>> +++ b/elf/dl-lookup-direct.c
>>> @@ -0,0 +1,116 @@
>>> +/* Look up a symbol in a single specified object.
>>> +   Copyright (C) 1995-2020 Free Software Foundation, Inc.
>>> +   This file is part of the GNU C Library.
>>> +
>>> +   The GNU C Library is free software; you can redistribute it and/or
>>> +   modify it under the terms of the GNU Lesser General Public
>>> +   License as published by the Free Software Foundation; either
>>> +   version 2.1 of the License, or (at your option) any later version.
>>> +
>>> +   The GNU C Library is distributed in the hope that it will be useful,
>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> +   Lesser General Public License for more details.
>>> +
>>> +   You should have received a copy of the GNU Lesser General Public
>>> +   License along with the GNU C Library; if not, see
>>> +   <https://www.gnu.org/licenses/>.  */
>>> +
>>> +#include <ldsodefs.h>
>>> +#include <string.h>
>>> +#include <elf_machine_sym_no_match.h>
>>> +#include <dl-hash.h>
>>> +
>>> +/* This function corresponds to do_lookup_x in elf/dl-lookup.c.  The
>>> +   variant here is simplified because it requires symbol
>>> +   versioning.  */
>>> +static const ElfW(Sym) *
>>> +check_match (const struct link_map *const map, const char *const undef_name,
>>> +             const char *version, uint32_t version_hash,
>>> +             const Elf_Symndx symidx)
>>> +{
>>> +  const ElfW(Sym) *symtab = (const void *) D_PTR (map, l_info[DT_SYMTAB]);
>>> +  const ElfW(Sym) *sym = &symtab[symidx];
>>> +
>>> +  unsigned int stt = ELFW(ST_TYPE) (sym->st_info);
>>> +  if (__glibc_unlikely ((sym->st_value == 0 /* No value.  */
>>> +                         && sym->st_shndx != SHN_ABS
>>> +                         && stt != STT_TLS)
>>> +                        || elf_machine_sym_no_match (sym)))
>>> +    return NULL;
>>> +
>>> +  /* Ignore all but STT_NOTYPE, STT_OBJECT, STT_FUNC,
>>> +     STT_COMMON, STT_TLS, and STT_GNU_IFUNC since these are no
>>> +     code/data definitions.  */
>>> +#define ALLOWED_STT \
>>> +  ((1 << STT_NOTYPE) | (1 << STT_OBJECT) | (1 << STT_FUNC) \
>>> +   | (1 << STT_COMMON) | (1 << STT_TLS) | (1 << STT_GNU_IFUNC))
>>> +  if (__glibc_unlikely (((1 << stt) & ALLOWED_STT) == 0))
>>> +    return NULL;
>>> +
>>> +  const char *strtab = (const void *) D_PTR (map, l_info[DT_STRTAB]);
>>> +
>>> +  if (strcmp (strtab + sym->st_name, undef_name) != 0)
>>> +    /* Not the symbol we are looking for.  */
>>> +    return NULL;
>>> +
>>> +  ElfW(Half) ndx = map->l_versyms[symidx] & 0x7fff;
>>> +  if (map->l_versions[ndx].hash != version_hash
>>> +      || strcmp (map->l_versions[ndx].name, version) != 0)
>>> +    /* It's not the version we want.  */
>>> +    return NULL;
>>> +
>>> +  return sym;
>>> +}
>>
>> OK.
> 
> The only reservation I have with this change it this code duplication,
> could we refactor to have this patch in a common place?
> 

I thought about asking this, but the code is so short, and the refactor
would be significantly more complicated with all the fallbacks. It's easier
to read this simplified version and maintain it because you know what it's
expected to do.

It *might* be possible to refactor it a bit more, but I wasn't going to push
on that as a requirement for the direct versioned symbol lookup.

-- 
Cheers,
Carlos.