Skip to content

Commit 9ed9df6

Browse files
committed
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Austin Clements <[email protected]>
1 parent e3b4b7b commit 9ed9df6

File tree

2 files changed

+13
-5
lines changed

2 files changed

+13
-5
lines changed

src/runtime/panic.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -729,10 +729,13 @@ func fatalpanic(msgs *_panic) {
729729
// It returns true if panic messages should be printed, or false if
730730
// the runtime is in bad shape and should just print stacks.
731731
//
732-
// It can have write barriers because the write barrier explicitly
733-
// ignores writes once dying > 0.
732+
// It must not have write barriers even though the write barrier
733+
// explicitly ignores writes once dying > 0. Write barriers still
734+
// assume that g.m.p != nil, and this function may not have P
735+
// in some contexts (e.g. a panic in a signal handler for a signal
736+
// sent to an M with no P).
734737
//
735-
//go:yeswritebarrierrec
738+
//go:nowritebarrierrec
736739
func startpanic_m() bool {
737740
_g_ := getg()
738741
if mheap_.cachealloc.size == 0 { // very early
@@ -752,8 +755,8 @@ func startpanic_m() bool {
752755

753756
switch _g_.m.dying {
754757
case 0:
758+
// Setting dying >0 has the side-effect of disabling this G's writebuf.
755759
_g_.m.dying = 1
756-
_g_.writebuf = nil
757760
atomic.Xadd(&panicking, 1)
758761
lock(&paniclk)
759762
if debug.schedtrace > 0 || debug.scheddetail > 0 {

src/runtime/print.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,12 @@ func gwrite(b []byte) {
8989
}
9090
recordForPanic(b)
9191
gp := getg()
92-
if gp == nil || gp.writebuf == nil {
92+
// Don't use the writebuf if gp.m is dying. We want anything
93+
// written through gwrite to appear in the terminal rather
94+
// than be written to in some buffer, if we're in a panicking state.
95+
// Note that we can't just clear writebuf in the gp.m.dying case
96+
// because a panic isn't allowed to have any write barriers.
97+
if gp == nil || gp.writebuf == nil || gp.m.dying > 0 {
9398
writeErr(b)
9499
return
95100
}

0 commit comments

Comments
 (0)