re PR target/83641 (-fstack-clash-protection generates incorrect CFI on i386)

	PR target/83641
	* config/i386/i386.c (ix86_adjust_stack_and_probe_stack_clash): For
	noreturn probe, use gen_pop instead of ix86_emit_restore_reg_using_pop,
	only set RTX_FRAME_RELATED_P on both the push and pop if cfa_reg is sp
	and add REG_CFA_ADJUST_CFA notes in that case to both insns.

	PR target/83641
	* config/i386/i386.c (ix86_adjust_stack_and_probe_stack_clash): Do not
	explicitly probe *sp in a noreturn function if there were any callee
	register saves or frame pointer is needed.

	PR target/83641
	* gcc.target/i386/stack-check-17.c: New test.
	* gcc.target/i386/stack-check-12.c: Drop unnecessary asm.

From-SVN: r256191

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 8df8f232d808ed22e24c85a187b742e060699561..7aa0920fc653608809e0710b5b7a5de5f5ae3ea0 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,17 @@
+2017-01-03  Jakub Jelinek  <jakub@redhat.com>
+           Jeff Law  <law@redhat.com>
+
+       PR target/83641
+       * config/i386/i386.c (ix86_adjust_stack_and_probe_stack_clash): For
+       noreturn probe, use gen_pop instead of ix86_emit_restore_reg_using_pop,
+       only set RTX_FRAME_RELATED_P on both the push and pop if cfa_reg is sp
+       and add REG_CFA_ADJUST_CFA notes in that case to both insns.
+
+       PR target/83641
+       * config/i386/i386.c (ix86_adjust_stack_and_probe_stack_clash): Do not
+       explicitly probe *sp in a noreturn function if there were any callee
+       register saves or frame pointer is needed.
+
 2018-01-03  Jakub Jelinek  <jakub@redhat.com>
 
        PR debug/83621

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 56baaa7e5e8a7a6e885be0f45fc16d2af2ea9e7d..c363de93e0238980213b2bbe443d1327b639d589 100644 (file)
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -12217,21 +12217,39 @@ ix86_adjust_stack_and_probe_stack_clash (const HOST_WIDE_INT size)
      pointer could be anywhere in the guard page.  The safe thing
      to do is emit a probe now.
 
+     The probe can be avoided if we have already emitted any callee
+     register saves into the stack or have a frame pointer (which will
+     have been saved as well).  Those saves will function as implicit
+     probes.
+
      ?!? This should be revamped to work like aarch64 and s390 where
      we track the offset from the most recent probe.  Normally that
      offset would be zero.  For a noreturn function we would reset
      it to PROBE_INTERVAL - (STACK_BOUNDARY / BITS_PER_UNIT).   Then
      we just probe when we cross PROBE_INTERVAL.  */
-  if (TREE_THIS_VOLATILE (cfun->decl))
+  if (TREE_THIS_VOLATILE (cfun->decl)
+      && !(m->frame.nregs || m->frame.nsseregs || frame_pointer_needed))
     {
       /* We can safely use any register here since we're just going to push
         its value and immediately pop it back.  But we do try and avoid
         argument passing registers so as not to introduce dependencies in
         the pipeline.  For 32 bit we use %esi and for 64 bit we use %rax.  */
       rtx dummy_reg = gen_rtx_REG (word_mode, TARGET_64BIT ? AX_REG : SI_REG);
-      rtx_insn *insn = emit_insn (gen_push (dummy_reg));
-      RTX_FRAME_RELATED_P (insn) = 1;
-      ix86_emit_restore_reg_using_pop (dummy_reg);
+      rtx_insn *insn_push = emit_insn (gen_push (dummy_reg));
+      rtx_insn *insn_pop = emit_insn (gen_pop (dummy_reg));
+      m->fs.sp_offset -= UNITS_PER_WORD;
+      if (m->fs.cfa_reg == stack_pointer_rtx)
+       {
+         m->fs.cfa_offset -= UNITS_PER_WORD;
+         rtx x = plus_constant (Pmode, stack_pointer_rtx, -UNITS_PER_WORD);
+         x = gen_rtx_SET (stack_pointer_rtx, x);
+         add_reg_note (insn_push, REG_CFA_ADJUST_CFA, x);
+         RTX_FRAME_RELATED_P (insn_push) = 1;
+         x = plus_constant (Pmode, stack_pointer_rtx, UNITS_PER_WORD);
+         x = gen_rtx_SET (stack_pointer_rtx, x);
+         add_reg_note (insn_pop, REG_CFA_ADJUST_CFA, x);
+         RTX_FRAME_RELATED_P (insn_pop) = 1;
+       }
       emit_insn (gen_blockage ());
     }
 

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 2c66a0d007e9e2523aa82e9bb7d7e70453e1cbbe..1b8e7ad6ea5a41e366af964a3847c3a568d3419d 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2018-01-03  Jeff Law  <law@redhat.com>
+
+       PR target/83641
+       * gcc.target/i386/stack-check-17.c: New test.
+       * gcc.target/i386/stack-check-12.c: Drop unnecessary asm.
+
 2018-01-03  Jakub Jelinek  <jakub@redhat.com>
 
        PR debug/83621

diff --git a/gcc/testsuite/gcc.target/i386/stack-check-12.c b/gcc/testsuite/gcc.target/i386/stack-check-12.c
index 980416946df3cebb065c725f41bbd17de7b1ae37..74d3a26cad1f2592a558fcd6045bdbebb122cce7 100644 (file)
--- a/gcc/testsuite/gcc.target/i386/stack-check-12.c
+++ b/gcc/testsuite/gcc.target/i386/stack-check-12.c
@@ -7,7 +7,6 @@ __attribute__ ((noreturn)) void exit (int);
 __attribute__ ((noreturn)) void
 f (void)
 {
-  asm volatile ("nop" ::: "edi");
   exit (1);
 }
 

diff --git a/gcc/testsuite/gcc.target/i386/stack-check-17.c b/gcc/testsuite/gcc.target/i386/stack-check-17.c
new file mode 100644 (file)
index 0000000..d2ef83b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/stack-check-17.c
@@ -0,0 +1,37 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fstack-clash-protection -mtune=generic -fomit-frame-pointer" } */
+/* { dg-require-effective-target supports_stack_clash_protection } */
+
+
+int x0, x1;
+void f1 (void);
+void f2 (int, int);
+
+__attribute__ ((noreturn))
+void
+f3 (void)
+{ 
+  int y0 = x0;
+  int y1 = x1;
+  f1 ();
+  f2 (y0, y1);
+  while (1);
+}
+
+/* Verify no explicit probes.  */
+/* { dg-final { scan-assembler-not "or\[ql\]" } } */
+
+/* We also want to verify we did not use a push/pop sequence
+   to probe *sp as the callee register saves are sufficient
+   to probe *sp.
+
+   y0/y1 are live across the call and thus must be allocated
+   into either a stack slot or callee saved register.  The former
+   would be rather dumb.  So assume it does not happen.
+
+   So search for two/four pushes for the callee register saves/argument
+   pushes and no pops (since the function has no reachable epilogue).  */
+/* { dg-final { scan-assembler-times "push\[ql\]" 2 { target { ! ia32 } } } }  */
+/* { dg-final { scan-assembler-times "push\[ql\]" 4 { target { ia32 } } } }  */
+/* { dg-final { scan-assembler-not "pop" } } */
+