Re: [PATCH] aarch64, libgcc: Fix unwinding from pac-ret to normal frames [PR94514]
The 04/22/2020 15:22, Christophe Lyon wrote:
> The new test fails with ilp32, not sure if that's supposed to work?
>
> FAIL: gcc.target/aarch64/pr94514.c (test for excess errors)
> Excess errors:
> /gcc/testsuite/gcc.target/aarch64/pr94514.c:27:4: warning: cast to
> pointer from integer of different size [-Wint-to-pointer-cast]
>
> spawn
> /aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/aarch64-none-elf/invoke-foundation-v8-bare-metal.sh
> ./pr94514.exe
> force_unwind_stop: CFA: 0xef80 PC: 0x80001304 actions: 10
> force_unwind_stop: CFA: 0xef90 PC: 0x8000133c actions: 10
> Terminated by exception.
>
> *** EXIT code 126
> gcc.target/aarch64/pr94514.c execution test (reason: TCL LOOKUP CHANNEL exp7)
> FAIL: gcc.target/aarch64/pr94514.c execution test
>
> (executed using the Foundation Model)
>
>
> The C++ test compiles without warnings, but fails at execution too
...
> Maybe you just want to skip the test for ilp32?
i didn't test ilp32, i would expect a compile time error for
__attribute__((target("branch-protection=pac-ret")))
on ilp32, or just ignoring it (which would have worked),
runtime error for this on a pac-enabled system is nasty.
i disabled the test on ilp32 as an obvious fix (attached),
and raised PR94729 for the attribute handling in ilp32.
thanks for catching this.
>From 744b3e4478df83f54543964b8eb7250eb9bb6d40 Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy
Date: Thu, 23 Apr 2020 11:26:10 +0100
Subject: [PATCH] aarch64: disable tests on ilp32 [PR94514]
branch-protection=pac-ret is only supported with lp64 abi.
gcc/testsuite/ChangeLog:
PR target/94514
* g++.target/aarch64/pr94514.C: Require lp64.
* gcc.target/aarch64/pr94514.c: Likewise.
---
gcc/testsuite/ChangeLog| 6 ++
gcc/testsuite/g++.target/aarch64/pr94514.C | 1 +
gcc/testsuite/gcc.target/aarch64/pr94514.c | 1 +
3 files changed, 8 insertions(+)
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 245c1512c76..7e676f053a5 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2020-04-23 Szabolcs Nagy
+
+ PR target/94514
+ * g++.target/aarch64/pr94514.C: Require lp64.
+ * gcc.target/aarch64/pr94514.c: Likewise.
+
2020-04-23 Jakub Jelinek
PR target/94707
diff --git a/gcc/testsuite/g++.target/aarch64/pr94514.C b/gcc/testsuite/g++.target/aarch64/pr94514.C
index 2a8c949ba30..ae925cafeb6 100644
--- a/gcc/testsuite/g++.target/aarch64/pr94514.C
+++ b/gcc/testsuite/g++.target/aarch64/pr94514.C
@@ -1,5 +1,6 @@
/* PR target/94514. Unwind across mixed pac-ret and non-pac-ret frames. */
/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
__attribute__((noinline, target("branch-protection=pac-ret")))
static void do_throw (void)
diff --git a/gcc/testsuite/gcc.target/aarch64/pr94514.c b/gcc/testsuite/gcc.target/aarch64/pr94514.c
index bbbf5a6b0b3..cbc940421d2 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr94514.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr94514.c
@@ -1,5 +1,6 @@
/* PR target/94514. Unwind across mixed pac-ret and non-pac-ret frames. */
/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
/* { dg-options "-fexceptions -O2" } */
#include
--
2.17.1
Re: [PATCH] aarch64, libgcc: Fix unwinding from pac-ret to normal frames [PR94514]
Hi, On Tue, 21 Apr 2020 at 18:52, Szabolcs Nagy wrote: > > The 04/17/2020 11:05, Kyrylo Tkachov wrote: > > Hi Szabolcs, > > > > > -Original Message- > > > From: Szabolcs Nagy > > > Sent: 09 April 2020 15:20 > > > To: gcc-patches@gcc.gnu.org > > > Cc: Richard Earnshaw ; Richard Sandiford > > > ; Kyrylo Tkachov > > > Subject: [PATCH] aarch64, libgcc: Fix unwinding from pac-ret to normal > > > frames [PR94514] > > > > > > With -mbranch-protection=pac-ret the debug info toggles the > > > signedness state of the return address so the unwinder knows when > > > the return address needs pointer authentication. > > > > > > The unwind context flags were not updated according to the dwarf > > > frame info. > > > > > > This causes unwinding across frames that were built without pac-ret > > > to incorrectly authenticate the return address wich corrupts the > > > return address on a system where PAuth is enabled. > > > > > > Note: This even affects systems where all code use pac-ret because > > > unwinding across a signal frame the return address is not signed. > > > > > > > Ok, I'm guessing this needs backporting? > > committed now, > > yes i think it has to go back to gcc-9 and gcc-8, > i will do that later. thanks. The new test fails with ilp32, not sure if that's supposed to work? FAIL: gcc.target/aarch64/pr94514.c (test for excess errors) Excess errors: /gcc/testsuite/gcc.target/aarch64/pr94514.c:27:4: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] spawn /aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/aarch64-none-elf/invoke-foundation-v8-bare-metal.sh ./pr94514.exe force_unwind_stop: CFA: 0xef80 PC: 0x80001304 actions: 10 force_unwind_stop: CFA: 0xef90 PC: 0x8000133c actions: 10 Terminated by exception. *** EXIT code 126 gcc.target/aarch64/pr94514.c execution test (reason: TCL LOOKUP CHANNEL exp7) FAIL: gcc.target/aarch64/pr94514.c execution test (executed using the Foundation Model) The C++ test compiles without warnings, but fails at execution too (without the force_unwind_stop traces): PASS: g++.target/aarch64/pr94514.C (test for excess errors) spawn /aci-gcc-fsf/builds/gcc-fsf-gccsrc/tools/aarch64-none-elf/invoke-foundation-v8-bare-metal.sh ./pr94514.exe Terminated by exception. *** EXIT code 126 g++.target/aarch64/pr94514.C execution test (reason: TCL LOOKUP CHANNEL exp7) FAIL: g++.target/aarch64/pr94514.C execution test Maybe you just want to skip the test for ilp32? Thanks, Christophe
Re: [PATCH] aarch64, libgcc: Fix unwinding from pac-ret to normal frames [PR94514]
The 04/17/2020 11:05, Kyrylo Tkachov wrote: > Hi Szabolcs, > > > -Original Message- > > From: Szabolcs Nagy > > Sent: 09 April 2020 15:20 > > To: gcc-patches@gcc.gnu.org > > Cc: Richard Earnshaw ; Richard Sandiford > > ; Kyrylo Tkachov > > Subject: [PATCH] aarch64, libgcc: Fix unwinding from pac-ret to normal > > frames [PR94514] > > > > With -mbranch-protection=pac-ret the debug info toggles the > > signedness state of the return address so the unwinder knows when > > the return address needs pointer authentication. > > > > The unwind context flags were not updated according to the dwarf > > frame info. > > > > This causes unwinding across frames that were built without pac-ret > > to incorrectly authenticate the return address wich corrupts the > > return address on a system where PAuth is enabled. > > > > Note: This even affects systems where all code use pac-ret because > > unwinding across a signal frame the return address is not signed. > > > > Ok, I'm guessing this needs backporting? committed now, yes i think it has to go back to gcc-9 and gcc-8, i will do that later. thanks.
RE: [PATCH] aarch64, libgcc: Fix unwinding from pac-ret to normal frames [PR94514]
Hi Szabolcs,
> -Original Message-
> From: Szabolcs Nagy
> Sent: 09 April 2020 15:20
> To: gcc-patches@gcc.gnu.org
> Cc: Richard Earnshaw ; Richard Sandiford
> ; Kyrylo Tkachov
> Subject: [PATCH] aarch64, libgcc: Fix unwinding from pac-ret to normal
> frames [PR94514]
>
> With -mbranch-protection=pac-ret the debug info toggles the
> signedness state of the return address so the unwinder knows when
> the return address needs pointer authentication.
>
> The unwind context flags were not updated according to the dwarf
> frame info.
>
> This causes unwinding across frames that were built without pac-ret
> to incorrectly authenticate the return address wich corrupts the
> return address on a system where PAuth is enabled.
>
> Note: This even affects systems where all code use pac-ret because
> unwinding across a signal frame the return address is not signed.
>
Ok, I'm guessing this needs backporting?
Thanks,
Kyrill
> gcc/testsuite/ChangeLog:
>
> 2020-04-XX Szabolcs Nagy
>
> PR target/94514
> * g++.target/aarch64/pr94514.C: New test.
> * gcc.target/aarch64/pr94514.c: New test.
>
> libgcc/ChangeLog:
>
> 2020-04-XX Szabolcs Nagy
>
> PR target/94514
> * config/aarch64/aarch64-unwind.h (aarch64_frob_update_context):
> Update context->flags accroding to the frame state.
> ---
> gcc/testsuite/g++.target/aarch64/pr94514.C | 26
> gcc/testsuite/gcc.target/aarch64/pr94514.c | 76 ++
> libgcc/config/aarch64/aarch64-unwind.h | 2 +
> 3 files changed, 104 insertions(+)
> create mode 100644 gcc/testsuite/g++.target/aarch64/pr94514.C
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr94514.c
>
> diff --git a/gcc/testsuite/g++.target/aarch64/pr94514.C
> b/gcc/testsuite/g++.target/aarch64/pr94514.C
> new file mode 100644
> index 000..2a8c949ba30
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/aarch64/pr94514.C
> @@ -0,0 +1,26 @@
> +/* PR target/94514. Unwind across mixed pac-ret and non-pac-ret frames.
> */
> +/* { dg-do run } */
> +
> +__attribute__((noinline, target("branch-protection=pac-ret")))
> +static void do_throw (void)
> +{
> + throw 42;
> + __builtin_abort ();
> +}
> +
> +__attribute__((noinline, target("branch-protection=none")))
> +static void no_pac_ret (void)
> +{
> + do_throw ();
> + __builtin_abort ();
> +}
> +
> +int main ()
> +{
> + try {
> +no_pac_ret ();
> + } catch (...) {
> +return 0;
> + }
> + __builtin_abort ();
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr94514.c
> b/gcc/testsuite/gcc.target/aarch64/pr94514.c
> new file mode 100644
> index 000..bbbf5a6b0b3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr94514.c
> @@ -0,0 +1,76 @@
> +/* PR target/94514. Unwind across mixed pac-ret and non-pac-ret frames.
> */
> +/* { dg-do run } */
> +/* { dg-options "-fexceptions -O2" } */
> +
> +#include
> +#include
> +#include
> +
> +#define die() \
> + do { \
> +printf ("%s:%d: reached unexpectedly.\n", __FILE__, __LINE__); \
> +fflush (stdout); \
> +abort (); \
> + } while (0)
> +
> +static struct _Unwind_Exception exc;
> +
> +static _Unwind_Reason_Code
> +force_unwind_stop (int version, _Unwind_Action actions,
> + _Unwind_Exception_Class exc_class,
> + struct _Unwind_Exception *exc_obj,
> + struct _Unwind_Context *context,
> + void *stop_parameter)
> +{
> + printf ("%s: CFA: %p PC: %p actions: %d\n",
> + __func__,
> + (void *)_Unwind_GetCFA (context),
> + (void *)_Unwind_GetIP (context),
> + (int)actions);
> + if (actions & _UA_END_OF_STACK)
> +die ();
> + return _URC_NO_REASON;
> +}
> +
> +static void force_unwind (void)
> +{
> +#ifndef __USING_SJLJ_EXCEPTIONS__
> + _Unwind_ForcedUnwind (, force_unwind_stop, 0);
> +#else
> + _Unwind_SjLj_ForcedUnwind (, force_unwind_stop, 0);
> +#endif
> +}
> +
> +__attribute__((noinline, target("branch-protection=pac-ret")))
> +static void f2_pac_ret (void)
> +{
> + force_unwind ();
> + die ();
> +}
> +
> +__attribute__((noinline, target("branch-protection=none")))
> +static void f1_no_pac_ret (void)
> +{
> + f2_pac_ret ();
> + die ();
> +}
> +
> +__attribute__((noinline, target("branch-protection=pac-ret")))
> +static void f0_pac_ret (void)
> +{
> + f1_no_pac_ret ();
> + die ();
> +}
> +
> +static void cleanup_handler (void *p)
> +{
> + printf ("
[PATCH] aarch64, libgcc: Fix unwinding from pac-ret to normal frames [PR94514]
With -mbranch-protection=pac-ret the debug info toggles the
signedness state of the return address so the unwinder knows when
the return address needs pointer authentication.
The unwind context flags were not updated according to the dwarf
frame info.
This causes unwinding across frames that were built without pac-ret
to incorrectly authenticate the return address wich corrupts the
return address on a system where PAuth is enabled.
Note: This even affects systems where all code use pac-ret because
unwinding across a signal frame the return address is not signed.
gcc/testsuite/ChangeLog:
2020-04-XX Szabolcs Nagy
PR target/94514
* g++.target/aarch64/pr94514.C: New test.
* gcc.target/aarch64/pr94514.c: New test.
libgcc/ChangeLog:
2020-04-XX Szabolcs Nagy
PR target/94514
* config/aarch64/aarch64-unwind.h (aarch64_frob_update_context):
Update context->flags accroding to the frame state.
---
gcc/testsuite/g++.target/aarch64/pr94514.C | 26
gcc/testsuite/gcc.target/aarch64/pr94514.c | 76 ++
libgcc/config/aarch64/aarch64-unwind.h | 2 +
3 files changed, 104 insertions(+)
create mode 100644 gcc/testsuite/g++.target/aarch64/pr94514.C
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr94514.c
diff --git a/gcc/testsuite/g++.target/aarch64/pr94514.C
b/gcc/testsuite/g++.target/aarch64/pr94514.C
new file mode 100644
index 000..2a8c949ba30
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/pr94514.C
@@ -0,0 +1,26 @@
+/* PR target/94514. Unwind across mixed pac-ret and non-pac-ret frames. */
+/* { dg-do run } */
+
+__attribute__((noinline, target("branch-protection=pac-ret")))
+static void do_throw (void)
+{
+ throw 42;
+ __builtin_abort ();
+}
+
+__attribute__((noinline, target("branch-protection=none")))
+static void no_pac_ret (void)
+{
+ do_throw ();
+ __builtin_abort ();
+}
+
+int main ()
+{
+ try {
+no_pac_ret ();
+ } catch (...) {
+return 0;
+ }
+ __builtin_abort ();
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/pr94514.c
b/gcc/testsuite/gcc.target/aarch64/pr94514.c
new file mode 100644
index 000..bbbf5a6b0b3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr94514.c
@@ -0,0 +1,76 @@
+/* PR target/94514. Unwind across mixed pac-ret and non-pac-ret frames. */
+/* { dg-do run } */
+/* { dg-options "-fexceptions -O2" } */
+
+#include
+#include
+#include
+
+#define die() \
+ do { \
+printf ("%s:%d: reached unexpectedly.\n", __FILE__, __LINE__); \
+fflush (stdout); \
+abort (); \
+ } while (0)
+
+static struct _Unwind_Exception exc;
+
+static _Unwind_Reason_Code
+force_unwind_stop (int version, _Unwind_Action actions,
+ _Unwind_Exception_Class exc_class,
+ struct _Unwind_Exception *exc_obj,
+ struct _Unwind_Context *context,
+ void *stop_parameter)
+{
+ printf ("%s: CFA: %p PC: %p actions: %d\n",
+ __func__,
+ (void *)_Unwind_GetCFA (context),
+ (void *)_Unwind_GetIP (context),
+ (int)actions);
+ if (actions & _UA_END_OF_STACK)
+die ();
+ return _URC_NO_REASON;
+}
+
+static void force_unwind (void)
+{
+#ifndef __USING_SJLJ_EXCEPTIONS__
+ _Unwind_ForcedUnwind (, force_unwind_stop, 0);
+#else
+ _Unwind_SjLj_ForcedUnwind (, force_unwind_stop, 0);
+#endif
+}
+
+__attribute__((noinline, target("branch-protection=pac-ret")))
+static void f2_pac_ret (void)
+{
+ force_unwind ();
+ die ();
+}
+
+__attribute__((noinline, target("branch-protection=none")))
+static void f1_no_pac_ret (void)
+{
+ f2_pac_ret ();
+ die ();
+}
+
+__attribute__((noinline, target("branch-protection=pac-ret")))
+static void f0_pac_ret (void)
+{
+ f1_no_pac_ret ();
+ die ();
+}
+
+static void cleanup_handler (void *p)
+{
+ printf ("%s: Success.\n", __func__);
+ exit (0);
+}
+
+int main ()
+{
+ char dummy __attribute__((cleanup (cleanup_handler)));
+ f0_pac_ret ();
+ die ();
+}
diff --git a/libgcc/config/aarch64/aarch64-unwind.h
b/libgcc/config/aarch64/aarch64-unwind.h
index 4c8790bae67..ed84a96db41 100644
--- a/libgcc/config/aarch64/aarch64-unwind.h
+++ b/libgcc/config/aarch64/aarch64-unwind.h
@@ -104,6 +104,8 @@ aarch64_frob_update_context (struct _Unwind_Context
*context,
if (fs->regs.reg[DWARF_REGNUM_AARCH64_RA_STATE].loc.offset & 0x1)
/* The flag is used for re-authenticating EH handler's address. */
context->flags |= RA_SIGNED_BIT;
+ else
+context->flags &= ~RA_SIGNED_BIT;
return;
}
--
2.17.1
