From: Michael Pratt <mpratt@google.com>
Date: Thu, 9 Jan 2025 17:22:53 +0000 (-0500)
Subject: runtime/pprof: hide map runtime frames from heap profiles
X-Git-Tag: go1.24rc3~2^2~55
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=d0c9142ce3;p=gostls13.git

runtime/pprof: hide map runtime frames from heap profiles

Heap profiles hide "runtime" frames like runtime.mapassign. This broke
in 1.24 because the map implementation moved to internal/runtime/maps,
and runtime/pprof only considered literal "runtime." when looking for
runtime frames.

It would be nice to use cmd/internal/objabi.PkgSpecial to find runtime
packages, but that is hidden away in cmd.

Fixes #71174.

Change-Id: I6a6a636cb42aa17539e47da16854bd3fd8cb1bfe
Reviewed-on: https://go-review.googlesource.com/c/go/+/641775
Auto-Submit: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
---

diff --git a/src/runtime/pprof/pprof.go b/src/runtime/pprof/pprof.go
index f6b4a5c367..b7680a13fd 100644
--- a/src/runtime/pprof/pprof.go
+++ b/src/runtime/pprof/pprof.go
@@ -555,7 +555,7 @@ func printStackRecord(w io.Writer, stk []uintptr, allFrames bool) {
 		if name == "" {
 			show = true
 			fmt.Fprintf(w, "#\t%#x\n", frame.PC)
-		} else if name != "runtime.goexit" && (show || !strings.HasPrefix(name, "runtime.")) {
+		} else if name != "runtime.goexit" && (show || !(strings.HasPrefix(name, "runtime.") || strings.HasPrefix(name, "internal/runtime/"))) {
 			// Hide runtime.goexit and any runtime functions at the beginning.
 			// This is useful mainly for allocation traces.
 			show = true
diff --git a/src/runtime/pprof/protomem.go b/src/runtime/pprof/protomem.go
index ab3550f43f..72aad82b30 100644
--- a/src/runtime/pprof/protomem.go
+++ b/src/runtime/pprof/protomem.go
@@ -36,7 +36,7 @@ func writeHeapProto(w io.Writer, p []profilerecord.MemProfileRecord, rate int64,
 			// what appendLocsForStack expects.
 			if hideRuntime {
 				for i, addr := range stk {
-					if f := runtime.FuncForPC(addr); f != nil && strings.HasPrefix(f.Name(), "runtime.") {
+					if f := runtime.FuncForPC(addr); f != nil && (strings.HasPrefix(f.Name(), "runtime.") || strings.HasPrefix(f.Name(), "internal/runtime/")) {
 						continue
 					}
 					// Found non-runtime. Show any runtime uses above it.
diff --git a/src/runtime/pprof/protomem_test.go b/src/runtime/pprof/protomem_test.go
index 885f4dca5b..4d08e67ddc 100644
--- a/src/runtime/pprof/protomem_test.go
+++ b/src/runtime/pprof/protomem_test.go
@@ -118,7 +118,7 @@ func locationToStrings(loc *profile.Location, funcs []string) []string {
 	return funcs
 }
 
-// This is a regression test for https://go.dev/issue/64528 .
+// This is a regression test for https://go.dev/issue/64528.
 func TestGenericsHashKeyInPprofBuilder(t *testing.T) {
 	if asan.Enabled {
 		t.Skip("extra allocations with -asan throw off the test; see #70079")
@@ -229,3 +229,61 @@ func TestGenericsInlineLocations(t *testing.T) {
 		t.Errorf("expected a location with at least 3 functions\n%s\ngot\n%s\n", expectedLocation, actual)
 	}
 }
+
+func growMap() {
+	m := make(map[int]int)
+	for i := range 512 {
+		m[i] = i
+	}
+}
+
+// Runtime frames are hidden in heap profiles.
+// This is a regression test for https://go.dev/issue/71174.
+func TestHeapRuntimeFrames(t *testing.T) {
+	previousRate := runtime.MemProfileRate
+	runtime.MemProfileRate = 1
+	defer func() {
+		runtime.MemProfileRate = previousRate
+	}()
+
+	growMap()
+
+	runtime.GC()
+	buf := bytes.NewBuffer(nil)
+	if err := WriteHeapProfile(buf); err != nil {
+		t.Fatalf("writing profile: %v", err)
+	}
+	p, err := profile.Parse(buf)
+	if err != nil {
+		t.Fatalf("profile.Parse: %v", err)
+	}
+
+	actual := profileToStrings(p)
+
+	// We must see growMap at least once.
+	foundGrowMap := false
+	for _, l := range actual {
+		if !strings.Contains(l, "runtime/pprof.growMap") {
+			continue
+		}
+		foundGrowMap = true
+
+		// Runtime frames like mapassign and map internals should be hidden.
+		if strings.Contains(l, "runtime.") {
+			t.Errorf("Sample got %s, want no runtime frames", l)
+		}
+		if strings.Contains(l, "internal/runtime/") {
+			t.Errorf("Sample got %s, want no runtime frames", l)
+		}
+		if strings.Contains(l, "runtime/internal/") {
+			t.Errorf("Sample got %s, want no runtime frames", l)
+		}
+		if strings.Contains(l, "mapassign") { // in case mapassign moves to a package not matching above paths.
+			t.Errorf("Sample got %s, want no mapassign frames", l)
+		}
+	}
+
+	if !foundGrowMap {
+		t.Errorf("Profile got:\n%s\nwant sample in runtime/pprof.growMap", strings.Join(actual, "\n"))
+	}
+}