runtime: preempt more aggressively when panicking

When we are crashing from an unrecovered panic, we freeze the
world, and print stack traces for all goroutines if GOTRACEBACK is
set to a high enough level. Freezing the world is best effort, so
there could still be goroutines that are not preempted, and so its
stack trace is unavailable and printed as "goroutine running on
other thread".

As we're crashing and not resuming execution on preempted
goroutines, we can make preemption more aggressive, preempting
cases that are not safe for resumption or stack scanning. This may
make goroutines more likely to be preempted in freezing the world
and have their stacks available.

Change-Id: Ie16269e2a05e007efa61368b695addc28d7a97ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/546135
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>

diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go
index 7e9bbd04f2758807ac59a4e9537ea4654cab1a5c..50f61c1d54162860ba4dcda056df41a3f8c8b0a7 100644 (file)
--- a/src/runtime/os_windows.go
+++ b/src/runtime/os_windows.go
@@ -1298,7 +1298,7 @@ func preemptM(mp *m) {
        // Does it want a preemption and is it safe to preempt?
        gp := gFromSP(mp, c.sp())
        if gp != nil && wantAsyncPreempt(gp) {
-               if ok, newpc := isAsyncSafePoint(gp, c.ip(), c.sp(), c.lr()); ok {
+               if ok, newpc := isAsyncSafePoint(gp, c.ip(), c.sp(), c.lr(), panicking.Load() != 0); ok {
                        // Inject call to asyncPreempt
                        targetPC := abi.FuncPCABI0(asyncPreempt)
                        switch GOARCH {

diff --git a/src/runtime/preempt.go b/src/runtime/preempt.go
index 82d85cd707619a5e140c018ad005f7e9e2c5779f..aedf7f3ff59e219666ba21790af7348de395e84b 100644 (file)
--- a/src/runtime/preempt.go
+++ b/src/runtime/preempt.go
@@ -359,7 +359,11 @@ func wantAsyncPreempt(gp *g) bool {
 // In some cases the PC is safe for asynchronous preemption but it
 // also needs to adjust the resumption PC. The new PC is returned in
 // the second result.
-func isAsyncSafePoint(gp *g, pc, sp, lr uintptr) (bool, uintptr) {
+//
+// If noResume is true, we know we're not going to resume execution
+// on this goroutine (as we're crashing), and thus we can preempt
+// more aggressively.
+func isAsyncSafePoint(gp *g, pc, sp, lr uintptr, noResume bool) (bool, uintptr) {
        mp := gp.m
 
        // Only user Gs can have safe-points. We check this first
@@ -370,7 +374,7 @@ func isAsyncSafePoint(gp *g, pc, sp, lr uintptr) (bool, uintptr) {
        }
 
        // Check M state.
-       if mp.p == 0 || !canPreemptM(mp) {
+       if mp.p == 0 || (!canPreemptM(mp) && !noResume) {
                return false, 0
        }
 
@@ -385,6 +389,13 @@ func isAsyncSafePoint(gp *g, pc, sp, lr uintptr) (bool, uintptr) {
                // Not Go code.
                return false, 0
        }
+       if noResume && f.flag&abi.FuncFlagAsm == 0 {
+               // We're not going to resume execution and not going to scan the
+               // stack for GC, so we don't care whether it is a safe point, and
+               // also don't care the resumption PC.
+               // TODO: maybe we can preempt non-SPWRITE assembly functions?
+               return true, pc
+       }
        if (GOARCH == "mips" || GOARCH == "mipsle" || GOARCH == "mips64" || GOARCH == "mips64le") && lr == pc+8 && funcspdelta(f, pc) == 0 {
                // We probably stopped at a half-executed CALL instruction,
                // where the LR is updated but the PC has not. If we preempt

diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go
index 84391d58ed7f22404ee33ba8ecf9174b513f8890..78e71c908697c445e2327f01b4db804ecef4a6d3 100644 (file)
--- a/src/runtime/signal_unix.go
+++ b/src/runtime/signal_unix.go
@@ -342,7 +342,7 @@ func doSigPreempt(gp *g, ctxt *sigctxt) {
        // Check if this G wants to be preempted and is safe to
        // preempt.
        if wantAsyncPreempt(gp) {
-               if ok, newpc := isAsyncSafePoint(gp, ctxt.sigpc(), ctxt.sigsp(), ctxt.siglr()); ok {
+               if ok, newpc := isAsyncSafePoint(gp, ctxt.sigpc(), ctxt.sigsp(), ctxt.siglr(), panicking.Load() != 0); ok {
                        // Adjust the PC and inject a call to asyncPreempt.
                        ctxt.pushCall(abi.FuncPCABI0(asyncPreempt), newpc)
                }