Skip to content

Commit 35c010a

Browse files
felixgeprattmic
authored andcommitted
[release-branch.go1.23] runtime: fix GoroutineProfile stacks not getting null terminated
Fix a regression introduced in CL 572396 causing goroutine stacks not getting null terminated. This bug impacts callers that reuse the []StackRecord slice for multiple calls to GoroutineProfile. See felixge/fgprof#33 for an example of the problem. Add a test case to prevent similar regressions in the future. Use null padding instead of null termination to be consistent with other profile types and because it's less code to implement. Also fix the ThreadCreateProfile code path. Fixes #69258 Change-Id: I0b9414f6c694c304bc03a5682586f619e9bf0588 Reviewed-on: https://go-review.googlesource.com/c/go/+/609815 Reviewed-by: Tim King <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Pratt <[email protected]> (cherry picked from commit 49e542a) Reviewed-on: https://go-review.googlesource.com/c/go/+/621277 Reviewed-by: Cherry Mui <[email protected]>
1 parent 6495ce0 commit 35c010a

File tree

2 files changed

+86
-12
lines changed

2 files changed

+86
-12
lines changed

src/runtime/mprof.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,7 +1270,8 @@ func pprof_mutexProfileInternal(p []profilerecord.BlockProfileRecord) (n int, ok
12701270
// of calling ThreadCreateProfile directly.
12711271
func ThreadCreateProfile(p []StackRecord) (n int, ok bool) {
12721272
return threadCreateProfileInternal(len(p), func(r profilerecord.StackRecord) {
1273-
copy(p[0].Stack0[:], r.Stack)
1273+
i := copy(p[0].Stack0[:], r.Stack)
1274+
clear(p[0].Stack0[i:])
12741275
p = p[1:]
12751276
})
12761277
}
@@ -1649,7 +1650,8 @@ func GoroutineProfile(p []StackRecord) (n int, ok bool) {
16491650
return
16501651
}
16511652
for i, mr := range records[0:n] {
1652-
copy(p[i].Stack0[:], mr.Stack)
1653+
l := copy(p[i].Stack0[:], mr.Stack)
1654+
clear(p[i].Stack0[l:])
16531655
}
16541656
return
16551657
}

src/runtime/pprof/pprof_test.go

Lines changed: 82 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2441,16 +2441,7 @@ func TestTimeVDSO(t *testing.T) {
24412441
}
24422442

24432443
func TestProfilerStackDepth(t *testing.T) {
2444-
// Disable sampling, otherwise it's difficult to assert anything.
2445-
oldMemRate := runtime.MemProfileRate
2446-
runtime.MemProfileRate = 1
2447-
runtime.SetBlockProfileRate(1)
2448-
oldMutexRate := runtime.SetMutexProfileFraction(1)
2449-
t.Cleanup(func() {
2450-
runtime.MemProfileRate = oldMemRate
2451-
runtime.SetBlockProfileRate(0)
2452-
runtime.SetMutexProfileFraction(oldMutexRate)
2453-
})
2444+
t.Cleanup(disableSampling())
24542445

24552446
const depth = 128
24562447
go produceProfileEvents(t, depth)
@@ -2742,3 +2733,84 @@ runtime/pprof.inlineA`,
27422733
})
27432734
}
27442735
}
2736+
2737+
func TestProfileRecordNullPadding(t *testing.T) {
2738+
// Produce events for the different profile types.
2739+
t.Cleanup(disableSampling())
2740+
memSink = make([]byte, 1) // MemProfile
2741+
<-time.After(time.Millisecond) // BlockProfile
2742+
blockMutex(t) // MutexProfile
2743+
runtime.GC()
2744+
2745+
// Test that all profile records are null padded.
2746+
testProfileRecordNullPadding(t, "MutexProfile", runtime.MutexProfile)
2747+
testProfileRecordNullPadding(t, "GoroutineProfile", runtime.GoroutineProfile)
2748+
testProfileRecordNullPadding(t, "BlockProfile", runtime.BlockProfile)
2749+
testProfileRecordNullPadding(t, "MemProfile/inUseZero=true", func(p []runtime.MemProfileRecord) (int, bool) {
2750+
return runtime.MemProfile(p, true)
2751+
})
2752+
testProfileRecordNullPadding(t, "MemProfile/inUseZero=false", func(p []runtime.MemProfileRecord) (int, bool) {
2753+
return runtime.MemProfile(p, false)
2754+
})
2755+
// Not testing ThreadCreateProfile because it is broken, see issue 6104.
2756+
}
2757+
2758+
func testProfileRecordNullPadding[T runtime.StackRecord | runtime.MemProfileRecord | runtime.BlockProfileRecord](t *testing.T, name string, fn func([]T) (int, bool)) {
2759+
stack0 := func(sr *T) *[32]uintptr {
2760+
switch t := any(sr).(type) {
2761+
case *runtime.StackRecord:
2762+
return &t.Stack0
2763+
case *runtime.MemProfileRecord:
2764+
return &t.Stack0
2765+
case *runtime.BlockProfileRecord:
2766+
return &t.Stack0
2767+
default:
2768+
panic(fmt.Sprintf("unexpected type %T", sr))
2769+
}
2770+
}
2771+
2772+
t.Run(name, func(t *testing.T) {
2773+
var p []T
2774+
for {
2775+
n, ok := fn(p)
2776+
if ok {
2777+
p = p[:n]
2778+
break
2779+
}
2780+
p = make([]T, n*2)
2781+
for i := range p {
2782+
s0 := stack0(&p[i])
2783+
for j := range s0 {
2784+
// Poison the Stack0 array to identify lack of zero padding
2785+
s0[j] = ^uintptr(0)
2786+
}
2787+
}
2788+
}
2789+
2790+
if len(p) == 0 {
2791+
t.Fatal("no records found")
2792+
}
2793+
2794+
for _, sr := range p {
2795+
for i, v := range stack0(&sr) {
2796+
if v == ^uintptr(0) {
2797+
t.Fatalf("record p[%d].Stack0 is not null padded: %+v", i, sr)
2798+
}
2799+
}
2800+
}
2801+
})
2802+
}
2803+
2804+
// disableSampling configures the profilers to capture all events, otherwise
2805+
// it's difficult to assert anything.
2806+
func disableSampling() func() {
2807+
oldMemRate := runtime.MemProfileRate
2808+
runtime.MemProfileRate = 1
2809+
runtime.SetBlockProfileRate(1)
2810+
oldMutexRate := runtime.SetMutexProfileFraction(1)
2811+
return func() {
2812+
runtime.MemProfileRate = oldMemRate
2813+
runtime.SetBlockProfileRate(0)
2814+
runtime.SetMutexProfileFraction(oldMutexRate)
2815+
}
2816+
}

0 commit comments

Comments
 (0)