proc: Improve 'next' functionality

Instead of trying to be clever and make an 'educated guess' as to where
the flow of control may go next, simple do the more naive, yet correct,
approach of setting a breakpoint everywhere we can in the function and
seeing where we end up. On top of this we were already setting a
breakpoint at the return address and deferred functions, so that remains
the same.

This removes a lot of gnarly, hard to maintain code and takes all the
guesswork out of this command.

Fixes #281
This commit is contained in:
Derek Parker 2015-10-22 10:07:24 -07:00
parent 28ede53b31
commit 28e0a322bf
8 changed files with 63 additions and 528 deletions

@ -1,72 +0,0 @@
package main
import "fmt"
func main() {
for {
for i := 0; i < 5; i++ {
if i == 0 {
fmt.Println("it is zero!")
} else if i == 1 {
fmt.Println("it is one")
} else {
fmt.Println("wat")
}
switch i {
case 3:
fmt.Println("three")
case 4:
fmt.Println("four")
}
}
fmt.Println("done")
}
{
fmt.Println("useless line")
}
fmt.Println("end")
}
func noop() {
var (
i = 1
j = 2
)
if j == 3 {
fmt.Println(i)
}
fmt.Println(j)
}
func looptest() {
for {
fmt.Println("wat")
if false {
fmt.Println("uh, wat")
break
}
}
fmt.Println("dun")
}
func endlesslooptest() {
for {
fmt.Println("foo")
fmt.Println("foo")
}
}
func decltest() {
var foo = "bar"
var baz = 9
fmt.Println(foo, baz)
}
func defertest() {
defer func() {
fmt.Println("this is a useless defer")
}()
fmt.Println("I should get here")
}

