Re: [PATCH glibc 2/2] aarch64: handle STO_AARCH64_VARIANT_PCS

[Szabolcs Nagy <Szabolcs dot Nagy at arm dot com>]

On 23/05/2019 12:35, Florian Weimer wrote:
> * Szabolcs Nagy:
> 
>>    /* Check for unexpected PLT reloc type.  */
>>    if (__builtin_expect (r_type == AARCH64_R(JUMP_SLOT), 1))
>>      {
>> -      if (__builtin_expect (map->l_mach.plt, 0) == 0)
>> -	*reloc_addr += l_addr;
>> -      else
>> -	*reloc_addr = map->l_mach.plt;
>> +      if (map->l_mach.plt == 0)
>> +	{
>> +	  /* Prelinking.  */
>> +	  *reloc_addr += l_addr;
>> +	  return;
>> +	}
>> +
>> +      if (__glibc_unlikely (map->l_mach.variant_pcs))
>> +	{
>> +	  /* Check the symbol table for variant PCS symbols.  */
>> +	  const Elf_Symndx symndx = ELFW (R_SYM) (reloc->r_info);
>> +	  const ElfW (Sym) *symtab =
>> +	    (const void *)D_PTR (map, l_info[DT_SYMTAB]);
>> +	  const ElfW (Sym) *sym = &symtab[symndx];
>> +	  if (__glibc_unlikely (sym->st_other & STO_AARCH64_VARIANT_PCS))
>> +	    {
>> +	      /* Avoid lazy resolution of variant PCS symbols.  */
>> +	      const struct r_found_version *version = NULL;
>> +	      if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL)
>> +		{
>> +		  const ElfW (Half) *vernum =
>> +		    (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]);
>> +		  version = &map->l_versions[vernum[symndx] & 0x7fff];
>> +		}
>> +	      elf_machine_rela (map, reloc, sym, version, reloc_addr,
>> +				skip_ifunc);
>> +	      return;
>> +	    }
>> +	}
>> +
>> +      *reloc_addr = map->l_mach.plt;
> 
> Looking at this, I wonder if it would be more natural to handle this as
> a new relocation type.  This way, you will get an error from old dynamic
> linkers.

yes, but that's a more intrusive change.

R_*_JUMP_SLOT is handled specially at various places
which require changes if we introduce a new dynamic
reloc.

the symbol marking is still required: static linker
needs to know which symbols are special (could be
done with static relocations, but then almost all
relocations would need a 'variant pcs' alternative
which is a lot of changes in static linkers)

so it seemed that DT_ tag + st_other marking is
minimally intrusive and sufficient, but i agree
that and R_* dynamic reloc + st_other marking
would be another feasible option (i didnt prototype
that so i cant tell how much more work that is).