runtime: make morestack less subtle

morestack writes the context pointer to gobuf.ctxt, but since
morestack is written in assembly (and has to be very careful with
state), it does *not* invoke the requisite write barrier for this
write. Instead, we patch this up later, in newstack, where we invoke
an explicit write barrier for ctxt.

This already requires some subtle reasoning, and it's going to get a
lot hairier with the hybrid barrier.

Fix this by simplifying the whole mechanism. Instead of writing
gobuf.ctxt in morestack, just pass the value of the context register
to newstack and let it write it to gobuf.ctxt. This is a normal Go
pointer write, so it gets the normal Go write barrier. No subtle
reasoning required.

Updates #17503.

Change-Id: Ia6bf8459bfefc6828f53682ade32c02412e4db63
Reviewed-on: https://go-review.googlesource.com/31550
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

diff --git a/src/runtime/asm_386.s b/src/runtime/asm_386.s
index 4ef738eacbf1664eec52c7cb69830a443888f35c..67b4cab77e5ae1a0204dfad87fbec53ba64ee60f 100644 (file)
--- a/src/runtime/asm_386.s
+++ b/src/runtime/asm_386.s
@@ -381,7 +381,7 @@ TEXT runtime·morestack(SB),NOSPLIT,$0-0
        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)
+       // newstack will fill gobuf.ctxt.
 
        // Call newstack on m->g0's stack.
        MOVL    m_g0(BX), BP
@@ -389,8 +389,10 @@ TEXT runtime·morestack(SB),NOSPLIT,$0-0
        MOVL    (g_sched+gobuf_sp)(BP), AX
        MOVL    -4(AX), BX      // fault if CALL would, before smashing SP
        MOVL    AX, SP
+       PUSHL   DX      // ctxt argument
        CALL    runtime·newstack(SB)
        MOVL    $0, 0x1003      // crash if newstack returns
+       POPL    DX      // keep balance check happy
        RET
 
 TEXT runtime·morestack_noctxt(SB),NOSPLIT,$0-0

diff --git a/src/runtime/asm_amd64.s b/src/runtime/asm_amd64.s
index 34da3bda9f4ec8ce86f960c69a79de864706edf0..398b14888f7bd1ff00bd9eae9db6343daaee6b73 100644 (file)
--- a/src/runtime/asm_amd64.s
+++ b/src/runtime/asm_amd64.s
@@ -358,15 +358,17 @@ TEXT runtime·morestack(SB),NOSPLIT,$0-0
        MOVQ    SI, (g_sched+gobuf_g)(SI)
        LEAQ    8(SP), AX // f's SP
        MOVQ    AX, (g_sched+gobuf_sp)(SI)
-       MOVQ    DX, (g_sched+gobuf_ctxt)(SI)
        MOVQ    BP, (g_sched+gobuf_bp)(SI)
+       // newstack will fill gobuf.ctxt.
 
        // Call newstack on m->g0's stack.
        MOVQ    m_g0(BX), BX
        MOVQ    BX, g(CX)
        MOVQ    (g_sched+gobuf_sp)(BX), SP
+       PUSHQ   DX      // ctxt argument
        CALL    runtime·newstack(SB)
        MOVQ    $0, 0x1003      // crash if newstack returns
+       POPQ    DX      // keep balance check happy
        RET
 
 // morestack but not preserving ctxt.

diff --git a/src/runtime/asm_amd64p32.s b/src/runtime/asm_amd64p32.s
index 4e3c0cd2b67f017077ae9b5aa6a50691a51347ef..fab6c0db5d73b911588f790bb0cbf95c5ee02ecf 100644 (file)
--- a/src/runtime/asm_amd64p32.s
+++ b/src/runtime/asm_amd64p32.s
@@ -276,14 +276,16 @@ TEXT runtime·morestack(SB),NOSPLIT,$0-0
        MOVL    SI, (g_sched+gobuf_g)(SI)
        LEAL    8(SP), AX // f's SP
        MOVL    AX, (g_sched+gobuf_sp)(SI)
-       MOVL    DX, (g_sched+gobuf_ctxt)(SI)
+       // newstack will fill gobuf.ctxt.
 
        // Call newstack on m->g0's stack.
        MOVL    m_g0(BX), BX
        MOVL    BX, g(CX)
        MOVL    (g_sched+gobuf_sp)(BX), SP
+       PUSHQ   DX      // ctxt argument
        CALL    runtime·newstack(SB)
        MOVL    $0, 0x1003      // crash if newstack returns
+       POPQ    DX      // keep balance check happy
        RET
 
 // morestack trampolines

