Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Intel syntax in the assembly reports #520

Merged
merged 9 commits into from
Apr 13, 2020
Merged

Support Intel syntax in the assembly reports #520

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a9ea908
Provide more helpful error message and require input for every non-bo…
wyk9787 Apr 8, 2020
c08f654
Add support for output assembly in Intel Syntax
wyk9787 Apr 10, 2020
bb2dcc0
Refactor to use varargs
wyk9787 Apr 10, 2020
3a9481b
Revert "Provide more helpful error message and require input for ever…
wyk9787 Apr 10, 2020
e4a7364
Fix nits
wyk9787 Apr 10, 2020
40bead3
Fix testDisasm
wyk9787 Apr 10, 2020
4f27ada
Merge branch 'master' into assembly
aalexand Apr 11, 2020
c3b8c71
Fix a few nits
wyk9787 Apr 12, 2020
1d913b5
Merge branch 'assembly' of github.com:wyk9787/pprof into assembly
wyk9787 Apr 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions driver/driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ type ObjTool interface {

// Disasm disassembles the named object file, starting at
// the start address and stopping at (before) the end address.
Disasm(file string, start, end uint64) ([]Inst, error)
Disasm(file string, start, end uint64, intelSyntax bool) ([]Inst, error)
kalyanac marked this conversation as resolved.
Show resolved Hide resolved
}

// An Inst is a single instruction in an assembly listing.
Expand Down Expand Up @@ -269,8 +269,8 @@ func (f *internalObjFile) Symbols(r *regexp.Regexp, addr uint64) ([]*plugin.Sym,
return pluginSyms, nil
}

