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

[Richard Sandiford <rdsandiford at googlemail dot com>]

Faraz Shahbazker <faraz.shahbazker@imgtec.com> writes:
> @@ -579,25 +584,31 @@ 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.  */

We still need the "For section symbols..." bit onwards (and the
associated code) for STB_LOCAL in the case where DT_MIPS_GENERAL_GOTNO
isn't defined.  STB_LOCAL symbols shouldn't get the error.

We should probably drop the last sentence though :-)  I don't see there's
anything to gain by dropping support for old objects.  We simply don't
get here for new objects, so changing the calculation wouldn't have
any significant benefit.

> +		/* Linker will not normally emit symbolic relocations for
> +		   locally-defined global symbols.  If we reach here, we must
> +		   be dealing with relocation to a global symbols which is
> +		   explicitly relocated via the general GOT region and
> +		   pre-emptible.  */

How about something like:

		/* The original MIPS ABI required every global symbol used
		   in a relocation to be in the global GOT.  We would then
		   only expect to get here for local symbols.

(this part is just a rewording of the first part of the "This wouldn't
work..." comment above)

		   This restriction is removed for objects that use
		   DT_MIPS_GENERAL_GOTNO, since newer relocations and
		   symbol types do not fit easily in the original ABI scheme.
		   Relocations against symbols below DT_MIPS_GOTSYM bind in
		   just the same way as relocations against symbols in the
		   global GOT; the only difference is that we are not able
		   to use the global GOT as a directly-indexed lookup cache.
		   Symbols below DT_MIPS_GOTSYM might be in the general GOT
		   region or might not have a GOT entry at all.  */

>  #ifndef RTLD_BOOTSTRAP
> -		if (map != &GL(dl_rtld_map))
> +		if (map->l_info[DT_MIPS (GENERAL_GOTNO)] == NULL)

I think an unlikely tag would be justified here (even though it isn't
when finding the start of the local GOT).  Only very old objects will
have local relocations against symbols other than 0.

> +		  {
> +		    const char *strtab;
> +		    strtab = (const void *) D_PTR (map, l_info[DT_STRTAB]);
> +
> +		    _dl_error_printf ("\
> +%s: Explicitly relocated symbol `%s' requires dynamic tag MIPS_GENERAL_GOTNO\n",
> +				      RTLD_PROGNAME, strtab + sym->st_name);
> +		  }
> +		else
> +		  {
> +		    struct link_map *rmap = RESOLVE_MAP (&sym, version, r_type);
> +		    if ((ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC))
> +		      reloc_value = elf_ifunc_invoke (sym->st_value
> +						      + rmap->l_addr);
> +		    else
> +		      reloc_value = sym->st_value + rmap->l_addr;

It might be worth having a comment at the start of this "else" along
the lines of:

		    /* The behavior for section symbols described above
		       is now so firmly established that it is explicitly
		       adopted by objects with DT_MIPS_GLOBAL_GOTNO.
		       We therefore don't have a special case for
		       section symbols.  */

Looks good to me otherwise from an ABI POV.  (Not an approval -- I can't
do that.)

Thanks for your patience with this.

Richard