diff --git a/src/runtime/asm_arm.s b/src/runtime/asm_arm.s
index 0c7d580163c0366e7684a40de8149d71a2311ccb..3bfa250c99ec3d4f37451cb779f1743ef810ce79 100644 (file)
--- a/src/runtime/asm_arm.s
+++ b/src/runtime/asm_arm.s
@@ -294,10 +294,10 @@ TEXT runtime·morestack(SB),NOSPLIT,$-4-0
 
        // Called from f.
        // Set g->sched to context in f.
-       MOVW    R7, (g_sched+gobuf_ctxt)(g)
        MOVW    R13, (g_sched+gobuf_sp)(g)
        MOVW    LR, (g_sched+gobuf_pc)(g)
        MOVW    R3, (g_sched+gobuf_lr)(g)
+       // newstack will fill gobuf.ctxt.
 
        // Called from f.
        // Set m->morebuf to f's caller.
@@ -310,6 +310,9 @@ TEXT runtime·morestack(SB),NOSPLIT,$-4-0
        MOVW    m_g0(R8), R0
        BL      setg<>(SB)
        MOVW    (g_sched+gobuf_sp)(g), R13
+       MOVW    $0, R0
+       MOVW.W  R0, -8(R13)     // create a call frame on g0
+       MOVW    R7, 4(R13)      // ctxt argument
        BL      runtime·newstack(SB)
 
        // Not reached, but make sure the return PC from the call to newstack

diff --git a/src/runtime/asm_arm64.s b/src/runtime/asm_arm64.s
index bd2b18385e479804dafe34286e911f5713a82451..2d73052c23ee19be65106fb8fc093b5013fdacae 100644 (file)
--- a/src/runtime/asm_arm64.s
+++ b/src/runtime/asm_arm64.s
@@ -269,11 +269,11 @@ TEXT runtime·morestack(SB),NOSPLIT,$-8-0
 
        // Called from f.
        // Set g->sched to context in f
-       MOVD    R26, (g_sched+gobuf_ctxt)(g)
        MOVD    RSP, R0
        MOVD    R0, (g_sched+gobuf_sp)(g)
        MOVD    LR, (g_sched+gobuf_pc)(g)
        MOVD    R3, (g_sched+gobuf_lr)(g)
+       // newstack will fill gobuf.ctxt.
 
        // Called from f.
        // Set m->morebuf to f's callers.
@@ -287,6 +287,8 @@ TEXT runtime·morestack(SB),NOSPLIT,$-8-0
        BL      runtime·save_g(SB)
        MOVD    (g_sched+gobuf_sp)(g), R0
        MOVD    R0, RSP
+       MOVD.W  $0, -16(RSP)    // create a call frame on g0
+       MOVD    R26, 8(RSP)     // ctxt argument
        BL      runtime·newstack(SB)
 
        // Not reached, but make sure the return PC from the call to newstack

diff --git a/src/runtime/asm_mips64x.s b/src/runtime/asm_mips64x.s
index e29522367d5671c5dee45378c9ae53c7912d8181..79378df22ceba4bd009a679f6f11fcb8591ae4d9 100644 (file)
--- a/src/runtime/asm_mips64x.s
+++ b/src/runtime/asm_mips64x.s
@@ -243,10 +243,10 @@ TEXT runtime·morestack(SB),NOSPLIT,$-8-0
 
        // Called from f.
        // Set g->sched to context in f.
-       MOVV    REGCTXT, (g_sched+gobuf_ctxt)(g)
        MOVV    R29, (g_sched+gobuf_sp)(g)
        MOVV    R31, (g_sched+gobuf_pc)(g)
        MOVV    R3, (g_sched+gobuf_lr)(g)
+       // newstack will fill gobuf.ctxt.
 
        // Called from f.
        // Set m->morebuf to f's caller.
@@ -258,6 +258,10 @@ TEXT runtime·morestack(SB),NOSPLIT,$-8-0
        MOVV    m_g0(R7), g
        JAL     runtime·save_g(SB)
        MOVV    (g_sched+gobuf_sp)(g), R29
+       // Create a stack frame on g0 to call newstack.
+       MOVV    R0, -16(R29)    // Zero saved LR in frame
+       ADDV    $-16, R29
+       MOVV    REGCTXT, 8(R29) // ctxt argument
        JAL     runtime·newstack(SB)
 
        // Not reached, but make sure the return PC from the call to newstack

diff --git a/src/runtime/asm_ppc64x.s b/src/runtime/asm_ppc64x.s
index 85f73a88b49629d78eb8f412d4e60d78ae13e0ab..1ce7b2d90335502a4def267fc16a3e01942ff919 100644 (file)
--- a/src/runtime/asm_ppc64x.s
+++ b/src/runtime/asm_ppc64x.s
@@ -297,11 +297,11 @@ TEXT runtime·morestack(SB),NOSPLIT|NOFRAME,$0-0
 
        // Called from f.
        // Set g->sched to context in f.
