internal/report: include the last byte in disassembly

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

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 {

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

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)
 			}
 		}

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)