Skip to content

Commit

Permalink
runtime/pprof: fix incorrect assumption in TestMapping
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
hyangah committed Jun 18, 2018
1 parent 741dad2 commit 88b442f
Showing 1 changed file with 26 additions and 20 deletions.
46 changes: 26 additions & 20 deletions src/runtime/pprof/proto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 88b442f

Please sign in to comment.