runtime: fix dangling pointer in readyExecute

readyExecute passes a closure to mcall that captures an argument to
readyExecute. Since mcall is marked noescape, this closure lives on
the stack of the calling goroutine. However, the closure puts the
calling goroutine on the run queue (and switches to a new
goroutine). If the calling goroutine gets scheduled before the mcall
returns, this stack-allocated closure will become invalid while it's
still executing. One consequence of this we've observed is that the
captured gp variable can get overwritten before the call to
execute(gp), causing execute(gp) to segfault.

Fix this by passing the currently captured gp variable through a field
in the calling goroutine's g struct so that the func is no longer a
closure.

To prevent problems like this in the future, this change also removes
the go:noescape annotation from mcall. Due to a compiler bug, this
will currently cause a func closure passed to mcall to be implicitly
allocated rather than refusing the implicit allocation. However, this
is okay because there are no other closures passed to mcall right now
and the compiler bug will be fixed shortly.

Fixes #10428.

Change-Id: I49b48b85de5643323b89e9eaa4df63854e968c32
Reviewed-on: https://go-review.googlesource.com/8866
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>

src/runtime/proc1.go

@@ -158,7 +158,15 @@ func ready(gp *g, traceskip int) {
 // readyExecute marks gp ready to run, preempt the current g, and execute gp.
 // This is used to start concurrent GC promptly when we reach its trigger.
 func readyExecute(gp *g, traceskip int) {
+	// Squirrel away gp so we don't allocate a closure for the
+	// mcall'd func below. If we allocate a closure, it could go
+	// away as soon as we put _g_ on the runqueue.
+	getg().readyg = gp
+
 	mcall(func(_g_ *g) {
+		gp := _g_.readyg
+		_g_.readyg = nil
+
 		if trace.enabled {
 			traceGoUnpark(gp, traceskip)
 			traceGoSched()

src/runtime/runtime2.go

@@ -243,6 +243,7 @@ type g struct {
 	startpc      uintptr // pc of goroutine function
 	racectx      uintptr
 	waiting      *sudog // sudog structures this g is waiting on (that have a valid elem ptr)
+	readyg       *g     // scratch for readyExecute
 }
 
 type mts struct {

src/runtime/stubs.go

@@ -33,7 +33,10 @@ func getg() *g
 // run other goroutines.
 //
 // mcall can only be called from g stacks (not g0, not gsignal).
-//go:noescape
+//
+// This must NOT be go:noescape: if fn is a stack-allocated closure,
+// fn puts g on a run queue, and g executes before fn returns, the
+// closure will be invalidated while it is still executing.
 func mcall(fn func(*g))
 
 // systemstack runs fn on a system stack.