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 b3c01ba10456fdc42fc8ae8c7552ef725bdfbb5e..23b727f4d8db565b1377b1b212f55646891ff0cc 100644 (file)
--- a/src/runtime/lockrank.go
+++ b/src/runtime/lockrank.go
@@ -44,7 +44,6 @@ const (
        lockRankPollDesc
        lockRankSched
        lockRankDeadlock
-       lockRankPanic
        lockRankAllg
        lockRankAllp
 
@@ -92,6 +91,7 @@ const (
        // 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 @@ var lockNames = []string{
        lockRankPollDesc: "pollDesc",
        lockRankSched:    "sched",
        lockRankDeadlock: "deadlock",
-       lockRankPanic:    "panic",
        lockRankAllg:     "allg",
        lockRankAllp:     "allp",
 
@@ -162,6 +161,7 @@ var lockNames = []string{
 
        lockRankGFree:     "gFree",
        lockRankHchanLeaf: "hchanLeaf",
+       lockRankPanic:     "panic",
 
        lockRankNewmHandoff:   "newmHandoff.lock",
        lockRankDebugPtrmask:  "debugPtrmask.lock",
@@ -202,8 +202,7 @@ var lockPartialOrder [][]lockRank = [][]lockRank{
        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 @@ var lockPartialOrder [][]lockRank = [][]lockRank{
 
        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 7d45debaca27f2f2db5a6264f5dcb6324be105b6..3958d9eeaa92af6822688a2a816605ca4af1da94 100644 (file)
--- a/src/runtime/lockrank_on.go
+++ b/src/runtime/lockrank_on.go
@@ -65,12 +65,11 @@ func lockWithRank(l *mutex, rank lockRank) {
                // 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
        }