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

[Faraz Shahbazker <faraz dot shahbazker at imgtec dot com>]

Hi Richard,

I am still confused :(
Lets back-up a bit to your earlier comment.

On 08/16/2015 12:17 PM, Richard Sandiford wrote:
> Faraz Shahbazker <faraz.shahbazker@imgtec.com> writes:
>> @@ -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 check whether the function is an IFUNC here because we are about to 
attempting preemption for a symbol which would otherwise not be preemptible,
as a consequence of (symidx < gotsym).

If I understand correctly, we want to attempt preemption for all global
symbols that use the explicit GOT region, not just for IFUNCs. However,
this (symidx < gotsym) case is a pre-existing code block and I see nothing that
precludes control from reaching here for another locally-defined global symbol
(which should not be preempted). We have no means to distinguish the
preemptible GENERAL_GOT symbols from the non-preemptible ones. The best we 
can do is check if (PLTGOT < reloc_offset < PLTGOT+GENERAL_GOTNO) in order
to decide which symbols we should attempt to preempt.

The caveat is that there could be other REL32 relocations for these same
symbols that modify a data section (eg: function pointer to an IFUNC).
The  condition (reloc_offset < PLTGOT + GENERAL_GOTNO) does not hold for 
such relocations, but we still need to attempt preemption. At best, 
we would end up with code like this, where the IFUNC condition is 
present, but not compulsory:

if (symidx < gotsym)
{
	const ElfW(Addr) *got = (const ElfW(Addr) *) D_PTR (map, l_info[DT_PLTGOT]);
	const ElfW(Word) general_gotno = 0;

	if (map->l_info[DT_MIPS (GENERAL_GOTNO)])
		general_gotno = const ElfW(Word)) map->l_info[DT_MIPS (GENERAL_GOTNO)]->d_un.d_val;

	if (general_gotno != 0 
		&& ((reloc_addr > got && reloc_addr < got + general_gotno)
			|| (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC)))
	{
		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;
	}
	else if (general_gotno == 0 && ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC))
		_dl_error_printf ("%s: Global IFUNC symbol `%s' has relocation but "
			"no explicitly relocated GOT entry.\n",
			RTLD_PROGNAME, strtab + sym->st_name);
	else /* pre-existing block for locally-defined global symbols */
#ifndef RTLD_BOOTSTRAP
		if (map != &GL(dl_rtld_map))
#endif
			reloc_value += sym->st_value + map->l_addr;
}

The only likeliness we should assign in this code is to the error condition 
being unlikely. Only the presence of GLOBAL_GOTNO is checked and no specific value/range 
check is imposed up on it.

Does that cover everything?

Regards,
Faraz Shahbazker