Patchwork libgo patch committed: Fix TLS stack size problem again

login
register
mail settings
Submitter Ian Taylor
Date June 7, 2012, 12:55 a.m.
Message ID <mcrbokwos8n.fsf@dhcp-172-18-216-180.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/163448/
State New
Headers show

Comments

Ian Taylor - June 7, 2012, 12:55 a.m.
This patch to libgo fixes the TLS stack size problem again, this time
using a documented interface.  The problem, to repeat, is that glibc
subtracts the TLS size out of the stack size when creating a thread with
a specified stack size.  That causes libgo to fail in a rather obscure
manner when used with a program with a large TLS.  This patch addresses
the problem by using dl_iterate_phdr, if available, to determine the TLS
size, and adds it to the stack size.  This patch should be safe, in that
it does nothing if dl_iterate_phdr is not available, and if it fails to
compute the TLS size, we are at least no worse of than before.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline and 4.7 branch.

Ian

Patch

diff -r e674f3abde0b libgo/configure.ac
--- a/libgo/configure.ac	Tue Jun 05 06:11:42 2012 -0700
+++ b/libgo/configure.ac	Wed Jun 06 17:49:18 2012 -0700
@@ -481,7 +481,7 @@ 
 
 AM_CONDITIONAL(HAVE_SYS_MMAN_H, test "$ac_cv_header_sys_mman_h" = yes)
 
-AC_CHECK_FUNCS(strerror_r strsignal wait4 mincore setenv _dl_get_tls_static_info)
+AC_CHECK_FUNCS(strerror_r strsignal wait4 mincore setenv dl_iterate_phdr)
 AM_CONDITIONAL(HAVE_STRERROR_R, test "$ac_cv_func_strerror_r" = yes)
 AM_CONDITIONAL(HAVE_WAIT4, test "$ac_cv_func_wait4" = yes)
 
diff -r e674f3abde0b libgo/runtime/proc.c
--- a/libgo/runtime/proc.c	Tue Jun 05 06:11:42 2012 -0700
+++ b/libgo/runtime/proc.c	Wed Jun 06 17:49:18 2012 -0700
@@ -8,6 +8,11 @@ 
 #include <unistd.h>
 
 #include "config.h"
+
+#ifdef HAVE_DL_ITERATE_PHDR
+#include <link.h>
+#endif
+
 #include "runtime.h"
 #include "arch.h"
 #include "defs.h"
@@ -36,12 +41,12 @@ 
 
 #endif
 
+#ifndef PTHREAD_STACK_MIN
+# define PTHREAD_STACK_MIN 8192
+#endif
+
 #if defined(USING_SPLIT_STACK) && defined(LINKER_SUPPORTS_SPLIT_STACK)
-# ifdef PTHREAD_STACK_MIN
-#  define StackMin PTHREAD_STACK_MIN
-# else
-#  define StackMin 8192
-# endif
+# define StackMin PTHREAD_STACK_MIN
 #else
 # define StackMin 2 * 1024 * 1024
 #endif
@@ -138,6 +143,46 @@ 
 
 int32	runtime_gcwaiting;
 
+// The static TLS size.  See runtime_newm.
+static int tlssize;
+
+#ifdef HAVE_DL_ITERATE_PHDR
+
+// Called via dl_iterate_phdr.
+
+static int
+addtls(struct dl_phdr_info* info, size_t size __attribute__ ((unused)), void *data)
+{
+	size_t *total = (size_t *)data;
+	unsigned int i;
+
+	for(i = 0; i < info->dlpi_phnum; ++i) {
+		if(info->dlpi_phdr[i].p_type == PT_TLS)
+			*total += info->dlpi_phdr[i].p_memsz;
+	}
+	return 0;
+}
+
+// Set the total TLS size.
+
+static void
+inittlssize()
+{
+	size_t total = 0;
+
+	dl_iterate_phdr(addtls, (void *)&total);
+	tlssize = total;
+}
+
+#else
+
+static void
+inittlssize()
+{
+}
+
+#endif
+
 // Go scheduler
 //
 // The go scheduler's job is to match ready-to-run goroutines (`g's)
@@ -393,6 +438,7 @@ 
 	g->m = m;
 
 	initcontext();
+	inittlssize();
 
 	m->nomemprof++;
 	runtime_mallocinit();
@@ -1116,34 +1162,15 @@ 
 	if(pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED) != 0)
 		runtime_throw("pthread_attr_setdetachstate");
 
-#ifndef PTHREAD_STACK_MIN
-#define PTHREAD_STACK_MIN 8192
-#endif
-
 	stacksize = PTHREAD_STACK_MIN;
 
-#if 0
-#ifdef HAVE__DL_GET_TLS_STATIC_INFO
-	{
-		/* On GNU/Linux the static TLS size is taken out of
-		   the stack size, and we get an error or a crash if
-		   there is not enough stack space left.  Add it back
-		   in if we can, in case the program uses a lot of TLS
-		   space.  */
-#ifndef internal_function
-#ifdef __i386__
-#define internal_function __attribute__ ((regparm (3), stdcall))
-#else
-#define internal_function
-#endif
-#endif
-		extern void _dl_get_tls_static_info(size_t*, size_t*) internal_function;
-		size_t tlssize, tlsalign;
-		_dl_get_tls_static_info(&tlssize, &tlsalign);
-		stacksize += tlssize;
-	}
-#endif
-#endif
+	// With glibc before version 2.16 the static TLS size is taken
+	// out of the stack size, and we get an error or a crash if
+	// there is not enough stack space left.  Add it back in if we
+	// can, in case the program uses a lot of TLS space.  FIXME:
+	// This can be disabled in glibc 2.16 and later, if the bug is
+	// indeed fixed then.
+	stacksize += tlssize;
 
 	if(pthread_attr_setstacksize(&attr, stacksize) != 0)
 		runtime_throw("pthread_attr_setstacksize");