-       MOVD    R11, (g_sched+gobuf_ctxt)(g)
        MOVD    R1, (g_sched+gobuf_sp)(g)
        MOVD    LR, R8
        MOVD    R8, (g_sched+gobuf_pc)(g)
        MOVD    R5, (g_sched+gobuf_lr)(g)
+       // newstack will fill gobuf.ctxt.
 
        // Called from f.
        // Set m->morebuf to f's caller.
@@ -313,6 +313,8 @@ TEXT runtime·morestack(SB),NOSPLIT|NOFRAME,$0-0
        MOVD    m_g0(R7), g
        BL      runtime·save_g(SB)
        MOVD    (g_sched+gobuf_sp)(g), R1
+       MOVDU   R0, -(FIXED_FRAME+8)(R1)        // create a call frame on g0
+       MOVD    R11, FIXED_FRAME+0(R1)  // ctxt argument
        BL      runtime·newstack(SB)
 
        // Not reached, but make sure the return PC from the call to newstack

diff --git a/src/runtime/asm_s390x.s b/src/runtime/asm_s390x.s
index d8f529ef906e85bb950a8b4c7743d2496406f937..198c565b79b29e886c3e8a56ed1e748c650f9d69 100644 (file)
--- a/src/runtime/asm_s390x.s
+++ b/src/runtime/asm_s390x.s
@@ -254,11 +254,11 @@ TEXT runtime·morestack(SB),NOSPLIT|NOFRAME,$0-0
 
        // Called from f.
        // Set g->sched to context in f.
-       MOVD    R12, (g_sched+gobuf_ctxt)(g)
        MOVD    R15, (g_sched+gobuf_sp)(g)
        MOVD    LR, R8
        MOVD    R8, (g_sched+gobuf_pc)(g)
        MOVD    R5, (g_sched+gobuf_lr)(g)
+       // newstack will fill gobuf.ctxt.
 
        // Called from f.
        // Set m->morebuf to f's caller.
@@ -270,6 +270,10 @@ TEXT runtime·morestack(SB),NOSPLIT|NOFRAME,$0-0
        MOVD    m_g0(R7), g
        BL      runtime·save_g(SB)
        MOVD    (g_sched+gobuf_sp)(g), R15
+       // Create a stack frame on g0 to call newstack.
+       MOVD    $0, -16(R15)    // Zero saved LR in frame
+       SUB     $16, R15
+       MOVD    R12, 8(R15)     // ctxt argument
        BL      runtime·newstack(SB)
 
        // Not reached, but make sure the return PC from the call to newstack

diff --git a/src/runtime/stack.go b/src/runtime/stack.go
index 90db4204a9919b7188fcf0c7910db714b6d6691c..49499d4433b488d22d1a032ddba689998f795bfd 100644 (file)
--- a/src/runtime/stack.go
+++ b/src/runtime/stack.go
@@ -925,7 +925,10 @@ func round2(x int32) int32 {
 //
 // g->atomicstatus will be Grunning or Gscanrunning upon entry.
 // If the GC is trying to stop this g then it will set preemptscan to true.
-func newstack() {
+//
+// ctxt is the value of the context register on morestack. newstack
+// will write it to g.sched.ctxt.
+func newstack(ctxt unsafe.Pointer) {
        thisg := getg()
        // TODO: double check all gp. shouldn't be getg().
        if thisg.m.morebuf.g.ptr().stackguard0 == stackFork {
@@ -937,8 +940,13 @@ func newstack() {
                traceback(morebuf.pc, morebuf.sp, morebuf.lr, morebuf.g.ptr())
                throw("runtime: wrong goroutine in newstack")
        }
+
+       gp := thisg.m.curg
+       // Write ctxt to gp.sched. We do this here instead of in
+       // morestack so it has the necessary write barrier.
+       gp.sched.ctxt = ctxt
+
        if thisg.m.curg.throwsplit {
-               gp := thisg.m.curg
                // Update syscallsp, syscallpc in case traceback uses them.
                morebuf := thisg.m.morebuf
                gp.syscallsp = morebuf.sp
@@ -951,7 +959,6 @@ func newstack() {
                throw("runtime: stack split at bad time")
        }
 
-       gp := thisg.m.curg
        morebuf := thisg.m.morebuf
        thisg.m.morebuf.pc = 0
        thisg.m.morebuf.lr = 0
@@ -1003,14 +1010,6 @@ func newstack() {
                throw("runtime: split stack overflow")
        }
 
-       if gp.sched.ctxt != nil {
-               // morestack wrote sched.ctxt on its way in here,
-               // without a write barrier. Run the write barrier now.
-               // It is not possible to be preempted between then
-               // and now, so it's okay.
-               writebarrierptr_nostore((*uintptr)(unsafe.Pointer(&gp.sched.ctxt)), uintptr(gp.sched.ctxt))
-       }
-
        if preempt {
                if gp == thisg.m.g0 {
                        throw("runtime: preempt g0")