proc: bugfix: Next and normal calls to deferred function

When a deferred function is called directly Next and StepOut should not
step into it.

Fixes #582

_fixtures/defercall.go

@@ -0,0 +1,29 @@
+package main
+
+var n = 0
+
+func sampleFunction() {
+	n++
+}
+
+func callAndDeferReturn() {
+	defer sampleFunction()
+	sampleFunction()
+	n++
+}
+
+func callAndPanic2() {
+	defer sampleFunction()
+	sampleFunction()
+	panic("panicking")
+}
+
+func callAndPanic() {
+	defer recover()
+	callAndPanic2()
+}
+
+func main() {
+	callAndDeferReturn()
+	callAndPanic()
+}

proc/breakpoints.go

@@ -33,7 +33,18 @@ 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
 
-	Cond ast.Expr // When Cond is not nil the breakpoint will be triggered only if evaluating Cond returns true
+	// 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
+	// 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
 }
 
 func (bp *Breakpoint) String() string {
@@ -122,6 +133,24 @@ func (bp *Breakpoint) checkCondition(thread *Thread) (bool, error) {
 	if bp.Cond == nil {
 		return true, nil
 	}
+	if bp.DeferCond {
+		frames, err := thread.Stacktrace(2)
+		if err == nil {
+			ispanic := len(frames) >= 3 && frames[2].Current.Fn != nil && frames[2].Current.Fn.Name == "runtime.gopanic"
+			isdeferreturn := false
+			if len(frames) >= 1 {
+				for _, pc := range bp.DeferReturns {
+					if frames[0].Ret == pc {
+						isdeferreturn = true
+						break
+					}
+				}
+			}
+			if !ispanic && !isdeferreturn {
+				return false, nil
+			}
+		}
+	}
 	scope, err := thread.Scope()
 	if err != nil {
 		return true, err

proc/proc_test.go

@@ -1907,3 +1907,30 @@ func TestTestvariables2Prologue(t *testing.T) {
 		}
 	})
 }
+
+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{
+		{9, 10},
+		{10, 11},
+		{11, 12},
+		{12, 13},
+		{13, 5},
+		{5, 6},
+		{6, 7},
+		{7, 13},
+		{13, 28}}, "main.callAndDeferReturn", t)
+}
+
+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{
+		{15, 16},
+		{16, 17},
+		{17, 18},
+		{18, 5}}, "main.callAndPanic2", t)
+}

proc/threads.go

@@ -162,8 +162,21 @@ 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.
 func (dbp *Process) next(g *G, topframe Stackframe) error {
-	pcs := dbp.lineInfo.AllPCsBetween(topframe.FDE.Begin(), topframe.FDE.End()-1, topframe.Current.File)
+	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 {
+		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)
+			}
+		}
+	}
+
+	// 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)
@@ -173,6 +186,20 @@ func (dbp *Process) next(g *G, topframe Stackframe) error {
 			return err
 		}
 	}
+	if deferpc != 0 {
+		bp, err := dbp.SetTempBreakpoint(deferpc, cond)
+		if err != nil {
+			if _, ok := err.(BreakpointExistsError); !ok {
+				dbp.ClearTempBreakpoints()
+				return err
+			}
+		}
+		bp.DeferCond = true
+		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 {
@@ -188,11 +215,10 @@ func (dbp *Process) next(g *G, topframe Stackframe) error {
 			return nil
 		}
 	}
-	if deferpc != 0 {
+
-		pcs = append(pcs, deferpc)
+	// Add a breakpoint on the return address for the current frame
-	}
 	pcs = append(pcs, topframe.Ret)
-	return dbp.setTempBreakpoints(topframe.Current.PC, pcs, sameGoroutineCondition(dbp.SelectedGoroutine))
+	return dbp.setTempBreakpoints(topframe.Current.PC, pcs, cond)
 }
 
 // Set a breakpoint at every reachable location, as well as the return address. Without
@@ -385,5 +411,11 @@ func (thread *Thread) onNextGoroutine() (bool, error) {
 	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
+	}()
 	return bp.checkCondition(thread)
 }