diff --git a/internal/driver/testdata/pprof.cpu.flat.addresses.weblist b/internal/driver/testdata/pprof.cpu.flat.addresses.weblist index f765648e..2eb6b7d4 100644 --- a/internal/driver/testdata/pprof.cpu.flat.addresses.weblist +++ b/internal/driver/testdata/pprof.cpu.flat.addresses.weblist @@ -20,14 +20,7 @@ h1 { .inlinesrc { color: #000066; } -.deadsrc { -cursor: pointer; -} -.deadsrc:hover { -background-color: #eeeeee; -} .livesrc { -color: #0000ff; cursor: pointer; } .livesrc:hover { @@ -63,12 +56,12 @@ Type: cpu
Duration: 10s, Total samples = 1.12s (11.20%)
Total: 1.12s

line1000

testdata/file1000.src

   Total:       1.10s      1.10s (flat, cum) 98.21%
-      1        1.10s      1.10s           line1                1.10s      1.10s     1000:     instruction one                                                              file1000.src:1
+      1        1.10s      1.10s           line1                1.10s      1.10s     1000:     instruction one                                                              file1000.src:1
                    .          .     1001:     instruction two                                                              file1000.src:1
                                      ⋮
                    .          .     1003:     instruction four                                                             file1000.src:1
 
-      2            .          .           line2                    .          .     1002:     instruction three                                                            file1000.src:2
+      2            .          .           line2                    .          .     1002:     instruction three                                                            file1000.src:2
 
       3            .          .           line3 
       4            .          .           line4 
@@ -84,13 +77,13 @@ Duration: 10s, Total samples = 1.12s (11.20%)
Total: 1.12s

line1000< 3 . . line3 4 . . line4 5 . . line5 - 6 10ms 1.01s line6 line5 file3000.src:5 + 6 10ms 1.01s line6 line5 file3000.src:5 line2 file3000.src:2 10ms 1.01s 3000: instruction one file3000.src:2 7 . . line7 8 . . line8 - 9 . 110ms line9 line8 file3000.src:8 + 9 . 110ms line9 line8 file3000.src:8 . 100ms 3001: instruction two file3000.src:8 line5 file3000.src:5 . 10ms 3002: instruction three file3000.src:5 diff --git a/internal/report/source.go b/internal/report/source.go index 6e2becb6..54245e5f 100644 --- a/internal/report/source.go +++ b/internal/report/source.go @@ -132,6 +132,7 @@ func printWebSource(w io.Writer, rpt *Report, obj plugin.ObjTool) error { // sourcePrinter holds state needed for generating source+asm HTML listing. type sourcePrinter struct { reader *sourceReader + synth *synthCode objectTool plugin.ObjTool objects map[string]plugin.ObjFile // Opened object files sym *regexp.Regexp // May be nil @@ -146,6 +147,12 @@ type sourcePrinter struct { prettyNames map[string]string } +// addrInfo holds information for an address we are interested in. +type addrInfo struct { + loc *profile.Location // Always non-nil + obj plugin.ObjFile // May be nil +} + // instructionInfo holds collected information for an instruction. type instructionInfo struct { objAddr uint64 // Address in object file (with base subtracted out) @@ -207,6 +214,7 @@ func PrintWebList(w io.Writer, rpt *Report, obj plugin.ObjTool, maxFiles int) er func newSourcePrinter(rpt *Report, obj plugin.ObjTool, sourcePath string) *sourcePrinter { sp := &sourcePrinter{ reader: newSourceReader(sourcePath, rpt.options.TrimPath), + synth: newSynthCode(rpt.prof.Mapping), objectTool: obj, objects: map[string]plugin.ObjFile{}, sym: rpt.options.Symbol, @@ -225,19 +233,21 @@ func newSourcePrinter(rpt *Report, obj plugin.ObjTool, sourcePath string) *sourc } } - addrs := map[uint64]bool{} + addrs := map[uint64]addrInfo{} flat := map[uint64]int64{} cum := map[uint64]int64{} // Record an interest in the function corresponding to lines[index]. - markInterest := func(addr uint64, lines []profile.Line, index int) { - fn := lines[index] + markInterest := func(addr uint64, loc *profile.Location, index int) { + fn := loc.Line[index] if fn.Function == nil { return } sp.interest[fn.Function.Name] = true sp.interest[fn.Function.SystemName] = true - addrs[addr] = true + if _, ok := addrs[addr]; !ok { + addrs[addr] = addrInfo{loc, sp.objectFile(loc.Mapping)} + } } // See if sp.sym matches line. @@ -270,15 +280,21 @@ func newSourcePrinter(rpt *Report, obj plugin.ObjTool, sourcePath string) *sourc sp.prettyNames[line.Function.SystemName] = line.Function.Name } - cum[loc.Address] += value + addr := loc.Address + if addr == 0 { + // Some profiles are missing valid addresses. + addr = sp.synth.address(loc) + } + + cum[addr] += value if i == 0 { - flat[loc.Address] += value + flat[addr] += value } - if sp.sym == nil || (address != nil && loc.Address == *address) { + if sp.sym == nil || (address != nil && addr == *address) { // Interested in top-level entry of stack. if len(loc.Line) > 0 { - markInterest(loc.Address, loc.Line, len(loc.Line)-1) + markInterest(addr, loc, len(loc.Line)-1) } continue } @@ -287,7 +303,7 @@ func newSourcePrinter(rpt *Report, obj plugin.ObjTool, sourcePath string) *sourc matchFile := (loc.Mapping != nil && sp.sym.MatchString(loc.Mapping.File)) for j, line := range loc.Line { if (j == 0 && matchFile) || matches(line) { - markInterest(loc.Address, loc.Line, j) + markInterest(addr, loc, j) } } } @@ -306,10 +322,11 @@ func (sp *sourcePrinter) close() { } } -func (sp *sourcePrinter) expandAddresses(rpt *Report, addrs map[uint64]bool, flat map[uint64]int64) { +func (sp *sourcePrinter) expandAddresses(rpt *Report, addrs map[uint64]addrInfo, flat map[uint64]int64) { // We found interesting addresses (ones with non-zero samples) above. // Get covering address ranges and disassemble the ranges. - ranges := sp.splitIntoRanges(rpt.prof, addrs, flat) + ranges, unprocessed := sp.splitIntoRanges(rpt.prof, addrs, flat) + sp.handleUnprocessed(addrs, unprocessed) // Trim ranges if there are too many. const maxRanges = 25 @@ -394,78 +411,115 @@ func (sp *sourcePrinter) expandAddresses(rpt *Report, addrs map[uint64]bool, fla frames = lastFrames } - // See if the stack contains a function we are interested in. - for i, f := range frames { - if !sp.interest[f.Func] { - continue - } + sp.addStack(addr, frames) + } + } +} - // Record sub-stack under frame's file/line. - fname := canonicalizeFileName(f.File) - file := sp.files[fname] - if file == nil { - file = &sourceFile{ - fname: fname, - lines: map[int][]sourceInst{}, - funcName: map[int]string{}, - } - sp.files[fname] = file - } - callees := frames[:i] - stack := make([]callID, 0, len(callees)) - for j := len(callees) - 1; j >= 0; j-- { // Reverse so caller is first - stack = append(stack, callID{ - file: callees[j].File, - line: callees[j].Line, - }) - } - file.lines[f.Line] = append(file.lines[f.Line], sourceInst{addr, stack}) +func (sp *sourcePrinter) addStack(addr uint64, frames []plugin.Frame) { + // See if the stack contains a function we are interested in. + for i, f := range frames { + if !sp.interest[f.Func] { + continue + } - // Remember the first function name encountered per source line - // and assume that that line belongs to that function. - if _, ok := file.funcName[f.Line]; !ok { - file.funcName[f.Line] = f.Func - } + // Record sub-stack under frame's file/line. + fname := canonicalizeFileName(f.File) + file := sp.files[fname] + if file == nil { + file = &sourceFile{ + fname: fname, + lines: map[int][]sourceInst{}, + funcName: map[int]string{}, } + sp.files[fname] = file + } + callees := frames[:i] + stack := make([]callID, 0, len(callees)) + for j := len(callees) - 1; j >= 0; j-- { // Reverse so caller is first + stack = append(stack, callID{ + file: callees[j].File, + line: callees[j].Line, + }) + } + file.lines[f.Line] = append(file.lines[f.Line], sourceInst{addr, stack}) + + // Remember the first function name encountered per source line + // and assume that that line belongs to that function. + if _, ok := file.funcName[f.Line]; !ok { + file.funcName[f.Line] = f.Func } } } -// splitIntoRanges converts the set of addresses we are interested in into a set of address -// ranges to disassemble. -func (sp *sourcePrinter) splitIntoRanges(prof *profile.Profile, set map[uint64]bool, flat map[uint64]int64) []addressRange { - // List of mappings so we can stop expanding address ranges at mapping boundaries. - mappings := append([]*profile.Mapping{}, prof.Mapping...) - sort.Slice(mappings, func(i, j int) bool { return mappings[i].Start < mappings[j].Start }) +// synthAsm is the special disassembler value used for instructions without an object file. +const synthAsm = "" + +// handleUnprocessed handles addresses that were skipped by splitIntoRanges because they +// did not belong to a known object file. +func (sp *sourcePrinter) handleUnprocessed(addrs map[uint64]addrInfo, unprocessed []uint64) { + // makeFrames synthesizes a []plugin.Frame list for the specified address. + // The result will typically have length 1, but may be longer if address corresponds + // to inlined calls. + makeFrames := func(addr uint64) []plugin.Frame { + loc := addrs[addr].loc + stack := make([]plugin.Frame, 0, len(loc.Line)) + for _, line := range loc.Line { + fn := line.Function + if fn == nil { + continue + } + stack = append(stack, plugin.Frame{ + Func: fn.Name, + File: fn.Filename, + Line: int(line.Line), + }) + } + return stack + } - var result []addressRange - addrs := make([]uint64, 0, len(set)) - for addr := range set { - addrs = append(addrs, addr) + for _, addr := range unprocessed { + frames := makeFrames(addr) + x := instructionInfo{ + objAddr: addr, + length: 1, + disasm: synthAsm, + } + if len(frames) > 0 { + x.file = frames[0].File + x.line = frames[0].Line + } + sp.insts[addr] = x + + sp.addStack(addr, frames) + } +} + +// splitIntoRanges converts the set of addresses we are interested in into a set of address +// ranges to disassemble. It also returns the set of addresses found that did not have an +// associated object file and were therefore not added to an address range. +func (sp *sourcePrinter) splitIntoRanges(prof *profile.Profile, addrMap map[uint64]addrInfo, flat map[uint64]int64) ([]addressRange, []uint64) { + // Partition addresses into two sets: ones with a known object file, and ones without. + var addrs, unprocessed []uint64 + for addr, info := range addrMap { + if info.obj != nil { + addrs = append(addrs, addr) + } else { + unprocessed = append(unprocessed, addr) + } } sort.Slice(addrs, func(i, j int) bool { return addrs[i] < addrs[j] }) - mappingIndex := 0 const expand = 500 // How much to expand range to pick up nearby addresses. + var result []addressRange for i, n := 0, len(addrs); i < n; { begin, end := addrs[i], addrs[i] sum := flat[begin] i++ - // Advance to mapping containing addrs[i] - for mappingIndex < len(mappings) && mappings[mappingIndex].Limit <= begin { - mappingIndex++ - } - if mappingIndex >= len(mappings) { - // TODO(sanjay): Report missed address and its samples. - break - } - m := mappings[mappingIndex] - obj := sp.objectFile(m) - if obj == nil { - // TODO(sanjay): Report missed address and its samples. - continue - } + info := addrMap[begin] + m := info.loc.Mapping + obj := info.obj // Non-nil because of the partitioning done above. // Find following addresses that are close enough to addrs[i]. for i < n && addrs[i] <= end+2*expand && addrs[i] < m.Limit { @@ -488,7 +542,7 @@ func (sp *sourcePrinter) splitIntoRanges(prof *profile.Profile, set map[uint64]b result = append(result, addressRange{begin, end, obj, m, sum}) } - return result + return result, unprocessed } func (sp *sourcePrinter) initSamples(flat, cum map[uint64]int64) { @@ -674,9 +728,12 @@ func (sp *sourcePrinter) functions(f *sourceFile) []sourceFunction { return funcs } -// objectFile return the object for the named file, opening it if necessary. +// objectFile return the object for the specified mapping, opening it if necessary. // It returns nil on error. func (sp *sourcePrinter) objectFile(m *profile.Mapping) plugin.ObjFile { + if m == nil { + return nil + } if object, ok := sp.objects[m.File]; ok { return object // May be nil if we detected an error earlier. } @@ -734,12 +791,28 @@ func printFunctionSourceLine(w io.Writer, lineNo int, flat, cum int64, lineConte return } + nestedInfo := false + cl := "deadsrc" + for _, an := range assembly { + if len(an.inlineCalls) > 0 || an.instruction != synthAsm { + nestedInfo = true + cl = "livesrc" + } + } + fmt.Fprintf(w, - " %6d %10s %10s %8s %s ", - lineNo, + " %6d %10s %10s %8s %s ", + lineNo, cl, valueOrDot(flat, rpt), valueOrDot(cum, rpt), "", template.HTMLEscapeString(lineContents)) - srcIndent := indentation(lineContents) + if nestedInfo { + srcIndent := indentation(lineContents) + printNested(w, srcIndent, assembly, reader, rpt) + } + fmt.Fprintln(w) +} + +func printNested(w io.Writer, srcIndent int, assembly []assemblyInstruction, reader *sourceReader, rpt *Report) { fmt.Fprint(w, "") var curCalls []callID for i, an := range assembly { @@ -772,6 +845,9 @@ func printFunctionSourceLine(w io.Writer, lineNo int, flat, cum int64, lineConte template.HTMLEscapeString(filepath.Base(c.file)), c.line) } curCalls = an.inlineCalls + if an.instruction == synthAsm { + continue + } text := strings.Repeat(" ", srcIndent+4+4*len(curCalls)) + an.instruction fmt.Fprintf(w, " %8s %10s %10s %8x: %s %s\n", "", valueOrDot(flat, rpt), valueOrDot(cum, rpt), an.address, @@ -781,7 +857,7 @@ func printFunctionSourceLine(w io.Writer, lineNo int, flat, cum int64, lineConte // would cause double-escaping of file name. fileline) } - fmt.Fprintln(w, "") + fmt.Fprint(w, "") } // printFunctionClosing prints the end of a function in a weblist report. diff --git a/internal/report/source_html.go b/internal/report/source_html.go index 26e8bdbb..17c9f6eb 100644 --- a/internal/report/source_html.go +++ b/internal/report/source_html.go @@ -40,14 +40,7 @@ h1 { .inlinesrc { color: #000066; } -.deadsrc { -cursor: pointer; -} -.deadsrc:hover { -background-color: #eeeeee; -} .livesrc { -color: #0000ff; cursor: pointer; } .livesrc:hover { diff --git a/internal/report/source_test.go b/internal/report/source_test.go index 78c9d477..1524eafb 100644 --- a/internal/report/source_test.go +++ b/internal/report/source_test.go @@ -16,6 +16,7 @@ package report import ( "bytes" + "fmt" "io/ioutil" "os" "path/filepath" @@ -53,6 +54,74 @@ func TestWebList(t *testing.T) { } } +func TestSourceSyntheticAddress(t *testing.T) { + testSourceMapping(t, true) +} + +func TestSourceMissingMapping(t *testing.T) { + testSourceMapping(t, false) +} + +// testSourceMapping checks that source info is found even when no applicable +// Mapping/objectFile exists. The locations used in the test are either zero +// (if zeroAddress is true), or non-zero (otherwise). +func testSourceMapping(t *testing.T, zeroAddress bool) { + nextAddr := uint64(0) + + makeLoc := func(name, fname string, line int64) *profile.Location { + if !zeroAddress { + nextAddr++ + } + return &profile.Location{ + Address: nextAddr, + Line: []profile.Line{ + { + Function: &profile.Function{Name: name, Filename: fname}, + Line: line, + }, + }, + } + } + + // Create profile that will need synthetic addresses since it has no mappings. + foo100 := makeLoc("foo", "foo.go", 100) + bar50 := makeLoc("bar", "bar.go", 50) + prof := &profile.Profile{ + Sample: []*profile.Sample{ + { + Value: []int64{9}, + Location: []*profile.Location{foo100, bar50}, + }, + { + Value: []int64{17}, + Location: []*profile.Location{bar50}, + }, + }, + } + rpt := &Report{ + prof: prof, + options: &Options{ + Symbol: regexp.MustCompile("foo|bar"), + SampleValue: func(s []int64) int64 { return s[0] }, + }, + formatValue: func(v int64) string { return fmt.Sprint(v) }, + } + + var out bytes.Buffer + err := PrintWebList(&out, rpt, nil, -1) + if err != nil { + t.Fatalf("PrintWebList returned unexpected error: %v", err) + } + got := out.String() + expect := regexp.MustCompile( + `(?s)` + // Allow "." to match newline + `bar\.go.* 50\b.* 17 +26 .*` + + `foo\.go.* 100\b.* 9 +9 `) + if !expect.MatchString(got) { + t.Errorf("expected regular expression %v does not match output:\n%s\n", expect, got) + } +} + func TestOpenSourceFile(t *testing.T) { tempdir, err := ioutil.TempDir("", "") if err != nil { diff --git a/internal/report/synth.go b/internal/report/synth.go new file mode 100644 index 00000000..7a35bbcd --- /dev/null +++ b/internal/report/synth.go @@ -0,0 +1,39 @@ +package report + +import ( + "github.com/google/pprof/profile" +) + +// synthCode assigns addresses to locations without an address. +type synthCode struct { + next uint64 + addr map[*profile.Location]uint64 // Synthesized address assigned to a location +} + +func newSynthCode(mappings []*profile.Mapping) *synthCode { + // Find a larger address than any mapping. + s := &synthCode{next: 1} + for _, m := range mappings { + if s.next < m.Limit { + s.next = m.Limit + } + } + return s +} + +// address returns the synthetic address for loc, creating one if needed. +func (s *synthCode) address(loc *profile.Location) uint64 { + if loc.Address != 0 { + panic("can only synthesize addresses for locations without an address") + } + if addr, ok := s.addr[loc]; ok { + return addr + } + if s.addr == nil { + s.addr = map[*profile.Location]uint64{} + } + addr := s.next + s.next++ + s.addr[loc] = addr + return addr +} diff --git a/internal/report/synth_test.go b/internal/report/synth_test.go new file mode 100644 index 00000000..4d8895a4 --- /dev/null +++ b/internal/report/synth_test.go @@ -0,0 +1,36 @@ +package report + +import ( + "testing" + + "github.com/google/pprof/profile" +) + +func TestSynthAddresses(t *testing.T) { + s := newSynthCode(nil) + l1 := &profile.Location{} + addr1 := s.address(l1) + if s.address(l1) != addr1 { + t.Errorf("different calls with same location returned different addresses") + } + + l2 := &profile.Location{} + addr2 := s.address(l2) + if addr2 == addr1 { + t.Errorf("same address assigned to different locations") + } + +} + +func TestSynthAvoidsMapping(t *testing.T) { + mappings := []*profile.Mapping{ + {Start: 100, Limit: 200}, + {Start: 300, Limit: 400}, + } + s := newSynthCode(mappings) + loc := &profile.Location{} + addr := s.address(loc) + if addr >= 100 && addr < 200 || addr >= 300 && addr < 400 { + t.Errorf("synthetic location %d overlaps mapping %v", addr, mappings) + } +}