Skip to content

Commit c64ca8c

Browse files
felixgecagedmantis
authored andcommitted
runtime: fix MutexProfile missing root frames
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 #69335 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]>
1 parent 8c8948c commit c64ca8c

File tree

3 files changed

+45
-6
lines changed

3 files changed

+45
-6
lines changed

src/runtime/mprof.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -1142,11 +1142,12 @@ func expandFrames(p []BlockProfileRecord) {
11421142
for i := range p {
11431143
cf := CallersFrames(p[i].Stack())
11441144
j := 0
1145-
for ; j < len(expandedStack); j++ {
1145+
for j < len(expandedStack) {
11461146
f, more := cf.Next()
11471147
// f.PC is a "call PC", but later consumers will expect
11481148
// "return PCs"
11491149
expandedStack[j] = f.PC + 1
1150+
j++
11501151
if !more {
11511152
break
11521153
}

src/runtime/pprof/mprof_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func TestMemoryProfiler(t *testing.T) {
145145
}
146146
t.Logf("Profile = %v", p)
147147

148-
stks := stacks(p)
148+
stks := profileStacks(p)
149149
for _, test := range tests {
150150
if !containsStack(stks, test.stk) {
151151
t.Fatalf("No matching stack entry for %q\n\nProfile:\n%v\n", test.stk, p)

src/runtime/pprof/pprof_test.go

+42-4
Original file line numberDiff line numberDiff line change
@@ -973,7 +973,7 @@ func TestBlockProfile(t *testing.T) {
973973
t.Fatalf("invalid profile: %v", err)
974974
}
975975

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

984984
}
985985

986-
func stacks(p *profile.Profile) (res [][]string) {
986+
func profileStacks(p *profile.Profile) (res [][]string) {
987987
for _, s := range p.Sample {
988988
var stk []string
989989
for _, l := range s.Location {
@@ -996,6 +996,22 @@ func stacks(p *profile.Profile) (res [][]string) {
996996
return res
997997
}
998998

999+
func blockRecordStacks(records []runtime.BlockProfileRecord) (res [][]string) {
1000+
for _, record := range records {
1001+
frames := runtime.CallersFrames(record.Stack())
1002+
var stk []string
1003+
for {
1004+
frame, more := frames.Next()
1005+
stk = append(stk, frame.Function)
1006+
if !more {
1007+
break
1008+
}
1009+
}
1010+
res = append(res, stk)
1011+
}
1012+
return res
1013+
}
1014+
9991015
func containsStack(got [][]string, want []string) bool {
10001016
for _, stk := range got {
10011017
if len(stk) < len(want) {
@@ -1280,7 +1296,7 @@ func TestMutexProfile(t *testing.T) {
12801296
t.Fatalf("invalid profile: %v", err)
12811297
}
12821298

1283-
stks := stacks(p)
1299+
stks := profileStacks(p)
12841300
for _, want := range [][]string{
12851301
{"sync.(*Mutex).Unlock", "runtime/pprof.blockMutexN.func1"},
12861302
} {
@@ -1320,6 +1336,28 @@ func TestMutexProfile(t *testing.T) {
13201336
t.Fatalf("profile samples total %v, want within range [%v, %v] (target: %v)", d, lo, hi, N*D)
13211337
}
13221338
})
1339+
1340+
t.Run("records", func(t *testing.T) {
1341+
// Record a mutex profile using the structured record API.
1342+
var records []runtime.BlockProfileRecord
1343+
for {
1344+
n, ok := runtime.MutexProfile(records)
1345+
if ok {
1346+
records = records[:n]
1347+
break
1348+
}
1349+
records = make([]runtime.BlockProfileRecord, n*2)
1350+
}
1351+
1352+
// Check that we see the same stack trace as the proto profile. For
1353+
// historical reason we expect a runtime.goexit root frame here that is
1354+
// omitted in the proto profile.
1355+
stks := blockRecordStacks(records)
1356+
want := []string{"sync.(*Mutex).Unlock", "runtime/pprof.blockMutexN.func1", "runtime.goexit"}
1357+
if !containsStack(stks, want) {
1358+
t.Errorf("No matching stack entry for %+v", want)
1359+
}
1360+
})
13231361
}
13241362

13251363
func TestMutexProfileRateAdjust(t *testing.T) {
@@ -2470,7 +2508,7 @@ func TestProfilerStackDepth(t *testing.T) {
24702508
}
24712509
t.Logf("Profile = %v", p)
24722510

2473-
stks := stacks(p)
2511+
stks := profileStacks(p)
24742512
var stk []string
24752513
for _, s := range stks {
24762514
if hasPrefix(s, test.prefix) {

0 commit comments

Comments
 (0)