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>

src/runtime/asm_386.s

@@ -275,10 +275,13 @@ TEXT runtime·asminit(SB),NOSPLIT,$0-0
 
 // void gogo(Gobuf*)
 // restore state from Gobuf; longjmp
-TEXT runtime·gogo(SB), NOSPLIT, $8-4
+TEXT runtime·gogo(SB), NOSPLIT, $0-4
 	MOVL	buf+0(FP), BX		// gobuf
 	MOVL	gobuf_g(BX), DX
 	MOVL	0(DX), CX		// make sure g != nil
+	JMP	gogo<>(SB)
+
+TEXT gogo<>(SB), NOSPLIT, $0
 	get_tls(CX)
 	MOVL	DX, g(CX)
 	MOVL	gobuf_sp(BX), SP	// restore SP
@@ -303,7 +306,6 @@ TEXT runtime·mcall(SB), NOSPLIT, $0-4
 	MOVL	BX, (g_sched+gobuf_pc)(AX)
 	LEAL	fn+0(FP), BX	// caller's SP
 	MOVL	BX, (g_sched+gobuf_sp)(AX)
-	MOVL	AX, (g_sched+gobuf_g)(AX)
 
 	// switch to m->g0 & its stack, call fn
 	MOVL	g(DX), BX
@@ -432,7 +434,6 @@ TEXT runtime·morestack(SB),NOSPLIT,$0-0
 	// Set g->sched to context in f.
 	MOVL	0(SP), AX	// f's PC
 	MOVL	AX, (g_sched+gobuf_pc)(SI)
-	MOVL	SI, (g_sched+gobuf_g)(SI)
 	LEAL	4(SP), AX	// f's SP
 	MOVL	AX, (g_sched+gobuf_sp)(SI)
 	MOVL	DX, (g_sched+gobuf_ctxt)(SI)

src/runtime/asm_amd64.s

@@ -256,10 +256,13 @@ TEXT runtime·asminit(SB),NOSPLIT,$0-0
 
 // func gogo(buf *gobuf)
 // restore state from Gobuf; longjmp
-TEXT runtime·gogo(SB), NOSPLIT, $16-8
+TEXT runtime·gogo(SB), NOSPLIT, $0-8
 	MOVQ	buf+0(FP), BX		// gobuf
 	MOVQ	gobuf_g(BX), DX
 	MOVQ	0(DX), CX		// make sure g != nil
+	JMP	gogo<>(SB)
+
+TEXT gogo<>(SB), NOSPLIT, $0
 	get_tls(CX)
 	MOVQ	DX, g(CX)
 	MOVQ	DX, R14		// set the g register
@@ -287,7 +290,6 @@ TEXT runtime·mcall(SB), NOSPLIT, $0-8
 	MOVQ	BX, (g_sched+gobuf_pc)(AX)
 	LEAQ	fn+0(FP), BX	// caller's SP
 	MOVQ	BX, (g_sched+gobuf_sp)(AX)
-	MOVQ	AX, (g_sched+gobuf_g)(AX)
 	MOVQ	BP, (g_sched+gobuf_bp)(AX)
 
 	// switch to m->g0 & its stack, call fn
@@ -418,7 +420,6 @@ TEXT runtime·morestack(SB),NOSPLIT,$0-0
 	// Set g->sched to context in f.
 	MOVQ	0(SP), AX // f's PC
 	MOVQ	AX, (g_sched+gobuf_pc)(SI)
-	MOVQ	SI, (g_sched+gobuf_g)(SI)
 	LEAQ	8(SP), AX // f's SP
 	MOVQ	AX, (g_sched+gobuf_sp)(SI)
 	MOVQ	BP, (g_sched+gobuf_bp)(SI)

src/runtime/asm_arm.s

@@ -208,21 +208,14 @@ TEXT runtime·asminit(SB),NOSPLIT,$0-0
 
 // void gogo(Gobuf*)
 // restore state from Gobuf; longjmp
-TEXT runtime·gogo(SB),NOSPLIT,$8-4
+TEXT runtime·gogo(SB),NOSPLIT|NOFRAME,$0-4
 	MOVW	buf+0(FP), R1
 	MOVW	gobuf_g(R1), R0
