[release-branch.go1.23] 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 #69865

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>
(cherry picked from commit c64ca8c6ef13723b9f25f4b5e1c7b6986b958d2e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/621276
Reviewed-by: Cherry Mui <cherryyz@google.com>

diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go
index 82b7fa68aecbde654cd3c4de43d5ee822daf8f4c..ee3e59a9aa99cedb0133f30b55ccfcb3453e3e68 100644 (file)
--- a/src/runtime/mprof.go
+++ b/src/runtime/mprof.go
@@ -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
                        }

diff --git a/src/runtime/pprof/mprof_test.go b/src/runtime/pprof/mprof_test.go
index 391588d4acd0ec61fda9dd562f4dfb033fa52b06..ef373b36848437ff02230f211691d686eb4d921f 100644 (file)
--- 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
index 41952ff147c4b7dc985a0fa7ab3fa6a57a02cc70..da4ad17d77e6fd6a9c510570969c096d10abe086 100644 (file)
--- a/src/runtime/pprof/pprof_test.go
+++ b/src/runtime/pprof/pprof_test.go
@@ -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)
@@ -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 {
@@ -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) {
@@ -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"},
                } {
@@ -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) {
@@ -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) {