diff mbox

libgcc patch committed: Fix for -fsplit-stack when using gold

Message ID mcrfwaokmtj.fsf@dhcp-172-18-216-180.mtv.corp.google.com
State New
Headers show

Commit Message

Ian Lance Taylor May 25, 2012, 8:50 p.m. UTC 
When using the gold linker, the linker will adjust the split-stack
prologue to call __morestack_non_split if it sees that the function
calls a different function compiled without -fsplit-stack.  The
__morestack_non_split function has an optimization I committed
2011-12-20 to avoid splitting the stack if it has a lot of space.
Unfortunately, I forgot that if the function calling
__morestack_non_split is a varargs function, then it expects %ebp to
have a particular value when __morestack_non_split returns.  There is no
reasonable way for __morestack_non_split to set %ebp to the right value
without actually splitting the stack.  This patch changes
__morestack_non_split to do a stack split when called by a varargs
function.  It detects a call from a varargs function by looking at the
instruction it is going to return to, to see if it uses %ebp.

Bootstrapped and ran Go testsuite and split-stack tests on
x86_64-unknown-linux-gnu, both 32-bit and 64-bit.  Added a new test to
check this case in the future.  With an unpatched libgcc, the test will
succeed with GNU ld, fail with gold.

Committed to mainline and 4.7 branch.

Ian


libgcc/:

2012-05-25  Ian Lance Taylor  <iant@google.com>

	* config/i386/morestack.S (__morestack_non_split): Check whether
	caller is varargs and needs %bp to hold the stack frame on return.

gcc/testsuite/:

2012-05-25  Ian Lance Taylor  <iant@google.com>

	* gcc.dg/split-6.c: New test.
diff mbox

Patch

Index: libgcc/config/i386/morestack.S
===================================================================
--- libgcc/config/i386/morestack.S	(revision 187020)
+++ libgcc/config/i386/morestack.S	(working copy)
@@ -1,5 +1,5 @@ 
 # x86/x86_64 support for -fsplit-stack.
-# Copyright (C) 2009, 2010, 2011 Free Software Foundation, Inc.
+# Copyright (C) 2009, 2010, 2011, 2012 Free Software Foundation, Inc.
 # Contributed by Ian Lance Taylor <iant@google.com>.
 
 # This file is part of GCC.
@@ -138,6 +138,24 @@  __morestack_non_split:
 	je	1f			# see above.
 	addl	$2,%eax
 1:	inc	%eax
+
+	# If the instruction that we return to is
+	#   leal  20(%ebp),{%eax,%ecx,%edx}
+	# then we have been called by a varargs function that expects
+	# %ebp to hold a real value.  That can only work if we do the
+	# full stack split routine.  FIXME: This is fragile.
+	cmpb	$0x8d,(%eax)
+	jne	3f
+	cmpb	$0x14,2(%eax)
+	jne	3f
+	cmpb	$0x45,1(%eax)
+	je	2f
+	cmpb	$0x4d,1(%eax)
+	je	2f
+	cmpb	$0x55,1(%eax)
+	je	2f
+
+3:	
 	movl	%eax,4(%esp)		# Update return address.
 
 	popl	%eax			# Restore %eax and stack.
@@ -175,18 +193,32 @@  __morestack_non_split:
 #else
 	cmpl	%fs:0x40,%eax
 #endif
-	popq	%rax			# Restore register.
-
-	.cfi_adjust_cfa_offset -8	# Adjust for popped register.
 
 	jb	2f			# Get more space if we need it.
 
 	# This breaks call/return prediction, as described above.
-	incq	(%rsp)			# Increment the return address.
+	incq	8(%rsp)			# Increment the return address.
+
+	# If the instruction that we return to is
+	#   leaq  24(%rbp), %r11n
+	# then we have been called by a varargs function that expects
+	# %ebp to hold a real value.  That can only work if we do the
+	# full stack split routine.  FIXME: This is fragile.
+	movq	8(%rsp),%rax
+	cmpl	$0x185d8d4c,(%rax)
+	je	2f
+
+	popq	%rax			# Restore register.
+
+	.cfi_adjust_cfa_offset -8	# Adjust for popped register.
 
 	ret				# Return to caller.
 
 2:
+	popq	%rax			# Restore register.
+
+	.cfi_adjust_cfa_offset -8	# Adjust for popped register.
+
 	addq	$0x5000+BACKOFF,%r10	# Increment space we request.
 
 	# Fall through into morestack.
Index: gcc/testsuite/gcc.dg/split-6.c
===================================================================
--- gcc/testsuite/gcc.dg/split-6.c	(revision 0)
+++ gcc/testsuite/gcc.dg/split-6.c	(revision 0)
@@ -0,0 +1,53 @@ 
+/* { dg-do run } */
+/* { dg-require-effective-target split_stack } */
+/* { dg-options "-fsplit-stack" } */
+
+/* This test is like split-3.c, but tests with a smaller stack frame,
+   since that uses a different prologue.  */
+
+#include <stdarg.h>
+#include <stdlib.h>
+
+/* Use a noinline function to ensure that the buffer is not removed
+   from the stack.  */
+static void use_buffer (char *buf) __attribute__ ((noinline));
+static void
+use_buffer (char *buf)
+{
+  buf[0] = '\0';
+}
+
+/* When using gold, the call to abort will force a stack split.  */
+
+static void
+down (int i, ...)
+{
+  char buf[1];
+  va_list ap;
+
+  va_start (ap, i);
+  if (va_arg (ap, int) != 1
+      || va_arg (ap, int) != 2
+      || va_arg (ap, int) != 3
+      || va_arg (ap, int) != 4
+      || va_arg (ap, int) != 5
+      || va_arg (ap, int) != 6
+      || va_arg (ap, int) != 7
+      || va_arg (ap, int) != 8
+      || va_arg (ap, int) != 9
+      || va_arg (ap, int) != 10)
+    abort ();
+
+  if (i > 0)
+    {
+      use_buffer (buf);
+      down (i - 1, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+    }
+}
+
+int
+main (void)
+{
+  down (1000, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+  return 0;
+}