This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] Add IFUNC support for MIPS (v3)


Faraz Shahbazker <faraz.shahbazker@imgtec.com> writes:
> -  /* got[0] is reserved. got[1] is also reserved for the dynamic object	\
> -     generated by gnu ld. Skip these reserved entries from		\
> -     relocation.  */							\
> -  i = (got[1] & ELF_MIPS_GNU_GOT1_MASK)? 2 : 1;				\
> +  if (__glibc_likely (map->l_info[DT_MIPS (GENERAL_GOTNO)] != NULL))	\
> +    i = map->l_info[DT_MIPS (GENERAL_GOTNO)]->d_un.d_val;		\
> +  else									\
> +    /* got[0] is reserved. got[1] is also reserved for the dynamic	\
> +       object generated by gnu ld. Skip these reserved entries from	\
> +       relocation.  */							\
> +    i = (got[1] & ELF_MIPS_GNU_GOT1_MASK)? 2 : 1;			\

I think we should just drop the likely/unlikely thing here.  Use in libc
will make "unlikely" incorrect, but many executables will probably still
not use ifuncs, so "likely" seems a stretch too.

> @@ -493,7 +497,8 @@ auto inline void
>  __attribute__ ((always_inline))
>  elf_machine_reloc (struct link_map *map, ElfW(Addr) r_info,
>  		   const ElfW(Sym) *sym, const struct r_found_version *version,
> -		   void *reloc_addr, ElfW(Addr) r_addend, int inplace_p)
> +		   void *reloc_addr, ElfW(Addr) r_addend, int inplace_p,
> +		   int skip_ifunc)
>  {
>    const unsigned long int r_type = ELFW(R_TYPE) (r_info);
>    ElfW(Addr) *addr_field = (ElfW(Addr) *) reloc_addr;
> @@ -579,21 +584,37 @@ elf_machine_reloc (struct link_map *map, ElfW(Addr) r_info,
>  
>  	    if ((ElfW(Word))symidx < gotsym)
>  	      {
> -		/* This wouldn't work for a symbol imported from other
> -		   libraries for which there's no GOT entry, but MIPS
> -		   requires every symbol referenced in a dynamic
> -		   relocation to have a GOT entry in the primary GOT,
> -		   so we only get here for locally-defined symbols.
> -		   For section symbols, we should *NOT* be adding
> -		   sym->st_value (per the definition of the meaning of
> -		   S in reloc expressions in the ELF64 MIPS ABI),
> -		   since it should have already been added to
> -		   reloc_value by the linker, but older versions of
> -		   GNU ld didn't add it, and newer versions don't emit
> -		   useless relocations to section symbols any more, so
> -		   it is safe to keep on adding sym->st_value, even
> -		   though it's not ABI compliant.  Some day we should
> -		   bite the bullet and stop doing this.  */
> +#ifndef RTLD_BOOTSTRAP
> +		/* Resolve IFUNC symbols with pre-emption.  */
> +		if (sym && __glibc_unlikely (ELFW(ST_TYPE) (sym->st_info)
> +					     == STT_GNU_IFUNC) && !skip_ifunc)

The idea of the DT_MIPS_GENERAL_GOTNO tag is to add a new region of the
GOT that is relocated normally.  So like I mentioned in my previous email,
I think we should handle all global symbols here if the tag is defined,
rather than restrict it to just ifuncs.

We should probably report an error in cases where we see a global symbol
and the tag isn't defined, rather than silently mislink the code.

Looked good to me otherwise FWIW (obviously modulo Andreas's comments).

Thanks,
Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]