From: Hana Kim <hakim@google.com>
Date: Mon, 18 Jun 2018 16:03:53 +0000 (-0400)
Subject: runtime/pprof: fix incorrect assumption in TestMapping
X-Git-Tag: go1.11beta1~63
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=88b442f5c08a2984e6800e83a483a6c7f4b24cd1;p=gostls13.git

runtime/pprof: fix incorrect assumption in TestMapping

TestMapping assumed that there was only one mapping entry corresponding
to /exe/main, but that is not always true.
This CL changes the test logic to examine whether all referenced mappings
are symbolized. Based on the result, the test determines whether the
corresponding mapping entries' HasFunctions fields to be true or false.

I initially attempted to create two mappings for referenced locations
(one for symbolized and another for unsymbolized) as described in the
TODO in proto.go as part of fixing this bug. But that change requires
non-trivial modification in the upstream profile package so I decided
to just fix the test for now.

Fixes #25891

Change-Id: Id27a5b07bb5b59e133755a0f863bf56c0a4f7f2b
Reviewed-on: https://go-review.googlesource.com/119455
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
---

diff --git a/src/runtime/pprof/proto_test.go b/src/runtime/pprof/proto_test.go
index 4a47111e57..36c345b6d9 100644
--- a/src/runtime/pprof/proto_test.go
+++ b/src/runtime/pprof/proto_test.go
@@ -261,37 +261,43 @@ func TestMapping(t *testing.T) {
 			if err != nil {
 				t.Fatalf("failed to parse the generated profile data: %v", err)
 			}
+			t.Logf("Profile: %s", prof)
 
-			allResolved := !hasUnresolvedSymbol(prof)
-			if allResolved && traceback != "GoOnly" {
-				t.Log("No non-Go samples were sampled")
+			hit := make(map[*profile.Mapping]bool)
+			miss := make(map[*profile.Mapping]bool)
+			for _, loc := range prof.Location {
+				if symbolized(loc) {
+					hit[loc.Mapping] = true
+				} else {
+					miss[loc.Mapping] = true
+				}
+			}
+			if len(miss) == 0 {
+				t.Log("no location with missing symbol info was sampled")
 			}
 
 			for _, m := range prof.Mapping {
-				if !strings.Contains(m.File, "/exe/main") {
+				if miss[m] && m.HasFunctions {
+					t.Errorf("mapping %+v has HasFunctions=true, but contains locations with failed symbolization", m)
 					continue
 				}
-				if allResolved && !m.HasFunctions {
-					t.Errorf("HasFunctions=%t when all sampled PCs were symbolized\n%s", m.HasFunctions, prof)
-				}
-				if !allResolved && m.HasFunctions {
-					t.Errorf("HasFunctions=%t when some sampled PCs were not symbolized\n%s", m.HasFunctions, prof)
+				if !miss[m] && hit[m] && !m.HasFunctions {
+					t.Errorf("mapping %+v has HasFunctions=false, but all referenced locations from this lapping were symbolized successfully", m)
+					continue
 				}
 			}
 		})
 	}
 }
 
-func hasUnresolvedSymbol(prof *profile.Profile) bool {
-	for _, loc := range prof.Location {
-		if len(loc.Line) == 0 {
-			return true
-		}
-		l := loc.Line[0]
-		f := l.Function
-		if l.Line == 0 || f == nil || f.Name == "" || f.Filename == "" {
-			return true
-		}
+func symbolized(loc *profile.Location) bool {
+	if len(loc.Line) == 0 {
+		return false
+	}
+	l := loc.Line[0]
+	f := l.Function
+	if l.Line == 0 || f == nil || f.Name == "" || f.Filename == "" {
+		return false
 	}
-	return false
+	return true
 }