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


Thanks for the patch.

Faraz Shahbazker <Faraz.Shahbazker@imgtec.com> writes:
> @@ -200,10 +201,13 @@ do {									\
>    if (__builtin_expect (map->l_addr == 0, 1))				\
>      break;								\
>  									\
> -  /* 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_unlikely (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;			\
>    n = map->l_info[DT_MIPS (LOCAL_GOTNO)]->d_un.d_val;			\

I'm not sure we should consider the new tag to be unlikely.  If the
intention is to use ifuncs in libc then it would eventually become
pretty common.

> +		/* Symbol pre-empted, use value from GOT.
> +		 2nd part of condition is redundant, explicit for clarity.  */
> +		if (__glibc_unlikely (rmap->l_relocated)
> +		    && __glibc_unlikely (rmap != map))
> +		  {
> +		    const ElfW(Addr) *got
> +		      = (const ElfW(Addr) *) D_PTR (rmap, l_info[DT_PLTGOT]);
> +		    const ElfW(Word) local_gotno
> +		      = (const ElfW(Word))
> +		      rmap->l_info[DT_MIPS (LOCAL_GOTNO)]->d_un.d_val;
> +		    symidx = (sym
> +			      - (ElfW(Sym) *) D_PTR(rmap, l_info[DT_SYMTAB]))
> +		      / sizeof (sym);
> +		    reloc_value = got[symidx + local_gotno - gotsym];
> +		  }
> +		/* Symbol pre-empted by not yet relocated, use best guess.  */
> +		else if (__glibc_unlikely (rmap != map))
> +		  reloc_value = sym->st_value + rmap->l_addr;
> +		  /* Resolve IFUNC in this link unit.  */
> +		else if (__glibc_likely (ELFW(ST_TYPE) (sym->st_info)
> +					 == STT_GNU_IFUNC))
> +		  reloc_value = elf_ifunc_invoke (sym->st_value + map->l_addr);
> +		else
> +		  reloc_value += sym->st_value + map->l_addr;
> +	      }
> +#endif
>  	    else
>  	      {
>  #ifndef RTLD_BOOTSTRAP

Does this mean that an STT_GNU_IFUNC can have two GOT entries,
one in the new explicitly-relocated region and one in the normal
global GOT?  The way I'd imagined it working is that STT_GNU_IFUNC
would be an exception to the usual ABI rule that every symbol involved
in a relocation needs a GOT entry.  Instead STT_GNU_IFUNCs would be
kept below the DT_MIPS_GOTSYM threshold and the:

	    if ((ElfW(Word))symidx < gotsym)
	      {
	      }

block would then handle global symbols too.  The IFUNC handling would be
very similar to other targets.

I suppose one downside of doing that for all global symbols is that we
would then silently accept broken objects that older ld.sos wouldn't.
If we really want to guard against that, we could raise a fatal error
if a global symbol below gotsym is seen in a relocation and if
DT_MIPS_GENERAL_GOTNO isn't set.  The new behaviour would then be
restricted to objects that use the new GOT layout.

> @@ -795,8 +844,18 @@ elf_machine_got_rel (struct link_map *map, int lazy)
>        const struct r_found_version *version __attribute__ ((unused))	  \
>  	= vernum ? &map->l_versions[vernum[sym_index] & 0x7fff] : NULL;	  \
>        struct link_map *sym_map;						  \
> +      ElfW(Addr) value = 0;						  \
>        sym_map = RESOLVE_MAP (&ref, version, reloc);			  \
> -      ref ? sym_map->l_addr + ref->st_value : 0;			  \
> +      if (__glibc_likely(ref != NULL))					  \
> +	{								  \
> +	  value = sym_map->l_addr + ref->st_value;			  \
> +	  if (__glibc_unlikely (ELFW(ST_TYPE) (ref->st_info)		  \
> +				== STT_GNU_IFUNC			  \
> +				&& sym_map != map			  \
> +				&& sym_map->l_relocated))		  \
> +	      value = elf_ifunc_invoke (value);				  \
> +	}								  \
> +      value;								  \
>      })

With that change, I think we should either:

(a) make STT_GNU_IFUNCs in the global GOT be a fatal error or

(b) call the resolver unconditionally, without the sym_map-based checks.
    We shouldn't resolve an IFUNC without calling the resolver.

It's a question of whether it's more defensive to support IFUNCs in the
ABI global GOT in the obvious way (b), in case we find a use for it later,
or whether it's more defensive to rule it out until we know it has
a use case (a).

Anything that did want to put IFUNCs in the ABI global GOT would need
to be sure that the resolver doesn't rely on unrelocated data.  But in
cases where that holds, traditional global entries might be (slightly)
more efficient.

Thanks,
Richard


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