]> Cypherpunks.ru repositories - gostls13.git/commitdiff
runtime: make throw safer to call
authorAustin Clements <austin@google.com>
Fri, 12 Jan 2018 17:39:22 +0000 (12:39 -0500)
committerAustin Clements <austin@google.com>
Thu, 8 Mar 2018 22:55:52 +0000 (22:55 +0000)
Currently, throw may grow the stack, which means whenever we call it
from a context where it's not safe to grow the stack, we first have to
switch to the system stack. This is pretty easy to get wrong.

Fix this by making throw switch to the system stack so it doesn't grow
the stack and is hence safe to call without a system stack switch at
the call site.

The only thing this complicates is badsystemstack itself, which would
now go into an infinite loop before printing anything (previously it
would also go into an infinite loop, but would at least print the
error first). Fix this by making badsystemstack do a direct write and
then crash hard.

Change-Id: Ic5b4a610df265e47962dcfa341cabac03c31c049
Reviewed-on: https://go-review.googlesource.com/93659
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
14 files changed:
src/runtime/asm_386.s
src/runtime/asm_amd64.s
src/runtime/asm_amd64p32.s
src/runtime/asm_arm.s
src/runtime/asm_arm64.s
src/runtime/asm_mips64x.s
src/runtime/asm_mipsx.s
src/runtime/asm_ppc64x.s
src/runtime/asm_s390x.s
src/runtime/cgocheck.go
src/runtime/panic.go
src/runtime/proc.go
src/runtime/stack.go
src/runtime/stubs.go

index d1935d28da07ca41ea9d08b05815f861d9bbabe7..6cea84837432a729e84729e5f538f5594ced84c3 100644 (file)
@@ -479,6 +479,7 @@ bad:
        // Hide call from linker nosplit analysis.
        MOVL    $runtime·badsystemstack(SB), AX
        CALL    AX
