1
0
mirror of https://github.com/golang/go synced 2024-11-07 18:06:27 -07:00

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>
This commit is contained in:
Michael Pratt 2020-11-17 12:28:40 -05:00
parent 280c735b07
commit 67b9ecb23b
2 changed files with 9 additions and 10 deletions

View File

@ -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: {},

View File

@ -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
}