[PATCH] Fix stack red zone bug (PR38644)

Jiangning Liu jiangning.liu@arm.com
Mon Sep 26 11:55:00 GMT 2011


This patch is fix PR38644, a 3-year-old bug. 

>From the discussions in mail list and bugzilla, I think the middle end fix
is a common view. Although there are stills some gaps on how to fix it in
middle end, I think this patch at least moves the problem from back-end to
middle-end, which makes GCC infrastructure stronger than before. Otherwise,
any back-end needs to "find" and fix this problem by itself.

Since this bug was pinged many times by customers, I think at least we can
move on with this patch. If necessary, we can then give follow-up to build a
even better solution in middle end later on.

The patch is tested on x86 and ARM.

ChangeLog:

        * config/i386/i386.c (ix86_stack_using_red_zone): Change inline
        to be extern.
        (TARGET_STACK_USING_RED_ZONE): New.
        * config/rs6000/rs6000.c (rs6000_stack_using_red_zone): New.
        (TARGET_STACK_USING_RED_ZONE): New.
        (offset_below_red_zone_p): Change to use new hook 
        TARGET_STACK_USING_RED_ZONE.
        * doc/tm.texi (TARGET_STACK_USING_RED_ZONE): New.
        * doc/tm.texi.in (TARGET_STACK_USING_RED_ZONE): New.
        * sched-deps.c (sched_analyze_1): If the stack pointer is being 
        modified and stack red zone is not supported for ports, flush out 
        all memory references as they may become invalid if moved across 
        the stack adjustment.
        * target.def (stack_using_red_zone): New.
        * testsuite/gcc.target/arm/stack-red-zone.c: New.

Thanks,
-Jiangning

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ff8c49f..ce486da 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2631,8 +2631,8 @@ static const char *const
cpu_names[TARGET_CPU_DEFAULT_max] =
 

 /* Return true if a red-zone is in use.  */
 
-static inline bool
-ix86_using_red_zone (void)
+extern bool
+ix86_stack_using_red_zone (void)
 {
   return TARGET_RED_ZONE && !TARGET_64BIT_MS_ABI;
 }
@@ -35958,6 +35958,9 @@ ix86_autovectorize_vector_sizes (void)
 #define TARGET_STACK_PROTECT_FAIL ix86_stack_protect_fail
 #endif
 
+#undef TARGET_STACK_USING_RED_ZONE
+#define TARGET_STACK_USING_RED_ZONE ix86_stack_using_red_zone
+
 #undef TARGET_FUNCTION_VALUE
 #define TARGET_FUNCTION_VALUE ix86_function_value
 
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 1ab57e5..411cb09 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -1537,6 +1537,9 @@ static const struct attribute_spec
rs6000_attribute_table[] =
 #define TARGET_STACK_PROTECT_FAIL rs6000_stack_protect_fail
 #endif
 
+#undef TARGET_STACK_USING_RED_ZONE
+#define TARGET_STACK_USING_RED_ZONE rs6000_stack_using_red_zone
+
 /* MPC604EUM 3.5.2 Weak Consistency between Multiple Processors
    The PowerPC architecture requires only weak consistency among
    processors--that is, memory accesses between processors need not be
@@ -20660,6 +20663,13 @@ rs6000_restore_saved_cr (rtx reg, int
using_mfcr_multiple)
 	}
 }
 
+/* Return true if the ABI allows red zone access.  */
+extern bool
+rs6000_stack_using_red_zone (void)
+{
+   return (DEFAULT_ABI != ABI_V4);
+}
+
 /* Return true if OFFSET from stack pointer can be clobbered by signals.
    V.4 doesn't have any stack cushion, AIX ABIs have 220 or 288 bytes
    below stack pointer not cloberred by signals.  */
