Hi Szabolcs,
> -----Original Message-----
> From: Gcc-patches <gcc-patches-boun...@gcc.gnu.org> On Behalf Of
> Szabolcs Nagy
> Sent: 26 June 2020 15:49
> To: gcc-patches@gcc.gnu.org
> Cc: fwei...@redhat.com; Richard Earnshaw <richard.earns...@arm.com>;
> Daniel Kiss <daniel.k...@arm.com>
> Subject: Re: [PATCH 2/4] aarch64: fix __builtin_eh_return with pac-ret
> [PR94891]
>
> The 06/05/2020 17:51, Szabolcs Nagy wrote:
> > The handler argument must not be signed since that may come from
> > outside the current module and exposing signed addresses is a pointer
> > ABI break. (The signed address also may not be representable as void *
> > which is why pac-ret is currently broken on ilp32.)
> >
> > There is no point protecting the eh return path with pointer auth
> > since arbitrary target can be reached with the instruction sequence
> > in the caller function anyway, however this is a big hammer solution
> > that turns off pac-ret for the caller completely not just on the eh
> > return path.
> >
> > 2020-06-04 Szabolcs Nagy <szabolcs.n...@arm.com>
> >
> > * config/aarch64/aarch64.c
> (aarch64_return_address_signing_enabled):
> > Disable return address signing if __builtin_eh_return is used.
>
> ping.
>
> this fixes a correctness bug in pac-ret, tested
> on aarch64, with only the following regressions:
>
> FAIL: gcc.target/aarch64/return_address_sign_1.c scan-assembler-times
> autiasp 4
> FAIL: gcc.target/aarch64/return_address_sign_1.c scan-assembler-times
> paciasp 4
> FAIL: gcc.target/aarch64/return_address_sign_b_1.c scan-assembler-times
> autibsp 4
> FAIL: gcc.target/aarch64/return_address_sign_b_1.c scan-assembler-times
> pacibsp 4
>
> which can be fixed by
>
> -/* { dg-final { scan-assembler-times "autiasp" 4 } } */
> -/* { dg-final { scan-assembler-times "paciasp" 4 } } */
> +/* { dg-final { scan-assembler-times "autiasp" 3 } } */
> +/* { dg-final { scan-assembler-times "paciasp" 3 } } */
>
> since __builtin_eh_return path no longer uses pac/aut.
This is ok but...
>
> > ---
> > gcc/config/aarch64/aarch64.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> > index 6a2f85c4af7..d9557f7c0a2 100644
> > --- a/gcc/config/aarch64/aarch64.c
> > +++ b/gcc/config/aarch64/aarch64.c
> > @@ -6954,6 +6954,10 @@ aarch64_return_address_signing_enabled (void)
> > /* This function should only be called after frame laid out. */
> > gcc_assert (cfun->machine->frame.laid_out);
> >
> > + /* TODO: Big hammer handling of __builtin_eh_return. */
... I don't think this comment is very useful. Please make it a bit more
descriptive. If you want to leave the TODO here, please give a more concrete
action plan.
Thanks,
Kyrill
> > + if (crtl->calls_eh_return)
> > + return false;
> > +
> > /* If signing scope is AARCH64_FUNCTION_NON_LEAF, we only sign a leaf
> function
> > if its LR is pushed onto stack. */
> > return (aarch64_ra_sign_scope == AARCH64_FUNCTION_ALL
> > --
> > 2.17.1
> >
>
> --
