[Patch v3] MIPS support for GNU hash

[Yann Droneaud]

Hi,

Le mardi 23 juillet 2019 à 14:44 +0200, Mihailo Stojanovic a écrit :
> Hello everyone,
> 
>   This patch is a reimplementation of [1], which was submitted back in
> 2015. Copyright issue has been sorted out [2] last year. It proposed a
> new section (.gnu.xhash) and related dynamic tag (DT_GNU_XHASH). The new
> section would be virtually identical to the existing .gnu.hash except
> for the translation table (xlat) which would contain correct MIPS
> .dynsym indexes corresponding to the hashvals in chains. This is because
> MIPS ABI imposes a different ordering of the dynyms than the one
> expected by the .gnu.hash section. Another addition would be a leading
> word which would contain the number of entries in the translation table.
> 
>   In this patch, the new section name and dynamic tag are changed to
> reflect the fact that the section should be treated as MIPS-specific
> (.MIPS.xhash and DT_MIPS_XHASH).
> 
>   This patch addresses the alignment issue as reported in [3] which is
> caused by the leading word added to the .MIPS.xhash section. Leading word
> is now removed in the corresponding binutils patch, and the number of
> entries in the translation table is computed using DT_MIPS_SYMTABNO
> dynamic tag.
> 
>   Since the MIPS specific dl-lookup.c file was removed since the initial
> patch, I opted for the definition of three new macros in the generic
> ldsodefs.h. ELF_MACHINE_GNU_HASH_ADDRIDX defines the index of the
> dynamic tag in the l_info array. ELF_MACHINE_HASH_SYMIDX is used to
> calculate the index of a symbol in GNU hash. On MIPS, it is defined to
> look up the symbol index in the translation table.
> ELF_MACHINE_XHASH_SETUP is defined for MIPS only. It initializes the
> .MIPS.xhash pointer in the link_map_machine struct.
> 
>   The other major change is reserving MIPS ABI version 5 for .MIPS.xhash,
> suggesting that the dynamic linker now supports .MIPS.xhash.
> 
>   The patch was tested by running the glibc testsuite for the three MIPS
> ABIs (o32, n32 and n64).
> 
>   The corresponding binutils patch can be found here:
> https://sourceware.org/ml/binutils/2019-07/msg00098.html
> 
> Regards,
> Mihailo
> 
> [1] https://sourceware.org/ml/binutils/2015-10/msg00057.html
> [2] https://sourceware.org/ml/binutils/2018-03/msg00025.html
> [3] https://sourceware.org/ml/binutils/2016-01/msg00006.html
> 
>         * elf/dl-addr.c (determine_info): Calculate the symbol index
>         using the newly defined ELF_MACHINE_HASH_SYMIDX macro.
>         * elf/dl-lookup.c (do_lookup_x): Calculate the symbol index
>         using the newly defined ELF_MACHINE_HASH_SYMIDX macro.
>         (_dl_setup_hash): Initialize MIPS xhash translation table.
>         * elf/elf.h (SHT_MIPS_XHASH): New define.
>         (DT_MIPS_XHASH): New define.
>         * sysdeps/generic/ldsodefs.h (ELF_MACHINE_GNU_HASH_ADDRIDX): New
>         define.
>         (ELF_MACHINE_HASH_SYMIDX): Ditto.
>         (ELF_MACHINE_XHASH_SETUP): Ditto.
>         *sysdeps/mips/ldsodefs.h (ELF_MACHINE_GNU_HASH_ADDRIDX): New
>         define.
>         (ELF_MACHINE_HASH_SYMIDX): Ditto.
>         (ELF_MACHINE_XHASH_SETUP): Ditto.
>         * sysdeps/mips/linkmap.h (struct link_map_machine): New member.
>         * sysdeps/unix/sysv/linux/mips/ldsodefs.h: Increment valid ABI
>         version.
>         * sysdeps/unix/sysv/linux/mips/libc-abis: New ABI version.
> ---
>  elf/dl-addr.c                           |  5 ++---
>  elf/dl-lookup.c                         | 10 +++++++---
>  elf/elf.h                               |  5 ++++-
>  sysdeps/generic/ldsodefs.h              | 16 ++++++++++++++++
>  sysdeps/mips/ldsodefs.h                 | 11 +++++++++++
>  sysdeps/mips/linkmap.h                  |  1 +
>  sysdeps/unix/sysv/linux/mips/ldsodefs.h |  2 +-
>  sysdeps/unix/sysv/linux/mips/libc-abis  |  3 +++
>  8 files changed, 45 insertions(+), 8 deletions(-)
> 
> diff --git a/elf/dl-addr.c b/elf/dl-addr.c
> index 9d285d7..5345c8d 100644
> --- a/elf/dl-addr.c
> +++ b/elf/dl-addr.c
> @@ -42,7 +42,7 @@ determine_info (const ElfW(Addr) addr, struct link_map *match, Dl_info *info,
>    ElfW(Word) strtabsize = match->l_info[DT_STRSZ]->d_un.d_val;
>  
>    const ElfW(Sym) *matchsym = NULL;
> -  if (match->l_info[ADDRIDX (DT_GNU_HASH)] != NULL)
> +  if (match->l_info[ELF_MACHINE_GNU_HASH_ADDRIDX] != NULL)
>      {
>        /* We look at all symbol table entries referenced by the hash
>  	 table.  */
> @@ -57,6 +57,7 @@ determine_info (const ElfW(Addr) addr, struct link_map *match, Dl_info *info,
>  		{
>  		  /* The hash table never references local symbols so
>  		     we can omit that test here.  */
> +		  symndx = ELF_MACHINE_HASH_SYMIDX(match, hasharr);
>  		  if ((symtab[symndx].st_shndx != SHN_UNDEF
>  		       || symtab[symndx].st_value != 0)
>  		      && symtab[symndx].st_shndx != SHN_ABS
> @@ -65,8 +66,6 @@ determine_info (const ElfW(Addr) addr, struct link_map *match, Dl_info *info,
>  					    matchsym, addr)
>  		      && symtab[symndx].st_name < strtabsize)
>  		    matchsym = (ElfW(Sym) *) &symtab[symndx];
> -
> -		  ++symndx;
>  		}
>  	      while ((*hasharr++ & 1u) == 0);
>  	    }


While I haven't reviewed them thoroughly nor tested them, those changes
are looking much better than the dl-addr.c duplication proposed in
first patch submission. In particular, the proposed changes seems
compatible with my tentative at optimizing dladdr(), see 
https://sourceware.org/bugzilla/show_bug.cgi?id=24561

So, thanks !

Regards.

-- 
Yann Droneaud
OPTEYA