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]

[PATCH][PR target/84064] Fix assertion with -fstack-clash-protection




stack-clash-protection will sometimes force a prologue to use pushes to
save registers.  We try to limit how often that happens as it constrains
options for generating efficient prologue sequences for common cases.

We check the value of TO_ALLOCATE in ix86_compute_frame_layout and if
it's greater than the probing interval, then we force the prologue to
use pushes to save integer registers.  That is conservatively correct
and allows freedom in the most common cases.  This is good.

In expand_prologue we assert that the integer registers were saved via a
push anytime we *might* generate a probe, even if the size of the
allocated frame is too small to require explicit probing.  Naturally,
this conflicts with the code in ix86_compute_frame_layout that tries to
avoid forcing pushes instead of moves.

This patch moves the assertion inside expand_prologue down to the points
where it actually needs to be true.  Specifically when we're generating
probes in a loop for -fstack-check or -fstack-clash-protection.

[ Probing via calls to chkstk_ms is not affected by this change. ]

Sorry to have to change this stuff for the 3rd time!

Bootstrapped and regression tested on x86_64 and i686.  Also verified
that if stack-clash checking is enabled by default that both compilers
will bootstrap.

OK for the trunk?

THanks,
Jeff
	* i386.c (ix86_adjust_stack_and_probe_stack_clash): New argument
	INT_REGISTERS_SAVED.  Check it prior to calling
	get_scratch_register_on_entry.
	(ix86_adjust_stack_and_probe): Similarly.
	(ix86_emit_probe_stack_range): Similarly.
	(ix86_expand_prologue): Corresponding changes.

	* gcc.target/i386/pr84064: New test.

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 3653ddd..fef34a1 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -12591,10 +12591,14 @@ release_scratch_register_on_entry (struct scratch_reg *sr)
    This differs from the next routine in that it tries hard to prevent
    attacks that jump the stack guard.  Thus it is never allowed to allocate
    more than PROBE_INTERVAL bytes of stack space without a suitable
-   probe.  */
+   probe.
+
+   INT_REGISTERS_SAVED is true if integer registers have already been
+   pushed on the stack.  */
 
 static void
-ix86_adjust_stack_and_probe_stack_clash (const HOST_WIDE_INT size)
+ix86_adjust_stack_and_probe_stack_clash (const HOST_WIDE_INT size,
+					 const bool int_registers_saved)
 {
   struct machine_function *m = cfun->machine;
 
@@ -12700,6 +12704,12 @@ ix86_adjust_stack_and_probe_stack_clash (const HOST_WIDE_INT size)
     }
   else
     {
+      /* We expect the GP registers to be saved when probes are used
+	 as the probing sequences might need a scratch register and
+	 the routine to allocate one assumes the integer registers
+	 have already been saved.  */
+      gcc_assert (int_registers_saved);
+
       struct scratch_reg sr;
       get_scratch_register_on_entry (&sr);
 
@@ -12758,10 +12768,14 @@ ix86_adjust_stack_and_probe_stack_clash (const HOST_WIDE_INT size)
   emit_insn (gen_blockage ());
 }
 
-/* Emit code to adjust the stack pointer by SIZE bytes while probing it.  */
+/* Emit code to adjust the stack pointer by SIZE bytes while probing it.
+
+   INT_REGISTERS_SAVED is true if integer registers have already been
+   pushed on the stack.  */
 
 static void
