diff --git a/_fixtures/issue561.go b/_fixtures/issue561.go new file mode 100644 index 0000000000..82f4ba0e56 --- /dev/null +++ b/_fixtures/issue561.go @@ -0,0 +1,11 @@ +package main + +import "fmt" + +func testfunction() { + fmt.Printf("here!\n") +} + +func main() { + testfunction() +} diff --git a/_fixtures/teststepconcurrent.go b/_fixtures/teststepconcurrent.go new file mode 100644 index 0000000000..1b69e99dda --- /dev/null +++ b/_fixtures/teststepconcurrent.go @@ -0,0 +1,42 @@ +package main + +import ( + "fmt" + "time" + "sync" +) + +var v int = 99 +var wg sync.WaitGroup +var s string + +func Foo(x, y int) (z int) { + //s = fmt.Sprintf("x=%d, y=%d, z=%d\n", x, y, z) + z = x + y + return +} + +func Threads(fn func(x, y int) int) { + for j := 0; j < 100; j++ { + wg.Add(1) + go func() { + for k := 0; k < 100; k++ { + fn(1, 2) + time.Sleep(10 * time.Millisecond) + } + wg.Done() + }() + } +} + +func main() { + x := v + y := x * x + var z int + Threads(Foo) + for i := 0; i < 100; i++ { + z = Foo(x, y) + } + fmt.Printf("z=%d\n", z) + wg.Wait() +} diff --git a/_fixtures/teststepprog.go b/_fixtures/teststepprog.go new file mode 100644 index 0000000000..76971c8bc4 --- /dev/null +++ b/_fixtures/teststepprog.go @@ -0,0 +1,22 @@ +package main + +var n = 0 + +func CallFn2() { + n++ +} + +func CallFn(fn func()) { + fn() +} + +func CallEface(eface interface{}) { + if eface != nil { + n++ + } +} + +func main() { + CallFn(CallFn2) + CallEface(n) +} diff --git a/proc/breakpoints.go b/proc/breakpoints.go index 4a78808363..0e6186e3f4 100644 --- a/proc/breakpoints.go +++ b/proc/breakpoints.go @@ -17,11 +17,11 @@ type Breakpoint struct { File string Line int - Addr uint64 // Address breakpoint is set for. - OriginalData []byte // If software breakpoint, the data we replace with breakpoint instruction. - Name string // User defined name of the breakpoint - ID int // Monotonically increasing ID. - Temp bool // Whether this is a temp breakpoint (for next'ing). + Addr uint64 // Address breakpoint is set for. + OriginalData []byte // If software breakpoint, the data we replace with breakpoint instruction. + Name string // User defined name of the breakpoint + ID int // Monotonically increasing ID. + Kind BreakpointKind // Whether this is a temp breakpoint (for next'ing or stepping). // Breakpoint information Tracepoint bool // Tracepoint flag @@ -33,20 +33,41 @@ type Breakpoint struct { HitCount map[int]uint64 // Number of times a breakpoint has been reached in a certain goroutine TotalHitCount uint64 // Number of times a breakpoint has been reached - // When DeferCond is set the breakpoint will only trigger - // if the caller is runtime.gopanic or if the return address - // is in the DeferReturns array. - // Next sets DeferCond for the breakpoint it sets on the + // DeferReturns: when kind == NextDeferBreakpoint this breakpoint + // will also check if the caller is runtime.gopanic or if the return + // address is in the DeferReturns array. + // Next uses NextDeferBreakpoints for the breakpoint it sets on the // deferred function, DeferReturns is populated with the // addresses of calls to runtime.deferreturn in the current // function. This insures that the breakpoint on the deferred // function only triggers on panic or on the defer call to // the function, not when the function is called directly - DeferCond bool DeferReturns []uint64 - Cond ast.Expr // When Cond is not nil the breakpoint will be triggered only if evaluating Cond returns true + // Cond: if not nil the breakpoint will be triggered only if evaluating Cond returns true + Cond ast.Expr } +// Breakpoint Kind determines the behavior of delve when the +// breakpoint is reached. +type BreakpointKind int + +const ( + // UserBreakpoint is a user set breakpoint + UserBreakpoint BreakpointKind = iota + // NextBreakpoint is a breakpoint set by Next, Continue + // will stop on it and delete it + NextBreakpoint + // NextDeferBreakpoint is a breakpoint set by Next on the + // first deferred function. In addition to checking their condition + // breakpoints of this kind will also check that the function has been + // called by runtime.gopanic or through runtime.deferreturn. + NextDeferBreakpoint + // StepBreakpoint is a breakpoint set by Step on a CALL instruction, + // Continue will set a new breakpoint (of NextBreakpoint kind) on the + // destination of CALL, delete this breakpoint and then continue again + StepBreakpoint +) + func (bp *Breakpoint) String() string { return fmt.Sprintf("Breakpoint %d at %#v %s:%d (%d)", bp.ID, bp.Addr, bp.File, bp.Line, bp.TotalHitCount) } @@ -82,7 +103,7 @@ func (iae InvalidAddressError) Error() string { return fmt.Sprintf("Invalid address %#v\n", iae.address) } -func (dbp *Process) setBreakpoint(tid int, addr uint64, temp bool) (*Breakpoint, error) { +func (dbp *Process) setBreakpoint(tid int, addr uint64, kind BreakpointKind) (*Breakpoint, error) { if bp, ok := dbp.FindBreakpoint(addr); ok { return nil, BreakpointExistsError{bp.File, bp.Line, bp.Addr} } @@ -97,12 +118,12 @@ func (dbp *Process) setBreakpoint(tid int, addr uint64, temp bool) (*Breakpoint, File: f, Line: l, Addr: addr, - Temp: temp, + Kind: kind, Cond: nil, HitCount: map[int]uint64{}, } - if temp { + if kind != UserBreakpoint { dbp.tempBreakpointIDCounter++ newBreakpoint.ID = dbp.tempBreakpointIDCounter } else { @@ -133,7 +154,7 @@ func (bp *Breakpoint) checkCondition(thread *Thread) (bool, error) { if bp.Cond == nil { return true, nil } - if bp.DeferCond { + if bp.Kind == NextDeferBreakpoint { frames, err := thread.Stacktrace(2) if err == nil { ispanic := len(frames) >= 3 && frames[2].Current.Fn != nil && frames[2].Current.Fn.Name == "runtime.gopanic" @@ -168,6 +189,11 @@ func (bp *Breakpoint) checkCondition(thread *Thread) (bool, error) { return constant.BoolVal(v.Value), nil } +// Internal returns true for breakpoints not set directly by the user. +func (bp *Breakpoint) Internal() bool { + return bp.Kind != UserBreakpoint +} + // NoBreakpointError is returned when trying to // clear a breakpoint that does not exist. type NoBreakpointError struct { diff --git a/proc/proc.go b/proc/proc.go index fc3a11c43f..300deed597 100644 --- a/proc/proc.go +++ b/proc/proc.go @@ -228,12 +228,12 @@ func (dbp *Process) SetBreakpoint(addr uint64) (*Breakpoint, error) { if dbp.exited { return nil, &ProcessExitedError{} } - return dbp.setBreakpoint(dbp.CurrentThread.ID, addr, false) + return dbp.setBreakpoint(dbp.CurrentThread.ID, addr, UserBreakpoint) } // SetTempBreakpoint sets a temp breakpoint. Used during 'next' operations. -func (dbp *Process) SetTempBreakpoint(addr uint64, cond ast.Expr) (*Breakpoint, error) { - bp, err := dbp.setBreakpoint(dbp.CurrentThread.ID, addr, true) +func (dbp *Process) SetTempBreakpoint(addr uint64, kind BreakpointKind, cond ast.Expr) (*Breakpoint, error) { + bp, err := dbp.setBreakpoint(dbp.CurrentThread.ID, addr, kind) if err != nil { return nil, err } @@ -271,12 +271,12 @@ func (dbp *Process) Next() (err error) { return &ProcessExitedError{} } for i := range dbp.Breakpoints { - if dbp.Breakpoints[i].Temp { + if dbp.Breakpoints[i].Internal() { return fmt.Errorf("next while nexting") } } - if err = dbp.setNextBreakpoints(); err != nil { + if err = dbp.next(false); err != nil { switch err.(type) { case ThreadBlockedError, NoReturnAddr: // Noop default: @@ -300,13 +300,12 @@ func (dbp *Process) Continue() error { return err } - var trapthread *Thread - var err error + dbp.allGCache = nil + for _, th := range dbp.Threads { + th.clearBreakpointState() + } - dbp.run(func() error { - trapthread, err = dbp.trapWait(-1) - return nil - }) + trapthread, err := dbp.trapWait(-1) if err != nil { return err } @@ -332,11 +331,31 @@ func (dbp *Process) Continue() error { } return dbp.conditionErrors() case dbp.CurrentThread.onTriggeredTempBreakpoint(): - err := dbp.ClearTempBreakpoints() - if err != nil { - return err + if dbp.CurrentThread.CurrentBreakpoint.Kind == StepBreakpoint { + // See description of proc.(*Process).next for the meaning of StepBreakpoints + if err := dbp.conditionErrors(); err != nil { + return err + } + pc, err := dbp.CurrentThread.PC() + if err != nil { + return err + } + text, err := dbp.CurrentThread.Disassemble(pc, pc+maxInstructionLength, true) + if err != nil { + return err + } + // here we either set a breakpoint into the destination of the CALL + // instruction or we determined that the called function is hidden, + // either way we need to resume execution + if err = dbp.setStepIntoBreakpoint(text, sameGoroutineCondition(dbp.SelectedGoroutine)); err != nil { + return err + } + } else { + if err := dbp.ClearTempBreakpoints(); err != nil { + return err + } + return dbp.conditionErrors() } - return dbp.conditionErrors() case dbp.CurrentThread.onTriggeredBreakpoint(): onNextGoroutine, err := dbp.CurrentThread.onNextGoroutine() if err != nil { @@ -393,67 +412,24 @@ func (dbp *Process) pickCurrentThread(trapthread *Thread) error { // Step will continue until another source line is reached. // Will step into functions. func (dbp *Process) Step() (err error) { - if dbp.SelectedGoroutine != nil && dbp.SelectedGoroutine.thread == nil { - // Step called on parked goroutine - return dbp.stepToPC(dbp.SelectedGoroutine.PC) - } - fn := func() error { - var nloc *Location - th := dbp.CurrentThread - loc, err := th.Location() - if err != nil { - return err - } - for { - pc, err := dbp.CurrentThread.PC() - if err != nil { - return err - } - text, err := dbp.CurrentThread.Disassemble(pc, pc+maxInstructionLength, true) - if err == nil && len(text) > 0 && text[0].IsCall() && text[0].DestLoc != nil && text[0].DestLoc.Fn != nil { - fn := text[0].DestLoc.Fn - // Ensure PC and Entry match, otherwise StepInto is likely to set - // its breakpoint before DestLoc.PC and hence run too far ahead. - // Calls to runtime.duffzero and duffcopy have this problem. - if fn.Entry == text[0].DestLoc.PC { - return dbp.StepInto(fn) - } - } - - err = dbp.CurrentThread.StepInstruction() - if err != nil { - return err - } - nloc, err = th.Location() - if err != nil { - return err - } - if nloc.File != loc.File { - return nil - } - if nloc.File == loc.File && nloc.Line != loc.Line { - return nil - } - } + if dbp.exited { + return &ProcessExitedError{} } - return dbp.run(fn) -} - -// StepInto sets a temp breakpoint after the prologue of fn and calls Continue -func (dbp *Process) StepInto(fn *gosym.Func) error { for i := range dbp.Breakpoints { - if dbp.Breakpoints[i].Temp { + if dbp.Breakpoints[i].Internal() { return fmt.Errorf("next while nexting") } } - pc, _ := dbp.FirstPCAfterPrologue(fn, false) - return dbp.stepToPC(pc) -} -func (dbp *Process) stepToPC(pc uint64) error { - if _, err := dbp.SetTempBreakpoint(pc, sameGoroutineCondition(dbp.SelectedGoroutine)); err != nil { - return err + if err = dbp.next(true); err != nil { + switch err.(type) { + case ThreadBlockedError, NoReturnAddr: // Noop + default: + dbp.ClearTempBreakpoints() + return + } } + return dbp.Continue() } @@ -485,9 +461,21 @@ func (dbp *Process) StepInstruction() (err error) { } if dbp.SelectedGoroutine.thread == nil { // Step called on parked goroutine - return dbp.stepToPC(dbp.SelectedGoroutine.PC) + if _, err := dbp.SetTempBreakpoint(dbp.SelectedGoroutine.PC, NextBreakpoint, sameGoroutineCondition(dbp.SelectedGoroutine)); err != nil { + return err + } + return dbp.Continue() } - return dbp.run(dbp.SelectedGoroutine.thread.StepInstruction) + dbp.allGCache = nil + if dbp.exited { + return &ProcessExitedError{} + } + dbp.SelectedGoroutine.thread.clearBreakpointState() + err = dbp.SelectedGoroutine.thread.StepInstruction() + if err != nil { + return err + } + return dbp.SelectedGoroutine.thread.SetCurrentBreakpoint() } // SwitchThread changes from current thread to the thread specified by `tid`. @@ -740,7 +728,7 @@ func initializeDebugProcess(dbp *Process, path string, attach bool) (*Process, e func (dbp *Process) ClearTempBreakpoints() error { for _, bp := range dbp.Breakpoints { - if !bp.Temp { + if !bp.Internal() { continue } if _, err := dbp.ClearBreakpoint(bp.Addr); err != nil { @@ -748,29 +736,13 @@ func (dbp *Process) ClearTempBreakpoints() error { } } for i := range dbp.Threads { - if dbp.Threads[i].CurrentBreakpoint != nil && dbp.Threads[i].CurrentBreakpoint.Temp { + if dbp.Threads[i].CurrentBreakpoint != nil && dbp.Threads[i].CurrentBreakpoint.Internal() { dbp.Threads[i].CurrentBreakpoint = nil } } return nil } -func (dbp *Process) run(fn func() error) error { - dbp.allGCache = nil - if dbp.exited { - return fmt.Errorf("process has already exited") - } - for _, th := range dbp.Threads { - th.CurrentBreakpoint = nil - th.BreakpointConditionMet = false - th.BreakpointConditionError = nil - } - if err := fn(); err != nil { - return err - } - return nil -} - func (dbp *Process) handlePtraceFuncs() { // We must ensure here that we are running on the same thread during // while invoking the ptrace(2) syscall. This is due to the fact that ptrace(2) expects diff --git a/proc/proc_test.go b/proc/proc_test.go index 48efedc9f7..bc0b691b21 100644 --- a/proc/proc_test.go +++ b/proc/proc_test.go @@ -307,9 +307,25 @@ func countBreakpoints(p *Process) int { return bpcount } -func testnext(program string, testcases []nextTest, initialLocation string, t *testing.T) { +type contFunc int + +const ( + contNext contFunc = iota + contStep +) + +func testseq(program string, contFunc contFunc, testcases []nextTest, initialLocation string, t *testing.T) { withTestProcess(program, t, func(p *Process, fixture protest.Fixture) { - bp, err := setFunctionBreakpoint(p, initialLocation) + var bp *Breakpoint + var err error + if initialLocation != "" { + bp, err = setFunctionBreakpoint(p, initialLocation) + } else { + var pc uint64 + pc, err = p.FindFileLocation(fixture.Source, testcases[0].begin) + assertNoError(err, t, "FindFileLocation()") + bp, err = p.SetBreakpoint(pc) + } assertNoError(err, t, "SetBreakpoint()") assertNoError(p.Continue(), t, "Continue()") p.ClearBreakpoint(bp.Addr) @@ -317,15 +333,22 @@ func testnext(program string, testcases []nextTest, initialLocation string, t *t f, ln := currentLineNumber(p, t) for _, tc := range testcases { + pc, _ := p.CurrentThread.PC() if ln != tc.begin { t.Fatalf("Program not stopped at correct spot expected %d was %s:%d", tc.begin, filepath.Base(f), ln) } - assertNoError(p.Next(), t, "Next() returned an error") + switch contFunc { + case contNext: + assertNoError(p.Next(), t, "Next() returned an error") + case contStep: + assertNoError(p.Step(), t, "Step() returned an error") + } f, ln = currentLineNumber(p, t) + pc, _ = p.CurrentThread.PC() if ln != tc.end { - t.Fatalf("Program did not continue to correct next location expected %d was %s:%d", tc.end, filepath.Base(f), ln) + t.Fatalf("Program did not continue to correct next location expected %d was %s:%d (%#x)", tc.end, filepath.Base(f), ln, pc) } } @@ -379,7 +402,7 @@ func TestNextGeneral(t *testing.T) { } } - testnext("testnextprog", testcases, "main.testnext", t) + testseq("testnextprog", contNext, testcases, "main.testnext", t) } func TestNextConcurrent(t *testing.T) { @@ -478,7 +501,7 @@ func TestNextFunctionReturn(t *testing.T) { {14, 15}, {15, 35}, } - testnext("testnextprog", testcases, "main.helloworld", t) + testseq("testnextprog", contNext, testcases, "main.helloworld", t) } func TestNextFunctionReturnDefer(t *testing.T) { @@ -490,7 +513,7 @@ func TestNextFunctionReturnDefer(t *testing.T) { {6, 7}, {7, 8}, } - testnext("testnextdefer", testcases, "main.main", t) + testseq("testnextdefer", contNext, testcases, "main.main", t) } func TestNextNetHTTP(t *testing.T) { @@ -1276,7 +1299,6 @@ func TestBreakpointCountsWithDetection(t *testing.T) { } assertNoError(err, t, "Continue()") } - fmt.Printf("Continue returned %d\n", bp.TotalHitCount) for _, th := range p.Threads { if th.CurrentBreakpoint == nil { continue @@ -1290,7 +1312,6 @@ func TestBreakpointCountsWithDetection(t *testing.T) { assertNoError(err, t, "evalVariable") id, _ := constant.Int64Val(v.Value) m[id] = i - fmt.Printf("\tgoroutine (%d) %d: %d\n", th.ID, id, i) } total := int64(0) @@ -1912,7 +1933,7 @@ func TestNextDeferReturnAndDirectCall(t *testing.T) { // Next should not step into a deferred function if it is called // directly, only if it is called through a panic or a deferreturn. // Here we test the case where the function is called by a deferreturn - testnext("defercall", []nextTest{ + testseq("defercall", contNext, []nextTest{ {9, 10}, {10, 11}, {11, 12}, @@ -1928,9 +1949,250 @@ func TestNextPanicAndDirectCall(t *testing.T) { // Next should not step into a deferred function if it is called // directly, only if it is called through a panic or a deferreturn. // Here we test the case where the function is called by a panic - testnext("defercall", []nextTest{ + testseq("defercall", contNext, []nextTest{ {15, 16}, {16, 17}, {17, 18}, {18, 5}}, "main.callAndPanic2", t) } + +func TestStepCall(t *testing.T) { + testseq("testnextprog", contStep, []nextTest{ + {34, 13}, + {13, 14}}, "", t) +} + +func TestStepCallPtr(t *testing.T) { + // Tests that Step works correctly when calling functions with a + // function pointer. + testseq("teststepprog", contStep, []nextTest{ + {9, 10}, + {10, 5}, + {5, 6}, + {6, 7}, + {7, 11}}, "", t) +} + +func TestStepReturnAndPanic(t *testing.T) { + // Tests that Step works correctly when returning from functions + // and when a deferred function is called when panic'ing. + testseq("defercall", contStep, []nextTest{ + {17, 5}, + {5, 6}, + {6, 7}, + {7, 18}, + {18, 5}, + {5, 6}, + {6, 7}}, "", t) +} + +func TestStepDeferReturn(t *testing.T) { + // Tests that Step works correctly when a deferred function is + // called during a return. + testseq("defercall", contStep, []nextTest{ + {11, 5}, + {5, 6}, + {6, 7}, + {7, 12}, + {12, 13}, + {13, 5}, + {5, 6}, + {6, 7}, + {7, 13}, + {13, 28}}, "", t) +} + +func TestStepIgnorePrivateRuntime(t *testing.T) { + // Tests that Step will ignore calls to private runtime functions + // (such as runtime.convT2E in this case) + ver, _ := ParseVersionString(runtime.Version()) + + if ver.Major < 0 || ver.AfterOrEqual(GoVersion{1, 7, -1, 0, 0}) { + testseq("teststepprog", contStep, []nextTest{ + {21, 13}, + {13, 14}, + {14, 15}, + {15, 14}, + {14, 17}, + {17, 22}}, "", t) + } else { + testseq("teststepprog", contStep, []nextTest{ + {21, 13}, + {13, 14}, + {14, 15}, + {15, 17}, + {17, 22}}, "", t) + } +} + +func TestIssue561(t *testing.T) { + // Step fails to make progress when PC is at a CALL instruction + // where a breakpoint is also set. + withTestProcess("issue561", t, func(p *Process, fixture protest.Fixture) { + _, err := setFunctionBreakpoint(p, "main.main") + assertNoError(err, t, "setFunctionBreakpoint()") + assertNoError(p.Continue(), t, "Continue()") + assertNoError(p.Step(), t, "Step()") + _, ln := currentLineNumber(p, t) + if ln != 5 { + t.Fatalf("wrong line number after Step, expected 5 got %d", ln) + } + }) +} + +func TestStepConcurrentDirect(t *testing.T) { + withTestProcess("teststepconcurrent", t, func(p *Process, fixture protest.Fixture) { + pc, err := p.FindFileLocation(fixture.Source, 37) + assertNoError(err, t, "FindFileLocation()") + bp, err := p.SetBreakpoint(pc) + assertNoError(err, t, "SetBreakpoint()") + + assertNoError(p.Continue(), t, "Continue()") + _, err = p.ClearBreakpoint(bp.Addr) + assertNoError(err, t, "ClearBreakpoint()") + + gid := p.SelectedGoroutine.ID + + seq := []int{37, 38, 13, 15, 16, 38} + + i := 0 + count := 0 + for { + f, ln := currentLineNumber(p, t) + if ln != seq[i] { + if i == 1 && ln == 40 { + // loop exited + break + } + t.Fatalf("Program did not continue at expected location (%d) %s:%d", seq[i], f, ln) + } + if p.SelectedGoroutine.ID != gid { + t.Fatalf("Step switched to different goroutine %d %d\n", gid, p.SelectedGoroutine.ID) + } + i = (i + 1) % len(seq) + if i == 0 { + count++ + } + assertNoError(p.Step(), t, "Step()") + } + + if count != 100 { + t.Fatalf("Program did not loop expected number of times: %d", count) + } + }) +} + +func nextInProgress(p *Process) bool { + for _, th := range p.Threads { + if th.CurrentBreakpoint != nil && th.CurrentBreakpoint.Internal() { + return true + } + } + return false +} + +func TestStepConcurrentPtr(t *testing.T) { + withTestProcess("teststepconcurrent", t, func(p *Process, fixture protest.Fixture) { + pc, err := p.FindFileLocation(fixture.Source, 24) + assertNoError(err, t, "FindFileLocation()") + _, err = p.SetBreakpoint(pc) + assertNoError(err, t, "SetBreakpoint()") + + kvals := map[int]int64{} + count := 0 + for { + err := p.Continue() + _, exited := err.(ProcessExitedError) + if exited { + break + } + assertNoError(err, t, "Continue()") + + f, ln := currentLineNumber(p, t) + if ln != 24 { + t.Fatalf("Program did not continue at expected location (24): %s:%d", f, ln) + } + + gid := p.SelectedGoroutine.ID + + kvar, err := evalVariable(p, "k") + assertNoError(err, t, "EvalVariable()") + k, _ := constant.Int64Val(kvar.Value) + + if oldk, ok := kvals[gid]; ok { + if oldk >= k { + t.Fatalf("Goroutine %d did not make progress?") + } + } + kvals[gid] = k + + assertNoError(p.Step(), t, "Step()") + for nextInProgress(p) { + if p.SelectedGoroutine.ID == gid { + t.Fatalf("step did not step into function call (but temp breakpoints still active?) (%d %d)", gid, p.SelectedGoroutine.ID) + } + assertNoError(p.Continue(), t, "Continue()") + } + + f, ln = currentLineNumber(p, t) + if ln != 13 { + t.Fatalf("Step did not step into function call (13): %s:%d (gid: %d)", f, ln, p.SelectedGoroutine.ID) + } + + if p.SelectedGoroutine.ID != gid { + t.Fatalf("Step switched goroutines (%d %d)", gid, p.SelectedGoroutine.ID) + } + + count++ + if count > 50 { + // this test could potentially go on for 10000 cycles, since that's + // too slow we cut the execution after 50 cycles + break + } + } + + if count == 0 { + t.Fatalf("Breakpoint never hit") + } + }) +} + +func TestStepOnCallPtrInstr(t *testing.T) { + withTestProcess("teststepprog", t, func(p *Process, fixture protest.Fixture) { + pc, err := p.FindFileLocation(fixture.Source, 10) + assertNoError(err, t, "FindFileLocation()") + _, err = p.SetBreakpoint(pc) + assertNoError(err, t, "SetBreakpoint()") + + assertNoError(p.Continue(), t, "Continue()") + + found := false + + for { + _, ln := currentLineNumber(p, t) + if ln != 10 { + break + } + pc, err := p.CurrentThread.PC() + assertNoError(err, t, "PC()") + text, err := p.CurrentThread.Disassemble(pc, pc+maxInstructionLength, true) + assertNoError(err, t, "Disassemble()") + if text[0].IsCall() { + found = true + break + } + assertNoError(p.StepInstruction(), t, "StepInstruction()") + } + + if !found { + t.Fatal("Could not find CALL instruction") + } + + assertNoError(p.Step(), t, "Step()") + + f, ln := currentLineNumber(p, t) + if ln != 5 { + t.Fatalf("Step continued to wrong line, expected 5 was %s:%d", f, ln) + } + }) +} diff --git a/proc/threads.go b/proc/threads.go index 2b106ae5b1..a8acec41c8 100644 --- a/proc/threads.go +++ b/proc/threads.go @@ -9,6 +9,7 @@ import ( "path/filepath" "reflect" "runtime" + "strings" "golang.org/x/debug/dwarf" ) @@ -145,99 +146,165 @@ func topframe(g *G, thread *Thread) (Stackframe, error) { return frames[0], nil } -// Set breakpoints for potential next lines. -func (dbp *Process) setNextBreakpoints() (err error) { +// Set breakpoints at every line, and the return address. Also look for +// a deferred function and set a breakpoint there too. +// If stepInto is true it will also set breakpoints inside all +// functions called on the current source line, for non-absolute CALLs +// a breakpoint of kind StepBreakpoint is set on the CALL instruction, +// Continue will take care of setting a breakpoint to the destination +// once the CALL is reached. +func (dbp *Process) next(stepInto bool) error { topframe, err := topframe(dbp.SelectedGoroutine, dbp.CurrentThread) if err != nil { return err } - if filepath.Ext(topframe.Current.File) != ".go" { - return dbp.cnext(topframe) + csource := filepath.Ext(topframe.Current.File) != ".go" + thread := dbp.CurrentThread + currentGoroutine := false + if dbp.SelectedGoroutine != nil && dbp.SelectedGoroutine.thread != nil { + thread = dbp.SelectedGoroutine.thread + currentGoroutine = true } - return dbp.next(dbp.SelectedGoroutine, topframe) -} + text, err := thread.Disassemble(topframe.FDE.Begin(), topframe.FDE.End(), currentGoroutine) + if err != nil && stepInto { + return err + } -// Set breakpoints at every line, and the return address. Also look for -// a deferred function and set a breakpoint there too. -func (dbp *Process) next(g *G, topframe Stackframe) error { cond := sameGoroutineCondition(dbp.SelectedGoroutine) - // Disassembles function to find all runtime.deferreturn locations - // See documentation of Breakpoint.DeferCond for why this is necessary - deferreturns := []uint64{} - text, err := dbp.CurrentThread.Disassemble(topframe.FDE.Begin(), topframe.FDE.End(), false) - if err == nil { + if stepInto { for _, instr := range text { - if instr.IsCall() && instr.DestLoc != nil && instr.DestLoc.Fn != nil && instr.DestLoc.Fn.Name == "runtime.deferreturn" { - deferreturns = append(deferreturns, instr.Loc.PC) + if instr.Loc.File != topframe.Current.File || instr.Loc.Line != topframe.Current.Line || !instr.IsCall() { + continue + } + + if instr.DestLoc != nil && instr.DestLoc.Fn != nil { + if err := dbp.setStepIntoBreakpoint([]AsmInstruction{instr}, cond); err != nil { + dbp.ClearTempBreakpoints() + return err + } + } else { + // Non-absolute call instruction, set a StepBreakpoint here + if _, err := dbp.SetTempBreakpoint(instr.Loc.PC, StepBreakpoint, cond); err != nil { + if _, ok := err.(BreakpointExistsError); !ok { + dbp.ClearTempBreakpoints() + return err + } + } } } } - // Set breakpoint on the most recently deferred function (if any) - var deferpc uint64 = 0 - if g != nil && g.DeferPC != 0 { - _, _, deferfn := dbp.goSymTable.PCToLine(g.DeferPC) - var err error - deferpc, err = dbp.FirstPCAfterPrologue(deferfn, false) - if err != nil { - return err + if !csource { + deferreturns := []uint64{} + + // Find all runtime.deferreturn locations in the function + // See documentation of Breakpoint.DeferCond for why this is necessary + for _, instr := range text { + if instr.IsCall() && instr.DestLoc != nil && instr.DestLoc.Fn != nil && instr.DestLoc.Fn.Name == "runtime.deferreturn" { + deferreturns = append(deferreturns, instr.Loc.PC) + } } - } - if deferpc != 0 { - bp, err := dbp.SetTempBreakpoint(deferpc, cond) - if err != nil { - if _, ok := err.(BreakpointExistsError); !ok { - dbp.ClearTempBreakpoints() + + // Set breakpoint on the most recently deferred function (if any) + var deferpc uint64 = 0 + if dbp.SelectedGoroutine != nil && dbp.SelectedGoroutine.DeferPC != 0 { + _, _, deferfn := dbp.goSymTable.PCToLine(dbp.SelectedGoroutine.DeferPC) + var err error + deferpc, err = dbp.FirstPCAfterPrologue(deferfn, false) + if err != nil { return err } } - bp.DeferCond = true - bp.DeferReturns = deferreturns + if deferpc != 0 && deferpc != topframe.Current.PC { + bp, err := dbp.SetTempBreakpoint(deferpc, NextDeferBreakpoint, cond) + if err != nil { + if _, ok := err.(BreakpointExistsError); !ok { + dbp.ClearTempBreakpoints() + return err + } + } + if bp != nil { + bp.DeferReturns = deferreturns + } + } } // Add breakpoints on all the lines in the current function pcs := dbp.lineInfo.AllPCsBetween(topframe.FDE.Begin(), topframe.FDE.End()-1, topframe.Current.File) - var covered bool - for i := range pcs { - if topframe.FDE.Cover(pcs[i]) { - covered = true - break + if !csource { + var covered bool + for i := range pcs { + if topframe.FDE.Cover(pcs[i]) { + covered = true + break + } } - } - if !covered { - fn := dbp.goSymTable.PCToFunc(topframe.Ret) - if g != nil && fn != nil && fn.Name == "runtime.goexit" { - return nil + if !covered { + fn := dbp.goSymTable.PCToFunc(topframe.Ret) + if dbp.SelectedGoroutine != nil && fn != nil && fn.Name == "runtime.goexit" { + return nil + } } } // Add a breakpoint on the return address for the current frame pcs = append(pcs, topframe.Ret) - return dbp.setTempBreakpoints(topframe.Current.PC, pcs, cond) + return dbp.setTempBreakpoints(topframe.Current.PC, pcs, NextBreakpoint, cond) } -// Set a breakpoint at every reachable location, as well as the return address. Without -// the benefit of an AST we can't be sure we're not at a branching statement and thus -// cannot accurately predict where we may end up. -func (dbp *Process) cnext(topframe Stackframe) error { - pcs := dbp.lineInfo.AllPCsBetween(topframe.FDE.Begin(), topframe.FDE.End(), topframe.Current.File) - pcs = append(pcs, topframe.Ret) - return dbp.setTempBreakpoints(topframe.Current.PC, pcs, sameGoroutineCondition(dbp.SelectedGoroutine)) +func (dbp *Process) setStepIntoBreakpoint(text []AsmInstruction, cond ast.Expr) error { + if len(text) <= 0 { + return nil + } + + instr := text[0] + + if instr.DestLoc == nil || instr.DestLoc.Fn == nil { + return nil + } + + fn := instr.DestLoc.Fn + + // Ensure PC and Entry match, otherwise StepInto is likely to set + // its breakpoint before DestLoc.PC and hence run too far ahead. + // Calls to runtime.duffzero and duffcopy have this problem. + if fn.Entry != instr.DestLoc.PC { + return nil + } + + // Skip unexported runtime functions + if strings.HasPrefix(fn.Name, "runtime.") && !isExportedRuntime(fn.Name) { + return nil + } + + //TODO(aarzilli): if we want to let users hide functions + // or entire packages from being stepped into with 'step' + // those extra checks should be done here. + + // Set a breakpoint after the function's prologue + pc, _ := dbp.FirstPCAfterPrologue(fn, false) + if _, err := dbp.SetTempBreakpoint(pc, NextBreakpoint, cond); err != nil { + if _, ok := err.(BreakpointExistsError); !ok { + return err + } + } + + return nil } // setTempBreakpoints sets a breakpoint to all addresses specified in pcs // skipping over curpc and curpc-1 -func (dbp *Process) setTempBreakpoints(curpc uint64, pcs []uint64, cond ast.Expr) error { +func (dbp *Process) setTempBreakpoints(curpc uint64, pcs []uint64, kind BreakpointKind, cond ast.Expr) error { for i := range pcs { if pcs[i] == curpc || pcs[i] == curpc-1 { continue } - if _, err := dbp.SetTempBreakpoint(pcs[i], cond); err != nil { + if _, err := dbp.SetTempBreakpoint(pcs[i], kind, cond); err != nil { if _, ok := err.(BreakpointExistsError); !ok { dbp.ClearTempBreakpoints() return err @@ -384,12 +451,18 @@ func (thread *Thread) SetCurrentBreakpoint() error { return nil } +func (thread *Thread) clearBreakpointState() { + thread.CurrentBreakpoint = nil + thread.BreakpointConditionMet = false + thread.BreakpointConditionError = nil +} + func (thread *Thread) onTriggeredBreakpoint() bool { return (thread.CurrentBreakpoint != nil) && thread.BreakpointConditionMet } func (thread *Thread) onTriggeredTempBreakpoint() bool { - return thread.onTriggeredBreakpoint() && thread.CurrentBreakpoint.Temp + return thread.onTriggeredBreakpoint() && thread.CurrentBreakpoint.Internal() } func (thread *Thread) onRuntimeBreakpoint() bool { @@ -404,18 +477,20 @@ func (thread *Thread) onRuntimeBreakpoint() bool { func (thread *Thread) onNextGoroutine() (bool, error) { var bp *Breakpoint for i := range thread.dbp.Breakpoints { - if thread.dbp.Breakpoints[i].Temp { + if thread.dbp.Breakpoints[i].Internal() { bp = thread.dbp.Breakpoints[i] + break } } if bp == nil { return false, nil } - // we just want to check the condition on the goroutine id here - dc := bp.DeferCond - bp.DeferCond = false - defer func() { - bp.DeferCond = dc - }() + if bp.Kind == NextDeferBreakpoint { + // we just want to check the condition on the goroutine id here + bp.Kind = NextBreakpoint + defer func() { + bp.Kind = NextDeferBreakpoint + }() + } return bp.checkCondition(thread) } diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 4e7ed83f77..0c1501035f 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -167,7 +167,7 @@ func (d *Debugger) state() (*api.DebuggerState, error) { } for _, bp := range d.process.Breakpoints { - if bp.Temp { + if bp.Internal() { state.NextInProgress = true break } @@ -297,7 +297,7 @@ func (d *Debugger) Breakpoints() []*api.Breakpoint { func (d *Debugger) breakpoints() []*api.Breakpoint { bps := []*api.Breakpoint{} for _, bp := range d.process.Breakpoints { - if bp.Temp { + if bp.Internal() { continue } bps = append(bps, api.ConvertBreakpoint(bp))