runtime/pprof: clean up call/return PCs in memory profiles

Proto profile conversion is inconsistent about call vs return PCs in
profile locations. The proto defines locations to be call PCs. This is
what we do when proto-izing CPU profiles, but we fail to convert the
return PCs in memory and count profile stacks to call PCs when
converting them to proto locations.

Fix this in the heap and count profile conversion functions.
TestConvertMemProfile also hard-codes this failure to convert from
return PCs to call PCs, so fix up the addresses in the synthesized
profile to be return PCs while checking that we get call PCs out of
the conversion.

Change-Id: If1fc028b86fceac6d71a2d9fa6c41ff442c89296
Reviewed-on: https://go-review.googlesource.com/42951
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>

src/runtime/pprof/pprof.go

@@ -398,10 +398,11 @@ func printCountProfile(w io.Writer, debug int, name string, p countProfile) erro
 	for _, k := range keys {
 		values[0] = int64(count[k])
 		locs = locs[:0]
-		for i, addr := range p.Stack(index[k]) {
-			if false && i > 0 { // TODO: why disabled?
-				addr--
-			}
+		for _, addr := range p.Stack(index[k]) {
+			// For count profiles, all stack addresses are
+			// return PCs. Adjust them to be call PCs for
+			// locForPC.
+			addr--
 			locs = append(locs, b.locForPC(addr))
 		}
 		b.pbSample(values, locs, nil)

src/runtime/pprof/proto.go

@@ -183,6 +183,7 @@ func (b *profileBuilder) pbMapping(tag int, id, base, limit, offset uint64, file
 }
 
 // locForPC returns the location ID for addr.
+// addr must be a call address (not a return address).
 // It may emit to b.pb, so there must be no message encoding in progress.
 func (b *profileBuilder) locForPC(addr uintptr) uint64 {
 	id := uint64(b.locs[addr])

src/runtime/pprof/protomem.go

@@ -27,10 +27,11 @@ func writeHeapProto(w io.Writer, p []runtime.MemProfileRecord, rate int64) error
 		locs = locs[:0]
 		hideRuntime := true
 		for tries := 0; tries < 2; tries++ {
-			for i, addr := range r.Stack() {
-				if false && i > 0 { // TODO: why disabled?
-					addr--
-				}
+			for _, addr := range r.Stack() {
+				// For heap profiles, all stack
+				// addresses are return PCs. Adjust
+				// them to be call PCs for locForPC.
+				addr--
 				if hideRuntime {
 					if f := runtime.FuncForPC(addr); f != nil && strings.HasPrefix(f.Name(), "runtime.") {
 						continue

src/runtime/pprof/protomem_test.go

@@ -15,7 +15,11 @@ func TestConvertMemProfile(t *testing.T) {
 	addr1, addr2, map1, map2 := testPCs(t)
 
 	var buf bytes.Buffer
-	a1, a2 := uintptr(addr1), uintptr(addr2)
+	// MemProfileRecord stacks are return PCs, so add one to the
+	// addresses recorded in the "profile". The proto profile
+	// locations are call PCs, so conversion will subtract one
+	// from these and get back to addr1 and addr2.
+	a1, a2 := uintptr(addr1)+1, uintptr(addr2)+1
 	rate := int64(512 * 1024)
 	rec := []runtime.MemProfileRecord{
 		{AllocBytes: 4096, FreeBytes: 1024, AllocObjects: 4, FreeObjects: 1, Stack0: [32]uintptr{a1, a2}},