From: Russ Cox Date: Mon, 15 Feb 2021 14:25:55 +0000 (-0500) Subject: runtime: remove unnecessary writes to gp.sched.g X-Git-Tag: go1.17beta1~1519 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=aa0388f2ed937669e9f938da8a65c75ea144ebfd;p=gostls13.git 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 Reviewed-by: Cherry Zhang --- diff --git a/src/runtime/asm_386.s b/src/runtime/asm_386.s index a59054226c..fcf74a03cf 100644 --- a/src/runtime/asm_386.s +++ b/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) diff --git a/src/runtime/asm_amd64.s b/src/runtime/asm_amd64.s index cd5ce3effb..8ee2ac2123 100644 --- a/src/runtime/asm_amd64.s +++ b/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) diff --git a/src/runtime/asm_arm.s b/src/runtime/asm_arm.s index c8c53e70db..92d7854306 100644 --- a/src/runtime/asm_arm.s +++ b/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. - // Until LR and PC are also updated, the g/SP/LR/PC quadruple - // 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. +TEXT gogo<>(SB),NOSPLIT|NOFRAME,$0 + BL setg<>(SB) 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) diff --git a/src/runtime/asm_arm64.s b/src/runtime/asm_arm64.s index 31a6fe57b9..4f0a680fa4 100644 --- a/src/runtime/asm_arm64.s +++ b/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) diff --git a/src/runtime/asm_mips64x.s b/src/runtime/asm_mips64x.s index 75bb223066..f6d8931a15 100644 --- a/src/runtime/asm_mips64x.s +++ b/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) diff --git a/src/runtime/asm_mipsx.s b/src/runtime/asm_mipsx.s index 341da8e8d7..cf4b1b42cc 100644 --- a/src/runtime/asm_mipsx.s +++ b/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 - JAL runtime·save_g(SB) + MOVW gobuf_g(R3), R4 + 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) diff --git a/src/runtime/asm_ppc64x.s b/src/runtime/asm_ppc64x.s index a99a61fd88..90f14d8e54 100644 --- a/src/runtime/asm_ppc64x.s +++ b/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 diff --git a/src/runtime/asm_riscv64.s b/src/runtime/asm_riscv64.s index fb7c6530dc..d06c77b948 100644 --- a/src/runtime/asm_riscv64.s +++ b/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) diff --git a/src/runtime/asm_s390x.s b/src/runtime/asm_s390x.s index 43244d961f..203754f32c 100644 --- a/src/runtime/asm_s390x.s +++ b/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) diff --git a/src/runtime/asm_wasm.s b/src/runtime/asm_wasm.s index cf3d961b74..3765c756b3 100644 --- a/src/runtime/asm_wasm.s +++ b/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) 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.