runtime: update paniclk ordering
Now that allglock is no longer taken in throw, paniclk can move to the
bottom of the lock order where it belongs.
There is no fundamental reason that we really need to skip checks on
paniclk in lockWithRank (despite the recursive throws that could be
caused by lock rank checking, startpanic_m would still allow the crash
to complete). However, the partial order of lockRankPanic should be
every single lock that may be held before a throw, nil dereference,
out-of-bounds access, which our partial order doesn't cover.
Updates #42669
Change-Id: Ic3efaea873dc2dd9fd5b0d6ccdd5319730b29a22
Reviewed-on: https://go-review.googlesource.com/c/go/+/270862
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
diff --git a/src/runtime/lockrank.go b/src/runtime/lockrank.go
index b3c01ba..23b727f 100644
--- a/src/runtime/lockrank.go
+++ b/src/runtime/lockrank.go
@@ -44,7 +44,6 @@
lockRankPollDesc
lockRankSched
lockRankDeadlock
- lockRankPanic
lockRankAllg
lockRankAllp
@@ -92,6 +91,7 @@
// rank, we don't allow any further locks to be acquired other than more
// hchan locks.
lockRankHchanLeaf
+ lockRankPanic
// Leaf locks with no dependencies, so these constants are not actually used anywhere.
// There are other architecture-dependent leaf locks as well.
@@ -123,7 +123,6 @@
lockRankPollDesc: "pollDesc",
lockRankSched: "sched",
lockRankDeadlock: "deadlock",
- lockRankPanic: "panic",
lockRankAllg: "allg",
lockRankAllp: "allp",
@@ -162,6 +161,7 @@
lockRankGFree: "gFree",
lockRankHchanLeaf: "hchanLeaf",
+ lockRankPanic: "panic",
lockRankNewmHandoff: "newmHandoff.lock",
lockRankDebugPtrmask: "debugPtrmask.lock",
@@ -202,8 +202,7 @@
lockRankPollDesc: {},
lockRankSched: {lockRankSysmon, lockRankScavenge, lockRankForcegc, lockRankSweepWaiters, lockRankAssistQueue, lockRankCpuprof, lockRankSweep, lockRankPollDesc},
lockRankDeadlock: {lockRankDeadlock},
- lockRankPanic: {lockRankDeadlock},
- lockRankAllg: {lockRankSysmon, lockRankSched, lockRankPanic},
+ lockRankAllg: {lockRankSysmon, lockRankSched},
lockRankAllp: {lockRankSysmon, lockRankSched},
lockRankTimers: {lockRankSysmon, lockRankScavenge, lockRankSched, lockRankAllp, lockRankPollDesc, lockRankTimers},
lockRankItab: {},
@@ -237,6 +236,7 @@
lockRankGFree: {lockRankSched},
lockRankHchanLeaf: {lockRankGscan, lockRankHchanLeaf},
+ lockRankPanic: {lockRankDeadlock}, // plus any other lock held on throw.
lockRankNewmHandoff: {},
lockRankDebugPtrmask: {},
diff --git a/src/runtime/lockrank_on.go b/src/runtime/lockrank_on.go
index 7d45deb..3958d9e 100644
--- a/src/runtime/lockrank_on.go
+++ b/src/runtime/lockrank_on.go
@@ -65,12 +65,11 @@
// rank recording for it, since print/println are used when
// printing out a lock ordering problem below.
//
- // paniclk has an ordering problem, since it can be acquired
- // during a panic with any other locks held (especially if the
- // panic is because of a directed segv), and yet also allg is
- // acquired after paniclk in tracebackothers()). This is a genuine
- // problem, so for now we don't do lock rank recording for paniclk
- // either.
+ // paniclk is only used for fatal throw/panic. Don't do lock
+ // ranking recording for it, since we throw after reporting a
+ // lock ordering problem. Additionally, paniclk may be taken
+ // after effectively any lock (anywhere we might panic), which
+ // the partial order doesn't cover.
lock2(l)
return
}
