diff --git a/driver/driver.go b/driver/driver.go index 657a673521..ae37b18629 100644 --- a/driver/driver.go +++ b/driver/driver.go @@ -143,8 +143,8 @@ type ObjTool interface { // corresponding to the start address. Open(file string, start, limit, offset uint64, relocationSymbol string) (ObjFile, error) - // Disasm disassembles the named object file, starting at - // the start address and stopping at (before) the end address. + // Disasm disassembles the named object file in the address range + // [start, end), including start and excluding end. Disasm(file string, start, end uint64, intelSyntax bool) ([]Inst, error) } @@ -178,7 +178,7 @@ type ObjFile interface { // If r is not nil, Symbols restricts the list to symbols // with names matching the regular expression. // If addr is not zero, Symbols restricts the list to symbols - // containing that address. + // containing that address in the inclusive range [Sym.Start, Sym.End]. Symbols(r *regexp.Regexp, addr uint64) ([]*Sym, error) // Close closes the file, releasing associated resources. @@ -194,7 +194,8 @@ type Frame struct { StartLine int // start line of function (if available) } -// A Sym describes a single symbol in an object file. +// A Sym describes a single symbol in an object file. Its address range +// [Start, End] includes both endpoints. type Sym struct { Name []string // names of symbol (many if symbol was dedup'ed) File string // object file containing symbol diff --git a/internal/binutils/binutils.go b/internal/binutils/binutils.go index 7a7fd0ff1a..b7f402445b 100644 --- a/internal/binutils/binutils.go +++ b/internal/binutils/binutils.go @@ -271,8 +271,8 @@ func findExe(cmd string, paths []string) (string, bool) { return cmd, false } -// Disasm returns the assembly instructions for the specified address range -// of a binary. +// Disasm returns the assembly instructions in the address range [start, end) +// of a binary, including start and excluding end. func (bu *Binutils) Disasm(file string, start, end uint64, intelSyntax bool) ([]plugin.Inst, error) { b := bu.get() if !b.objdumpFound { diff --git a/internal/plugin/plugin.go b/internal/plugin/plugin.go index e6f0827633..914afd2ad0 100644 --- a/internal/plugin/plugin.go +++ b/internal/plugin/plugin.go @@ -114,8 +114,8 @@ type ObjTool interface { // corresponding to the start address. Open(file string, start, limit, offset uint64, relocationSymbol string) (ObjFile, error) - // Disasm disassembles the named object file, starting at - // the start address and stopping at (before) the end address. + // Disasm disassembles the named object file in the address range + // [start, end), including start and excluding end. Disasm(file string, start, end uint64, intelSyntax bool) ([]Inst, error) } @@ -150,7 +150,7 @@ type ObjFile interface { // If r is not nil, Symbols restricts the list to symbols // with names matching the regular expression. // If addr is not zero, Symbols restricts the list to symbols - // containing that address. + // containing that address in the inclusive range [Sym.Start, Sym.End]. Symbols(r *regexp.Regexp, addr uint64) ([]*Sym, error) // Close closes the file, releasing associated resources. @@ -166,7 +166,8 @@ type Frame struct { StartLine int // start line of function (if available) } -// A Sym describes a single symbol in an object file. +// A Sym describes a single symbol in an object file. Its address range +// [Start, End] includes both endpoints. type Sym struct { Name []string // names of symbol (many if symbol was dedup'ed) File string // object file containing symbol diff --git a/internal/report/report.go b/internal/report/report.go index 02a836c5ad..47155baeba 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -454,7 +454,13 @@ func PrintAssembly(w io.Writer, rpt *Report, obj plugin.ObjTool, maxFuncs int) e flatSum, cumSum := sns.Sum() // Get the function assembly. - insts, err := obj.Disasm(s.sym.File, s.sym.Start, s.sym.End, o.IntelSyntax) + // Symbol ends are inclusive, but Disasm expects an exclusive end. + // Keep the unknown end of the final nm symbol from wrapping to zero. + end := s.sym.End + if end != ^uint64(0) { + end++ + } + insts, err := obj.Disasm(s.sym.File, s.sym.Start, end, o.IntelSyntax) if err != nil { return err } @@ -588,12 +594,14 @@ func (o orderSyms) Less(i, j int) bool { return o.less(o.v[i], o.v[j]) } func (o orderSyms) Swap(i, j int) { o.v[i], o.v[j] = o.v[j], o.v[i] } // nodesPerSymbol classifies nodes into a group of symbols. +// A node belongs to a symbol when its object address is in the inclusive +// range [sym.Start, sym.End]. func nodesPerSymbol(ns graph.Nodes, symbols []*objSymbol) map[*objSymbol]graph.Nodes { symNodes := make(map[*objSymbol]graph.Nodes) for _, s := range symbols { // Gather samples for this symbol. for _, n := range ns { - if address, err := s.file.ObjAddr(n.Info.Address); err == nil && address >= s.sym.Start && address < s.sym.End { + if address, err := s.file.ObjAddr(n.Info.Address); err == nil && address >= s.sym.Start && address <= s.sym.End { symNodes[s] = append(symNodes[s], n) } } diff --git a/internal/report/report_test.go b/internal/report/report_test.go index 678fef9e49..7fc9e53015 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -28,6 +28,7 @@ import ( "github.com/google/pprof/internal/binutils" "github.com/google/pprof/internal/graph" + "github.com/google/pprof/internal/plugin" "github.com/google/pprof/internal/proftest" "github.com/google/pprof/profile" ) @@ -509,6 +510,96 @@ func TestComputeTotal(t *testing.T) { } } +func TestPrintAssemblySymbolEnd(t *testing.T) { + for _, tc := range []struct { + name string + end uint64 + sampleAddr uint64 + lastAddr uint64 + }{ + {"last instruction", 0x1001, 0x1000, 0x1001}, + {"sample at last byte", 0x1001, 0x1001, 0x1001}, + {"single byte symbol", 0x1000, 0x1000, 0x1000}, + {"unknown symbol end", ^uint64(0), 0x1000, 0x1001}, + } { + t.Run(tc.name, func(t *testing.T) { + mapping := &profile.Mapping{ID: 1, File: "testbin", Start: 0x1000, Limit: 0x2000} + function := &profile.Function{ID: 1, Name: "function"} + location := &profile.Location{ + ID: 1, Mapping: mapping, Address: tc.sampleAddr, + Line: []profile.Line{{Function: function}}, + } + prof := &profile.Profile{ + SampleType: []*profile.ValueType{{Type: "samples", Unit: "count"}}, + Sample: []*profile.Sample{{Location: []*profile.Location{location}, Value: []int64{7}}}, + Mapping: []*profile.Mapping{mapping}, + Location: []*profile.Location{location}, + Function: []*profile.Function{function}, + } + obj := assemblyObjTool{ + symbol: &plugin.Sym{Name: []string{"function"}, File: "testbin", Start: 0x1000, End: tc.end}, + insts: []plugin.Inst{{Addr: tc.lastAddr, Text: "RET"}, {Addr: tc.lastAddr + 1, Text: "next symbol"}}, + } + if tc.lastAddr > obj.symbol.Start { + obj.insts = append([]plugin.Inst{{Addr: obj.symbol.Start, Text: "NOP"}}, obj.insts...) + } + rpt := NewDefault(prof, Options{OutputFormat: Dis, Symbol: regexp.MustCompile("function")}) + var buf bytes.Buffer + if err := PrintAssembly(&buf, rpt, obj, -1); err != nil { + t.Fatal(err) + } + output := buf.String() + if want := fmt.Sprintf("%x: RET", tc.lastAddr); !strings.Contains(output, want) { + t.Errorf("missing last instruction %q in:\n%s", want, output) + } + if tc.end != ^uint64(0) && strings.Contains(output, "next symbol") { + t.Errorf("disassembled beyond the symbol:\n%s", output) + } + if want := fmt.Sprintf("%10s %10s (flat, cum)", "7", "7"); !strings.Contains(output, want) { + t.Errorf("missing sample totals %q in:\n%s", want, output) + } + if tc.sampleAddr == tc.lastAddr { + if want := fmt.Sprintf("%10s %10s %10x: RET", "7", "7", tc.lastAddr); !strings.Contains(output, want) { + t.Errorf("missing last-instruction sample %q in:\n%s", want, output) + } + } + }) + } +} + +type assemblyObjTool struct { + symbol *plugin.Sym + insts []plugin.Inst +} + +func (o assemblyObjTool) Open(string, uint64, uint64, uint64, string) (plugin.ObjFile, error) { + return assemblyObjFile{symbol: o.symbol}, nil +} + +func (o assemblyObjTool) Disasm(_ string, start, end uint64, _ bool) ([]plugin.Inst, error) { + var insts []plugin.Inst + for _, inst := range o.insts { + if inst.Addr >= start && inst.Addr < end { + insts = append(insts, inst) + } + } + return insts, nil +} + +type assemblyObjFile struct { + symbol *plugin.Sym +} + +func (o assemblyObjFile) Symbols(*regexp.Regexp, uint64) ([]*plugin.Sym, error) { + return []*plugin.Sym{o.symbol}, nil +} + +func (assemblyObjFile) ObjAddr(addr uint64) (uint64, error) { return addr, nil } +func (assemblyObjFile) Close() error { return nil } +func (o assemblyObjFile) Name() string { return o.symbol.File } +func (assemblyObjFile) BuildID() string { return "" } +func (assemblyObjFile) SourceLine(uint64) ([]plugin.Frame, error) { return nil, nil } + func TestPrintAssemblyErrorMessage(t *testing.T) { profile := readProfile(filepath.Join("testdata", "sample.cpu"), t)