* [PATCH][GCC] arm: Add support for new frame unwinding instruction "0xb5".
@ 2022-11-10 10:37 Srinath Parvathaneni
2022-11-17 20:27 ` Ramana Radhakrishnan
2023-01-20 16:59 ` Richard Earnshaw
0 siblings, 2 replies; 6+ messages in thread
From: Srinath Parvathaneni @ 2022-11-10 10:37 UTC (permalink / raw)
To: gcc-patches; +Cc: richard.earnshaw, kyrylo.tkachov
[-- Attachment #1: Type: text/plain, Size: 2367 bytes --]
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/ehabi32.pdf
Regression tested on arm-none-eabi target and found no regressions.
Ok for master?
Regards,
Srinath.
gcc/ChangeLog:
2022-11-09 Srinath Parvathaneni <srinath.parvathaneni@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..73e4942a39b34a83c2da85def6b13e82ec501552 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. */
[-- Attachment #2: rb16528.patch --]
[-- Type: text/plain, Size: 1559 bytes --]
diff --git a/libgcc/config/arm/pr-support.c b/libgcc/config/arm/pr-support.c
index e48854587c667a959aa66ccc4982231f63333ecc..73e4942a39b34a83c2da85def6b13e82ec501552 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. */
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH][GCC] arm: Add support for new frame unwinding instruction "0xb5".
2022-11-10 10:37 [PATCH][GCC] arm: Add support for new frame unwinding instruction "0xb5" Srinath Parvathaneni
@ 2022-11-17 20:27 ` Ramana Radhakrishnan
2022-11-18 9:33 ` Srinath Parvathaneni
2023-01-20 16:59 ` Richard Earnshaw
1 sibling, 1 reply; 6+ messages in thread
From: Ramana Radhakrishnan @ 2022-11-17 20:27 UTC (permalink / raw)
To: Srinath Parvathaneni; +Cc: gcc-patches, richard.earnshaw, kyrylo.tkachov
On Thu, Nov 10, 2022 at 10:38 AM Srinath Parvathaneni via Gcc-patches
<gcc-patches@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/ehabi32.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 ?
regards
Ramana
> Regards,
> Srinath.
>
> gcc/ChangeLog:
>
> 2022-11-09 Srinath Parvathaneni <srinath.parvathaneni@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..73e4942a39b34a83c2da85def6b13e82ec501552 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. */
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH][GCC] arm: Add support for new frame unwinding instruction "0xb5".
2022-11-17 20:27 ` Ramana Radhakrishnan
@ 2022-11-18 9:33 ` Srinath Parvathaneni
2022-11-20 22:48 ` Ramana Radhakrishnan
0 siblings, 1 reply; 6+ messages in thread
From: Srinath Parvathaneni @ 2022-11-18 9:33 UTC (permalink / raw)
To: Ramana Radhakrishnan; +Cc: gcc-patches, Richard Earnshaw, Kyrylo Tkachov
Hi,
> -----Original Message-----
> From: Ramana Radhakrishnan <ramana.gcc@googlemail.com>
> Sent: Thursday, November 17, 2022 8:27 PM
> To: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>
> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@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-
> patches@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).
Regards,
Srinath.
> regards
> Ramana
>
>
> > Regards,
> > Srinath.
> >
> > gcc/ChangeLog:
> >
> > 2022-11-09 Srinath Parvathaneni <srinath.parvathaneni@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. */
> >
> >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH][GCC] arm: Add support for new frame unwinding instruction "0xb5".
2022-11-18 9:33 ` Srinath Parvathaneni
@ 2022-11-20 22:48 ` Ramana Radhakrishnan
2023-01-18 17:56 ` Srinath Parvathaneni
0 siblings, 1 reply; 6+ messages in thread
From: Ramana Radhakrishnan @ 2022-11-20 22:48 UTC (permalink / raw)
To: Srinath Parvathaneni; +Cc: gcc-patches, Richard Earnshaw, Kyrylo Tkachov
On Fri, Nov 18, 2022 at 9:33 AM Srinath Parvathaneni
<Srinath.Parvathaneni@arm.com> wrote:
>
> Hi,
>
> > -----Original Message-----
> > From: Ramana Radhakrishnan <ramana.gcc@googlemail.com>
> > Sent: Thursday, November 17, 2022 8:27 PM
> > To: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>
> > Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> > <Richard.Earnshaw@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@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-
> > patches@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.parvathaneni@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. */
> > >
> > >
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH][GCC] arm: Add support for new frame unwinding instruction "0xb5".
2022-11-20 22:48 ` Ramana Radhakrishnan
@ 2023-01-18 17:56 ` Srinath Parvathaneni
0 siblings, 0 replies; 6+ messages in thread
From: Srinath Parvathaneni @ 2023-01-18 17:56 UTC (permalink / raw)
To: Ramana Radhakrishnan; +Cc: gcc Patches, Richard Earnshaw, Kyrylo Tkachov
Hi Ramana,
> -----Original Message-----
> From: Ramana Radhakrishnan <ramana.gcc@googlemail.com>
> Sent: Sunday, November 20, 2022 10:48 PM
> To: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>
> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@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.Parvathaneni@arm.com> wrote:
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: Ramana Radhakrishnan <ramana.gcc@googlemail.com>
> > > Sent: Thursday, November 17, 2022 8:27 PM
> > > To: Srinath Parvathaneni <Srinath.Parvathaneni@arm.com>
> > > Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> > > <Richard.Earnshaw@arm.com>; Kyrylo Tkachov
> <Kyrylo.Tkachov@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- patches@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.parvathaneni@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. */
> > > >
> > > >
> > > >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][GCC] arm: Add support for new frame unwinding instruction "0xb5".
2022-11-10 10:37 [PATCH][GCC] arm: Add support for new frame unwinding instruction "0xb5" Srinath Parvathaneni
2022-11-17 20:27 ` Ramana Radhakrishnan
@ 2023-01-20 16:59 ` Richard Earnshaw
1 sibling, 0 replies; 6+ messages in thread
From: Richard Earnshaw @ 2023-01-20 16:59 UTC (permalink / raw)
To: Srinath Parvathaneni, gcc-patches; +Cc: richard.earnshaw, kyrylo.tkachov
On 10/11/2022 10:37, Srinath Parvathaneni via Gcc-patches 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/ehabi32.pdf
>
> Regression tested on arm-none-eabi target and found no regressions.
>
> Ok for master?
>
> Regards,
> Srinath.
>
> gcc/ChangeLog:
>
> 2022-11-09 Srinath Parvathaneni <srinath.parvathaneni@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..73e4942a39b34a83c2da85def6b13e82ec501552 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;
I don't think you need to worry about the case when set_pac is false; in
fact, I don't think you need to even test set_pac here. It's harmless
if this opcode appears and then we never do the authentication, so just
record the SP value at this point.
> + set_pac_sp = 1;
> + continue;
> + }
> +
> + if ((op & 0xfd) == 0xb6) /* Obsolete FPA. */
No, this is logically impossible (0xfd is binary 1111_1101, while 0xb6
is binary 1011_110 and thus bit 2 will never be set after the mask).
But you don't need to change the condition here at all, because we've
already taken out the case you're worried about immediately above (and
ended that block with a 'continue').
> return _URC_FAILURE;
> > /* op & 0xf8 == 0xb8. */
>
>
>
R.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-20 16:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-10 10:37 [PATCH][GCC] arm: Add support for new frame unwinding instruction "0xb5" Srinath Parvathaneni
2022-11-17 20:27 ` Ramana Radhakrishnan
2022-11-18 9:33 ` Srinath Parvathaneni
2022-11-20 22:48 ` Ramana Radhakrishnan
2023-01-18 17:56 ` Srinath Parvathaneni
2023-01-20 16:59 ` Richard Earnshaw
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).