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 ? For the future , it would certainly be worth being explicit about this in your patch submission :) 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. */ > > > > > > > > >