-ix86_adjust_stack_and_probe (const HOST_WIDE_INT size)
+ix86_adjust_stack_and_probe (const HOST_WIDE_INT size,
+			     const bool int_registers_saved)
 {
   /* We skip the probe for the first interval + a small dope of 4 words and
      probe that many bytes past the specified size to maintain a protection
@@ -12822,6 +12836,12 @@ ix86_adjust_stack_and_probe (const HOST_WIDE_INT size)
      equality test for the loop condition.  */
   else
     {
+      /* We expect the GP registers to be saved when probes are used
+	 as the probing sequences might need a scratch register and
+	 the routine to allocate one assumes the integer registers
+	 have already been saved.  */
+      gcc_assert (int_registers_saved);
+
       HOST_WIDE_INT rounded_size;
       struct scratch_reg sr;
 
@@ -12949,10 +12969,14 @@ output_adjust_stack_and_probe (rtx reg)
 }
 
 /* Emit code to probe a range of stack addresses from FIRST to FIRST+SIZE,
-   inclusive.  These are offsets from the current stack pointer.  */
+   inclusive.  These are offsets from the current stack pointer.
+
+   INT_REGISTERS_SAVED is true if integer registers have already been
+   pushed on the stack.  */
 
 static void
-ix86_emit_probe_stack_range (HOST_WIDE_INT first, HOST_WIDE_INT size)
+ix86_emit_probe_stack_range (HOST_WIDE_INT first, HOST_WIDE_INT size,
+			     const bool int_registers_saved)
 {
   /* See if we have a constant small number of probes to generate.  If so,
      that's the easy case.  The run-time loop is made up of 6 insns in the
@@ -12980,6 +13004,12 @@ ix86_emit_probe_stack_range (HOST_WIDE_INT first, HOST_WIDE_INT size)
      equality test for the loop condition.  */
   else
     {
+      /* We expect the GP registers to be saved when probes are used
+	 as the probing sequences might need a scratch register and
+	 the routine to allocate one assumes the integer registers
+	 have already been saved.  */
+      gcc_assert (int_registers_saved);
+
       HOST_WIDE_INT rounded_size, last;
       struct scratch_reg sr;
 
@@ -13733,15 +13763,10 @@ ix86_expand_prologue (void)
       && (flag_stack_check == STATIC_BUILTIN_STACK_CHECK
 	  || flag_stack_clash_protection))
     {
-      /* We expect the GP registers to be saved when probes are used
-	 as the probing sequences might need a scratch register and
-	 the routine to allocate one assumes the integer registers
-	 have already been saved.  */
-      gcc_assert (int_registers_saved);
-
       if (flag_stack_clash_protection)
 	{
-	  ix86_adjust_stack_and_probe_stack_clash (allocate);
+	  ix86_adjust_stack_and_probe_stack_clash (allocate,
+						   int_registers_saved);
 	  allocate = 0;
 	}
       else if (STACK_CHECK_MOVING_SP)
@@ -13749,7 +13774,7 @@ ix86_expand_prologue (void)
 	  if (!(crtl->is_leaf && !cfun->calls_alloca
 		&& allocate <= get_probe_interval ()))
 	    {
-	      ix86_adjust_stack_and_probe (allocate);
+	      ix86_adjust_stack_and_probe (allocate, int_registers_saved);
 	      allocate = 0;
 	    }
 	}
@@ -13765,11 +13790,12 @@ ix86_expand_prologue (void)
 	      if (crtl->is_leaf && !cfun->calls_alloca)
 		{
 		  if (size > get_probe_interval ())
-		    ix86_emit_probe_stack_range (0, size);
+		    ix86_emit_probe_stack_range (0, size, int_registers_saved);
 		}
 	      else
 		ix86_emit_probe_stack_range (0,
-					     size + get_stack_check_protect ());
+					     size + get_stack_check_protect (),
+					     int_registers_saved);
 	    }
 	  else
 	    {
@@ -13778,10 +13804,13 @@ ix86_expand_prologue (void)
 		  if (size > get_probe_interval ()
 		      && size > get_stack_check_protect ())
 		    ix86_emit_probe_stack_range (get_stack_check_protect (),
-						 size - get_stack_check_protect ());
+						 (size
+						  - get_stack_check_protect ()),
+						 int_registers_saved);
 		}
 	      else
-		ix86_emit_probe_stack_range (get_stack_check_protect (), size);
+		ix86_emit_probe_stack_range (get_stack_check_protect (), size,
+					     int_registers_saved);
 	    }
 	}
     }
diff --git a/gcc/testsuite/gcc.target/i386/pr84064.c b/gcc/testsuite/gcc.target/i386/pr84064.c
new file mode 100644
index 0000000..01f8d9e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr84064.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=i686 -fstack-clash-protection" } */
+/* { dg-require-effective-target ia32 } */
+
+void
+f (void *p1, void *p2)
+{
+  __builtin_memcpy (p1, p2, 1000);
+}
+

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