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>

diff --git a/src/runtime/panic.go b/src/runtime/panic.go
index 81ff21113f98d83e18ec618f62eaf016598b48d4..bb83be4715d3a450b6f289aee9f2e5dfefb13532 100644 (file)
--- a/src/runtime/panic.go
+++ b/src/runtime/panic.go
@@ -729,10 +729,13 @@ func fatalpanic(msgs *_panic) {
 // It returns true if panic messages should be printed, or false if
 // the runtime is in bad shape and should just print stacks.
 //
-// It can have write barriers because the write barrier explicitly
-// ignores writes once dying > 0.
+// It must not have write barriers even though the write barrier
+// 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 {
        _g_ := getg()
        if mheap_.cachealloc.size == 0 { // very early
@@ -752,8 +755,8 @@ func startpanic_m() bool {
 
        switch _g_.m.dying {
        case 0:
+               // Setting dying >0 has the side-effect of disabling this G's writebuf.
                _g_.m.dying = 1
-               _g_.writebuf = nil
                atomic.Xadd(&panicking, 1)
                lock(&paniclk)
                if debug.schedtrace > 0 || debug.scheddetail > 0 {

diff --git a/src/runtime/print.go b/src/runtime/print.go
index 7b2e4f40ffe06c4797163df895310d799d8e8a9e..e605eb34cb90a52a60849e55eb445b7d2fa895c0 100644 (file)
--- a/src/runtime/print.go
+++ b/src/runtime/print.go
@@ -89,7 +89,12 @@ func gwrite(b []byte) {
        }
        recordForPanic(b)
        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)
                return
        }