PR 50654 points out that many Go tests fail on systems that use emutls.
This turns out to be a subtle issue involving the use of setcontext and
getcontext. When a particular invocation is moved to run on a different
thread via getcontext and setcontext, it must reload the thread-local
variables m and g. This happens naturally, because the function call
makes gcc think that they might have changed (as indeed they might
have). However, gcc knows that the address of the thread-local
variables can not change. Or at least it thinks it does; if setcontext
causes the function to start running on a different thread, then the
address actually does change. This means that gcc may cache the address
on the stack in some cases where it must not.
The same issue arises for ordinary TLS, of course, and I have already
fixed most cases. However, I missed one case. That case was working
for ordinary TLS because the function refers to both m and g, and gcc
compiles the code such that it holds a pointer to the thread-specific
area and references m and g off that pointer. This happens to work even
if the function starts running on a different thread. However, it does
not work when using emultls, for which gcc uses a different compilation
strategy.
This patch fixes the problem. Bootstrapped and ran Go testsuite on
x86_64-unknonw-linux-gnu, with both regular TLS and emutls. Committed
to mainline.
Ian
diff -r 5b77b481d6f9 libgo/runtime/proc.c
--- a/libgo/runtime/proc.c Mon Feb 13 16:29:13 2012 -0800
+++ b/libgo/runtime/proc.c Mon Feb 13 16:30:31 2012 -0800
@@ -309,6 +309,8 @@
static void
runtime_mcall(void (*pfn)(G*))
{
+ M *mp;
+ G *gp;
#ifndef USING_SPLIT_STACK
int i;
#endif
@@ -317,28 +319,45 @@
// collector.
__builtin_unwind_init();
- if(g == m->g0)
+ mp = m;
+ gp = g;
+ if(gp == mp->g0)
runtime_throw("runtime: mcall called on m->g0 stack");
- if(g != nil) {
+ if(gp != nil) {
#ifdef USING_SPLIT_STACK
__splitstack_getcontext(&g->stack_context[0]);
#else
- g->gcnext_sp = &i;
+ gp->gcnext_sp = &i;
#endif
- g->fromgogo = false;
- getcontext(&g->context);
+ gp->fromgogo = false;
+ getcontext(&gp->context);
+
+ // When we return from getcontext, we may be running
+ // in a new thread. That means that m and g may have
+ // changed. They are global variables so we will
+ // reload them, but the addresses of m and g may be
+ // cached in our local stack frame, and those
+ // addresses may be wrong. Call functions to reload
+ // the values for this thread.
+ mp = runtime_m();
+ gp = runtime_g();
}
- if (g == nil || !g->fromgogo) {
+ if (gp == nil || !gp->fromgogo) {
#ifdef USING_SPLIT_STACK
- __splitstack_setcontext(&m->g0->stack_context[0]);
+ __splitstack_setcontext(&mp->g0->stack_context[0]);
#endif
- m->g0->entry = (byte*)pfn;
- m->g0->param = g;
- g = m->g0;
- fixcontext(&m->g0->context);
- setcontext(&m->g0->context);
+ mp->g0->entry = (byte*)pfn;
+ mp->g0->param = gp;
+
+ // It's OK to set g directly here because this case
+ // can not occur if we got here via a setcontext to
+ // the getcontext call just above.
+ g = mp->g0;
+
+ fixcontext(&mp->g0->context);
+ setcontext(&mp->g0->context);
runtime_throw("runtime: mcall function returned");
}
}