-	BL	setg<>(SB)
+	MOVW	0(R0), R2	// make sure g != nil
+	B	gogo<>(SB)
 
-	// NOTE: We updated g above, and we are about to update SP.
+TEXT gogo<>(SB),NOSPLIT|NOFRAME,$0
-	// Until LR and PC are also updated, the g/SP/LR/PC quadruple
+	BL	setg<>(SB)
-	// are out of sync and must not be used as the basis of a traceback.
-	// Sigprof skips the traceback when SP is not within g's bounds,
-	// and when the PC is inside this function, runtime.gogo.
-	// Since we are about to update SP, until we complete runtime.gogo
-	// we must not leave this function. In particular, no calls
-	// after this point: it must be straight-line code until the
-	// final B instruction.
-	// See large comment in sigprof for more details.
 	MOVW	gobuf_sp(R1), R13	// restore SP==R13
 	MOVW	gobuf_lr(R1), LR
 	MOVW	gobuf_ret(R1), R0
@@ -246,7 +239,6 @@ TEXT runtime·mcall(SB),NOSPLIT|NOFRAME,$0-4
 	MOVW	LR, (g_sched+gobuf_pc)(g)
 	MOVW	$0, R11
 	MOVW	R11, (g_sched+gobuf_lr)(g)
-	MOVW	g, (g_sched+gobuf_g)(g)
 
 	// Switch to m->g0 & its stack, call fn.
 	MOVW	g, R1
@@ -537,7 +529,6 @@ TEXT gosave_systemstack_switch<>(SB),NOSPLIT|NOFRAME,$0
 	ADD	$4, R11 // get past push {lr}
 	MOVW	R11, (g_sched+gobuf_pc)(g)
 	MOVW	R13, (g_sched+gobuf_sp)(g)
-	MOVW	g, (g_sched+gobuf_g)(g)
 	MOVW	$0, R11
 	MOVW	R11, (g_sched+gobuf_lr)(g)
 	MOVW	R11, (g_sched+gobuf_ret)(g)

src/runtime/asm_arm64.s

@@ -115,12 +115,16 @@ TEXT runtime·asminit(SB),NOSPLIT|NOFRAME,$0-0
 
 // void gogo(Gobuf*)
 // restore state from Gobuf; longjmp
-TEXT runtime·gogo(SB), NOSPLIT, $24-8
+TEXT runtime·gogo(SB), NOSPLIT|NOFRAME, $0-8
 	MOVD	buf+0(FP), R5
-	MOVD	gobuf_g(R5), g
+	MOVD	gobuf_g(R5), R6
+	MOVD	0(R6), R4	// make sure g != nil
+	B	gogo<>(SB)
+
+TEXT gogo<>(SB), NOSPLIT|NOFRAME, $0
+	MOVD	R6, g
 	BL	runtime·save_g(SB)
 
-	MOVD	0(g), R4	// make sure g is not nil
 	MOVD	gobuf_sp(R5), R0
 	MOVD	R0, RSP
 	MOVD	gobuf_bp(R5), R29
@@ -147,7 +151,6 @@ TEXT runtime·mcall(SB), NOSPLIT|NOFRAME, $0-8
 	MOVD	R29, (g_sched+gobuf_bp)(g)
 	MOVD	LR, (g_sched+gobuf_pc)(g)
 	MOVD	$0, (g_sched+gobuf_lr)(g)
-	MOVD	g, (g_sched+gobuf_g)(g)
 
 	// Switch to m->g0 & its stack, call fn.
 	MOVD	g, R3
@@ -861,7 +864,6 @@ TEXT gosave_systemstack_switch<>(SB),NOSPLIT|NOFRAME,$0
 	MOVD	R29, (g_sched+gobuf_bp)(g)
 	MOVD	$0, (g_sched+gobuf_lr)(g)
 	MOVD	$0, (g_sched+gobuf_ret)(g)
-	MOVD	g, (g_sched+gobuf_g)(g)
 	// Assert ctxt is zero. See func save.
 	MOVD	(g_sched+gobuf_ctxt)(g), R0
 	CBZ	R0, 2(PC)

