runtime: scan caller-saved registers for non-split-stack

While testing a patch on Solaris, which does not support split-stack, I
    ran across a bug in the handling of caller-saved registers for the
    garbage collector.  For non-split-stack systems, runtime_mcall is
    responsible for saving all caller-saved registers on the stack so that
    the GC stack scan will see them.  It does this by calling
    __builtin_unwind_init and setting the g's gcnextsp field to point to the
    current stack.  The garbage collector then scans the stack from gcnextsp
    to the top of stack.
    
    Unfortunately, the code was setting gcnextsp to point to runtime_mcall's
    argument, which meant that even though runtime_mcall was careful to
    store all caller-saved registers on the stack, the GC never saw them.
    This is, of course, only a problem if a value lives only in a
    caller-saved register, and not anywhere else on the stack or heap.  And
    it is only a problem if that caller-saved register manages to make it
    all the way down to runtime_mcall without being saved by any function on
    the way.  This is moderately unlikely but it turns out that the recent
    changes to keep values on the stack when compiling the runtime package
    caused it to happen for the local variable `s` in `notifyListWait` in
    runtime/sema.go.  That function calls goparkunlock which is simple
    enough to not require all registers, and itself calls runtime_mcall.  So
    it was possible for `s` to be released by the GC before the goroutine
    returned from goparkunlock, which eventually caused a dangling pointer
    to be passed to releaseSudog.
    
    This is not a problem on split-stack systems, which use
    __splitstack_get_context, which saves a stack pointer low enough on the
    stack to scan the registers saved by runtime_mcall.
    
    Reviewed-on: https://go-review.googlesource.com/31323

From-SVN: r241304
This commit is contained in:
Ian Lance Taylor 2016-10-18 13:29:37 +00:00
parent 1b32951078
commit 421a8ed412
2 changed files with 7 additions and 2 deletions

View File

@ -1,4 +1,4 @@
314ba28067383516c213ba84c931f93325a48c39
0a49b1dadd862215bdd38b9725a6e193b0d8fd0b
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.

View File

@ -283,6 +283,9 @@ runtime_mcall(void (*pfn)(G*))
{
M *mp;
G *gp;
#ifndef USING_SPLIT_STACK
void *afterregs;
#endif
// Ensure that all registers are on the stack for the garbage
// collector.
@ -298,7 +301,9 @@ runtime_mcall(void (*pfn)(G*))
#ifdef USING_SPLIT_STACK
__splitstack_getcontext(&g->stackcontext[0]);
#else
gp->gcnextsp = &pfn;
// We have to point to an address on the stack that is
// below the saved registers.
gp->gcnextsp = &afterregs;
#endif
gp->fromgogo = false;
getcontext(ucontext_arg(&gp->context[0]));