proc: change 'step' command so that it steps through go statements (#3686)

Change 'step' command so that when stepping into a 'go statement' the
debugger will stop on the newly created goroutine, instead of just
stepping over the go statement.

pkg/proc/breakpoints.go

@@ -135,7 +135,11 @@ const (
 	// been loaded and we should try to enable suspended breakpoints.
 	PluginOpenBreakpoint
 
-	steppingMask = NextBreakpoint | NextDeferBreakpoint | StepBreakpoint
+	// StepIntoNewProc is a breakpoint used to step into a newly created
+	// goroutine.
+	StepIntoNewProcBreakpoint
+
+	steppingMask = NextBreakpoint | NextDeferBreakpoint | StepBreakpoint | StepIntoNewProcBreakpoint
 )
 
 // WatchType is the watchpoint type
@@ -210,6 +214,8 @@ func (bp *Breakpoint) VerboseDescr() []string {
 			r = append(r, fmt.Sprintf("StackResizeBreakpoint Cond=%q", exprToString(breaklet.Cond)))
 		case PluginOpenBreakpoint:
 			r = append(r, "PluginOpenBreakpoint")
+		case StepIntoNewProcBreakpoint:
+			r = append(r, "StepIntoNewProcBreakpoint")
 		default:
 			r = append(r, fmt.Sprintf("Unknown %d", breaklet.Kind))
 		}
@@ -304,7 +310,7 @@ func (bpstate *BreakpointState) checkCond(tgt *Target, breaklet *Breaklet, threa
 			}
 		}
 
-	case StackResizeBreakpoint, PluginOpenBreakpoint:
+	case StackResizeBreakpoint, PluginOpenBreakpoint, StepIntoNewProcBreakpoint:
 		// no further checks
 
 	default:

pkg/proc/proc_test.go

@@ -549,6 +549,10 @@ func testseq2Args(wd string, args []string, buildFlags protest.BuildFlags, t *te
 				// do nothing
 			}
 
+			if err := p.CurrentThread().Breakpoint().CondError; err != nil {
+				t.Logf("breakpoint condition error: %v", err)
+			}
+
 			f, ln = currentLineNumber(p, t)
 			regs, _ = p.CurrentThread().Registers()
 			pc := regs.PC()
@@ -6215,3 +6219,18 @@ func TestReadClosure(t *testing.T) {
 		}
 	})
 }
+
+func TestStepIntoGoroutine(t *testing.T) {
+	testseq2(t, "goroutinestackprog", "", []seqTest{
+		{contContinue, 23},
+		{contStep, 7},
+		{contNothing, func(p *proc.Target) {
+			vari := api.ConvertVar(evalVariable(p, t, "i"))
+			varis := vari.SinglelineString()
+			t.Logf("i = %s", varis)
+			if varis != "0" {
+				t.Fatalf("wrong value for variable i: %s", vari.SinglelineString())
+			}
+		}},
+	})
+}

pkg/proc/target_exec.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"go/ast"
+	"go/constant"
 	"go/token"
 	"path/filepath"
 	"strings"
