Arm's SVE PCS and LD_AUDIT support?

[Szabolcs Nagy]

On 03/02/2020 15:43, Carlos O'Donell wrote:
> On 2/3/20 6:35 AM, Szabolcs Nagy wrote:
>> On 01/02/2020 03:37, Carlos O'Donell wrote:
>>> One of the things I want to refactor is to move some of the LD_AUDIT
>>> support up a level inside the dynamic loader and have it depend
>>> less on the lazy-binding semantics.
>>>
>>> I want only la_pltenter() and la_pltexit() to be affected by the
>>> binding semantics, but today because lazy was the default, we aren't
>>> there yet.
>>
>> iirc currently any load time bound pltgot entries
>> will not go via plt hook of ldaudit because it
>> uses the same entry mechanism as lazy binding
>> (plt0 jumps to GOT[2]) so vpcs is not using ld
>> audit now.
> 
> I'm sorry, I don't quite understand this sentence, but I think you
> are saying:
> 
> - Currently the STO_AARCH64_VARIANT_PCS symbols cannot support
>   la_pltenter() and la_pltexit() because they do not call through
>   the loader's PLT hook.
> 
> I agree this is the current state.

yes, that's the current state and as far as i was
concerned this is not broken: ld audit still works,
just variant pcs symbols are not hooked.

i thought you might be planing something that makes
this behaviour broken (e.g. force all symbols through
a different hooking mechanism instead of GOT[2]).

>> will that change?
> 
> I'm asking _you_ the question if we can change things to support 
> LD_AUDIT and SVE PCS and _how_ we might change things.
> 
> I think you answer the _how_ below, by saying we could create an
> alternate hook to avoid looking up the symbol's type.

my idea was that the audit plt hooks were not very
reliable or widely used anyway so the current level
of support was a reasonable trade-off.

i think adding ld audit support for variant pcs is
doable and likely preferable to the current state,
but i would only spend significant abi design effort
on it if we expect that it will be used.

e.g. is it ok to just always save/restore the full
register state during ld audit (but only expose to
users a subset of the regs so the callback abi is
unchanged)?

the reason i wanted to avoid full save/restore during
lazy binding was to avoid stack usage issues, but
i'm less concerned about that in case of audit.
(should i be concerned? doesn't it currently use a
large amount of stack anyway?)

will ld audit work on '-z now' binaries?
will distros build binaries with -z now?
will users build tooling on top of ld audit?
do we plan to make compiler changes to make plt
calls more reliable?

i.e. i'm not sure what level of support i should
aim for and what to optimize for.

>>> Florian and I were wondering if we couldn't implement the following:
>>>
>>> - Leave PLT in place for SVE PCS but unused.
>>>
>>> - Enable full save-restore in plt enter/exit conservatively for
>>>   STO_AARCH64_VARIANT_PCS if LD_AUDIT is in use, possibly routing
>>>   those symbols to a different _dl_profile_fixup_full_save?
>>>
>>> Would that work? 
>>
>> i don't understand what is the new ld audit mechanism
>> for hooking into the plt if not GOT[1] & GOT[2].
> 
> You are asking for implementation details which I did not provide :-)
> 
> In dl-machine.h:elf_machine_lazy_rel() when we fully resolve the
> STO_AARCH64_VARIANT_PCS symbol, we would instead need to point the
> symbol at something new like a reserved GOT[3]/GOT[4].
> 
> So you define it like this:
> DT_PLTGOT = GOT[0]
> GOT[1] = link map
> GOT[2] = hook for all symbols
> GOT[3] = link map
> GOT[4] = hook for STO_AARCH64_VARIANT_PCS
> 
> This would be lower-cost to develop but similar to DT_AARCH64_VPCS_PLTGOT 
> and DT_AARCH64_VPCS_PLT.
> 
> For the sake of upgrades we want to use DT_AARCH64_VPCS_PLTGOT
> to indicate the availability of the feature and define that it points
> to DT_PLTGOT+2, and we use GOT[1]/GOT[2] as expected (really GOT[3]/GOT[4]).
> 
> This way old binaries keep working without LD_AUDIT, but new binaries
> can redirect VPCS symbols into the second hook.

i will have to refresh my memory about the details,
but something along these lines can work and can be
added to the psabi. (e.g. i'm not sure if the second
entry can be generic entry point ldso may use however
it likes to, or the semantics needs to be tied to
vpcs/audit, it may depend on what other tools like
debuggers do with got[1]/got[2])

