runtime: make throw safer to call

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>
This commit is contained in:
Austin Clements 2018-01-12 12:39:22 -05:00
parent 9d59234cbe
commit 7f1b2738bb
14 changed files with 27 additions and 26 deletions

View file

@ -479,6 +479,7 @@ bad:
// Hide call from linker nosplit analysis. // Hide call from linker nosplit analysis.
MOVL $runtime·badsystemstack(SB), AX MOVL $runtime·badsystemstack(SB), AX
CALL AX CALL AX
INT $3
/* /*
* support for morestack * support for morestack

View file

@ -424,6 +424,7 @@ bad:
// Bad: g is not gsignal, not g0, not curg. What is it? // Bad: g is not gsignal, not g0, not curg. What is it?
MOVQ $runtime·badsystemstack(SB), AX MOVQ $runtime·badsystemstack(SB), AX
CALL AX CALL AX
INT $3
/* /*

View file

@ -310,6 +310,7 @@ bad:
// Hide call from linker nosplit analysis. // Hide call from linker nosplit analysis.
MOVL $runtime·badsystemstack(SB), AX MOVL $runtime·badsystemstack(SB), AX
CALL AX CALL AX
INT $3
/* /*
* support for morestack * support for morestack

View file

@ -317,6 +317,7 @@ TEXT runtime·systemstack(SB),NOSPLIT,$0-4
// Hide call from linker nosplit analysis. // Hide call from linker nosplit analysis.
MOVW $runtime·badsystemstack(SB), R0 MOVW $runtime·badsystemstack(SB), R0
BL (R0) BL (R0)
B runtime·abort(SB)
switch: switch:
// save our state in g->sched. Pretend to // save our state in g->sched. Pretend to

View file

@ -201,6 +201,7 @@ TEXT runtime·systemstack(SB), NOSPLIT, $0-8
// Hide call from linker nosplit analysis. // Hide call from linker nosplit analysis.
MOVD $runtime·badsystemstack(SB), R3 MOVD $runtime·badsystemstack(SB), R3
BL (R3) BL (R3)
B runtime·abort(SB)
switch: switch:
// save our state in g->sched. Pretend to // save our state in g->sched. Pretend to

View file

@ -179,6 +179,7 @@ TEXT runtime·systemstack(SB), NOSPLIT, $0-8
// Hide call from linker nosplit analysis. // Hide call from linker nosplit analysis.
MOVV $runtime·badsystemstack(SB), R4 MOVV $runtime·badsystemstack(SB), R4
JAL (R4) JAL (R4)
JAL runtime·abort(SB)
switch: switch:
// save our state in g->sched. Pretend to // save our state in g->sched. Pretend to

View file

@ -180,6 +180,7 @@ TEXT runtime·systemstack(SB),NOSPLIT,$0-4
// Hide call from linker nosplit analysis. // Hide call from linker nosplit analysis.
MOVW $runtime·badsystemstack(SB), R4 MOVW $runtime·badsystemstack(SB), R4
JAL (R4) JAL (R4)
JAL runtime·abort(SB)
switch: switch:
// save our state in g->sched. Pretend to // save our state in g->sched. Pretend to

View file

@ -222,6 +222,7 @@ TEXT runtime·systemstack(SB), NOSPLIT, $0-8
MOVD $runtime·badsystemstack(SB), R12 MOVD $runtime·badsystemstack(SB), R12
MOVD R12, CTR MOVD R12, CTR
BL (CTR) BL (CTR)
BL runtime·abort(SB)
switch: switch:
// save our state in g->sched. Pretend to // save our state in g->sched. Pretend to

View file

@ -266,6 +266,7 @@ TEXT runtime·systemstack(SB), NOSPLIT, $0-8
// Hide call from linker nosplit analysis. // Hide call from linker nosplit analysis.
MOVD $runtime·badsystemstack(SB), R3 MOVD $runtime·badsystemstack(SB), R3
BL (R3) BL (R3)
BL runtime·abort(SB)
switch: switch:
// save our state in g->sched. Pretend to // save our state in g->sched. Pretend to

View file

@ -148,9 +148,7 @@ func cgoCheckTypedBlock(typ *_type, src unsafe.Pointer, off, size uintptr) {
if i >= off && bits&bitPointer != 0 { if i >= off && bits&bitPointer != 0 {
v := *(*unsafe.Pointer)(add(src, i)) v := *(*unsafe.Pointer)(add(src, i))
if cgoIsGoPointer(v) { if cgoIsGoPointer(v) {
systemstack(func() {
throw(cgoWriteBarrierFail) throw(cgoWriteBarrierFail)
})
} }
} }
hbits = hbits.next() hbits = hbits.next()
@ -183,9 +181,7 @@ func cgoCheckBits(src unsafe.Pointer, gcbits *byte, off, size uintptr) {
if bits&1 != 0 { if bits&1 != 0 {
v := *(*unsafe.Pointer)(add(src, i)) v := *(*unsafe.Pointer)(add(src, i))
if cgoIsGoPointer(v) { if cgoIsGoPointer(v) {
systemstack(func() {
throw(cgoWriteBarrierFail) throw(cgoWriteBarrierFail)
})
} }
} }
} }

View file

@ -586,7 +586,11 @@ func sync_throw(s string) {
//go:nosplit //go:nosplit
func throw(s string) { func throw(s string) {
// 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") print("fatal error: ", s, "\n")
})
gp := getg() gp := getg()
if gp.m.throwing == 0 { if gp.m.throwing == 0 {
gp.m.throwing = 1 gp.m.throwing = 1

View file

@ -796,9 +796,7 @@ func casgstatus(gp *g, oldval, newval uint32) {
// GC time to finish and change the state to oldval. // GC time to finish and change the state to oldval.
for i := 0; !atomic.Cas(&gp.atomicstatus, oldval, newval); i++ { for i := 0; !atomic.Cas(&gp.atomicstatus, oldval, newval); i++ {
if oldval == _Gwaiting && gp.atomicstatus == _Grunnable { 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. // Help GC if needed.
// if gp.preemptscan && !gp.gcworkdone && (oldval == _Grunning || oldval == _Gsyscall) { // if gp.preemptscan && !gp.gcworkdone && (oldval == _Grunning || oldval == _Gsyscall) {
@ -2925,21 +2923,14 @@ func exitsyscall(dummy int32) {
_g_.m.locks++ // see comment in entersyscall _g_.m.locks++ // see comment in entersyscall
if getcallersp(unsafe.Pointer(&dummy)) > _g_.syscallsp { 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 _g_.waitsince = 0
oldp := _g_.m.p.ptr() oldp := _g_.m.p.ptr()
if exitsyscallfast() { if exitsyscallfast() {
if _g_.m.mcache == nil { if _g_.m.mcache == nil {
systemstack(func() {
throw("lost mcache") throw("lost mcache")
})
} }
if trace.enabled { if trace.enabled {
if oldp != _g_.m.p.ptr() || _g_.m.syscalltick != _g_.m.p.ptr().syscalltick { if oldp != _g_.m.p.ptr() || _g_.m.syscalltick != _g_.m.p.ptr().syscalltick {
@ -2986,9 +2977,7 @@ func exitsyscall(dummy int32) {
mcall(exitsyscall0) mcall(exitsyscall0)
if _g_.m.mcache == nil { if _g_.m.mcache == nil {
systemstack(func() {
throw("lost mcache") throw("lost mcache")
})
} }
// Scheduler returned, so we're allowed to run now. // Scheduler returned, so we're allowed to run now.

View file

@ -1184,7 +1184,5 @@ func freeStackSpans() {
//go:nosplit //go:nosplit
func morestackc() { func morestackc() {
systemstack(func() {
throw("attempt to execute system stack code on user stack") throw("attempt to execute system stack code on user stack")
})
} }

View file

@ -53,8 +53,13 @@ func mcall(fn func(*g))
//go:noescape //go:noescape
func systemstack(fn func()) func systemstack(fn func())
var badsystemstackMsg = "fatal: systemstack called from unexpected goroutine"
//go:nosplit
//go:nowritebarrierrec
func badsystemstack() { 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. // memclrNoHeapPointers clears n bytes starting at ptr.