Hi Szabolcs,
> -----Original Message-----
> From: Szabolcs Nagy <szabolcs.n...@arm.com>
> Sent: 09 April 2020 15:20
> To: gcc-patches@gcc.gnu.org
> Cc: Richard Earnshaw <richard.earns...@arm.com>; Richard Sandiford
> <richard.sandif...@arm.com>; Kyrylo Tkachov <kyrylo.tkac...@arm.com>
> Subject: [PATCH] aarch64: Fix .cfi_window_save with pac-ret [PR94515]
>
> On aarch64 -mbranch-protection=pac-ret reuses the dwarf
> opcode for window_save to mean "toggle the return address
> mangle state", but in the dwarf2cfi internal logic the
> state was not properly recorded so the remember/restore
> logic was broken.
>
> This can cause the unwinder not to authenticate return
> addresses that were signed (or vice versa) which means
> a runtime crash on a pauth enabled system.
>
> Currently only aarch64 pac-ret uses REG_CFA_TOGGLE_RA_MANGLE.
I think this is ok, but this code is in the midend so I've CC'ed Jason who,
from what I gather from MAINTAINERS, is maintainer for this file.
Thanks,
Kyrill
>
> gcc/ChangeLog:
>
> 2020-04-XX Szabolcs Nagy <szabolcs.n...@arm.com>
>
> PR target/94515
> * dwarf2cfi.c (dwarf2out_frame_debug): Record RA
> mangle state in cur_row->window_save.
>
> gcc/testsuite/ChangeLog:
>
> 2020-04-XX Szabolcs Nagy <szabolcs.n...@arm.com>
>
> PR target/94515
> * g++.target/aarch64/pr94515.C: New test.
> ---
> gcc/dwarf2cfi.c | 3 ++
> gcc/testsuite/g++.target/aarch64/pr94515.C | 43 ++++++++++++++++++++++
> 2 files changed, 46 insertions(+)
> create mode 100644 gcc/testsuite/g++.target/aarch64/pr94515.C
>
> diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
> index 229fbfacc30..22989a6c2fb 100644
> --- a/gcc/dwarf2cfi.c
> +++ b/gcc/dwarf2cfi.c
> @@ -2145,6 +2145,9 @@ dwarf2out_frame_debug (rtx_insn *insn)
> case REG_CFA_TOGGLE_RA_MANGLE:
> /* This uses the same DWARF opcode as the next operation. */
> dwarf2out_frame_debug_cfa_window_save (true);
> + /* Keep track of RA mangle state by toggling the window_save bit.
> + This is needed so .cfi_window_save is emitted correctly. */
> + cur_row->window_save = !cur_row->window_save;
> handled_one = true;
> break;
>
> diff --git a/gcc/testsuite/g++.target/aarch64/pr94515.C
> b/gcc/testsuite/g++.target/aarch64/pr94515.C
> new file mode 100644
> index 00000000000..7707c773a72
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/aarch64/pr94515.C
> @@ -0,0 +1,43 @@
> +/* PR target/94515. Check .cfi_window_save with multiple return paths. */
> +/* { dg-do run } */
> +/* { dg-additional-options "-O2 -mbranch-protection=pac-ret --save-temps" }
> */
> +
> +volatile int zero = 0;
> +
> +__attribute__((noinline, target("branch-protection=none")))
> +void unwind (void)
> +{
> + if (zero == 0)
> + throw 42;
> +}
> +
> +__attribute__((noinline, noipa, target("branch-protection=pac-ret")))
> +int test (int z)
> +{
> + if (z) {
> + asm volatile ("":::"x20","x21");
> + unwind ();
> + return 1;
> + } else {
> + unwind ();
> + return 2;
> + }
> +}
> +
> +__attribute__((target("branch-protection=none")))
> +int main ()
> +{
> + try {
> + test (zero);
> + __builtin_abort ();
> + } catch (...) {
> + return 0;
> + }
> + __builtin_abort ();
> +}
> +
> +/* This check only works if there are two return paths in test and
> + cfi_window_save is used for both instead of cfi_remember_state
> + plus cfi_restore_state. This is currently the case with -O2. */
> +
> +/* { dg-final { scan-assembler-times {\t\.cfi_window_save\n} 4 } } */
> --
> 2.17.1