+       INT     $3
 
 /*
  * support for morestack
index ab5407bbcd0692a8b38c348e3f01212b8c38d124..953f1181464336355476dd469c510298c1a57f21 100644 (file)
@@ -424,6 +424,7 @@ bad:
        // Bad: g is not gsignal, not g0, not curg. What is it?
        MOVQ    $runtime·badsystemstack(SB), AX
        CALL    AX
+       INT     $3
 
 
 /*
index 0c104b23e73665cef0ff1d80ebdf5bc3774785f4..1fbc6c42188fe94367c5713298532886443db1d7 100644 (file)
@@ -310,6 +310,7 @@ bad:
        // Hide call from linker nosplit analysis.
        MOVL    $runtime·badsystemstack(SB), AX
        CALL    AX
+       INT     $3
 
 /*
  * support for morestack
index d54dc62ba4208e7ae0ca9ce09d8882691838f5cf..c51e0f0b7826c409f2e35853efe8bf97839b1679 100644 (file)
@@ -317,6 +317,7 @@ TEXT runtime·systemstack(SB),NOSPLIT,$0-4
        // Hide call from linker nosplit analysis.
        MOVW    $runtime·badsystemstack(SB), R0
        BL      (R0)
+       B       runtime·abort(SB)
 
 switch:
        // save our state in g->sched. Pretend to
index ef32beecb5b73f5f1570c2c45604eca0be7bb292..e88532728a383ea06e1e6d2b2568dea8fc54d4bd 100644 (file)
@@ -201,6 +201,7 @@ TEXT runtime·systemstack(SB), NOSPLIT, $0-8
        // Hide call from linker nosplit analysis.
        MOVD    $runtime·badsystemstack(SB), R3
        BL      (R3)
+       B       runtime·abort(SB)
 
 switch:
        // save our state in g->sched. Pretend to
index 00a7951fc1171c07a66d2eee0d4715bb01c24baf..e4a5a32ad0f99f8a859ada04a53b658d18f7ccc6 100644 (file)
@@ -179,6 +179,7 @@ TEXT runtime·systemstack(SB), NOSPLIT, $0-8
        // Hide call from linker nosplit analysis.
        MOVV    $runtime·badsystemstack(SB), R4
        JAL     (R4)
+       JAL     runtime·abort(SB)
 
 switch:
        // save our state in g->sched. Pretend to
index 0eb9bb1e6cdeb65f3ee8b0c4dc37ddca7a06f676..ef63f4128927d0ee6fda4ef151b4a4de4c66459f 100644 (file)
@@ -180,6 +180,7 @@ TEXT runtime·systemstack(SB),NOSPLIT,$0-4
        // Hide call from linker nosplit analysis.
        MOVW    $runtime·badsystemstack(SB), R4
        JAL     (R4)
+       JAL     runtime·abort(SB)
 
 switch:
        // save our state in g->sched.  Pretend to
index fef603dc300999757dc8d261f90c32567973364b..b565a1370a613aed3cb48b0079ef3b4c2c02149d 100644 (file)
@@ -222,6 +222,7 @@ TEXT runtime·systemstack(SB), NOSPLIT, $0-8
        MOVD    $runtime·badsystemstack(SB), R12
        MOVD    R12, CTR
        BL      (CTR)
+       BL      runtime·abort(SB)
 
 switch:
        // save our state in g->sched. Pretend to
index 1c7e44cdae4c6c75044a2ef90c1af149ab48ad54..1d8c4032e50b358d8060973cdbd6a0c6faee262b 100644 (file)
@@ -266,6 +266,7 @@ TEXT runtime·systemstack(SB), NOSPLIT, $0-8
        // Hide call from linker nosplit analysis.
        MOVD    $runtime·badsystemstack(SB), R3
        BL      (R3)
+       BL      runtime·abort(SB)
 
 switch:
        // save our state in g->sched.  Pretend to
index 95f6522e94172453b1fe135c32045ce6a1e70f79..73cb6ecae2b49dd87ea65700b3b09524f0506bc3 100644 (file)
@@ -148,9 +148,7 @@ func cgoCheckTypedBlock(typ *_type, src unsafe.Pointer, off, size uintptr) {
                if i >= off && bits&bitPointer != 0 {
                        v := *(*unsafe.Pointer)(add(src, i))
                        if cgoIsGoPointer(v) {
-                               systemstack(func() {
-                                       throw(cgoWriteBarrierFail)
-                               })
+                               throw(cgoWriteBarrierFail)
                        }
                }
                hbits = hbits.next()
@@ -183,9 +181,7 @@ func cgoCheckBits(src unsafe.Pointer, gcbits *byte, off, size uintptr) {
                        if bits&1 != 0 {
                                v := *(*unsafe.Pointer)(add(src, i))
                                if cgoIsGoPointer(v) {
-                                       systemstack(func() {
-                                               throw(cgoWriteBarrierFail)
-                                       })
+                                       throw(cgoWriteBarrierFail)
                                }
                        }
                }
index cd2b18cc51f70fd16d90d02128048dfeceeb96b3..d9fa512530ba1350380aaebe7596b8fc69b9679f 100644 (file)
@@ -586,7 +586,11 @@ func sync_throw(s string) {
 
 //go:nosplit
 func throw(s string) {
-       print("fatal error: ", s, "\n")
+       // Everything throw does should be recursively nosplit so it
+       // can be called even when it's unsafe to grow the stack.
+       systemstack(func() {
+               print("fatal error: ", s, "\n")
+       })
        gp := getg()
        if gp.m.throwing == 0 {
                gp.m.throwing = 1
index c3c64ebfafd1566b9f750a577cd264845a98ad99..9ed8c14e7a5e69f92eb437b181ceabe3d1d4ba54 100644 (file)
@@ -796,9 +796,7 @@ func casgstatus(gp *g, oldval, newval uint32) {
        // GC time to finish and change the state to oldval.
        for i := 0; !atomic.Cas(&gp.atomicstatus, oldval, newval); i++ {
                if oldval == _Gwaiting && gp.atomicstatus == _Grunnable {
-                       systemstack(func() {
-                               throw("casgstatus: waiting for Gwaiting but is Grunnable")
-                       })
+                       throw("casgstatus: waiting for Gwaiting but is Grunnable")
                }
                // Help GC if needed.
                // if gp.preemptscan && !gp.gcworkdone && (oldval == _Grunning || oldval == _Gsyscall) {
@@ -2925,21 +2923,14 @@ func exitsyscall(dummy int32) {
 
        _g_.m.locks++ // see comment in entersyscall
        if getcallersp(unsafe.Pointer(&dummy)) > _g_.syscallsp {
-               // throw calls print which may try to grow the stack,
-               // but throwsplit == true so the stack can not be grown;
-               // use systemstack to avoid that possible problem.
-               systemstack(func() {
-                       throw("exitsyscall: syscall frame is no longer valid")
-               })
+               throw("exitsyscall: syscall frame is no longer valid")
        }
 
        _g_.waitsince = 0
        oldp := _g_.m.p.ptr()
        if exitsyscallfast() {
                if _g_.m.mcache == nil {
-                       systemstack(func() {
-                               throw("lost mcache")
-                       })
+                       throw("lost mcache")
                }
                if trace.enabled {
                        if oldp != _g_.m.p.ptr() || _g_.m.syscalltick != _g_.m.p.ptr().syscalltick {
@@ -2986,9 +2977,7 @@ func exitsyscall(dummy int32) {
        mcall(exitsyscall0)
 
        if _g_.m.mcache == nil {
-               systemstack(func() {
-                       throw("lost mcache")
-               })
+               throw("lost mcache")
        }
 
        // Scheduler returned, so we're allowed to run now.
index b5dda0d9e61225e18eb3519e34d5a35e5d7d1a9a..5a6259c6e2f89812fbdf05ae01caf8466266ffdf 100644 (file)
@@ -1184,7 +1184,5 @@ func freeStackSpans() {
 
 //go:nosplit
 func morestackc() {
-       systemstack(func() {
-               throw("attempt to execute system stack code on user stack")
-       })
+       throw("attempt to execute system stack code on user stack")
 }
index e83064166ae6a901838c53bf77ed8097c57b0393..6019005fbe211ff49e1ddc0412a1dc64028f8f5f 100644 (file)
@@ -53,8 +53,13 @@ func mcall(fn func(*g))
 //go:noescape
 func systemstack(fn func())
 
+var badsystemstackMsg = "fatal: systemstack called from unexpected goroutine"
+
+//go:nosplit
+//go:nowritebarrierrec
 func badsystemstack() {
-       throw("systemstack called from unexpected goroutine")
+       sp := stringStructOf(&badsystemstackMsg)
+       write(2, sp.str, int32(sp.len))
 }
 
 // memclrNoHeapPointers clears n bytes starting at ptr.