runtime/pprof: clarify recursive inline heuristic

Following CL 226818, the compiler will allow inlining a single cycle in
an inline chain. Immediately-recursive functions are still disallowed,
which is what this heuristic refers to.

Add a regression test for this case.

Note that in addition to this check, if the compiler were to inline
multiple cycles via a loop (i.e., rather than appending duplicate code),
much more work would be required here to handle a single address
appearing in multiple different inline frames.

Updates #29737

Change-Id: I88de15cfbeabb9c04381e1c12cc36778623132a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/227346
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>

src/runtime/pprof/pprof_test.go

@@ -227,6 +227,25 @@ func recursionCallee(n, x int) int {
 	return y * recursionCallee(n-1, x)
 }
 
+func recursionChainTop(x int, pcs []uintptr) {
+	if x < 0 {
+		return
+	}
+	recursionChainMiddle(x, pcs)
+}
+
+func recursionChainMiddle(x int, pcs []uintptr) {
+	recursionChainBottom(x, pcs)
+}
+
+func recursionChainBottom(x int, pcs []uintptr) {
+	// This will be called each time, we only care about the last. We
+	// can't make this conditional or this function won't be inlined.
+	dumpCallers(pcs)
+
+	recursionChainTop(x - 1, pcs)
+}
+
 func parseProfile(t *testing.T, valBytes []byte, f func(uintptr, []*profile.Location, map[string][]string)) *profile.Profile {
 	p, err := profile.Parse(bytes.NewReader(valBytes))
 	if err != nil {
@@ -1158,11 +1177,12 @@ func TestTracebackAll(t *testing.T) {
 	}
 }
 
-// TestTryAdd tests the cases that's hard to test with real program execution.
+// TestTryAdd tests the cases that are hard to test with real program execution.
-// For example, the current go compilers may not inline functions involved in recursion
+//
-// but that may not be true in the future compilers. This tests such cases by
+// For example, the current go compilers may not always inline functions
-// using fake call sequences and forcing the profile build utilizing
+// involved in recursion but that may not be true in the future compilers. This
-// translateCPUProfile defined in proto_test.go
+// tests such cases by using fake call sequences and forcing the profile build
+// utilizing translateCPUProfile defined in proto_test.go
 func TestTryAdd(t *testing.T) {
 	if _, found := findInlinedCall(inlinedCallerDump, 4<<10); !found {
 		t.Skip("Can't determine whether anything was inlined into inlinedCallerDump.")
@@ -1177,6 +1197,23 @@ func TestTryAdd(t *testing.T) {
 		inlinedCallerStack[i] = uint64(pcs[i])
 	}
 
+	if _, found := findInlinedCall(recursionChainBottom, 4<<10); !found {
+		t.Skip("Can't determine whether anything was inlined into recursionChainBottom.")
+	}
+
+	// recursionChainTop
+	//   recursionChainMiddle
+	//     recursionChainBottom
+	//       recursionChainTop
+	//         recursionChainMiddle
+	//           recursionChainBottom
+	pcs = make([]uintptr, 6)
+	recursionChainTop(1, pcs)
+	recursionStack := make([]uint64, len(pcs))
+	for i := range pcs {
+		recursionStack[i] = uint64(pcs[i])
+	}
+
 	period := int64(2000 * 1000) // 1/500*1e9 nanosec.
 
 	testCases := []struct {
@@ -1225,14 +1262,14 @@ func TestTryAdd(t *testing.T) {
 			{Value: []int64{4242, 4242 * period}, Location: []*profile.Location{{ID: 1}}},
 		},
 	}, {
-		// If a function is called recursively then it must not be
+		// If a function is directly called recursively then it must
-		// inlined in the caller.
+		// not be inlined in the caller.
 		//
 		// N.B. We're generating an impossible profile here, with a
 		// recursive inlineCalleeDump call. This is simulating a non-Go
 		// function that looks like an inlined Go function other than
 		// its recursive property. See pcDeck.tryAdd.
-		name: "recursive_func_is_not_inlined",
+		name: "directly_recursive_func_is_not_inlined",
 		input: []uint64{
 			3, 0, 500, // hz = 500. Must match the period.
 			5, 0, 30, inlinedCallerStack[0], inlinedCallerStack[0],
@@ -1245,6 +1282,28 @@ func TestTryAdd(t *testing.T) {
 			{Value: []int64{30, 30 * period}, Location: []*profile.Location{{ID: 1}, {ID: 1}, {ID: 2}}},
 			{Value: []int64{40, 40 * period}, Location: []*profile.Location{{ID: 1}, {ID: 2}}},
 		},
+	}, {
+		name: "recursion_chain_inline",
+		input: []uint64{
+			3, 0, 500, // hz = 500. Must match the period.
+			9, 0, 10, recursionStack[0], recursionStack[1], recursionStack[2], recursionStack[3], recursionStack[4], recursionStack[5],
+		},
+		wantLocs: [][]string{
+			{"runtime/pprof.recursionChainBottom"},
+			{
+				"runtime/pprof.recursionChainMiddle",
+				"runtime/pprof.recursionChainTop",
+				"runtime/pprof.recursionChainBottom",
+			},
+			{
+				"runtime/pprof.recursionChainMiddle",
+				"runtime/pprof.recursionChainTop",
+				"runtime/pprof.TestTryAdd", // inlined into the test.
+			},
+		},
+		wantSamples: []*profile.Sample{
+			{Value: []int64{10, 10 * period}, Location: []*profile.Location{{ID: 1}, {ID: 2}, {ID: 3}}},
+		},
 	}, {
 		name: "truncated_stack_trace_later",
 		input: []uint64{

src/runtime/pprof/proto.go

@@ -461,7 +461,7 @@ func (b *profileBuilder) appendLocsForStack(locs []uint64, stk []uintptr) (newLo
 // have the following properties:
 //   Frame's Func is nil (note: also true for non-Go functions), and
 //   Frame's Entry matches its entry function frame's Entry (note: could also be true for recursive calls and non-Go functions), and
-//   Frame's Name does not match its entry function frame's name (note: inlined functions cannot be recursive).
+//   Frame's Name does not match its entry function frame's name (note: inlined functions cannot be directly recursive).
 //
 // As reading and processing the pcs in a stack trace one by one (from leaf to the root),
 // we use pcDeck to temporarily hold the observed pcs and their expanded frames