Re: Go patch committed: Multiplex goroutines onto OS threads

[Rainer Orth <ro at CeBiTec dot Uni-Bielefeld dot DE>]

Ian Lance Taylor <iant@google.com> writes:

> Ian Lance Taylor <iant@google.com> writes:
>
>> Right now it looks like there is a bug, or at least an incompatibility,
>> in the 64-bit versions of getcontext and setcontext.  It looks like
>> calling setcontext on the 32-bit version does not change the value of
>> TLS variables, which is also the case on GNU/Linux.  In the 64-bit
>> version, calling setcontext does change the value of TLS variables.
>> That is, on the 64-bit version, getcontext preserves the value of TLS
>> variables and setcontext restores the old value.  (Of course it's really
>> the pointer, not the TLS variables themselves).  This incompatibility
>> has to be a bug, and of course I would prefer that the incompatibility
>> be resolved in favor of the implementation used on GNU/Linux.
>
> Well, I thought I could fix this in a sensible way, but I really
> couldn't.  It's a fairly serious bug.  Calling setcontext in thread T2
> when getcontext was called in thread T1 causes T2 to start using T1's
> TLS variables, T1's pthread_getspecific values, and even T1's
> pthread_self.  I couldn't figure out any way that T2 could do any thread
> specific.
>
> So I cheated and wrote a system-specific hack.  With this patch applied,
> I see only 4 failures running the testsuite on x86/x86_64 Solaris.
> Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu, where the
> code is not used.  Committed to mainline.

Excellent, works like a charm, thanks.  The patches for Solaris 10 and
11 are expected within the next month or two.  I only noticed two
problems:

* On Solaris 8/x86 with native TLS, SETCONTEXT_CLOBBERS_TLS was
  incorrectly defined, causing a proc.c compilation failure:

/vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:105:4: error: #error unknown case for SETCONTEXT_CLOBBERS_TLS
/vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c: In function 'runtime_gogo':
/vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:299:2: error: implicit declaration of function 'fixcontext' [-Werror=implicit-function-declaration]
/vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c: In function 'runtime_schedinit':
/vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:366:2: error: implicit declaration of function 'initcontext' [-Werror=implicit-function-declaration]
cc1: all warnings being treated as errors
make[4]: *** [proc.lo] Error 1

  The configure test failed like this:

configure:14894: ./conftest
ld.so.1: conftest: fatal: conftest: object requires TLS, but TLS failed to initi
alize
/vol/gcc/src/hg/trunk/local/libgo/configure[20]: 28491 Killed

  The problem is that on Solaris 8 one needs to link with -pthread which
  takes care of finding the correct libthread.so.  The following patch
  takes care of that and gives decent testsuite results.

* In the cross-compilation case, one needs to test both the
  i?86-*-solaris2.1[01] and x86_64-*-solaris2.1[10] target triplets, but
  only for 64-bit.  The first is the default, while one can configure
  for the second if need be.  The patch contains the necessary change,
  adapted from libgcc/configure.ac's type size detection.  Besides, the
  bug affects both Solaris 10 and 11, so the workaround should trigger
  on either, too.

	Rainer


diff --git a/libgo/configure.ac b/libgo/configure.ac
--- a/libgo/configure.ac
+++ b/libgo/configure.ac
@@ -584,8 +584,12 @@ fi
 dnl See whether setcontext changes the value of TLS variables.
 AC_CACHE_CHECK([whether setcontext clobbers TLS variables],
 [libgo_cv_lib_setcontext_clobbers_tls],
-[LIBS_hold="$LIBS"
+[CFLAGS_hold="$CFLAGS"
+CFLAGS="$PTHREAD_CFLAGS"
+LIBS_hold="$LIBS"
 LIBS="$LIBS $PTHREAD_LIBS"
+AC_CHECK_SIZEOF([void *])
+AS_VAR_ARITH([ptr_type_size], [$ac_cv_sizeof_void_p \* 8])
 AC_RUN_IFELSE(
   [AC_LANG_SOURCE([
 #include <pthread.h>
@@ -650,11 +654,14 @@ main ()
 ])],
 [libgo_cv_lib_setcontext_clobbers_tls=no],
 [libgo_cv_lib_setcontext_clobbers_tls=yes],
-[case "$target" in
-  x86_64*-*-solaris2.10) libgo_cv_lib_setcontext_clobbers_tls=yes ;;
-  *) libgo_cv_lib_setcontext_clobbers_tls=no ;;
+[case "$target:$ptr_type_size" in
+  i?86-*-solaris2.1[[01]]:64 | x86_64*-*-solaris2.1[[01]]:64)
+    libgo_cv_lib_setcontext_clobbers_tls=yes ;;
+  *)
+    libgo_cv_lib_setcontext_clobbers_tls=no ;;
  esac
 ])
+CFLAGS="$CFLAGS_hold"
 LIBS="$LIBS_hold"
 ])
 if test "$libgo_cv_lib_setcontext_clobbers_tls" = "yes"; then

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University