From f00e947cdf5cdac794d571c87a81ea8878a6fe4f Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Sun, 23 Apr 2023 12:25:47 -0700 Subject: [PATCH] runtime: add raceFiniLock to lock ranking Also preserve the PC/SP in reentersyscall when doing lock ranking. The test is TestDestructorCallbackRace with the staticlockranking experiment enabled. For #59711 Change-Id: I87ac1d121ec0d399de369666834891ab9e7d11b0 Reviewed-on: https://go-review.googlesource.com/c/go/+/487955 Run-TryBot: Ian Lance Taylor Reviewed-by: Michael Knyszek TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor Auto-Submit: Ian Lance Taylor Run-TryBot: Ian Lance Taylor --- src/runtime/lockrank.go | 3 +++ src/runtime/lockrank_off.go | 2 ++ src/runtime/lockrank_on.go | 10 ++++++++-- src/runtime/mklockrank.go | 2 ++ src/runtime/proc.go | 6 ++++++ src/runtime/race.go | 4 ++-- 6 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/runtime/lockrank.go b/src/runtime/lockrank.go index 284a61e336..c2d85ef11b 100644 --- a/src/runtime/lockrank.go +++ b/src/runtime/lockrank.go @@ -62,6 +62,7 @@ const ( lockRankTraceStackTab lockRankPanic lockRankDeadlock + lockRankRaceFini ) // lockRankLeafRank is the rank of lock that does not have a declared rank, @@ -115,6 +116,7 @@ var lockNames = []string{ lockRankTraceStackTab: "traceStackTab", lockRankPanic: "panic", lockRankDeadlock: "deadlock", + lockRankRaceFini: "raceFini", } func (rank lockRank) String() string { @@ -181,4 +183,5 @@ var lockPartialOrder [][]lockRank = [][]lockRank{ lockRankTraceStackTab: {lockRankSysmon, lockRankScavenge, lockRankForcegc, lockRankDefer, lockRankSweepWaiters, lockRankAssistQueue, lockRankSweep, lockRankPollDesc, lockRankCpuprof, lockRankSched, lockRankAllg, lockRankAllp, lockRankTimers, lockRankNetpollInit, lockRankHchan, lockRankNotifyList, lockRankSudog, lockRankRwmutexW, lockRankRwmutexR, lockRankRoot, lockRankItab, lockRankReflectOffs, lockRankUserArenaState, lockRankTraceBuf, lockRankTraceStrings, lockRankFin, lockRankGcBitsArenas, lockRankMspanSpecial, lockRankSpanSetSpine, lockRankProfInsert, lockRankProfBlock, lockRankProfMemActive, lockRankProfMemFuture, lockRankGscan, lockRankStackpool, lockRankStackLarge, lockRankWbufSpans, lockRankMheap, lockRankTrace}, lockRankPanic: {}, lockRankDeadlock: {lockRankPanic, lockRankDeadlock}, + lockRankRaceFini: {lockRankPanic}, } diff --git a/src/runtime/lockrank_off.go b/src/runtime/lockrank_off.go index bf046a1041..c86726f3dd 100644 --- a/src/runtime/lockrank_off.go +++ b/src/runtime/lockrank_off.go @@ -6,6 +6,8 @@ package runtime +const staticLockRanking = false + // // lockRankStruct is embedded in mutex, but is empty when staticklockranking is // disabled (the default) type lockRankStruct struct { diff --git a/src/runtime/lockrank_on.go b/src/runtime/lockrank_on.go index 5dcc79b15e..bf530eeb5b 100644 --- a/src/runtime/lockrank_on.go +++ b/src/runtime/lockrank_on.go @@ -11,6 +11,8 @@ import ( "unsafe" ) +const staticLockRanking = true + // worldIsStopped is accessed atomically to track world-stops. 1 == world // stopped. var worldIsStopped atomic.Uint32 @@ -49,7 +51,7 @@ func getLockRank(l *mutex) lockRank { // split on entry to lock2() would record stack split locks as taken after l, // even though l is not actually locked yet. func lockWithRank(l *mutex, rank lockRank) { - if l == &debuglock || l == &paniclk { + if l == &debuglock || l == &paniclk || l == &raceFiniLock { // debuglock is only used for println/printlock(). Don't do lock // rank recording for it, since print/println are used when // printing out a lock ordering problem below. @@ -59,6 +61,10 @@ func lockWithRank(l *mutex, rank lockRank) { // lock ordering problem. Additionally, paniclk may be taken // after effectively any lock (anywhere we might panic), which // the partial order doesn't cover. + // + // raceFiniLock is held while exiting when running + // the race detector. Don't do lock rank recording for it, + // since we are exiting. lock2(l) return } @@ -159,7 +165,7 @@ func checkRanks(gp *g, prevRank, rank lockRank) { // See comment on lockWithRank regarding stack splitting. func unlockWithRank(l *mutex) { - if l == &debuglock || l == &paniclk { + if l == &debuglock || l == &paniclk || l == &raceFiniLock { // See comment at beginning of lockWithRank. unlock2(l) return diff --git a/src/runtime/mklockrank.go b/src/runtime/mklockrank.go index bc15e57dd4..87328baf38 100644 --- a/src/runtime/mklockrank.go +++ b/src/runtime/mklockrank.go @@ -179,6 +179,8 @@ NONE < panic; # deadlock is not acquired while holding panic, but it also needs to be # below all other locks. panic < deadlock; +# raceFini is only held while exiting. +panic < raceFini; ` // cyclicRanks lists lock ranks that allow multiple locks of the same diff --git a/src/runtime/proc.go b/src/runtime/proc.go index fdbf888c4f..26bab27cb1 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -115,6 +115,7 @@ var ( g0 g mcache0 *mcache raceprocctx0 uintptr + raceFiniLock mutex ) // This slice records the initializing tasks that need to be @@ -3773,6 +3774,11 @@ func reentersyscall(pc, sp uintptr) { gp.syscallsp = sp gp.syscallpc = pc casgstatus(gp, _Grunning, _Gsyscall) + if staticLockRanking { + // When doing static lock ranking casgstatus can call + // systemstack which clobbers g.sched. + save(pc, sp) + } if gp.syscallsp < gp.stack.lo || gp.stack.hi < gp.syscallsp { systemstack(func() { print("entersyscall inconsistent ", hex(gp.syscallsp), " [", hex(gp.stack.lo), ",", hex(gp.stack.hi), "]\n") diff --git a/src/runtime/race.go b/src/runtime/race.go index 7c7b78c145..9120db28da 100644 --- a/src/runtime/race.go +++ b/src/runtime/race.go @@ -355,6 +355,8 @@ func isvalidaddr(addr unsafe.Pointer) bool { //go:nosplit func raceinit() (gctx, pctx uintptr) { + lockInit(&raceFiniLock, lockRankRaceFini) + // On most machines, cgo is required to initialize libc, which is used by race runtime. if !iscgo && GOOS != "darwin" { throw("raceinit: race build must use cgo") @@ -397,8 +399,6 @@ func raceinit() (gctx, pctx uintptr) { return } -var raceFiniLock mutex - //go:nosplit func racefini() { // racefini() can only be called once to avoid races. -- 2.50.0