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 updated when an opcode was emitted, the
currently present update logic is only valid for the
original sparc use of window_save so a separate bool is
used on aarch64 to track the state.

This bug 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.

This should be backported to gcc-9 and gcc-8 branches.

gcc/ChangeLog:

	Backport from mainline.
	2020-04-27  Szabolcs Nagy  <szabolcs.nagy@arm.com>

	PR target/94515
	* dwarf2cfi.c (struct GTY): Add ra_mangled.
	(cfi_row_equal_p): Check ra_mangled.
	(dwarf2out_frame_debug_cfa_window_save): Remove the argument,
	this only handles the sparc logic now.
	(dwarf2out_frame_debug_cfa_toggle_ra_mangle): New function for
	the aarch64 specific logic.
	(dwarf2out_frame_debug): Update to use the new subroutines.
	(change_cfi_row): Check ra_mangled.

gcc/testsuite/ChangeLog:

	Backport from mainline.
	2020-04-27  Szabolcs Nagy  <szabolcs.nagy@arm.com>

	PR target/94515
	* g++.target/aarch64/pr94515-1.C: New test.
	* g++.target/aarch64/pr94515-2.C: New test.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 85beb08220361c2fc29aea9b9eb6ad8f187106e1..b36c9d53d26bd606c9f59c24830244aa68d0d727 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,18 @@
+2020-05-14  Szabolcs Nagy  <szabolcs.nagy@arm.com>
+
+       Backport from mainline.
+       2020-04-27  Szabolcs Nagy  <szabolcs.nagy@arm.com>
+
+       PR target/94515
+       * dwarf2cfi.c (struct GTY): Add ra_mangled.
+       (cfi_row_equal_p): Check ra_mangled.
+       (dwarf2out_frame_debug_cfa_window_save): Remove the argument,
+       this only handles the sparc logic now.
+       (dwarf2out_frame_debug_cfa_toggle_ra_mangle): New function for
+       the aarch64 specific logic.
+       (dwarf2out_frame_debug): Update to use the new subroutines.
+       (change_cfi_row): Check ra_mangled.
+
 2020-05-12  David Edelsohn  <dje.gcc@gmail.com>
 
        Backport from mainline

diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
index d6aed358484594fd2f741d97e7c3447f9ee7655a..3831979e26df661582caccdd7208616a19c92d3f 100644 (file)
--- a/gcc/dwarf2cfi.c
+++ b/gcc/dwarf2cfi.c
@@ -71,6 +71,9 @@ struct GTY(()) dw_cfi_row
 
   /* True if the register window is saved.  */
   bool window_save;
+
+  /* True if the return address is in a mangled state.  */
+  bool ra_mangled;
 };
 
 /* The caller's ORIG_REG is saved in SAVED_IN_REG.  */
@@ -772,6 +775,9 @@ cfi_row_equal_p (dw_cfi_row *a, dw_cfi_row *b)
   if (a->window_save != b->window_save)
     return false;
 
+  if (a->ra_mangled != b->ra_mangled)
+    return false;
+
   return true;
 }
 
@@ -1370,20 +1376,33 @@ dwarf2out_frame_debug_cfa_restore (rtx reg)
 }
 
 /* A subroutine of dwarf2out_frame_debug, process a REG_CFA_WINDOW_SAVE.
-   FAKE is true if this is not really a window save but something else.
 
    ??? Perhaps we should note in the CIE where windows are saved (instead
    of assuming 0(cfa)) and what registers are in the window.  */
 
 static void
