From 4a8b9bd2646a5b297197ffd1c412ef6afebe5c0d Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Thu, 26 Mar 2020 15:10:21 -0400 Subject: [PATCH] runtime/pprof: increment fake overflow record PC gentraceback generates PCs which are usually following the CALL instruction. For those that aren't, it fixes up the PCs so that functions processing the output can unconditionally decrement the PC. runtime_expandInlineFrames does this unconditional decrement when looking up the function. However, the fake stack frame generated for overflow records fails to meet the contract, and decrementing the PC results in a PC in the previous function. If that function contains inlined call, runtime_expandInlineFrames will not short-circuit and will panic trying to look up a PC that doesn't exist. Note that the added test does not fail at HEAD. It will only fail (with a panic) if the function preceeding lostProfileEvent contains inlined function calls. At the moment (on linux/amd64), that is runtime/pprof.addMaxRSS, which does not. Fixes #38096 Change-Id: Iad0819f23c566011c920fd9a5b1254719228da0b Reviewed-on: https://go-review.googlesource.com/c/go/+/225661 Reviewed-by: Hyang-Ah Hana Kim Reviewed-by: Heschi Kreinick Reviewed-by: Keith Randall Run-TryBot: Michael Pratt TryBot-Result: Gobot Gobot --- src/runtime/pprof/pprof_test.go | 12 ++++++++++++ src/runtime/pprof/proto.go | 5 ++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go index 5bfc3b6134..83b3152d68 100644 --- a/src/runtime/pprof/pprof_test.go +++ b/src/runtime/pprof/pprof_test.go @@ -1171,6 +1171,18 @@ func TestTryAdd(t *testing.T) { {Value: []int64{10, 10 * period}, Location: []*profile.Location{{ID: 1}, {ID: 1}}}, {Value: []int64{20, 20 * period}, Location: []*profile.Location{{ID: 1}}}, }, + }, { + name: "bug38096", + input: []uint64{ + 3, 0, 500, // hz = 500. Must match the period. + // count (data[2]) == 0 && len(stk) == 1 is an overflow + // entry. The "stk" entry is actually the count. + 4, 0, 0, 4242, + }, + wantLocs: [][]string{{"runtime/pprof.lostProfileEvent"}}, + wantSamples: []*profile.Sample{ + {Value: []int64{4242, 4242 * period}, Location: []*profile.Location{{ID: 1}}}, + }, }, { // If a function is called recursively then it must not be // inlined in the caller. diff --git a/src/runtime/pprof/proto.go b/src/runtime/pprof/proto.go index 416ace7ab2..bb63153a70 100644 --- a/src/runtime/pprof/proto.go +++ b/src/runtime/pprof/proto.go @@ -322,7 +322,10 @@ func (b *profileBuilder) addCPUData(data []uint64, tags []unsafe.Pointer) error // overflow record count = uint64(stk[0]) stk = []uint64{ - uint64(funcPC(lostProfileEvent)), + // gentraceback guarantees that PCs in the + // stack can be unconditionally decremented and + // still be valid, so we must do the same. + uint64(funcPC(lostProfileEvent)+1), } } b.m.lookup(stk, tag).count += int64(count)