proc: Implement Step using Continue

Instead of repeatedly calling StepInstruction set breakpoints to the
destination of CALL instructions (or on the CALL instructions
themselves for indirect CALLs), then call Continue.
Calls to unexported runtime functions are skipped.
Reduces the number of code paths managing inferior state from 3 to 2
(StepInstruction, Continue).

Fixes #561

_fixtures/issue561.go

+package main
+
+import "fmt"
+
+func testfunction() {
+	fmt.Printf("here!\n")
+}
+
+func main() {
+	testfunction()
+}

_fixtures/teststepconcurrent.go

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

_fixtures/teststepprog.go

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

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 {

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

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

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

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