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

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

On 04/06/2019 18:17, Szabolcs Nagy wrote:
> On 04/06/2019 12:39, Szabolcs Nagy wrote:
>> On 23/05/2019 13:31, Szabolcs Nagy wrote:
>>> 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.
>>
>> to expand a bit: i want old dynamic linkers to
>> continue to work if they do not use lazy binding.
> 
> Rich Felker commented that it's possible to force bind now
> semantics for marked symbols in the static linker.
> PLT entries may still be needed (e.g. in the main exe for
> pointer equality), but those can use R_*_GLOB_DAT relocs
> and appear outside of the normal PLT entries that are
> associated with R_*_JUMP_SLOT relocs.
> 
> then no dynamic linker change is needed, but lazy binding
> (and ld_audit) cannot be supported later for vector calls.
> 
> no dynamic linker change sounds preferable to me, but
> i have to think about the details.

i think this approach (forcing marked symbols to bind now
without dynamic linker change) might work.

it's similar to what i described in
https://groups.google.com/d/msg/generic-abi/Bfb2CwX-u4M/2qbM5fDbDQAJ

i dropped that idea because it required two dynamic relocs,
one of which is unused (and the interaction with local ifuncs
was a bit messy).

but the same can be done with one dyn reloc if marked plt
entries are sorted to the end of the plt section (which
requires another pass over the symbols in the static linker,
but i think that's fine).

what i can do is revert the binutils patch, write a new
abi draft which uses similar symbol marking, but require
static linkers not to emit lazy JUMP_SLOT relocs (i.e. in
DT_JMPREL) for marked symbols.

then a new binary would work with an old dynamic linker
(and an old toolchain can build libmvec.so other than the
symbol markings, but those are not critical for interop).

if this plan fails i will go back to the previous plan.