runtime: remove unnecessary writes to gp.sched.g

A g's sched.g is set in newproc1:

	newg.sched.g = guintptr(unsafe.Pointer(newg))

After that, it never changes. Yet lots of assembly code does
"g.sched.g = g" unnecessarily. Remove all those lines to avoid
confusion about whether it ever changes.

Also, split gogo into two functions, one that does the nil g check
and a second that does the actual switch. This way, if the nil g check
fails, we get a stack trace showing the call stack that led to the failure.
(The SP write would otherwise cause the stack trace to abort.)

Also restore the proper nil g check in a handful of assembly functions.
(There is little point in checking for nil g *after* installing it as the real g.)

Change-Id: I22866b093f901f765de1d074e36eeec10366abfb
Reviewed-on: https://go-review.googlesource.com/c/go/+/292109
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

1 files changed, 20 insertions, 4 deletions

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 1dbd01ed40..388d843004 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -1281,6 +1281,9 @@ func mstart() {
 	mexit(osStack)
 }
 
+// The go:noinline is to guarantee the getcallerpc/getcallersp below are safe,
+// so that we can set up g0.sched to return to the call of mstart1 above.
+//go:noinline
 func mstart1() {
 	_g_ := getg()
 
@@ -1288,11 +1291,16 @@ func mstart1() {
 		throw("bad runtime·mstart")
 	}
 
-	// Record the caller for use as the top of stack in mcall and
-	// for terminating the thread.
+	// Set up m.g0.sched as a label returning returning to just
+	// after the mstart1 call in mstart0 above, for use by goexit0 and mcall.
 	// We're never coming back to mstart1 after we call schedule,
 	// so other calls can reuse the current frame.
-	save(getcallerpc(), getcallersp())
+	// And goexit0 does a gogo that needs to return from mstart1
+	// and let mstart0 exit the thread.
+	_g_.sched.g = guintptr(unsafe.Pointer(_g_))
+	_g_.sched.pc = getcallerpc()
+	_g_.sched.sp = getcallersp()
+
 	asminit()
 	minit()
 
@@ -3445,11 +3453,19 @@ func goexit0(gp *g) {
 func save(pc, sp uintptr) {
 	_g_ := getg()
 
+	if _g_ == _g_.m.g0 || _g_ == _g_.m.gsignal {
+		// m.g0.sched is special and must describe the context
+		// for exiting the thread. mstart1 writes to it directly.
+		// m.gsignal.sched should not be used at all.
+		// This check makes sure save calls do not accidentally
+		// run in contexts where they'd write to system g's.
+		throw("save on system g not allowed")
+	}
+
 	_g_.sched.pc = pc
 	_g_.sched.sp = sp
 	_g_.sched.lr = 0
 	_g_.sched.ret = 0
-	_g_.sched.g = guintptr(unsafe.Pointer(_g_))
 	// We need to ensure ctxt is zero, but can't have a write
 	// barrier here. However, it should always already be zero.
 	// Assert that.