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>

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},
-	lockRankAllg:          {lockRankSysmon, lockRankSched, lockRankPanic},
 	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:  {},

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
+		// paniclk is only used for fatal throw/panic. Don't do lock
-		// during a panic with any other locks held (especially if the
+		// ranking recording for it, since we throw after reporting a
-		// panic is because of a directed segv), and yet also allg is
+		// lock ordering problem. Additionally, paniclk may be taken
-		// acquired after paniclk in tracebackothers()). This is a genuine
+		// after effectively any lock (anywhere we might panic), which
-		// problem, so for now we don't do lock rank recording for paniclk
+		// the partial order doesn't cover.
-		// either.
 		lock2(l)
 		return
 	}