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"); } }