@@ -20667,7 +20677,7 @@ rs6000_restore_saved_cr (rtx reg, int
using_mfcr_multiple)
 static inline bool
 offset_below_red_zone_p (HOST_WIDE_INT offset)
 {
-  return offset < (DEFAULT_ABI == ABI_V4
+  return offset < (!TARGET_STACK_USING_RED_ZONE
 		   ? 0
 		   : TARGET_32BIT ? -220 : -288);
 }
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 335c1d1..c848806 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -11332,6 +11332,22 @@ to the stack.  Therefore, this hook should return
true in general, but
 false for naked functions.  The default implementation always returns true.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_STACK_USING_RED_ZONE (void)
+This hook returns true if the target has a red zone (an area beyond the
+current extent of the stack that cannot be modified by asynchronous events
+on the processor).
+
+If this hook returns false then the compiler mid-end will not move an
access
+to memory in the stack frame past a stack adjustment insn.
+
+If this hook returns true then the compiler mid-end will assume that it is
+safe to move an access to memory in the stack frame past a stack adjustment
+insn.  The target back-end must emit scheduling barrier insns when this is
+unsafe.
+
+The default is to return false which is safe and appropriate for most
targets.
+@end deftypefn
+
 @deftypevr {Target Hook} {unsigned HOST_WIDE_INT} TARGET_CONST_ANCHOR
 On some architectures it can take multiple instructions to synthesize
 a constant.  If there is another constant already in a register that
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 6783826..0fa9e10 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -11223,6 +11223,22 @@ to the stack.  Therefore, this hook should return
true in general, but
 false for naked functions.  The default implementation always returns true.
 @end deftypefn
 
+@hook TARGET_STACK_USING_RED_ZONE
+This hook returns true if the target has a red zone (an area beyond the
+current extent of the stack that cannot be modified by asynchronous events
+on the processor).
+
+If this hook returns false then the compiler mid-end will not move an
access
+to memory in the stack frame past a stack adjustment insn.
+
+If this hook returns true then the compiler mid-end will assume that it is
+safe to move an access to memory in the stack frame past a stack adjustment
+insn.  The target back-end must emit scheduling barrier insns when this is
+unsafe.
+
+The default is to return false which is safe and appropriate for most
targets.
+@end deftypefn
+
 @hook TARGET_CONST_ANCHOR
 On some architectures it can take multiple instructions to synthesize
 a constant.  If there is another constant already in a register that
diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c
index ed592c8..575b20c 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -2332,6 +2332,14 @@ sched_analyze_1 (struct deps_desc *deps, rtx x, rtx
insn)
 			       FIRST_STACK_REG);
 	}
 #endif
+      /* If the stack pointer is being modified, flush out all memory
+         references as they may become invalid if moved across the
+         stack adjustment.  */
+      if (!targetm.calls.stack_using_red_zone ()
+          && (dest == stack_pointer_rtx))
+        {
+          flush_pending_lists (deps, insn, true, true);
+        }
     }
   else if (MEM_P (dest))
     {
diff --git a/gcc/target.def b/gcc/target.def
index 1e09ba7..5e2cd38 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -2110,6 +2110,13 @@ DEFHOOK
  bool, (void),
  hook_bool_void_true)
 
+/* Return true if target uses stack red zone.  */
+DEFHOOK
+(stack_using_red_zone,
+ "",
+ bool, (void),
+ hook_bool_void_false)
+
 /* Return an rtx for the static chain for FNDECL.  If INCOMING_P is true,
        then it should be for the callee; otherwise for the caller.  */
 DEFHOOK
diff --git a/gcc/testsuite/gcc.target/arm/stack-red-zone.c
b/gcc/testsuite/gcc.target/arm/stack-red-zone.c
new file mode 100644
index 0000000..97f7aae
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/stack-red-zone.c
@@ -0,0 +1,12 @@
+/* No stack red zone.  PR38644.  */
+/* { dg-options "-mthumb -O2" } */
+/* { dg-final { scan-assembler "ldrb\[^\n\]*\\n\[\t \]*add\[\t
\]*sp\[^\n\]*\\n\[\t \]*@\[^\n\]*\\n\[\t \]*pop" } } */
+
+extern int doStreamReadBlock (int *, char *, int size, int);
+
+char readStream (int *s)
+{
+       char c = 0;
+       doStreamReadBlock (s, &c, 1, *s);
+       return c;
+}





More information about the Gcc-patches mailing list