src/runtime/asm_mips64x.s

@@ -91,9 +91,14 @@ TEXT runtime·asminit(SB),NOSPLIT|NOFRAME,$0-0
 
 // void gogo(Gobuf*)
 // restore state from Gobuf; longjmp
-TEXT runtime·gogo(SB), NOSPLIT, $16-8
+TEXT runtime·gogo(SB), NOSPLIT|NOFRAME, $0-8
 	MOVV	buf+0(FP), R3
-	MOVV	gobuf_g(R3), g	// make sure g is not nil
+	MOVV	gobuf_g(R3), R4
+	MOVV	0(R4), R5	// make sure g != nil
+	JMP	gogo<>(SB)
+
+TEXT gogo<>(SB), NOSPLIT|NOFRAME, $0
+	MOVV	R5, g
 	JAL	runtime·save_g(SB)
 
 	MOVV	0(g), R2
@@ -117,7 +122,6 @@ TEXT runtime·mcall(SB), NOSPLIT|NOFRAME, $0-8
 	MOVV	R29, (g_sched+gobuf_sp)(g)
 	MOVV	R31, (g_sched+gobuf_pc)(g)
 	MOVV	R0, (g_sched+gobuf_lr)(g)
-	MOVV	g, (g_sched+gobuf_g)(g)
 
 	// Switch to m->g0 & its stack, call fn.
 	MOVV	g, R1
@@ -404,7 +408,6 @@ TEXT gosave_systemstack_switch<>(SB),NOSPLIT|NOFRAME,$0
 	MOVV	R29, (g_sched+gobuf_sp)(g)
 	MOVV	R0, (g_sched+gobuf_lr)(g)
 	MOVV	R0, (g_sched+gobuf_ret)(g)
-	MOVV	g, (g_sched+gobuf_g)(g)
 	// Assert ctxt is zero. See func save.
 	MOVV	(g_sched+gobuf_ctxt)(g), R1
 	BEQ	R1, 2(PC)

src/runtime/asm_mipsx.s

@@ -92,12 +92,15 @@ TEXT runtime·asminit(SB),NOSPLIT,$0-0
 
 // void gogo(Gobuf*)
 // restore state from Gobuf; longjmp
-TEXT runtime·gogo(SB),NOSPLIT,$8-4
+TEXT runtime·gogo(SB),NOSPLIT|NOFRAME,$0-4
 	MOVW	buf+0(FP), R3
-	MOVW	gobuf_g(R3), g	// make sure g is not nil
+	MOVW	gobuf_g(R3), R4
-	JAL	runtime·save_g(SB)
+	MOVW	0(R4), R5	// make sure g != nil
+	JMP	gogo<>(SB)
 
-	MOVW	0(g), R2
+TEXT gogo<>(SB),NOSPLIT|NOFRAME,$0
+	MOVW	R4, g
+	JAL	runtime·save_g(SB)
 	MOVW	gobuf_sp(R3), R29
 	MOVW	gobuf_lr(R3), R31
 	MOVW	gobuf_ret(R3), R1
@@ -118,7 +121,6 @@ TEXT runtime·mcall(SB),NOSPLIT|NOFRAME,$0-4
 	MOVW	R29, (g_sched+gobuf_sp)(g)
 	MOVW	R31, (g_sched+gobuf_pc)(g)
 	MOVW	R0, (g_sched+gobuf_lr)(g)
-	MOVW	g, (g_sched+gobuf_g)(g)
 
 	// Switch to m->g0 & its stack, call fn.
 	MOVW	g, R1
@@ -404,7 +406,6 @@ TEXT gosave_systemstack_switch<>(SB),NOSPLIT|NOFRAME,$0
 	MOVW	R29, (g_sched+gobuf_sp)(g)
 	MOVW	R0, (g_sched+gobuf_lr)(g)
 	MOVW	R0, (g_sched+gobuf_ret)(g)
