Re: [PATCH] Fix runtime linker auditing on aarch64

[Szabolcs Nagy <szabolcs.nagy@arm.com>]

The 09/23/2020 08:10, Ben Coyote Woodard wrote:
> On Wed, Sep 23, 2020 at 13:34, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > The 09/22/2020 18:16, Ben Woodard via Libc-alpha wrote:
> > >   /* Return values for calls from PLT on AArch64.  */
> > >   typedef struct La_aarch64_retval
> > >   {
> > >  -  /* Up to two integer registers can be used for a return value.
> > > */
> > >  -  uint64_t lrv_xreg[2];
> > >  -  /* Up to four D registers can be used for a return value.  */
> > >  -  uint64_t lrv_dreg[4];
> > >  +  /* Up to eight integer registers and the indirect result
> > > location register
> > >  +     can be used for a return value.  */
> > >  +  uint64_t    lrv_xreg[9];
> > 
> > x8 is not preserved so recording it at function exit
> > is not useful. (on entry it points to where results
> > are stored but on exit it can be clobbered)
> 
> OK that was not clear to me reading the AAPCS. Do you want to ping you're
> colleagues the tech writers over at arm and see if they can tighten up the
> language a bit.

aapcs is now openly developed (on github)
so you can submit bug reports easily ;)

in this case section 6.1.1 does not say
if x8 is preserved or not, but 6.5 is quite
explicit i think:

https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#result-return

> > >  +  /* Up to eight V registers can be used for a return value.  */
> > >  +  __uint128_t lrv_vreg[8];
> > > 
> > >   } La_aarch64_retval;
> > >   __BEGIN_DECLS
> > 
> > note: i don't like to use non-standard types in
> > public apis (like __uint128_t), but we already
> > made this mistake in the linux sigcontext, so this
> > is probably ok.
> > 
> > (my preference normally is to use a standard type
> > e.g. long double or char[] with alignment attr,
> > but in practice __uint128_t is probably easier to
> > deal with)
> > 
> 
> I kind of prefer "long double" here as well. It is after all what it likely
> is. I'm not really attached to __uint128_t; the previous version of the
> interface had uint64_t and so when making the registers the correct size I
> changed it to uint128_t but that didn't compile and then when I grepped the
> source I found __uint128_t. It wasn't like I put a lot of thought into that
> decision.

hm, i think the common case is to inspect float
or double arguments in the v regs, so the
__uint128_t is probably easier for the user
to deal with. (and sigcontext etc already
uses that for save/restore of the registers
so it is better for interop and least surprise)

it's unfortunate that there is no standard
uint128_t type. (i think the arm way would
be to use "uint8x16_t" or similar type from
arm_neon.h but in practice that's harder
to use)

so keep this as is.

(but this reminds me that the current hooks
are broken for long double functions, not
just for functions taking neon vector args)