diff --git a/_fixtures/testvisitorprog.go b/_fixtures/testvisitorprog.go deleted file mode 100644 index 50b20fdb..00000000 --- a/_fixtures/testvisitorprog.go +++ /dev/null @@ -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") -} diff --git a/dwarf/line/state_machine.go b/dwarf/line/state_machine.go index b9255f85..5776f4a3 100644 --- a/dwarf/line/state_machine.go +++ b/dwarf/line/state_machine.go @@ -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) } } diff --git a/proc/proc.go b/proc/proc.go index 3e9302cd..d27e12f0 100644 --- a/proc/proc.go +++ b/proc/proc.go @@ -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 diff --git a/proc/proc_test.go b/proc/proc_test.go index b6223b1a..4cea2c97 100644 --- a/proc/proc_test.go +++ b/proc/proc_test.go @@ -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) } diff --git a/proc/threads.go b/proc/threads.go index 6600d892..2db494e5 100644 --- a/proc/threads.go +++ b/proc/threads.go @@ -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]) { diff --git a/service/test/integration_test.go b/service/test/integration_test.go index c38e027f..d54b81c6 100644 --- a/service/test/integration_test.go +++ b/service/test/integration_test.go @@ -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) }) diff --git a/source/source.go b/source/source.go deleted file mode 100644 index 41c858c5..00000000 --- a/source/source.go +++ /dev/null @@ -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 -} diff --git a/source/source_test.go b/source/source_test.go deleted file mode 100644 index d2059510..00000000 --- a/source/source_test.go +++ /dev/null @@ -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) - } - } - } -}