-	MOVW	g, (g_sched+gobuf_g)(g)
 	// Assert ctxt is zero. See func save.
 	MOVW	(g_sched+gobuf_ctxt)(g), R1
 	BEQ	R1, 2(PC)

src/runtime/asm_ppc64x.s

@@ -130,12 +130,16 @@ TEXT runtime·reginit(SB),NOSPLIT|NOFRAME,$0-0
 
 // void gogo(Gobuf*)
 // restore state from Gobuf; longjmp
-TEXT runtime·gogo(SB), NOSPLIT, $16-8
+TEXT runtime·gogo(SB), NOSPLIT|NOFRAME, $0-8
 	MOVD	buf+0(FP), R5
-	MOVD	gobuf_g(R5), g	// make sure g is not nil
+	MOVD	gobuf_g(R5), R6
+	MOVD	0(R6), R4	// make sure g != nil
+	BR	gogo<>(SB)
+
+TEXT gogo<>(SB), NOSPLIT|NOFRAME, $0
+	MOVD	R6, g
 	BL	runtime·save_g(SB)
 
-	MOVD	0(g), R4
 	MOVD	gobuf_sp(R5), R1
 	MOVD	gobuf_lr(R5), R31
 #ifndef GOOS_aix
@@ -163,7 +167,6 @@ TEXT runtime·mcall(SB), NOSPLIT|NOFRAME, $0-8
 	MOVD	LR, R31
 	MOVD	R31, (g_sched+gobuf_pc)(g)
 	MOVD	R0, (g_sched+gobuf_lr)(g)
-	MOVD	g, (g_sched+gobuf_g)(g)
 
 	// Switch to m->g0 & its stack, call fn.
 	MOVD	g, R3
@@ -536,7 +539,6 @@ TEXT gosave_systemstack_switch<>(SB),NOSPLIT|NOFRAME,$0
 	MOVD	R1, (g_sched+gobuf_sp)(g)
 	MOVD	R0, (g_sched+gobuf_lr)(g)
 	MOVD	R0, (g_sched+gobuf_ret)(g)
-	MOVD	g, (g_sched+gobuf_g)(g)
 	// Assert ctxt is zero. See func save.
 	MOVD	(g_sched+gobuf_ctxt)(g), R31
 	CMP	R0, R31

src/runtime/asm_riscv64.s

@@ -224,12 +224,16 @@ TEXT runtime·return0(SB), NOSPLIT, $0
 // restore state from Gobuf; longjmp
 
 // func gogo(buf *gobuf)
-TEXT runtime·gogo(SB), NOSPLIT, $16-8
+TEXT runtime·gogo(SB), NOSPLIT|NOFRAME, $0-8
 	MOV	buf+0(FP), T0
-	MOV	gobuf_g(T0), g	// make sure g is not nil
+	MOV	gobuf_g(T0), T1
+	MOV	0(T1), ZERO // make sure g != nil
+	JMP	gogo<>(SB)
+
+TEXT gogo<>(SB), NOSPLIT|NOFRAME, $0
+	MOV	T1, g
 	CALL	runtime·save_g(SB)
 
-	MOV	(g), ZERO // make sure g is not nil
 	MOV	gobuf_sp(T0), X2
 	MOV	gobuf_lr(T0), RA
 	MOV	gobuf_ret(T0), A0
@@ -270,7 +274,6 @@ TEXT runtime·mcall(SB), NOSPLIT|NOFRAME, $0-8
 	MOV	X2, (g_sched+gobuf_sp)(g)
 	MOV	RA, (g_sched+gobuf_pc)(g)
 	MOV	ZERO, (g_sched+gobuf_lr)(g)
-	MOV	g, (g_sched+gobuf_g)(g)
 
 	// Switch to m->g0 & its stack, call fn.
 	MOV	g, T0
@@ -300,7 +303,6 @@ TEXT gosave_systemstack_switch<>(SB),NOSPLIT|NOFRAME,$0
 	MOV	X2, (g_sched+gobuf_sp)(g)
 	MOV	ZERO, (g_sched+gobuf_lr)(g)
 	MOV	ZERO, (g_sched+gobuf_ret)(g)
