From 88b442f5c08a2984e6800e83a483a6c7f4b24cd1 Mon Sep 17 00:00:00 2001 From: Hana Kim Date: Mon, 18 Jun 2018 12:03:53 -0400 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/runtime/pprof/proto_test.go | 46 +++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 20 deletions(-) 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 } -- 2.50.0