test(godeltaprof): repro for the mutex/block LocsForStack panic (#245) - #246
Draft
korniltsev-grafanista wants to merge 1 commit into
Draft
test(godeltaprof): repro for the mutex/block LocsForStack panic (#245)#246korniltsev-grafanista wants to merge 1 commit into
korniltsev-grafanista wants to merge 1 commit into
Conversation
godeltaprof passes raw block/mutex profile records to
profileBuilder.LocsForStack. Since Go 1.23 those records hold either
physical return addresses (frame pointer unwinding, the common case) or
logical PCs including the virtual PCs of inlined calls (full traceback,
used when GODEBUG=tracefpunwindoff=1 or when the M has cgo on its
stack). LocsForStack expects logical PCs only. runtime/pprof normalizes
every record with expandInlinedFrames before building locations,
godeltaprof does not.
Two failure modes, one end to end test each, in a package of their own
so that the recovered panics do not take the rest of the compat suite
with them:
- frames inlined into a physical frame are dropped from every mutex
and block profile, no cgo needed;
- a dump that mixes both shapes panics with "slice bounds out of
range", which is the panic reported in #245: a location cached from
a logical record claims more PCs than a physical record has left.
The mix comes from the runtime itself, by contending the same mutex
from a Go callback invoked from C and from plain Go goroutines.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
korniltsev-grafanista
force-pushed
the
repro/issue-245-mutex-inline-expansion
branch
from
August 24, 2026 08:24
fc4df88 to
7f2346b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Repro only, no fix yet. Both tests in the new package fail on purpose -- see Diagnosis and Proposed fix.
Closes-when-fixed: #245
What #245 actually is
The issue attributes the panic to golang/go#70529 (a rare SIGPROF misclassification window). It reproduces without any of that, and it is our bug: godeltaprof skips the stack normalization step that
runtime/pprofperforms for block and mutex records.Since Go 1.23
runtime.saveblockeventrecords contention stacks in one of two shapes:fpTracebackPartialExpandcallersGODEBUG=tracefpunwindoff=1, orm.hasCgoOnStack()-- i.e. the event was sampled while cgo was on the M's stackpprof_mutexProfileInternalhands out the raw bucket stack, so the consumer sees both shapes.runtime/pprof.printCountCycleProfilenormalizes each record withexpandInlinedFrames(runtime.CallersFramesover the whole stack) beforeappendLocsForStack.godeltaprof/internal/pprof.DeltaMutexProfiler.PrintCountCycleProfilepassesr.Stackstraight toLocsForStack, which assumes logical PCs.Diagnosis
Two consequences, one end to end test each.
1. Inlined frames are silently dropped from every mutex and block profile. No cgo involved, affects everyone.
allFramesexpands a physical PC withCallersFramesone PC at a time, andruntime.Frames.Nextonly inserts the virtual PCs of an inlined call when it can peek at the next PC of the stack (runtime/symtab.go, "So check to see if the implied virtual PC for this PC is the next PC in ci.callers"). Same contention, same process:2. A dump that mixes both shapes panics, which is #245. A location cached from a logical record records
len(pcs) == 6for a frame that a physical record spells as a single PC, sostk = stk[len(l.pcs):]at proto.go:375 runs off the end:On a
CGO_ENABLED=1service with Go callbacks (the reporter runs v8go) some contention events are sampled with cgo on the stack and some are not, so every dump mixes the two shapes and every dump is lost -- matching the reported "100% empty server-side, ~24 recovered panics/day". pyroscope-go recovers inSession.dumpMutexProfile, which is why it only shows up as a log line.The tests
New package
godeltaprof/compat/inlineexpansion, kept separate because the runtime never clears mutex buckets: once a poisoned record exists, every later dump in the process panics, which would take the rest of the compat suite down with it.dumpMutexProfilerecovers and callst.Errorf, so each failure is reported without killing the binary.Both tests are end to end: real contention, real runtime records, no hand-written PC lists.
TestMutexProfileDropsInlinedFramesreproMuthrough a chain the compiler inlines, asserts the inlined frames survive the dump (bug 1)TestMutexProfileCgoMixedTracebackVerified on go1.26.3 darwin/arm64, with and without
-race, and withCGO_ENABLED=0(the cgo test drops out, the other one still fails).Proposed fix
Do what
runtime/pprofdoes: normalize each block/mutex record before it reaches the builder, i.e.runtime.CallersFrames(stk)collectingf.PC+1, into a buffer reused across records. Open questions worth a maintainer decision, which is why this PR is repro only:d.m.Lookup. Before means the physical and logical spellings of one stack finally dedupe into a single series, and the delta map is keyed by normalized stacks.mProf_Mallocusescallers, always logical), so the change belongs inDeltaMutexProfiler.PrintCountCycleProfileonly.LocsForStackas well (panic("stack too short to match cached location"), runtime/pprof/proto.go), so a future drift fails loudly instead of as a slice bounds error. Thecheck_golang_profiler_changesnote in Mutex/block delta profiles are silently lost: LocsForStack slice-bounds panic (golang/go#70529), recovered and discarded by pyroscope-go #245 stands: the stdlib gained bothexpandInlinedFramesand that guard, the vendored copy got neither.Happy to push the fix onto this branch.
🤖 Generated with Claude Code