>> when the vpcs abi was designed we were thinking about
>> adding a second entry point somewhere (e.g.
>> DT_AARCH64_VPCS_PLT and DT_AARCH64_VPCS_PLTGOT)
>> instead of using GOT[1] as PLTGOT initializer which
>> then jumps to GOT[2], variant_pcs symbols could
>> use a different entry point which can do whatever
>> it takes to make lazy binding work.
> 
> That would be a very robust design.
> 
> I think I'm suggesting a subset of this design.
> 
>> but it seemed a bit too much hassle for something
>> we don't really plan to use (bind now for vpcs is
>> good enough) and in principle the entry point can
>> handle variant_pcs and normal symbols differently,
>> it's just ugly because checking for the STO_*
>> symbol table flag at runtime has to happen in
>> asm since we don't know the pcs yet.
> 
> - If in lazy-binding mode.
>   - Setup GOT[1]/GOT[2] to point to ld hook.
>   - Normal lazily bound symbols go to the ld hook.
>   - STO_AARCH64_VARIANT_PCS are immediately bound for performance.
> 
> - If in non-lazy binding mode.
>   - Do nothing since we will relocate all PLT entries.
>   - All work done in dl-machine.h:elf_machine_lazy_rel()
> 
> - If in ld-audit mode.
>   - Setup GOT[1]/GOT[2] to point to ld hook.
>   - Setup GOT[3]/GOT[4] to point to full-save ld hook.
>   - Additionally relocate all STO_AARCH64_VARIANT_PCS to GOT[3]/GOT[4]
>   - If no la_pltenter or la_pltexit is requested for the symbol we could
>     finalize the relocation to the real symbol and avoid the full save
>     for the hook.

will ld-audit mode work for binaries built with -z now?
(what if somebody used -z now in order to guarantee there
is no hooking because a magic pcs is in use somewhere?)

> 
> Notes:
> - If you really wanted we could use the alternative hook to support
>   lazy binding of STO_AARCH64_VARIANT_PCS, but I wouldn't bother.
> - Florian and I discussed offloading the problem of VPCS detection to
>   the user by moving la_symbind() really early and let the user, who
>   knows the calls, return LA_SYMB_FULLSAVE (new flag) from la_symbind
>   for those functions that need a full save and restore. The down side
>   to this approach is silent corruption if you get this wrong. You could
>   invert the meaning and say LA_SYMB_NOFULLSAVE and use it to speed up
>   all the other symbols during auditing on aarch64. I'm warry of this
>   approach because we could do a better job with just a little bit more
>   design work.
> 
>> adding such second entry is still possible, or
>> the ld audit hook can do something ugly in asm.
> 
> I think a second entry would be preferable to doing this all in asm.

ok.

>> if you can distinguish normal and vpcs syms in
>> the hook then i see no problem doing ld audit,
>> but the struct where the register state is saved
>> need to be scalable (currently exposed to the
>> user in the plt callbacks).
> 
> We would have to do the following:
> 
> - the la_aarch64_gnu_pltenter hook must inspect the symbol and detect
>   if it is STO_AARCH64_VARIANT_PCS, and if so, then use a different
>   scalable definition of La_aarch64_vpcs_regs and La_aarch64_vpcs_retval.
> 
> - Keep the La_aarch64_vpcs_regs and La_aarch64_vpcs_retval compatible
>   with the existing regs and retval, and just extend them.

note that there are bugs in this area, so ld audit does
not work correctly for base pcs on aarch64 (i haven't
gotten around applying
https://sourceware.org/ml/libc-alpha/2018-08/msg00019.html
)

i will have to think about how to best deal with
La_aarch64_vpcs_* types (it may well be that the
current setup is unusable and then we can break abi).

> In summary:
> - You suggest an alternative with DT_AARCH64_VPCS_PLT and DT_AARCH64_VPCS_PLTGOT
>   to enable both lazy binding and ld audit.
> - I suggest a crude DT_AARCH64_VPCS_PLTGOT-only solution just for ld audit.
>   - Could be extended to support DT_AARCH64_VPCS_PLT by changing the value
>     stored in DT_AARCH64_VPCS_PLTGOT.
> 
> Either solution requires:
> - New La_aarch64_vpcs_regs, and La_aarch64_vpcs_retval.
>