diff mbox

PATCH COMMITTED: Fix PR 46084: Allow for alignment with split stack

Message ID mcrhbfv8h83.fsf@google.com
State New
Headers show

Commit Message

Ian Lance Taylor Nov. 5, 2010, 11:48 p.m. UTC
When doing dynamic stack allocation when using -fsplit-stack, the new
space is allocated using malloc.  The alignment returned by malloc may
be less than the expected alignment of the stack, as is indeed the case
on x86.  Since the dynamic stack allocator aligns the address it
receives, this can cause the code to overrun the end of the buffer
allocated by malloc.  This is the problem behind PR 46084.

This patch fixes the problem by simply requesting extra space when
allocating from the heap, enough extra space to ensure that the
alignment will not cause a buffer overrun.

Bootstrapped and tested on x86_64-unknown-linux-gnu.  Committed to
mainline.

Ian


gcc/:
2010-11-05  Ian Lance Taylor  <iant@google.com>

	PR target/46084
	* explow.c (allocate_dynamic_stack_space): If flag_split_stack,
	request enough additional space for alignment, and force
	alignment.

gcc/testsuite:
2010-11-05  Ian Lance Taylor  <iant@google.com>

	PR target/46084
	* gcc.target/i386/pr46084.c: New test.

Comments

Richard Henderson Nov. 5, 2010, 11:55 p.m. UTC | #1
On 11/05/2010 04:48 PM, Ian Lance Taylor wrote:
> +      /* The __morestack_allocate_stack_space function will allocate
> +	 memory using malloc.  We don't know that the alignment of the
> +	 memory returned by malloc will meet REQUIRED_ALIGN.  Increase
> +	 SIZE to make sure we allocate enough space.  */
> +      ask = expand_binop (Pmode, add_optab, size,
> +			  GEN_INT (required_align / BITS_PER_UNIT - 1),
> +			  NULL_RTX, 1, OPTAB_LIB_WIDEN);

We do have MALLOC_ABI_ALIGNMENT.


r~
diff mbox

Patch

Index: gcc/explow.c
===================================================================
--- gcc/explow.c	(revision 166300)
+++ gcc/explow.c	(working copy)
@@ -1340,7 +1340,7 @@  allocate_dynamic_stack_space (rtx size, 
      least it doesn't cause a stack overflow.  */
   if (flag_split_stack)
     {
-      rtx available_label, space, func;
+      rtx available_label, ask, space, func;
 
       available_label = NULL_RTX;
 
@@ -1355,10 +1355,19 @@  allocate_dynamic_stack_space (rtx size, 
 	}
 #endif
 
+      /* The __morestack_allocate_stack_space function will allocate
+	 memory using malloc.  We don't know that the alignment of the
+	 memory returned by malloc will meet REQUIRED_ALIGN.  Increase
+	 SIZE to make sure we allocate enough space.  */
+      ask = expand_binop (Pmode, add_optab, size,
+			  GEN_INT (required_align / BITS_PER_UNIT - 1),
+			  NULL_RTX, 1, OPTAB_LIB_WIDEN);
+      must_align = true;
+
       func = init_one_libfunc ("__morestack_allocate_stack_space");
 
       space = emit_library_call_value (func, target, LCT_NORMAL, Pmode,
-				       1, size, Pmode);
+				       1, ask, Pmode);
 
       if (available_label == NULL_RTX)
 	return space;
Index: gcc/testsuite/gcc.target/i386/pr46084.c
===================================================================
--- gcc/testsuite/gcc.target/i386/pr46084.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/pr46084.c	(revision 0)
@@ -0,0 +1,69 @@ 
+/* This test needs to use setrlimit to set the stack size, so it can
+   only run on Unix.  */
+/* { dg-do run { target *-*-linux* *-*-solaris* *-*-darwin* } } */
+/* { dg-require-effective-target avx } */
+/* { dg-require-effective-target split_stack } */
+/* { dg-options "-fsplit-stack -O2 -mavx" } */
+
+#include <stdlib.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/resource.h>
+
+/* Use a noinline function to ensure that the buffer is not removed
+   from the stack.  */
+static void use_buffer (char *buf, size_t) __attribute__ ((noinline));
+static void
+use_buffer (char *buf, size_t c)
+{
+  size_t i;
+
+  for (i = 0; i < c; ++i)
+    buf[i] = (char) i;
+}
+
+/* Each recursive call uses 10 * i bytes.  We call it 1000 times,
+   using a total of 5,000,000 bytes.  If -fsplit-stack is not working,
+   that will overflow our stack limit.  */
+
+static void
+down1 (int i)
+{
+  char buf[10 * i];
+
+  if (i > 0)
+    {
+      use_buffer (buf, 10 * i);
+      down1 (i - 1);
+    }
+}
+
+/* Same thing, using alloca.  */
+
+static void
+down2 (int i)
+{
+  char *buf = alloca (10 * i);
+
+  if (i > 0)
+    {
+      use_buffer (buf, 10 * i);
+      down2 (i - 1);
+    }
+}
+
+int
+main (void)
+{
+  struct rlimit r;
+
+  /* We set a stack limit because we are usually invoked via make, and
+     make sets the stack limit to be as large as possible.  */
+  r.rlim_cur = 8192 * 1024;
+  r.rlim_max = 8192 * 1024;
+  if (setrlimit (RLIMIT_STACK, &r) != 0)
+    abort ();
+  down1 (1000);
+  down2 (1000);
+  return 0;
+}