Hi Ramana,
> -----Original Message-----
> From: Ramana Radhakrishnan <ramana....@googlemail.com>
> Sent: Sunday, November 20, 2022 10:48 PM
> To: Srinath Parvathaneni <srinath.parvathan...@arm.com>
> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> <richard.earns...@arm.com>; Kyrylo Tkachov <kyrylo.tkac...@arm.com>
> Subject: Re: [PATCH][GCC] arm: Add support for new frame unwinding
> instruction "0xb5".
>
> On Fri, Nov 18, 2022 at 9:33 AM Srinath Parvathaneni
> <srinath.parvathan...@arm.com> wrote:
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: Ramana Radhakrishnan <ramana....@googlemail.com>
> > > Sent: Thursday, November 17, 2022 8:27 PM
> > > To: Srinath Parvathaneni <srinath.parvathan...@arm.com>
> > > Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> > > <richard.earns...@arm.com>; Kyrylo Tkachov
> <kyrylo.tkac...@arm.com>
> > > Subject: Re: [PATCH][GCC] arm: Add support for new frame unwinding
> > > instruction "0xb5".
> > >
> > > On Thu, Nov 10, 2022 at 10:38 AM Srinath Parvathaneni via
> > > Gcc-patches <gcc- patc...@gcc.gnu.org> wrote:
> > > >
> > > > Hi,
> > > >
> > > > This patch adds support for Arm frame unwinding instruction "0xb5"
> > > > [1]. When an exception is taken and "0xb5" instruction is
> > > > encounter during runtime stack-unwinding, we use effective vsp as
> > > > modifier in pointer
> > > authentication.
> > > > On completion of stack unwinding if "0xb5" instruction is not
> > > > encountered then CFA will be used as modifier in pointer
> authentication.
> > > >
> > > > [1]
> > > > https://github.com/ARM-software/abi-
> > > aa/releases/download/2022Q3/ehabi3
> > > > 2.pdf
> > > >
> > > > Regression tested on arm-none-eabi target and found no regressions.
> > > >
> > > > Ok for master?
> > > >
> > >
> > > No, not yet.
> > >
> > > Presumably the logic to produce 0xb5 is in the source base and this
> > > was tested with suitable options that produce said opcode ? I see no
> > > logic in place to produce the said opcode in the backend in a quick
> > > read as the pacbti patches still seem to be in review. ?
> > >
> > > So what was the test suite run actually testing ?
> >
> > Sorry for the late response, the patch supporting the said opcode (directive
> ".pacspval)" is here:
> > https://gcc.gnu.org/pipermail/gcc-patches/2022-November/605524.html
> > (still under upstream review)
> >
> > and the patch to encode ".pacspval" with the mentioned opcode "0xb5" in
> binutils is here:
> > https://sourceware.org/pipermail/binutils/2022-November/124328.html
> (approved and committed to binutils).
>
> Thanks for the answer but perhaps I should make my question more explicit
> - are you saying that this patch was tested in combination with those and
> other dependent patches on a suitable simulator with suitable multilibs and
> C++ to test for this presumably for frame unwinding ?
>
Sorry for the late response, I'm re-spinning other pacbti patches on top of
which this
patch needs to be applied, so I could not respond to you.
I have applied this patch on top of all the pacbti and related multilib patches,
the patch applies cleanly, and the toolchain build is successful.
I have tested this patch with C testcase with nested function (which emits
.pacspval
directive in case of clobber IP) on a simulator which supports PACBTI and
executed the
binary successfully.
But I'm unable to test this patch for C++ frame unwinding for this opcode
because C++
doesn't support nested functions and with current pacbti code IP register is
clobbered
and we emit .pacspval directive only for nested function.
> For the future , it would certainly be worth being explicit about this in your
> patch submission :)
Thank you, I will keep this is in mind for my later patch submissions.
Regards,
Srinath.
> regards
> Ramana
>
> >
> > Regards,
> > Srinath.
> >
> > > regards
> > > Ramana
> > >
> > >
> > > > Regards,
> > > > Srinath.
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > > 2022-11-09 Srinath Parvathaneni <srinath.parvathan...@arm.com>
> > > >
> > > > * libgcc/config/arm/pr-support.c (__gnu_unwind_execute):
> > > > Decode
> > > opcode
> > > > "0xb5".
> > > >
> > > >
> > > > ############### Attachment also inlined for ease of reply
> > > ###############
> > > >
> > > >
> > > > diff --git a/libgcc/config/arm/pr-support.c
> > > > b/libgcc/config/arm/pr-support.c index
> > > >
> > >
> e48854587c667a959aa66ccc4982231f63333ecc..73e4942a39b34a83c2da85de
> > > f6b1
> > > > 3e82ec501552 100644
> > > > --- a/libgcc/config/arm/pr-support.c
> > > > +++ b/libgcc/config/arm/pr-support.c
> > > > @@ -107,7 +107,9 @@ __gnu_unwind_execute (_Unwind_Context *
> > > context, __gnu_unwind_state * uws)
> > > > _uw op;
> > > > int set_pc;
> > > > int set_pac = 0;
> > > > + int set_pac_sp = 0;
> > > > _uw reg;
> > > > + _uw sp;
> > > >
> > > > set_pc = 0;
> > > > for (;;)
> > > > @@ -124,10 +126,11 @@ __gnu_unwind_execute (_Unwind_Context *
> > > context,
> > > > __gnu_unwind_state * uws) #if defined(TARGET_HAVE_PACBTI)
> > > > if (set_pac)
> > > > {
> > > > - _uw sp;
> > > > _uw lr;
> > > > _uw pac;
> > > > - _Unwind_VRS_Get (context, _UVRSC_CORE, R_SP,
> > > _UVRSD_UINT32, &sp);
> > > > + if (!set_pac_sp)
> > > > + _Unwind_VRS_Get (context, _UVRSC_CORE, R_SP,
> > > _UVRSD_UINT32,
> > > > + &sp);
> > > > _Unwind_VRS_Get (context, _UVRSC_CORE, R_LR,
> > > > _UVRSD_UINT32,
> > > &lr);
> > > > _Unwind_VRS_Get (context, _UVRSC_PAC, R_IP,
> > > > _UVRSD_UINT32, &pac); @@ -259,7
> > > > +262,19 @@ __gnu_unwind_execute (_Unwind_Context * context,
> > > __gnu_unwind_state * uws)
> > > > continue;
> > > > }
> > > >
> > > > - if ((op & 0xfc) == 0xb4) /* Obsolete FPA. */
> > > > + /* Use current VSP as modifier in PAC validation. */
> > > > + if (op == 0xb5)
> > > > + {
> > > > + if (set_pac)
> > > > + _Unwind_VRS_Get (context, _UVRSC_CORE, R_SP,
> > > _UVRSD_UINT32,
> > > > + &sp);
> > > > + else
> > > > + return _URC_FAILURE;
> > > > + set_pac_sp = 1;
> > > > + continue;
> > > > + }
> > > > +
> > > > + if ((op & 0xfd) == 0xb6) /* Obsolete FPA. */
> > > > return _URC_FAILURE;
> > > >
> > > > /* op & 0xf8 == 0xb8. */
> > > >
> > > >
> > > >
