runtime: avoid write barrier in startpanic_m

startpanic_m could be called correctly in a context where there's a
valid G, a valid M, but no P, for example in a signal handler which
panics. Currently, startpanic_m has write barriers enabled because
write barriers are permitted if a G's M is dying. However, all the
current write barrier implementations assume the current G has a P.

Therefore, in this change we disable write barriers in startpanic_m,
remove the only pointer write which clears g.writebuf, and fix up gwrite
to ignore the writebuf if the current G's M is dying, rather than
relying on it being nil in the dying case.

Fixes #26575.

Change-Id: I9b29e6b9edf00d8e99ffc71770c287142ebae086
Reviewed-on: https://go-review.googlesource.com/c/154837
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This commit is contained in:
Michael Anthony Knyszek 2018-12-18 20:04:53 +00:00 committed by Michael Knyszek
parent e3b4b7baad
commit 9ed9df6ca2
2 changed files with 13 additions and 5 deletions

View file

@ -729,10 +729,13 @@ func fatalpanic(msgs *_panic) {
// It returns true if panic messages should be printed, or false if // It returns true if panic messages should be printed, or false if
// the runtime is in bad shape and should just print stacks. // the runtime is in bad shape and should just print stacks.
// //
// It can have write barriers because the write barrier explicitly // It must not have write barriers even though the write barrier
// ignores writes once dying > 0. // explicitly ignores writes once dying > 0. Write barriers still
// assume that g.m.p != nil, and this function may not have P
// in some contexts (e.g. a panic in a signal handler for a signal
// sent to an M with no P).
// //
//go:yeswritebarrierrec //go:nowritebarrierrec
func startpanic_m() bool { func startpanic_m() bool {
_g_ := getg() _g_ := getg()
if mheap_.cachealloc.size == 0 { // very early if mheap_.cachealloc.size == 0 { // very early
@ -752,8 +755,8 @@ func startpanic_m() bool {
switch _g_.m.dying { switch _g_.m.dying {
case 0: case 0:
// Setting dying >0 has the side-effect of disabling this G's writebuf.
_g_.m.dying = 1 _g_.m.dying = 1
_g_.writebuf = nil
atomic.Xadd(&panicking, 1) atomic.Xadd(&panicking, 1)
lock(&paniclk) lock(&paniclk)
if debug.schedtrace > 0 || debug.scheddetail > 0 { if debug.schedtrace > 0 || debug.scheddetail > 0 {

View file

@ -89,7 +89,12 @@ func gwrite(b []byte) {
} }
recordForPanic(b) recordForPanic(b)
gp := getg() gp := getg()
if gp == nil || gp.writebuf == nil { // Don't use the writebuf if gp.m is dying. We want anything
// written through gwrite to appear in the terminal rather
// than be written to in some buffer, if we're in a panicking state.
// Note that we can't just clear writebuf in the gp.m.dying case
// because a panic isn't allowed to have any write barriers.
if gp == nil || gp.writebuf == nil || gp.m.dying > 0 {
writeErr(b) writeErr(b)
return return
} }