@ -109,9 +109,10 @@ func (dbl *DebugLines) AllPCsForFileLine(f string, l int) (pcs []uint64) {
func (dbl *DebugLines) AllPCsBetween(begin, end uint64, filename string) []uint64 {
lineInfo := dbl.GetLineInfo(filename)
var (
pcs []uint64
sm = newStateMachine(lineInfo)
buf = bytes.NewBuffer(lineInfo.Instructions)
pcs []uint64
lastaddr uint64
sm = newStateMachine(lineInfo)
buf = bytes.NewBuffer(lineInfo.Instructions)
)
for b, err := buf.ReadByte(); err == nil; b, err = buf.ReadByte() {
@ -119,7 +120,8 @@ func (dbl *DebugLines) AllPCsBetween(begin, end uint64, filename string) []uint6
if sm.address > end {
break
}
if sm.address >= begin && sm.address <= end {
if sm.address >= begin && sm.address > lastaddr {
lastaddr = sm.address
pcs = append(pcs, sm.address)
}
}

@ -16,7 +16,6 @@ import (
"github.com/derekparker/delve/dwarf/frame"
"github.com/derekparker/delve/dwarf/line"
"github.com/derekparker/delve/dwarf/reader"
"github.com/derekparker/delve/source"
)
// Process represents all of the information the debugger
@ -47,7 +46,6 @@ type Process struct {
firstStart bool
os *OSProcessDetails
arch Arch
ast *source.Searcher
breakpointIDCounter int
tempBreakpointIDCounter int
halt bool
@ -63,7 +61,6 @@ func New(pid int) *Process {
Breakpoints: make(map[uint64]*Breakpoint),
firstStart: true,
os: new(OSProcessDetails),
ast: source.New(),
ptraceChan: make(chan func()),
ptraceDoneChan: make(chan interface{}),
}
@ -167,34 +164,39 @@ func (dbp *Process) FindFileLocation(fileName string, lineno int) (uint64, error
// Note that setting breakpoints at that address will cause surprising behavior:
// https://github.com/derekparker/delve/issues/170
func (dbp *Process) FindFunctionLocation(funcName string, firstLine bool, lineOffset int) (uint64, error) {
fn := dbp.goSymTable.LookupFunc(funcName)
if fn == nil {
origfn := dbp.goSymTable.LookupFunc(funcName)
if origfn == nil {
return 0, fmt.Errorf("Could not find function %s\n", funcName)
}
if firstLine {
filename, lineno, _ := dbp.goSymTable.PCToLine(fn.Entry)
filename, lineno, _ := dbp.goSymTable.PCToLine(origfn.Entry)
if filepath.Ext(filename) != ".go" {
return fn.Entry, nil
return origfn.Entry, nil
}
lines, err := dbp.ast.NextLines(filename, lineno)
if err != nil {
return 0, err
for {
lineno++
pc, fn, _ := dbp.goSymTable.LineToPC(filename, lineno)
if fn != nil {
if fn.Name != funcName {
if strings.Contains(fn.Name, funcName) {
continue
}
break
}
if fn.Name == funcName {
return pc, nil
}
}
}
if len(lines) > 0 {
linePC, _, err := dbp.goSymTable.LineToPC(filename, lines[0])
return linePC, err
}
return fn.Entry, nil
return origfn.Entry, nil
} else if lineOffset > 0 {
filename, lineno, _ := dbp.goSymTable.PCToLine(fn.Entry)
filename, lineno, _ := dbp.goSymTable.PCToLine(origfn.Entry)
breakAddr, _, err := dbp.goSymTable.LineToPC(filename, lineno+lineOffset)
return breakAddr, err
}
return fn.Entry, nil
return origfn.Entry, nil
}
// Sends out a request that the debugged process halt

@ -339,25 +339,20 @@ func TestNextConcurrent(t *testing.T) {
})
}
func TestNextGoroutine(t *testing.T) {
testcases := []nextTest{
{47, 42},
}
testnext("testnextprog", testcases, "main.testgoroutine", t)
}
func TestNextFunctionReturn(t *testing.T) {
testcases := []nextTest{
{14, 35},
{14, 15},
{15, 35},
}
testnext("testnextprog", testcases, "main.helloworld", t)
}
func TestNextFunctionReturnDefer(t *testing.T) {
testcases := []nextTest{
{9, 6},
{6, 7},
{7, 10},
{8, 9},
{9, 10},
{10, 7},
{7, 8},
}
testnext("testnextdefer", testcases, "main.main", t)
}
@ -547,8 +542,8 @@ func (l1 *loc) match(l2 Stackframe) bool {
func TestStacktrace(t *testing.T) {
stacks := [][]loc{
[]loc{{4, "main.stacktraceme"}, {8, "main.func1"}, {16, "main.main"}},
[]loc{{4, "main.stacktraceme"}, {8, "main.func1"}, {12, "main.func2"}, {17, "main.main"}},
{{4, "main.stacktraceme"}, {8, "main.func1"}, {16, "main.main"}},
{{4, "main.stacktraceme"}, {8, "main.func1"}, {12, "main.func2"}, {17, "main.main"}},
}
withTestProcess("stacktraceprog", t, func(p *Process, fixture protest.Fixture) {
bp, err := setFunctionBreakpoint(p, "main.stacktraceme")
@ -586,7 +581,7 @@ func TestStacktrace2(t *testing.T) {
locations, err := p.CurrentThread.Stacktrace(40)
assertNoError(err, t, "Stacktrace()")
if !stackMatch([]loc{loc{-1, "main.f"}, loc{16, "main.main"}}, locations) {
if !stackMatch([]loc{{-1, "main.f"}, {16, "main.main"}}, locations) {
for i := range locations {
t.Logf("\t%s:%d [%s]\n", locations[i].Call.File, locations[i].Call.Line, locations[i].Call.Fn.Name)
}
@ -596,7 +591,7 @@ func TestStacktrace2(t *testing.T) {
assertNoError(p.Continue(), t, "Continue()")
locations, err = p.CurrentThread.Stacktrace(40)
assertNoError(err, t, "Stacktrace()")
if !stackMatch([]loc{loc{-1, "main.g"}, loc{17, "main.main"}}, locations) {
if !stackMatch([]loc{{-1, "main.g"}, {17, "main.main"}}, locations) {
for i := range locations {
t.Logf("\t%s:%d [%s]\n", locations[i].Call.File, locations[i].Call.Line, locations[i].Call.Fn.Name)
}

@ -9,7 +9,6 @@ import (
sys "golang.org/x/sys/unix"
"github.com/derekparker/delve/dwarf/frame"
"github.com/derekparker/delve/source"
)
// Thread represents a single thread in the traced process
@ -116,19 +115,6 @@ func (tbe ThreadBlockedError) Error() string {
}
// Set breakpoints for potential next lines.
//
// There are two modes of operation for this method. First,
// if we are executing Go code, we can use the stdlib AST
// information to determine which lines we could potentially
// end up at. Parsing the source file into an AST and traversing
// it lets us gain insight into whether we're at a branch, and
// where that branch could end up at, etc...
//
// However, if we are executing C code, we use the DWARF
// debug_line information and essentially set a breakpoint
// at every single line within the current function, and
// another at the functions return address, in case we're at
// the end.
func (thread *Thread) setNextBreakpoints() (err error) {
if thread.blocked() {
return ThreadBlockedError{}
@ -137,15 +123,6 @@ func (thread *Thread) setNextBreakpoints() (err error) {
if err != nil {
return err
}
g, err := thread.GetG()
if err != nil {
return err
}
if g.DeferPC != 0 {
if _, err = thread.dbp.SetTempBreakpoint(g.DeferPC); err != nil {
return err
}
}
// Grab info on our current stack frame. Used to determine
// whether we may be stepping outside of the current function.
@ -176,12 +153,31 @@ func (ge GoroutineExitingError) Error() string {
return fmt.Sprintf("goroutine %d is exiting", ge.goid)
}
// Use the AST to determine potential next lines.
// Set breakpoints at every line, and the return address. Also look for
// a deferred function and set a breakpoint there too.
func (thread *Thread) next(curpc uint64, fde *frame.FrameDescriptionEntry, file string, line int) error {
lines, err := thread.dbp.ast.NextLines(file, line)
pcs := thread.dbp.lineInfo.AllPCsBetween(fde.Begin(), fde.End(), file)
g, err := thread.GetG()
if err != nil {
if _, ok := err.(source.NoNodeError); !ok {
return err
return err
}
if g.DeferPC != 0 {
f, lineno, _ := thread.dbp.goSymTable.PCToLine(g.DeferPC)
for {
lineno++
dpc, _, err := thread.dbp.goSymTable.LineToPC(f, lineno)
if err == nil {
// We want to avoid setting an actual breakpoint on the
// entry point of the deferred function so instead create
// a fake breakpoint which will be cleaned up later.
thread.dbp.Breakpoints[g.DeferPC] = new(Breakpoint)
defer func() { delete(thread.dbp.Breakpoints, g.DeferPC) }()
if _, err = thread.dbp.SetTempBreakpoint(dpc); err != nil {
return err
}
break
}
}
}
@ -190,11 +186,6 @@ func (thread *Thread) next(curpc uint64, fde *frame.FrameDescriptionEntry, file
return err
}
pcs := make([]uint64, 0, len(lines))
for i := range lines {
pcs = append(pcs, thread.dbp.lineInfo.AllPCsForFileLine(file, lines[i])...)
}
var covered bool
for i := range pcs {
if fde.Cover(pcs[i]) {

@ -232,16 +232,10 @@ func TestNextGeneral(t *testing.T) {
testnext(testcases, "main.testnext", t)
}
func TestNextGoroutine(t *testing.T) {
testcases := []nextTest{
{47, 42},
}
testnext(testcases, "main.testgoroutine", t)
}
func TestNextFunctionReturn(t *testing.T) {
testcases := []nextTest{
{14, 35},
{14, 15},
{15, 35},
}
testnext(testcases, "main.helloworld", t)
}
@ -598,7 +592,7 @@ func TestClientServer_FindLocations(t *testing.T) {
})
withTestClient("testnextdefer", t, func(c service.Client) {
firstMainLine := findLocationHelper(t, c, "testnextdefer.go:9", false, 1, 0)[0]
firstMainLine := findLocationHelper(t, c, "testnextdefer.go:8", false, 1, 0)[0]
findLocationHelper(t, c, "main.main", false, 1, firstMainLine)
})

@ -1,315 +0,0 @@
package source
import (
"fmt"
"go/ast"
"go/parser"
"go/token"
)
type Searcher struct {
fileset *token.FileSet
visited map[string]*ast.File
}
func New() *Searcher {
return &Searcher{fileset: token.NewFileSet(), visited: make(map[string]*ast.File)}
}
type NoNodeError struct {
f string
l int
}
func (n NoNodeError) Error() string {
return fmt.Sprintf("could not find node at %s:%d", n.f, n.l)
}
// Returns the first node at the given file:line.
func (s *Searcher) FirstNodeAt(fname string, line int) (*ast.File, ast.Node, error) {
var node ast.Node
f, err := s.parse(fname)
if err != nil {
return nil, nil, err
}
ast.Inspect(f, func(n ast.Node) bool {
if n == nil {
return true
}
position := s.fileset.Position(n.Pos())
if position.Line == line {
node = n
return false
}
return true
})
if node == nil {
return nil, nil, NoNodeError{f: fname, l: line}
}
return f, node, nil
}
type Done string
func (d Done) Error() string {
return string(d)
}
// Returns all possible lines that could be executed after the given file:line,
// within the same source file.
func (s *Searcher) NextLines(fname string, line int) (lines []int, err error) {
parsedFile, n, err := s.FirstNodeAt(fname, line)
if err != nil {
return lines, nil
}
switch x := n.(type) {
// Follow if statements
case *ast.IfStmt:
lines = removeDuplicateLines(s.parseIfStmtBlock(x, parsedFile))
return
// Follow case statements.
// Append line for first statement following each 'case' condition.
case *ast.SwitchStmt:
var switchEnd int
ast.Inspect(x, func(n ast.Node) bool {
if stmt, ok := n.(*ast.SwitchStmt); ok {
switchEnd = s.fileset.Position(stmt.End()).Line
return true
}
if switchEnd < s.fileset.Position(x.Pos()).Line {
return false
}
if stmt, ok := n.(*ast.CaseClause); ok {
p := stmt.Body[0].Pos()
pos := s.fileset.Position(p)
lines = append(lines, pos.Line)
return false
}
return true
})
lines = removeDuplicateLines(lines)
return
// Default case - find next source line.
default:
lines = removeDuplicateLines(s.parseDefaultBlock(x, parsedFile, line))
return
}
return lines, nil
}
// Parses file named by fname, caching files it has already parsed.
func (s *Searcher) parse(fname string) (*ast.File, error) {
if f, ok := s.visited[fname]; ok {
return f, nil
}
f, err := parser.ParseFile(s.fileset, fname, nil, 0)
if err != nil {
return nil, err
}
s.visited[fname] = f
return f, nil
}
func (s *Searcher) nextLineAfter(parsedFile *ast.File, line int) (nextLine int) {
var done bool
ast.Inspect(parsedFile, func(n ast.Node) bool {
if done || n == nil {
return false
}
pos := s.fileset.Position(n.Pos())
if line < pos.Line {
nextLine = pos.Line
done = true
return false
}
return true
})
return
}
// If we are at an 'if' statement, employ the following algorithm:
// * Follow all 'else if' statements, appending their line number
// * Follow any 'else' statement if it exists, appending the line
// number of the statement following the 'else'.
// * If there is no 'else' statement, append line of first statement
// following the entire 'if' block.
func (s *Searcher) parseIfStmtBlock(ifRoot *ast.IfStmt, parsedFile *ast.File) []int {
var (
rbrace int
ifStmtLine = s.fileset.Position(ifRoot.Body.List[0].Pos()).Line
lines = []int{ifStmtLine}
)
for {
if ifRoot.Else == nil {
// Grab first line after entire 'if' block
rbrace = s.fileset.Position(ifRoot.Body.Rbrace).Line
return append(lines, s.nextLineAfter(parsedFile, rbrace))
}
// Continue following 'else if' branches.
if elseStmt, ok := ifRoot.Else.(*ast.IfStmt); ok {
lines = append(lines, s.fileset.Position(elseStmt.Pos()).Line)
ifRoot = elseStmt
continue
}
// Grab next line after final 'else'.
pos := s.fileset.Position(ifRoot.Else.Pos())
return append(lines, s.nextLineAfter(parsedFile, pos.Line))
}
}
// We are not at a branch, employ the following algorithm:
// * Traverse tree, storing any loop as a parent
// * Find next source line after the given line
// * Check and see if we've passed the scope of any parent we've
// stored. If so, pop them off the stack. The last parent that
// is left get's appending to our list of lines since we could
// end up at the top of the loop again.
func (s *Searcher) parseDefaultBlock(ifRoot ast.Node, parsedFile *ast.File, line int) []int {
var (
found bool
lines []int
parents []*ast.BlockStmt
parentBlockBeginLine int
deferEndLine int
)
ast.Inspect(parsedFile, func(n ast.Node) bool {
if found || n == nil {
return false
}
pos := s.fileset.Position(n.Pos())
if line < pos.Line && deferEndLine != 0 {
p := s.fileset.Position(n.Pos())
if deferEndLine < p.Line {
found = true
lines = append(lines, p.Line)
return false
}
}
if stmt, ok := n.(*ast.ForStmt); ok {
pos := s.fileset.Position(stmt.Pos())
if stmt.Cond == nil {
nextLine := s.fileset.Position(stmt.Body.List[0].Pos()).Line
if line < nextLine {
lines = []int{nextLine}
found = true
parents = nil
return false
}
}
parents = append(parents, stmt.Body)
parentBlockBeginLine = pos.Line
}
if _, ok := n.(*ast.GenDecl); ok {
return true
}
if dn, ok := n.(*ast.DeferStmt); ok && line < pos.Line {
endpos := s.fileset.Position(dn.End())
deferEndLine = endpos.Line
return false
}
if st, ok := n.(*ast.DeclStmt); ok {
beginpos := s.fileset.Position(st.Pos())
endpos := s.fileset.Position(st.End())
if beginpos.Line < endpos.Line {
return true
}
}
// Check to see if we've found the "next" line.
if line < pos.Line {
if _, ok := n.(*ast.BlockStmt); ok {
return true
}
var (
parent *ast.BlockStmt
parentEndLine int
)
for len(parents) > 0 {
parent = parents[len(parents)-1]
// Grab the line number of the right brace of the parent block.
parentEndLine = s.fileset.Position(parent.Rbrace).Line
// Check to see if we're still within the parents block.
// If we are, we're done and that is our parent.
if parentEndLine > line {
parentBlockBeginLine = s.fileset.Position(parent.Pos()).Line
break
}
// If we weren't, and there is only 1 parent, we no longer have one.
if len(parents) == 1 {
parent = nil
break
}
// Remove that parent from the stack.
parents = parents[0 : len(parents)-1]
}
if parent != nil {
var (
endfound bool
beginFound bool
beginLine int
)
ast.Inspect(parsedFile, func(n ast.Node) bool {
if n == nil || endfound {
return false
}
if _, ok := n.(*ast.BlockStmt); ok {
return true
}
pos := s.fileset.Position(n.Pos())
if parentBlockBeginLine < pos.Line && !beginFound {
beginFound = true
beginLine = pos.Line
return true
}
if parentEndLine < pos.Line {
if _, ok := n.(*ast.FuncDecl); !ok {
lines = append(lines, beginLine, pos.Line)
}
endfound = true
return false
}
return true
})
lines = append(lines, parentBlockBeginLine)
}
switch n.(type) {
case *ast.BranchStmt, *ast.FuncDecl:
default:
lines = append(lines, pos.Line)
}
found = true
return false
}
return true
})
if len(lines) == 0 && 0 < len(parents) {
parent := parents[len(parents)-1]
lbrace := s.fileset.Position(parent.Lbrace).Line
pos := s.fileset.Position(parent.List[0].Pos())
lines = append(lines, lbrace, pos.Line)
}
return lines
}
func removeDuplicateLines(lines []int) []int {
nl := make([]int, 0, len(lines))
fnd := make(map[int]bool)
for _, l := range lines {
if _, ok := fnd[l]; !ok {
fnd[l] = true
nl = append(nl, l)
}
}
return nl
}

@ -1,62 +0,0 @@
package source
import (
"fmt"
"go/ast"
"path/filepath"
"testing"
)
func TestTokenAtLine(t *testing.T) {
var (
tf, _ = filepath.Abs("../_fixtures/testvisitorprog.go")
v = New()
)
_, n, err := v.FirstNodeAt(tf, 8)
if err != nil {
t.Fatal(err)
}
if _, ok := n.(*ast.IfStmt); !ok {
t.Fatal("Did not get correct node")
}
}
func TestNextLines(t *testing.T) {
var (
tf, _ = filepath.Abs("../_fixtures/testvisitorprog.go")
v = New()
)
cases := []struct {
line int
nextlines []int
}{
{5, []int{7}},
{8, []int{9, 10, 13}},
{15, []int{17, 19}},
{25, []int{27}},
{22, []int{7, 25, 6}},
{33, []int{36}},
{36, []int{37, 40}},
{47, []int{45, 51, 44}},
{57, []int{55}},
{30, []int{32}},
{62, []int{63}},
{67, []int{71}},
{68, []int{69}},
}
for i, c := range cases {
lines, err := v.NextLines(tf, c.line)
if err != nil {
t.Fatal(err)
}
if len(lines) != len(c.nextlines) {
fmt.Println(lines)
t.Fatalf("did not get correct number of lines back expected %d got %d for test case %d got %#v", len(c.nextlines), len(lines), i+1, lines)
}
for j, l := range lines {
if l != c.nextlines[j] {
t.Fatalf("expected index %d to be %d got %d for case %d", j, c.nextlines[j], l, i+1)
}
}
}
}