runtime: fix MutexProfile missing root frames

felixge · cagedmantis · commit c64ca8c6ef13 · 2024-09-25T17:30:33.000Z
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 <drchase@google.com> Reviewed-by: Nick Ripley <nick.ripley@datadoghq.com> Reviewed-by: Michael Pratt <mpratt@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go
@@ -1142,11 +1142,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
 			}
diff --git a/src/runtime/pprof/mprof_test.go b/src/runtime/pprof/mprof_test.go
@@ -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)
diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go
@@ -973,7 +973,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)
@@ -983,7 +983,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 {
@@ -996,6 +996,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) {
@@ -1280,7 +1296,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"},
 		} {
@@ -1320,6 +1336,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) {
@@ -2470,7 +2508,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) {

Original file line number	Diff line number	Diff line change
`@@ -145,7 +145,7 @@ func TestMemoryProfiler(t *testing.T) {`
`145`	`145`	`}`
`146`	`146`	`t.Logf("Profile = %v", p)`
`147`	`147`
`148`		`- stks := stacks(p)`
	`148`	`+ stks := profileStacks(p)`
`149`	`149`	`for _, test := range tests {`
`150`	`150`	`if !containsStack(stks, test.stk) {`
`151`	`151`	`t.Fatalf("No matching stack entry for %q\n\nProfile:\n%v\n", test.stk, p)`