Skip to content

Commit

Permalink
[release-branch.go1.23] runtime: fix MutexProfile missing root frames
Browse files Browse the repository at this point in the history
Fix a regression introduced in CL 598515 causing runtime.MutexProfile
stack traces to omit their root frames.

In most cases this was merely causing the `runtime.goexit` frame to go
missing. But in the case of runtime._LostContendedRuntimeLock, an empty
stack trace was being produced.

Add a test that catches this regression by checking for a stack trace
with the `runtime.goexit` frame.

Also fix a separate problem in expandFrame that could cause
out-of-bounds panics when profstackdepth is set to a value below 32.
There is no test for this fix because profstackdepth can't be changed at
runtime right now.

Fixes #69865

Change-Id: I1600fe62548ea84981df0916d25072c3ddf1ea1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/611615
Reviewed-by: David Chase <[email protected]>
Reviewed-by: Nick Ripley <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
(cherry picked from commit c64ca8c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/621276
Reviewed-by: Cherry Mui <[email protected]>
  • Loading branch information
felixge authored and prattmic committed Oct 30, 2024
1 parent 958f3a0 commit a0d15cb
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 6 deletions.
3 changes: 2 additions & 1 deletion src/runtime/mprof.go
Original file line number Diff line number Diff line change
Expand Up @@ -1136,11 +1136,12 @@ func expandFrames(p []BlockProfileRecord) {
for i := range p {
cf := CallersFrames(p[i].Stack())
j := 0
for ; j < len(expandedStack); j++ {
for j < len(expandedStack) {
f, more := cf.Next()
// f.PC is a "call PC", but later consumers will expect
// "return PCs"
expandedStack[j] = f.PC + 1
j++
if !more {
break
}
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/pprof/mprof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func TestMemoryProfiler(t *testing.T) {
}
t.Logf("Profile = %v", p)

stks := stacks(p)
stks := profileStacks(p)
for _, test := range tests {
if !containsStack(stks, test.stk) {
t.Fatalf("No matching stack entry for %q\n\nProfile:\n%v\n", test.stk, p)
Expand Down
46 changes: 42 additions & 4 deletions src/runtime/pprof/pprof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -982,7 +982,7 @@ func TestBlockProfile(t *testing.T) {
t.Fatalf("invalid profile: %v", err)
}

stks := stacks(p)
stks := profileStacks(p)
for _, test := range tests {
if !containsStack(stks, test.stk) {
t.Errorf("No matching stack entry for %v, want %+v", test.name, test.stk)
Expand All @@ -992,7 +992,7 @@ func TestBlockProfile(t *testing.T) {

}

func stacks(p *profile.Profile) (res [][]string) {
func profileStacks(p *profile.Profile) (res [][]string) {
for _, s := range p.Sample {
var stk []string
for _, l := range s.Location {
Expand All @@ -1005,6 +1005,22 @@ func stacks(p *profile.Profile) (res [][]string) {
return res
}

func blockRecordStacks(records []runtime.BlockProfileRecord) (res [][]string) {
for _, record := range records {
frames := runtime.CallersFrames(record.Stack())
var stk []string
for {
frame, more := frames.Next()
stk = append(stk, frame.Function)
if !more {
break
}
}
res = append(res, stk)
}
return res
}

func containsStack(got [][]string, want []string) bool {
for _, stk := range got {
if len(stk) < len(want) {
Expand Down Expand Up @@ -1289,7 +1305,7 @@ func TestMutexProfile(t *testing.T) {
t.Fatalf("invalid profile: %v", err)
}

stks := stacks(p)
stks := profileStacks(p)
for _, want := range [][]string{
{"sync.(*Mutex).Unlock", "runtime/pprof.blockMutexN.func1"},
} {
Expand Down Expand Up @@ -1329,6 +1345,28 @@ func TestMutexProfile(t *testing.T) {
t.Fatalf("profile samples total %v, want within range [%v, %v] (target: %v)", d, lo, hi, N*D)
}
})

t.Run("records", func(t *testing.T) {
// Record a mutex profile using the structured record API.
var records []runtime.BlockProfileRecord
for {
n, ok := runtime.MutexProfile(records)
if ok {
records = records[:n]
break
}
records = make([]runtime.BlockProfileRecord, n*2)
}

// Check that we see the same stack trace as the proto profile. For
// historical reason we expect a runtime.goexit root frame here that is
// omitted in the proto profile.
stks := blockRecordStacks(records)
want := []string{"sync.(*Mutex).Unlock", "runtime/pprof.blockMutexN.func1", "runtime.goexit"}
if !containsStack(stks, want) {
t.Errorf("No matching stack entry for %+v", want)
}
})
}

func TestMutexProfileRateAdjust(t *testing.T) {
Expand Down Expand Up @@ -2514,7 +2552,7 @@ func TestProfilerStackDepth(t *testing.T) {
}
t.Logf("Profile = %v", p)

stks := stacks(p)
stks := profileStacks(p)
var stk []string
for _, s := range stks {
if hasPrefix(s, test.prefix) {
Expand Down

0 comments on commit a0d15cb

Please sign in to comment.