diff mbox series

[v2] MIPS support for GNU hash

Message ID 1562843158-31390-1-git-send-email-mihailo.stojanovic@rt-rk.com
State New
Headers show
Series [v2] MIPS support for GNU hash | expand

Commit Message

Mihailo Stojanovic July 11, 2019, 11:05 a.m. UTC
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. This is
something of which I am not sure, especially after reading [4]. I am
confused on whether this ABI version is reserved for IFUNC, or it can be
used for this purpose.

  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
[4] https://sourceware.org/ml/libc-alpha/2016-12/msg00853.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.
        *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                           |  6 ++----
 elf/dl-lookup.c                         | 12 +++++++++---
 elf/elf.h                               |  5 ++++-
 sysdeps/generic/ldsodefs.h              | 11 +++++++++++
 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, 42 insertions(+), 9 deletions(-)

Comments

Mihailo Stojanovic July 17, 2019, 3:13 p.m. UTC | #1
Hi,

I would like to check the status of this patch, given the fact that
it's a release blocker for the glibc 2.30.

Cheers,
Mihailo
Adhemerval Zanella July 22, 2019, 8:43 p.m. UTC | #2
On 11/07/2019 08:05, Mihailo Stojanovic wrote:
> 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. This is
> something of which I am not sure, especially after reading [4]. I am
> confused on whether this ABI version is reserved for IFUNC, or it can be
> used for this purpose.
> 
>   The patch was tested by running the glibc testsuite for the three MIPS
> ABIs (o32, n32 and n64).

I focused the review on the changes that touch generic implementation, I
did not focused on MIPS specific bits neither if the changes matches what
binutils patch is aiming to do. 

The patch looks ok with on the change of ELF_MACHINE_XHASH_SETUP definition
below.

> 
>   The corresponding binutils patch can be found here:
> https://sourceware.org/ml/binutils/2019-07/msg00098.html

If I understood correctly this patch has not been pushed upstream, neither
in fact actually reviewed.  My understanding is we require upstream support
for either kernel or toolchain features before including it on glibc, although
not sure about this specific feature where it really tied with binutils
support.

Carlos, do you think we should wait for binutils?

> 
> 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
> [4] https://sourceware.org/ml/libc-alpha/2016-12/msg00853.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.
>         *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                           |  6 ++----
>  elf/dl-lookup.c                         | 12 +++++++++---
>  elf/elf.h                               |  5 ++++-
>  sysdeps/generic/ldsodefs.h              | 11 +++++++++++
>  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, 42 insertions(+), 9 deletions(-)
> 
> diff --git a/elf/dl-addr.c b/elf/dl-addr.c
> index 9d285d7..f504e03 100644
> --- a/elf/dl-addr.c
> +++ b/elf/dl-addr.c
> @@ -20,7 +20,6 @@
>  #include <stddef.h>
>  #include <ldsodefs.h>
>  
> -

Gratuitous line deletion, I personally try to avoid such changes.

>  static inline void
>  __attribute ((always_inline))
>  determine_info (const ElfW(Addr) addr, struct link_map *match, Dl_info *info,
> @@ -42,7 +41,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 +56,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

Ok, assignment will be expanded to:

  symndx = hasharr - hasharr->l_gnu_chain_zero;

> @@ -65,8 +65,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);
>  	    }

And then 'symndx' will be updated by 'hashrray' increment.

> diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c
> index e3f42a1..d717ed3 100644
> --- a/elf/dl-lookup.c
> +++ b/elf/dl-lookup.c
> @@ -403,7 +403,7 @@ do_lookup_x (const char *undef_name, uint_fast32_t new_hash,
>  		  do
>  		    if (((*hasharr ^ new_hash) >> 1) == 0)
>  		      {
> -			symidx = hasharr - map->l_gnu_chain_zero;
> +			symidx = ELF_MACHINE_HASH_SYMIDX (map, hasharr);
>  			sym = check_match (undef_name, ref, version, flags,
>  					   type_class, &symtab[symidx], symidx,
>  					   strtab, map, &versioned_sym,

Ok.

> @@ -937,10 +937,10 @@ _dl_setup_hash (struct link_map *map)
>  {
>    Elf_Symndx *hash;
>  
> -  if (__glibc_likely (map->l_info[ADDRIDX (DT_GNU_HASH)] != NULL))
> +  if (__glibc_likely (map->l_info[ELF_MACHINE_GNU_HASH_ADDRIDX] != NULL))
>      {
>        Elf32_Word *hash32
> -	= (void *) D_PTR (map, l_info[ADDRIDX (DT_GNU_HASH)]);
> +	= (void *) D_PTR (map, l_info[ELF_MACHINE_GNU_HASH_ADDRIDX]);
>        map->l_nbuckets = *hash32++;
>        Elf32_Word symbias = *hash32++;
>        Elf32_Word bitmask_nwords = *hash32++;

Ok.

> @@ -955,6 +955,12 @@ _dl_setup_hash (struct link_map *map)
>        map->l_gnu_buckets = hash32;
>        hash32 += map->l_nbuckets;
>        map->l_gnu_chain_zero = hash32 - symbias;
> +
> +      /* Initialize MIPS xhash translation table.  */
> +#ifdef ELF_MACHINE_XHASH_SETUP
> +      ELF_MACHINE_XHASH_SETUP (hash32, symbias, map)
> +#endif
> +
>        return;
>      }
>  

I think it is better we move away from ifdef such kind of logic where possible
and just define ELF_MACHINE_XHASH_SETUP in the generic as an empty macro.

> diff --git a/elf/elf.h b/elf/elf.h
> index bb13c87..b53d815 100644
> --- a/elf/elf.h
> +++ b/elf/elf.h
> @@ -1715,6 +1715,7 @@ typedef struct
>  #define SHT_MIPS_EH_REGION	0x70000027
>  #define SHT_MIPS_XLATE_OLD	0x70000028
>  #define SHT_MIPS_PDR_EXCEPTION	0x70000029
> +#define SHT_MIPS_XHASH		0x7000002b
>  
>  /* Legal values for sh_flags field of Elf32_Shdr.  */
>  

Ok, it matches the binutils patch.

> @@ -1962,7 +1963,9 @@ typedef struct
>     in a PIE as it stores a relative offset from the address of the tag
>     rather than an absolute address.  */
>  #define DT_MIPS_RLD_MAP_REL  0x70000035
> -#define DT_MIPS_NUM	     0x36
> +/* GNU-style hash table with xlat.  */
> +#define DT_MIPS_XHASH	     0x70000036
> +#define DT_MIPS_NUM	     0x37
>  
>  /* Legal values for DT_MIPS_FLAGS Elf32_Dyn entry.  */
>  

Ditto.

> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index b1fc5c3..e95d8cd 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -47,6 +47,17 @@ __BEGIN_DECLS
>  #define ADDRIDX(tag)	(DT_NUM + DT_THISPROCNUM + DT_VERSIONTAGNUM \
>  			 + DT_EXTRANUM + DT_VALNUM + DT_ADDRTAGIDX (tag))
>  
> +/* Type of GNU hash which the machine uses.  */
> +#ifndef ELF_MACHINE_GNU_HASH_ADDRIDX
> +# define ELF_MACHINE_GNU_HASH_ADDRIDX ADDRIDX (DT_GNU_HASH)
> +#endif
> +
> +/* Calculate the index of a symbol in GNU_HASH.  */
> +#ifndef ELF_MACHINE_HASH_SYMIDX
> +# define ELF_MACHINE_HASH_SYMIDX(map, hasharr) \
> +  ((hasharr) - (map)->l_gnu_chain_zero)
> +#endif
> +
>  /* We use this macro to refer to ELF types independent of the native wordsize.
>     `ElfW(TYPE)' is used in place of `Elf32_TYPE' or `Elf64_TYPE'.  */
>  #define ELFW(type)	_ElfW (ELF, __ELF_NATIVE_CLASS, type)

Ok, the define if not since the correct option since ldsodefs is included
in arch-specific versions with include_next.

> diff --git a/sysdeps/mips/ldsodefs.h b/sysdeps/mips/ldsodefs.h
> index f0acb02..0c6f027 100644
> --- a/sysdeps/mips/ldsodefs.h
> +++ b/sysdeps/mips/ldsodefs.h
> @@ -26,6 +26,17 @@ struct La_mips_32_retval;
>  struct La_mips_64_regs;
>  struct La_mips_64_retval;
>  
> +#define ELF_MACHINE_GNU_HASH_ADDRIDX (DT_MIPS_XHASH - DT_LOPROC + DT_NUM)
> +
> +/* Calculate the index of a symbol in MIPS_XHASH.  */
> +#define ELF_MACHINE_HASH_SYMIDX(map, hasharr) \
> +  ((map)->l_mach.mips_xlat_zero[(hasharr) - (map)->l_gnu_chain_zero])
> +
> +/* Setup MIPS_XHASH.  */
> +#define ELF_MACHINE_XHASH_SETUP(hash32, symbias, map) \
> +  (hash32) += (map)->l_info[DT_MIPS (SYMTABNO)]->d_un.d_val - (symbias); \
> +  (map)->l_mach.mips_xlat_zero = (hash32) - (symbias);
> +
>  #define ARCH_PLTENTER_MEMBERS						    \
>      Elf32_Addr (*mips_o32_gnu_pltenter) (Elf32_Sym *, unsigned int,	    \
>  					 uintptr_t *, uintptr_t *,	    \

Ok.

> diff --git a/sysdeps/mips/linkmap.h b/sysdeps/mips/linkmap.h
> index 1fb9678..1e640c3 100644
> --- a/sysdeps/mips/linkmap.h
> +++ b/sysdeps/mips/linkmap.h
> @@ -3,4 +3,5 @@ struct link_map_machine
>      ElfW(Addr) plt; /* Address of .plt */
>      ElfW(Word) fpabi; /* FP ABI of the object */
>      unsigned int odd_spreg; /* Does the object require odd_spreg support? */
> +    const Elf32_Word *mips_xlat_zero; /* .MIPS.xhash */
>    };
> diff --git a/sysdeps/unix/sysv/linux/mips/ldsodefs.h b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
> index 8dde855..ce7b2f9 100644
> --- a/sysdeps/unix/sysv/linux/mips/ldsodefs.h
> +++ b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
> @@ -34,7 +34,7 @@ extern void _dl_static_init (struct link_map *map);
>  #undef VALID_ELF_ABIVERSION
>  #define VALID_ELF_ABIVERSION(osabi,ver)			\
>    (ver == 0						\
> -   || (osabi == ELFOSABI_SYSV && ver < 4)		\
> +   || (osabi == ELFOSABI_SYSV && ver < 6)		\
>     || (osabi == ELFOSABI_GNU && ver < LIBC_ABI_MAX))
>  
>  #endif /* ldsodefs.h */

I presume the version 6 is to make room for the PT_GNU_STACK.

> diff --git a/sysdeps/unix/sysv/linux/mips/libc-abis b/sysdeps/unix/sysv/linux/mips/libc-abis
> index eaea558..83a667b 100644
> --- a/sysdeps/unix/sysv/linux/mips/libc-abis
> +++ b/sysdeps/unix/sysv/linux/mips/libc-abis
> @@ -16,3 +16,6 @@ UNIQUE
>  MIPS_O32_FP64   mips*-*-linux*
>  # Absolute (SHN_ABS) symbols working correctly.
>  ABSOLUTE
> +# GNU-style hash table with translation table.
> +MIPS_XHASH
> +
> 

Ok.
Mihailo Stojanovic July 23, 2019, 1:06 p.m. UTC | #3
Hello,

>>    The corresponding binutils patch can be found here:
>> https://sourceware.org/ml/binutils/2019-07/msg00098.html
> If I understood correctly this patch has not been pushed upstream, neither
> in fact actually reviewed.  My understanding is we require upstream support
> for either kernel or toolchain features before including it on glibc, although
> not sure about this specific feature where it really tied with binutils
> support.
>
> Carlos, do you think we should wait for binutils?
Binutils patch hasn't been pushed upstream, but it was reviewed in [1].
We are currently waiting on FSF to process the binutils assignment for
RT-RK [2].
>> @@ -955,6 +955,12 @@ _dl_setup_hash (struct link_map *map)
>>         map->l_gnu_buckets = hash32;
>>         hash32 += map->l_nbuckets;
>>         map->l_gnu_chain_zero = hash32 - symbias;
>> +
>> +      /* Initialize MIPS xhash translation table.  */
>> +#ifdef ELF_MACHINE_XHASH_SETUP
>> +      ELF_MACHINE_XHASH_SETUP (hash32, symbias, map)
>> +#endif
>> +
>>         return;
>>       }
>>   
> I think it is better we move away from ifdef such kind of logic where possible
> and just define ELF_MACHINE_XHASH_SETUP in the generic as an empty macro.
Acknowledged. Defined as an empty macro in the newly submitted
patch [3].
>> diff --git a/sysdeps/mips/linkmap.h b/sysdeps/mips/linkmap.h
>> index 1fb9678..1e640c3 100644
>> --- a/sysdeps/mips/linkmap.h
>> +++ b/sysdeps/mips/linkmap.h
>> @@ -3,4 +3,5 @@ struct link_map_machine
>>       ElfW(Addr) plt; /* Address of .plt */
>>       ElfW(Word) fpabi; /* FP ABI of the object */
>>       unsigned int odd_spreg; /* Does the object require odd_spreg support? */
>> +    const Elf32_Word *mips_xlat_zero; /* .MIPS.xhash */
>>     };
>> diff --git a/sysdeps/unix/sysv/linux/mips/ldsodefs.h b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
>> index 8dde855..ce7b2f9 100644
>> --- a/sysdeps/unix/sysv/linux/mips/ldsodefs.h
>> +++ b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
>> @@ -34,7 +34,7 @@ extern void _dl_static_init (struct link_map *map);
>>   #undef VALID_ELF_ABIVERSION
>>   #define VALID_ELF_ABIVERSION(osabi,ver)			\
>>     (ver == 0						\
>> -   || (osabi == ELFOSABI_SYSV && ver < 4)		\
>> +   || (osabi == ELFOSABI_SYSV && ver < 6)		\
>>      || (osabi == ELFOSABI_GNU && ver < LIBC_ABI_MAX))
>>   
>>   #endif /* ldsodefs.h */
> I presume the version 6 is to make room for the PT_GNU_STACK.

Actually, the jump from 4 to 5 should have happened when support
for absolute symbols was added to binutils [4], but it didn't, so I had
to address that here.

Cheers,
Mihailo

[1] https://sourceware.org/ml/binutils/2019-07/msg00055.html
[2] https://sourceware.org/ml/binutils/2019-07/msg00215.html
[3] https://sourceware.org/ml/libc-alpha/2019-07/msg00526.html
[4] 
https://github.com/bminor/binutils-gdb/blob/bfa2a36d94d124eb7b54fd271a543047579b47ee/bfd/elfxx-mips.c#L16606
Joseph Myers July 24, 2019, 4:13 p.m. UTC | #4
On Tue, 23 Jul 2019, Mihailo Stojanović wrote:

> Binutils patch hasn't been pushed upstream, but it was reviewed in [1].
> We are currently waiting on FSF to process the binutils assignment for
> RT-RK [2].

Then we should await that before considering this patch for glibc.

In my view, nothing should be entered as a candidate on a release blocker 
list unless the changes to all other relevant packages are upstream in 
those components *before the start of the release freeze* (as well as the 
glibc patch itself being essentially ready before the release freeze).

> > I presume the version 6 is to make room for the PT_GNU_STACK.
> 
> Actually, the jump from 4 to 5 should have happened when support
> for absolute symbols was added to binutils [4], but it didn't, so I had
> to address that here.

Bug fixes should not be mixed with new features like this.

Rather, post a bug fix patch that updates the version for the absolute 
symbols changes *and adds a testcase to the glibc testsuite that fails 
without the patch* (if there's some reason it's hard to test, that needs 
to be adequately explained in the proposed commit message for the patch).  
Once that's gone through review and some version has been accepted, then a 
GNU hash patch building on top of it can be considered.

The fix relating to absolute symbols might be considered for backporting 
to whatever release branches have the relevant feature but without the 
update to ABI versions accepted, whereas GNU hash support is a new feature 
clearly not appropriate for such backporting.
diff mbox series

Patch

diff --git a/elf/dl-addr.c b/elf/dl-addr.c
index 9d285d7..f504e03 100644
--- a/elf/dl-addr.c
+++ b/elf/dl-addr.c
@@ -20,7 +20,6 @@ 
 #include <stddef.h>
 #include <ldsodefs.h>
 
-
 static inline void
 __attribute ((always_inline))
 determine_info (const ElfW(Addr) addr, struct link_map *match, Dl_info *info,
@@ -42,7 +41,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 +56,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 +65,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);
 	    }
diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c
index e3f42a1..d717ed3 100644
--- a/elf/dl-lookup.c
+++ b/elf/dl-lookup.c
@@ -403,7 +403,7 @@  do_lookup_x (const char *undef_name, uint_fast32_t new_hash,
 		  do
 		    if (((*hasharr ^ new_hash) >> 1) == 0)
 		      {
-			symidx = hasharr - map->l_gnu_chain_zero;
+			symidx = ELF_MACHINE_HASH_SYMIDX (map, hasharr);
 			sym = check_match (undef_name, ref, version, flags,
 					   type_class, &symtab[symidx], symidx,
 					   strtab, map, &versioned_sym,
@@ -937,10 +937,10 @@  _dl_setup_hash (struct link_map *map)
 {
   Elf_Symndx *hash;
 
-  if (__glibc_likely (map->l_info[ADDRIDX (DT_GNU_HASH)] != NULL))
+  if (__glibc_likely (map->l_info[ELF_MACHINE_GNU_HASH_ADDRIDX] != NULL))
     {
       Elf32_Word *hash32
-	= (void *) D_PTR (map, l_info[ADDRIDX (DT_GNU_HASH)]);
+	= (void *) D_PTR (map, l_info[ELF_MACHINE_GNU_HASH_ADDRIDX]);
       map->l_nbuckets = *hash32++;
       Elf32_Word symbias = *hash32++;
       Elf32_Word bitmask_nwords = *hash32++;
@@ -955,6 +955,12 @@  _dl_setup_hash (struct link_map *map)
       map->l_gnu_buckets = hash32;
       hash32 += map->l_nbuckets;
       map->l_gnu_chain_zero = hash32 - symbias;
+
+      /* Initialize MIPS xhash translation table.  */
+#ifdef ELF_MACHINE_XHASH_SETUP
+      ELF_MACHINE_XHASH_SETUP (hash32, symbias, map)
+#endif
+
       return;
     }
 
diff --git a/elf/elf.h b/elf/elf.h
index bb13c87..b53d815 100644
--- a/elf/elf.h
+++ b/elf/elf.h
@@ -1715,6 +1715,7 @@  typedef struct
 #define SHT_MIPS_EH_REGION	0x70000027
 #define SHT_MIPS_XLATE_OLD	0x70000028
 #define SHT_MIPS_PDR_EXCEPTION	0x70000029
+#define SHT_MIPS_XHASH		0x7000002b
 
 /* Legal values for sh_flags field of Elf32_Shdr.  */
 
@@ -1962,7 +1963,9 @@  typedef struct
    in a PIE as it stores a relative offset from the address of the tag
    rather than an absolute address.  */
 #define DT_MIPS_RLD_MAP_REL  0x70000035
-#define DT_MIPS_NUM	     0x36
+/* GNU-style hash table with xlat.  */
+#define DT_MIPS_XHASH	     0x70000036
+#define DT_MIPS_NUM	     0x37
 
 /* Legal values for DT_MIPS_FLAGS Elf32_Dyn entry.  */
 
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index b1fc5c3..e95d8cd 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -47,6 +47,17 @@  __BEGIN_DECLS
 #define ADDRIDX(tag)	(DT_NUM + DT_THISPROCNUM + DT_VERSIONTAGNUM \
 			 + DT_EXTRANUM + DT_VALNUM + DT_ADDRTAGIDX (tag))
 
+/* Type of GNU hash which the machine uses.  */
+#ifndef ELF_MACHINE_GNU_HASH_ADDRIDX
+# define ELF_MACHINE_GNU_HASH_ADDRIDX ADDRIDX (DT_GNU_HASH)
+#endif
+
+/* Calculate the index of a symbol in GNU_HASH.  */
+#ifndef ELF_MACHINE_HASH_SYMIDX
+# define ELF_MACHINE_HASH_SYMIDX(map, hasharr) \
+  ((hasharr) - (map)->l_gnu_chain_zero)
+#endif
+
 /* We use this macro to refer to ELF types independent of the native wordsize.
    `ElfW(TYPE)' is used in place of `Elf32_TYPE' or `Elf64_TYPE'.  */
 #define ELFW(type)	_ElfW (ELF, __ELF_NATIVE_CLASS, type)
diff --git a/sysdeps/mips/ldsodefs.h b/sysdeps/mips/ldsodefs.h
index f0acb02..0c6f027 100644
--- a/sysdeps/mips/ldsodefs.h
+++ b/sysdeps/mips/ldsodefs.h
@@ -26,6 +26,17 @@  struct La_mips_32_retval;
 struct La_mips_64_regs;
 struct La_mips_64_retval;
 
+#define ELF_MACHINE_GNU_HASH_ADDRIDX (DT_MIPS_XHASH - DT_LOPROC + DT_NUM)
+
+/* Calculate the index of a symbol in MIPS_XHASH.  */
+#define ELF_MACHINE_HASH_SYMIDX(map, hasharr) \
+  ((map)->l_mach.mips_xlat_zero[(hasharr) - (map)->l_gnu_chain_zero])
+
+/* Setup MIPS_XHASH.  */
+#define ELF_MACHINE_XHASH_SETUP(hash32, symbias, map) \
+  (hash32) += (map)->l_info[DT_MIPS (SYMTABNO)]->d_un.d_val - (symbias); \
+  (map)->l_mach.mips_xlat_zero = (hash32) - (symbias);
+
 #define ARCH_PLTENTER_MEMBERS						    \
     Elf32_Addr (*mips_o32_gnu_pltenter) (Elf32_Sym *, unsigned int,	    \
 					 uintptr_t *, uintptr_t *,	    \
diff --git a/sysdeps/mips/linkmap.h b/sysdeps/mips/linkmap.h
index 1fb9678..1e640c3 100644
--- a/sysdeps/mips/linkmap.h
+++ b/sysdeps/mips/linkmap.h
@@ -3,4 +3,5 @@  struct link_map_machine
     ElfW(Addr) plt; /* Address of .plt */
     ElfW(Word) fpabi; /* FP ABI of the object */
     unsigned int odd_spreg; /* Does the object require odd_spreg support? */
+    const Elf32_Word *mips_xlat_zero; /* .MIPS.xhash */
   };
diff --git a/sysdeps/unix/sysv/linux/mips/ldsodefs.h b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
index 8dde855..ce7b2f9 100644
--- a/sysdeps/unix/sysv/linux/mips/ldsodefs.h
+++ b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
@@ -34,7 +34,7 @@  extern void _dl_static_init (struct link_map *map);
 #undef VALID_ELF_ABIVERSION
 #define VALID_ELF_ABIVERSION(osabi,ver)			\
   (ver == 0						\
-   || (osabi == ELFOSABI_SYSV && ver < 4)		\
+   || (osabi == ELFOSABI_SYSV && ver < 6)		\
    || (osabi == ELFOSABI_GNU && ver < LIBC_ABI_MAX))
 
 #endif /* ldsodefs.h */
diff --git a/sysdeps/unix/sysv/linux/mips/libc-abis b/sysdeps/unix/sysv/linux/mips/libc-abis
index eaea558..83a667b 100644
--- a/sysdeps/unix/sysv/linux/mips/libc-abis
+++ b/sysdeps/unix/sysv/linux/mips/libc-abis
@@ -16,3 +16,6 @@  UNIQUE
 MIPS_O32_FP64   mips*-*-linux*
 # Absolute (SHN_ABS) symbols working correctly.
 ABSOLUTE
+# GNU-style hash table with translation table.
+MIPS_XHASH
+