This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][AARCH64] PR target/84521 Fix frame pointer corruption with -fomit-frame-pointer with __builtin_setjmp


Hi Eric

On 27/06/18 12:22, Wilco Dijkstra wrote:
Eric Botcazou wrote:

This test can easily be changed not to use optimize since it doesn't look
like it needs it. We really need to tests these builtins properly,
otherwise they will continue to fail on most targets.

As far as I can see PR target/84521 has been reported only for Aarch64 so I'd
just leave the other targets alone (and avoid propagating FUD if possible).

It's quite obvious from PR84521 that this is an issue affecting all targets.
Adding better generic tests for __builtin_setjmp can only be a good thing.

Wilco


This conversation seems to have died down and I would like to
start it again. I would agree with Wilco's suggestion about
keeping the test in the generic folder. I have removed the
optimize attribute and the effect is still the same. It passes
on AArch64 with this patch and it currently fails on x86
trunk (gcc version 9.0.0 20180712 (experimental) (GCC))
on -O1 and above.

Thanks
Sudi
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index f284e74..9792d28 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -473,7 +473,9 @@ extern unsigned aarch64_architecture_version;
 #define EH_RETURN_STACKADJ_RTX	gen_rtx_REG (Pmode, R4_REGNUM)
 #define EH_RETURN_HANDLER_RTX  aarch64_eh_return_handler_rtx ()
 
-/* Don't use __builtin_setjmp until we've defined it.  */
+/* Don't use __builtin_setjmp until we've defined it.
+   CAUTION: This macro is only used during exception unwinding.
+   Don't fall for its name.  */
 #undef DONT_USE_BUILTIN_SETJMP
 #define DONT_USE_BUILTIN_SETJMP 1
 
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 01f35f8..4266a3d 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3998,7 +3998,7 @@ static bool
 aarch64_needs_frame_chain (void)
 {
   /* Force a frame chain for EH returns so the return address is at FP+8.  */
-  if (frame_pointer_needed || crtl->calls_eh_return)
+  if (frame_pointer_needed || crtl->calls_eh_return || cfun->has_nonlocal_label)
     return true;
 
   /* A leaf function cannot have calls or write LR.  */
@@ -12218,6 +12218,13 @@ aarch64_expand_builtin_va_start (tree valist, rtx nextarg ATTRIBUTE_UNUSED)
   expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL);
 }
 
+/* Implement TARGET_BUILTIN_SETJMP_FRAME_VALUE.  */
+static rtx
+aarch64_builtin_setjmp_frame_value (void)
+{
+  return hard_frame_pointer_rtx;
+}
+
 /* Implement TARGET_GIMPLIFY_VA_ARG_EXPR.  */
 
 static tree
@@ -17744,6 +17751,9 @@ aarch64_run_selftests (void)
 #undef TARGET_FOLD_BUILTIN
 #define TARGET_FOLD_BUILTIN aarch64_fold_builtin
 
+#undef TARGET_BUILTIN_SETJMP_FRAME_VALUE
+#define TARGET_BUILTIN_SETJMP_FRAME_VALUE aarch64_builtin_setjmp_frame_value
+
 #undef TARGET_FUNCTION_ARG
 #define TARGET_FUNCTION_ARG aarch64_function_arg
 
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index a014a01..d5f33d8 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -6087,6 +6087,30 @@
   DONE;
 })
 
+;; This is broadly similar to the builtins.c except that it uses
+;; temporaries to load the incoming SP and FP.
+(define_expand "nonlocal_goto"
+  [(use (match_operand 0 "general_operand"))
+   (use (match_operand 1 "general_operand"))
+   (use (match_operand 2 "general_operand"))
+   (use (match_operand 3 "general_operand"))]
+  ""
+{
+    rtx label_in = copy_to_reg (operands[1]);
+    rtx fp_in = copy_to_reg (operands[3]);
+    rtx sp_in = copy_to_reg (operands[2]);
+
+    emit_move_insn (hard_frame_pointer_rtx, fp_in);
+    emit_stack_restore (SAVE_NONLOCAL, sp_in);
+
+    emit_use (hard_frame_pointer_rtx);
+    emit_use (stack_pointer_rtx);
+
+    emit_indirect_jump (label_in);
+
+    DONE;
+})
+
 ;; Helper for aarch64.c code.
 (define_expand "set_clobber_cc"
   [(parallel [(set (match_operand 0)
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr84521.c b/gcc/testsuite/gcc.c-torture/execute/pr84521.c
new file mode 100644
index 0000000..564ef14
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr84521.c
@@ -0,0 +1,53 @@
+/* { dg-require-effective-target indirect_jumps } */
+
+#include <string.h>
+#include <alloca.h>
+#include <setjmp.h>
+
+jmp_buf buf;
+
+int uses_longjmp (void)
+{
+  jmp_buf buf2;
+  memcpy (buf2, buf, sizeof (buf));
+  __builtin_longjmp (buf2, 1);
+}
+
+int gl;
+void after_longjmp (void)
+{
+  gl = 5;
+}
+
+int
+test_1 (int n)
+{
+  volatile int *p = alloca (n);
+  if (__builtin_setjmp (buf))
+    {
+      after_longjmp ();
+    }
+  else
+    {
+      uses_longjmp ();
+    }
+
+  return 0;
+}
+
+int
+test_2 (int n)
+{
+  int i;
+  int *ptr = (int *)__builtin_alloca (sizeof (int) * n);
+  for (i = 0; i < n; i++)
+    ptr[i] = i;
+  test_1 (n);
+  return 0;
+}
+
+int main (int argc, const char **argv)
+{
+  __builtin_memset (&buf, 0xaf, sizeof (buf));
+  test_2 (100);
+}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]