@@ -812,6 +813,7 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error {
 }
 
 func setStepIntoBreakpoints(dbp *Target, curfn *Function, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr) error {
+	gostmt := false
 	for _, instr := range text {
 		if instr.Loc.File != topframe.Current.File || instr.Loc.Line != topframe.Current.Line || !instr.IsCall() {
 			continue
@@ -821,6 +823,12 @@ func setStepIntoBreakpoints(dbp *Target, curfn *Function, text []AsmInstruction,
 			if err := setStepIntoBreakpoint(dbp, curfn, []AsmInstruction{instr}, sameGCond); err != nil {
 				return err
 			}
+			if curfn != nil && curfn.Name != "runtime." && instr.DestLoc.Fn != nil && instr.DestLoc.Fn.Name == "runtime.newproc" {
+				// The current statement is a go statement, i.e. "go somecall()"
+				// We are excluding this check inside the runtime package because
+				// functions in the runtime package can call runtime.newproc directly.
+				gostmt = true
+			}
 		} else {
 			// Non-absolute call instruction, set a StepBreakpoint here
 			bp, err := allowDuplicateBreakpoint(dbp.SetBreakpoint(0, instr.Loc.PC, StepBreakpoint, sameGCond))
@@ -831,6 +839,9 @@ func setStepIntoBreakpoints(dbp *Target, curfn *Function, text []AsmInstruction,
 			breaklet.callback = stepIntoCallback
 		}
 	}
+	if gostmt {
+		setStepIntoNewProcBreakpoint(dbp, sameGCond)
+	}
 	return nil
 }
 
@@ -979,7 +990,7 @@ func setStepIntoBreakpoint(dbp *Target, curfn *Function, text []AsmInstruction,
 		return nil
 	}
 
-	fn, pc = skipAutogeneratedWrappersIn(dbp, fn, pc)
+	fn, pc = skipAutogeneratedWrappersIn(dbp, fn, pc, false)
 
 	// We want to skip the function prologue but we should only do it if the
 	// destination address of the CALL instruction is the entry point of the
@@ -999,6 +1010,108 @@ func setStepIntoBreakpoint(dbp *Target, curfn *Function, text []AsmInstruction,
 	return nil
 }
 
+// setStepIntoNewProcBreakpoint sets a temporary breakpoint on
+// runtime.newproc that, when hit, clears all temporary breakpoints and sets
+// a new temporary breakpoint on the starting function for the new
+// goroutine.
+func setStepIntoNewProcBreakpoint(p *Target, sameGCond ast.Expr) {
+	const (
+		runtimeNewprocFunc1 = "runtime.newproc.func1"
+		runtimeRunqput      = "runtime.runqput"
+	)
+	rnf := p.BinInfo().LookupFunc()[runtimeNewprocFunc1]
+	if len(rnf) != 1 {
+		logflags.DebuggerLogger().Error("could not find " + runtimeNewprocFunc1)
+		return
+	}
+	text, err := Disassemble(p.Memory(), nil, p.Breakpoints(), p.BinInfo(), rnf[0].Entry, rnf[0].End)
+	if err != nil {
+		logflags.DebuggerLogger().Errorf("could not disassemble "+runtimeNewprocFunc1+": %v", err)
+		return
+	}
+
+	callfile, callline := "", 0
+	for _, instr := range text {
+		if instr.Kind == CallInstruction && instr.DestLoc != nil && instr.DestLoc.Fn != nil && instr.DestLoc.Fn.Name == runtimeRunqput {
+			callfile = instr.Loc.File
+			callline = instr.Loc.Line
+			break
+		}
+	}
+	if callfile == "" {
+		logflags.DebuggerLogger().Error("could not find " + runtimeRunqput + " call in " + runtimeNewprocFunc1)
+		return
+	}
+	var pc uint64
+	for _, pcstmt := range rnf[0].cu.lineInfo.LineToPCs(callfile, callline) {
+		if pcstmt.Stmt {
+			pc = pcstmt.PC
+			break
+		}
+	}
+	if pc == 0 {
+		logflags.DebuggerLogger().Errorf("could not set newproc breakpoint: location not found for " + runtimeRunqput + " call")
+		return
+	}
+
+	bp, err := p.SetBreakpoint(0, pc, StepIntoNewProcBreakpoint, sameGCond)
+	if err != nil {
+		logflags.DebuggerLogger().Errorf("could not set StepIntoNewProcBreakpoint: %v", err)
+		return
+	}
+	blet := bp.Breaklets[len(bp.Breaklets)-1]
+	blet.callback = func(th Thread, p *Target) (bool, error) {
+		// Clear temp breakpoints that exist and set a new one for goroutine
+		// newg.goid on the go statement's target
+		scope, err := ThreadScope(p, th)
+		if err != nil {
+			return false, err
+		}
+		v, err := scope.EvalExpression("newg.goid", loadSingleValue)
+		if err != nil {
+			return false, err
+		}
+		if v.Unreadable != nil {
+			return false, v.Unreadable
+		}
+		newGGoID, _ := constant.Int64Val(v.Value)
+
+		v, err = scope.EvalExpression("newg.startpc", loadSingleValue)
+		if err != nil {
+			return false, err
+		}
+		if v.Unreadable != nil {
+			return false, v.Unreadable
+		}
+		startpc, _ := constant.Int64Val(v.Value)
+
+		// Temp breakpoints must be cleared because the current goroutine could
+		// hit one of them before the new goroutine manages to start.
+		err = p.ClearSteppingBreakpoints()
+		if err != nil {
+			return false, err
+		}
+
+		newGCond := astutil.Eql(astutil.Sel(astutil.PkgVar("runtime", "curg"), "goid"), astutil.Int(newGGoID))
+
+		// We don't want to use startpc directly because it will be an
+		// autogenerated wrapper on some versions of Go. Addditionally, once we
+		// have the correct function we must also skip to prologue.
+		startfn := p.BinInfo().PCToFunc(uint64(startpc))
+		if startfn2, _ := skipAutogeneratedWrappersIn(p, startfn, uint64(startpc), true); startfn2 != nil {
+			startfn = startfn2
+		}
+		if startpc2, err := FirstPCAfterPrologue(p, startfn, false); err == nil {
+			startpc = int64(startpc2)
+		}
+
+		// The new breakpoint must have 'NextBreakpoint' kind because we want to
+		// stop on it.
+		_, err = p.SetBreakpoint(0, uint64(startpc), NextBreakpoint, newGCond)
+		return false, err // we don't want to stop at this breakpoint if there is no error
+	}
+}
+
 func allowDuplicateBreakpoint(bp *Breakpoint, err error) (*Breakpoint, error) {
 	if err != nil {
 		//lint:ignore S1020 this is clearer
@@ -1019,8 +1132,10 @@ func isAutogeneratedOrDeferReturn(loc Location) bool {
 
 // skipAutogeneratedWrappersIn skips autogenerated wrappers when setting a
 // step-into breakpoint.
+// If alwaysSkipFirst is set the first function is always skipped if it is
+// autogenerated, even if it isn't a wrapper for the function it is calling.
 // See genwrapper in: $GOROOT/src/cmd/compile/internal/gc/subr.go
-func skipAutogeneratedWrappersIn(p Process, startfn *Function, startpc uint64) (*Function, uint64) {
+func skipAutogeneratedWrappersIn(p Process, startfn *Function, startpc uint64, alwaysSkipFirst bool) (*Function, uint64) {
 	if startfn == nil {
 		return nil, startpc
 	}
@@ -1072,7 +1187,10 @@ func skipAutogeneratedWrappersIn(p Process, startfn *Function, startpc uint64) (
 		}
 
 		tgtfn := tgtfns[0]
-		if strings.TrimSuffix(tgtfn.BaseName(), "-fm") != strings.TrimSuffix(fn.BaseName(), "-fm") {
+		if alwaysSkipFirst {
+			alwaysSkipFirst = false
+			startfn, startpc = tgtfn, tgtfn.Entry
+		} else if strings.TrimSuffix(tgtfn.BaseName(), "-fm") != strings.TrimSuffix(fn.BaseName(), "-fm") {
 			return startfn, startpc
 		}
 		fn = tgtfn