func (o *internalObjTool) Disasm(file string, start, end uint64) ([]plugin.Inst, error) {
insts, err := o.ObjTool.Disasm(file, start, end)
func (o *internalObjTool) Disasm(file string, start, end uint64, intelSyntax bool) ([]plugin.Inst, error) {
insts, err := o.ObjTool.Disasm(file, start, end, intelSyntax)
if err != nil {
return nil, err
}
Expand Down
16 changes: 13 additions & 3 deletions internal/binutils/binutils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,22 @@ func findExe(cmd string, paths []string) (string, bool) {

// Disasm returns the assembly instructions for the specified address range
// of a binary.
func (bu *Binutils) Disasm(file string, start, end uint64) ([]plugin.Inst, error) {
func (bu *Binutils) Disasm(file string, start, end uint64, intelSyntax bool) ([]plugin.Inst, error) {
b := bu.get()
cmd := exec.Command(b.objdump, "-d", "-C", "--no-show-raw-insn", "-l",
var cmd *exec.Cmd
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this var, and use "cmd :=" below, as prior to to this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

args := []string{"-d", "-C", "--no-show-raw-insn", "-l",
fmt.Sprintf("--start-address=%#x", start),
fmt.Sprintf("--stop-address=%#x", end),
file)
file}

if intelSyntax {
if runtime.GOOS == "darwin" {
args = append([]string{"-x86-asm-syntax=intel"}, args...)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I personally would append this flag at the end, and move appending file to after this code. It doesn't really matter much in this case, but appending "args..." is O(N), unnecessarily. Again, this doesn't really matter here since the list is fixed len and tiny. So feel free to leave this as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense. Now changed.

} else {
args = append([]string{"-M", "intel"}, args...)
}
}
cmd = exec.Command(b.objdump, args...)
out, err := cmd.Output()
if err != nil {
return nil, fmt.Errorf("%v: %v", cmd.Args, err)
Expand Down
11 changes: 8 additions & 3 deletions internal/binutils/binutils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,9 @@ func skipUnlessDarwinAmd64(t *testing.T) {
}
}

func TestDisasm(t *testing.T) {
skipUnlessLinuxAmd64(t)
func testDisasm(t *testing.T, intelSyntax bool) {
bu := &Binutils{}
insts, err := bu.Disasm(filepath.Join("testdata", "exe_linux_64"), 0, math.MaxUint64)
insts, err := bu.Disasm(filepath.Join("testdata", "exe_linux_64"), 0, math.MaxUint64, intelSyntax)
if err != nil {
t.Fatalf("Disasm: unexpected error %v", err)
}
Expand All @@ -206,6 +205,12 @@ func TestDisasm(t *testing.T) {
}
}

func TestDisasm(t *testing.T) {
skipUnlessLinuxAmd64(t)
testDisasm(t, true)
testDisasm(t, false)
}

func findSymbol(syms []*plugin.Sym, name string) *plugin.Sym {
for _, s := range syms {
for _, n := range s.Name {
Expand Down
3 changes: 3 additions & 0 deletions internal/driver/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ var pprofVariables = variables{
"compact_labels": &variable{boolKind, "f", "", "Show minimal headers"},
"source_path": &variable{stringKind, "", "", "Search path for source files"},
"trim_path": &variable{stringKind, "", "", "Path to trim from source paths before search"},
"intel_syntax": &variable{boolKind, "f", "", helpText(
"Show assembly in Intel syntax",
"Only applicable to commands `disasm` and `weblist`")},

// Filtering options
"nodecount": &variable{intKind, "-1", "", helpText(
Expand Down
2 changes: 2 additions & 0 deletions internal/driver/driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@ func reportOptions(p *profile.Profile, numLabelUnits map[string]string, vars var

SourcePath: vars["source_path"].stringValue(),
TrimPath: vars["trim_path"].stringValue(),

IntelSyntax: vars["intel_syntax"].boolValue(),
}

if len(p.Mapping) > 0 && p.Mapping[0].File != "" {
Expand Down
2 changes: 1 addition & 1 deletion internal/driver/driver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1562,7 +1562,7 @@ func (*mockObjTool) Open(file string, start, limit, offset uint64) (plugin.ObjFi
return &mockFile{file, "abcdef", 0}, nil
}

func (m *mockObjTool) Disasm(file string, start, end uint64) ([]plugin.Inst, error) {
func (m *mockObjTool) Disasm(file string, start, end uint64, intelSyntax bool) ([]plugin.Inst, error) {
switch start {
case 0x1000:
return []plugin.Inst{
Expand Down
4 changes: 3 additions & 1 deletion internal/driver/fetch_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,9 @@ func (o testObj) Open(file string, start, limit, offset uint64) (plugin.ObjFile,
func (testObj) Demangler(_ string) func(names []string) (map[string]string, error) {
return func(names []string) (map[string]string, error) { return nil, nil }
}
func (testObj) Disasm(file string, start, end uint64) ([]plugin.Inst, error) { return nil, nil }
func (testObj) Disasm(file string, start, end uint64, intelSyntax bool) ([]plugin.Inst, error) {
return nil, nil
}

type testFile struct{ name, buildID string }

Expand Down
2 changes: 1 addition & 1 deletion internal/driver/webui_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func (obj fakeObjTool) Open(file string, start, limit, offset uint64) (plugin.Ob
return fakeObj{}, nil
}

func (obj fakeObjTool) Disasm(file string, start, end uint64) ([]plugin.Inst, error) {
func (obj fakeObjTool) Disasm(file string, start, end uint64, intelSyntax bool) ([]plugin.Inst, error) {
return []plugin.Inst{
{Addr: addrBase + 0, Text: "f1:asm", Function: "F1"},
{Addr: addrBase + 10, Text: "f2:asm", Function: "F2"},
Expand Down
2 changes: 1 addition & 1 deletion internal/plugin/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ type ObjTool interface {

// Disasm disassembles the named object file, starting at
// the start address and stopping at (before) the end address.
Disasm(file string, start, end uint64) ([]Inst, error)
Disasm(file string, start, end uint64, intelSyntax bool) ([]Inst, error)
}

// An Inst is a single instruction in an assembly listing.
Expand Down
4 changes: 3 additions & 1 deletion internal/report/report.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ type Options struct {
Symbol *regexp.Regexp // Symbols to include on disassembly report.
SourcePath string // Search path for source files.
TrimPath string // Paths to trim from source file paths.

IntelSyntax bool // Whether or not to print assembly in Intel syntax.
}

// Generate generates a report as directed by the Report.
Expand Down Expand Up @@ -438,7 +440,7 @@ 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)
insts, err := obj.Disasm(s.sym.File, s.sym.Start, s.sym.End, o.IntelSyntax)
if err != nil {
return err
}
Expand Down
6 changes: 3 additions & 3 deletions internal/report/source.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func PrintWebList(w io.Writer, rpt *Report, obj plugin.ObjTool, maxFiles int) er
ff := fileFunction{n.Info.File, n.Info.Name}
fns := fileNodes[ff]

asm := assemblyPerSourceLine(symbols, fns, ff.fileName, obj)
asm := assemblyPerSourceLine(symbols, fns, ff.fileName, obj, o.IntelSyntax)
start, end := sourceCoordinates(asm)

fnodes, path, err := getSourceFromFile(ff.fileName, reader, fns, start, end)
Expand Down Expand Up @@ -239,7 +239,7 @@ func sourceCoordinates(asm map[int][]assemblyInstruction) (start, end int) {
// assemblyPerSourceLine disassembles the binary containing a symbol
// and classifies the assembly instructions according to its
// corresponding source line, annotating them with a set of samples.
func assemblyPerSourceLine(objSyms []*objSymbol, rs graph.Nodes, src string, obj plugin.ObjTool) map[int][]assemblyInstruction {
func assemblyPerSourceLine(objSyms []*objSymbol, rs graph.Nodes, src string, obj plugin.ObjTool, intelSyntax bool) map[int][]assemblyInstruction {
assembly := make(map[int][]assemblyInstruction)
// Identify symbol to use for this collection of samples.
o := findMatchingSymbol(objSyms, rs)
Expand All @@ -248,7 +248,7 @@ func assemblyPerSourceLine(objSyms []*objSymbol, rs graph.Nodes, src string, obj
}

// Extract assembly for matched symbol
insts, err := obj.Disasm(o.sym.File, o.sym.Start, o.sym.End)
insts, err := obj.Disasm(o.sym.File, o.sym.Start, o.sym.End, intelSyntax)
if err != nil {
return assembly
}
Expand Down
2 changes: 1 addition & 1 deletion internal/symbolizer/symbolizer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ func (mockObjTool) Open(file string, start, limit, offset uint64) (plugin.ObjFil
return mockObjFile{frames: mockAddresses}, nil
}

func (mockObjTool) Disasm(file string, start, end uint64) ([]plugin.Inst, error) {
func (mockObjTool) Disasm(file string, start, end uint64, intelSyntax bool) ([]plugin.Inst, error) {
return nil, fmt.Errorf("disassembly not supported")
}

Expand Down