From 421a8ed41201b3e6e379f8fda5834ce8617b94f1 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 18 Oct 2016 13:29:37 +0000 Subject: [PATCH] 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 --- gcc/go/gofrontend/MERGE | 2 +- libgo/runtime/proc.c | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE index 9c1839d58c2..c13c10dd953 100644 --- a/gcc/go/gofrontend/MERGE +++ b/gcc/go/gofrontend/MERGE @@ -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. diff --git a/libgo/runtime/proc.c b/libgo/runtime/proc.c index 9838c7f7635..a9f3b837334 100644 --- a/libgo/runtime/proc.c +++ b/libgo/runtime/proc.c @@ -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])); -- 2.30.2