-	MOV	g, (g_sched+gobuf_g)(g)
 	// Assert ctxt is zero. See func save.
 	MOV	(g_sched+gobuf_ctxt)(g), X31
 	BEQ	ZERO, X31, 2(PC)

src/runtime/asm_s390x.s

@@ -176,9 +176,14 @@ TEXT runtime·asminit(SB),NOSPLIT|NOFRAME,$0-0
 
 // void gogo(Gobuf*)
 // restore state from Gobuf; longjmp
-TEXT runtime·gogo(SB), NOSPLIT, $16-8
+TEXT runtime·gogo(SB), NOSPLIT|NOFRAME, $0-8
 	MOVD	buf+0(FP), R5
-	MOVD	gobuf_g(R5), g	// make sure g is not nil
+	MOVD	gobuf_g(R5), R6
+	MOVD	0(R6), R7	// make sure g != nil
+	BR	gogo<>(SB)
+
+TEXT gogo<>(SB), NOSPLIT|NOFRAME, $0
+	MOVD	R6, g
 	BL	runtime·save_g(SB)
 
 	MOVD	0(g), R4
@@ -203,7 +208,6 @@ TEXT runtime·mcall(SB), NOSPLIT, $-8-8
 	MOVD	R15, (g_sched+gobuf_sp)(g)
 	MOVD	LR, (g_sched+gobuf_pc)(g)
 	MOVD	$0, (g_sched+gobuf_lr)(g)
-	MOVD	g, (g_sched+gobuf_g)(g)
 
 	// Switch to m->g0 & its stack, call fn.
 	MOVD	g, R3
@@ -500,7 +504,6 @@ TEXT gosave_systemstack_switch<>(SB),NOSPLIT|NOFRAME,$0
 	MOVD	R15, (g_sched+gobuf_sp)(g)
 	MOVD	$0, (g_sched+gobuf_lr)(g)
 	MOVD	$0, (g_sched+gobuf_ret)(g)
-	MOVD	g, (g_sched+gobuf_g)(g)
 	// Assert ctxt is zero. See func save.
 	MOVD	(g_sched+gobuf_ctxt)(g), R1
 	CMPBEQ	R1, $0, 2(PC)

src/runtime/asm_wasm.s

@@ -34,7 +34,9 @@ TEXT ·checkASM(SB), NOSPLIT, $0-1
 
 TEXT runtime·gogo(SB), NOSPLIT, $0-8
 	MOVD buf+0(FP), R0
-	MOVD gobuf_g(R0), g
+	MOVD gobuf_g(R0), R1
+	MOVD 0(R1), R2	// make sure g != nil
+	MOVD R1, g
 	MOVD gobuf_sp(R0), SP
 
 	// Put target PC at -8(SP), wasm_pc_f_loop will pick it up
@@ -69,7 +71,6 @@ TEXT runtime·mcall(SB), NOSPLIT, $0-8
 	// save state in g->sched
 	MOVD 0(SP), g_sched+gobuf_pc(g)     // caller's PC
 	MOVD $fn+0(FP), g_sched+gobuf_sp(g) // caller's SP
-	MOVD g, g_sched+gobuf_g(g)
 
 	// if g == g0 call badmcall
 	Get g
@@ -143,7 +144,6 @@ TEXT runtime·systemstack(SB), NOSPLIT, $0-8
 	MOVD $runtime·systemstack_switch(SB), g_sched+gobuf_pc(g)
 
 	MOVD SP, g_sched+gobuf_sp(g)
-	MOVD g, g_sched+gobuf_g(g)
 
 	// switch to g0
 	MOVD R2, g
@@ -270,7 +270,6 @@ TEXT runtime·morestack(SB), NOSPLIT, $0-0
 
 	// Set g->sched to context in f.
 	MOVD 0(SP), g_sched+gobuf_pc(g)
-	MOVD g, g_sched+gobuf_g(g)
 	MOVD $8(SP), g_sched+gobuf_sp(g) // f's SP
 	MOVD CTXT, g_sched+gobuf_ctxt(g)
 

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
+	// Set up m.g0.sched as a label returning returning to just
-	// for terminating the thread.
+	// 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.