-dwarf2out_frame_debug_cfa_window_save (bool fake)
+dwarf2out_frame_debug_cfa_window_save (void)
 {
   dw_cfi_ref cfi = new_cfi ();
 
   cfi->dw_cfi_opc = DW_CFA_GNU_window_save;
   add_cfi (cfi);
-  if (!fake)
-    cur_row->window_save = true;
+  cur_row->window_save = true;
+}
+
+/* A subroutine of dwarf2out_frame_debug, process a REG_CFA_TOGGLE_RA_MANGLE.
+   Note: DW_CFA_GNU_window_save dwarf opcode is reused for toggling RA mangle
+   state, this is a target specific operation on AArch64 and can only be used
+   on other targets if they don't use the window save operation otherwise.  */
+
+static void
+dwarf2out_frame_debug_cfa_toggle_ra_mangle (void)
+{
+  dw_cfi_ref cfi = new_cfi ();
+
+  cfi->dw_cfi_opc = DW_CFA_GNU_window_save;
+  add_cfi (cfi);
+  cur_row->ra_mangled = !cur_row->ra_mangled;
 }
 
 /* Record call frame debugging information for an expression EXPR,
@@ -2143,13 +2162,12 @@ dwarf2out_frame_debug (rtx_insn *insn)
        break;
 
       case REG_CFA_TOGGLE_RA_MANGLE:
-       /* This uses the same DWARF opcode as the next operation.  */
-       dwarf2out_frame_debug_cfa_window_save (true);
+       dwarf2out_frame_debug_cfa_toggle_ra_mangle ();
        handled_one = true;
        break;
 
       case REG_CFA_WINDOW_SAVE:
-       dwarf2out_frame_debug_cfa_window_save (false);
+       dwarf2out_frame_debug_cfa_window_save ();
        handled_one = true;
        break;
 
@@ -2218,6 +2236,17 @@ change_cfi_row (dw_cfi_row *old_row, dw_cfi_row *new_row)
     {
       dw_cfi_ref cfi = new_cfi ();
 
+      gcc_assert (!old_row->ra_mangled && !new_row->ra_mangled);
+      cfi->dw_cfi_opc = DW_CFA_GNU_window_save;
+      add_cfi (cfi);
+    }
+
+  if (old_row->ra_mangled != new_row->ra_mangled)
+    {
+      dw_cfi_ref cfi = new_cfi ();
+
+      gcc_assert (!old_row->window_save && !new_row->window_save);
+      /* DW_CFA_GNU_window_save is reused for toggling RA mangle state.  */
       cfi->dw_cfi_opc = DW_CFA_GNU_window_save;
       add_cfi (cfi);
     }

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 847b998e8dab33d849087a6209a323660ba580df..a642d0f9a19ca0a2c7e642ac3fe8015238ada1b8 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,12 @@
+2020-05-14  Szabolcs Nagy  <szabolcs.nagy@arm.com>
+
+       Backport from mainline.
+       2020-04-27  Szabolcs Nagy  <szabolcs.nagy@arm.com>
+
+       PR target/94515
+       * g++.target/aarch64/pr94515-1.C: New test.
+       * g++.target/aarch64/pr94515-2.C: New test.
+
 2020-05-14  Szabolcs Nagy  <szabolcs.nagy@arm.com>
 
        Backport from mainline.

diff --git a/gcc/testsuite/g++.target/aarch64/pr94515-1.C b/gcc/testsuite/g++.target/aarch64/pr94515-1.C
new file mode 100644 (file)
index 0000000..20ae812
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/pr94515-1.C
@@ -0,0 +1,44 @@
+/* PR target/94515. Check .cfi_window_save with multiple return paths.  */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
+/* { dg-additional-options "-O2 --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 } } */

diff --git a/gcc/testsuite/g++.target/aarch64/pr94515-2.C b/gcc/testsuite/g++.target/aarch64/pr94515-2.C
new file mode 100644 (file)
index 0000000..e73df49
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/pr94515-2.C
@@ -0,0 +1,41 @@
+/* PR target/94515. Check .cfi_window_save with multiple return paths.  */
+/* { dg-do run } */
+/* { dg-require-effective-target lp64 } */
+/* { dg-additional-options "-O2 -mbranch-protection=pac-ret" } */
+
+volatile int zero = 0;
+int global = 0;
+
+__attribute__((noinline))
+int bar(void)
+{
+  if (zero == 0) return 3;
+  return 0;
+}
+
+__attribute__((noinline, noreturn))
+void unwind (void)
+{
+  throw 42;
+}
+
+__attribute__((noinline, noipa))
+int test(int x)
+{
+  if (x==1) return 2; /* This return path may not use the stack.  */
+  int y = bar();
+  if (y > global) global=y;
+  if (y==3) unwind(); /* This return path must have RA mangle state set.  */
+  return 0;
+}
+
+int main ()
+{
+  try {
+    test (zero);
+    __builtin_abort ();
+  } catch (...) {
+    return 0;
+  }
+  __builtin_abort ();
+}