proc,terminal: allow setting suspended breakpoints (go-delve#3154)

Allows setting suspended breakpoints and try to enable them
automatically after every time a plugin is loaded.

Fixes go-delve#1653
Updates go-delve#2551

Documentation/cli/starlark.md

@@ -26,7 +26,7 @@ checkpoint(Where) | Equivalent to API call [Checkpoint](https://godoc.org/github
 clear_breakpoint(Id, Name) | Equivalent to API call [ClearBreakpoint](https://godoc.org/github.com/go-delve/delve/service/rpc2#RPCServer.ClearBreakpoint)
 clear_checkpoint(ID) | Equivalent to API call [ClearCheckpoint](https://godoc.org/github.com/go-delve/delve/service/rpc2#RPCServer.ClearCheckpoint)
 raw_command(Name, ThreadID, GoroutineID, ReturnInfoLoadConfig, Expr, UnsafeCall) | Equivalent to API call [Command](https://godoc.org/github.com/go-delve/delve/service/rpc2#RPCServer.Command)
-create_breakpoint(Breakpoint, LocExpr, SubstitutePathRules) | Equivalent to API call [CreateBreakpoint](https://godoc.org/github.com/go-delve/delve/service/rpc2#RPCServer.CreateBreakpoint)
+create_breakpoint(Breakpoint, LocExpr, SubstitutePathRules, Suspended) | Equivalent to API call [CreateBreakpoint](https://godoc.org/github.com/go-delve/delve/service/rpc2#RPCServer.CreateBreakpoint)
 create_ebpf_tracepoint(FunctionName) | Equivalent to API call [CreateEBPFTracepoint](https://godoc.org/github.com/go-delve/delve/service/rpc2#RPCServer.CreateEBPFTracepoint)
 create_watchpoint(Scope, Expr, Type) | Equivalent to API call [CreateWatchpoint](https://godoc.org/github.com/go-delve/delve/service/rpc2#RPCServer.CreateWatchpoint)
 detach(Kill) | Equivalent to API call [Detach](https://godoc.org/github.com/go-delve/delve/service/rpc2#RPCServer.Detach)

pkg/locspec/locations.go

@@ -270,6 +270,10 @@ func packageMatch(specPkg, symPkg string, packageMap map[string][]string) bool {
 // Find will search all functions in the target program and filter them via the
 // regex location spec. Only functions matching the regex will be returned.
 func (loc *RegexLocationSpec) Find(t *proc.Target, _ []string, scope *proc.EvalScope, locStr string, includeNonExecutableLines bool, _ [][2]string) ([]api.Location, error) {
+	if scope == nil {
+		//TODO(aarzilli): this needs only the list of function we should make it work
+		return nil, fmt.Errorf("could not determine location (scope is nil)")
+	}
 	funcs := scope.BinInfo.Functions
 	matches, err := regexFilterFuncs(loc.FuncRegex, funcs)
 	if err != nil {
@@ -390,7 +394,10 @@ func (loc *NormalLocationSpec) Find(t *proc.Target, processArgs []string, scope
 		candidateFuncs = loc.findFuncCandidates(t.BinInfo(), limit)
 	}
 
-	if matching := len(candidateFiles) + len(candidateFuncs); matching == 0 && scope != nil {
+	if matching := len(candidateFiles) + len(candidateFuncs); matching == 0 {
+		if scope == nil {
+			return nil, fmt.Errorf("location \"%s\" not found", locStr)
+		}
 		// if no result was found this locations string could be an
 		// expression that the user forgot to prefix with '*', try treating it as
 		// such.

pkg/proc/bininfo.go

@@ -392,6 +392,27 @@ func FirstPCAfterPrologue(p Process, fn *Function, sameline bool) (uint64, error
 	return pc, nil
 }
 
+func findRetPC(t *Target, name string) ([]uint64, error) {
+	fn := t.BinInfo().LookupFunc[name]
+	if fn == nil {
+		return nil, fmt.Errorf("could not find %s", name)
+	}
+	text, err := Disassemble(t.Memory(), nil, t.Breakpoints(), t.BinInfo(), fn.Entry, fn.End)
+	if err != nil {
+		return nil, err
+	}
+	r := []uint64{}
+	for _, instr := range text {
+		if instr.IsRet() {
+			r = append(r, instr.Loc.PC)
+		}
+	}
+	if len(r) == 0 {
+		return nil, fmt.Errorf("could not find return instruction in %s", name)
+	}
+	return r, nil
+}
+
 // cpuArch is a stringer interface representing CPU architectures.
 type cpuArch interface {
 	String() string

pkg/proc/breakpoints.go

@@ -131,6 +131,10 @@ const (
 	// adjust the watchpoint of stack variables.
 	StackResizeBreakpoint
 
+	// PluginOpenBreakpoint is a breakpoint used to detect that a plugin has
+	// been loaded and we should try to enable suspended breakpoints.
+	PluginOpenBreakpoint
+
 	steppingMask = NextBreakpoint | NextDeferBreakpoint | StepBreakpoint
 )
 
@@ -204,6 +208,8 @@ func (bp *Breakpoint) VerboseDescr() []string {
 			r = append(r, fmt.Sprintf("WatchOutOfScope Cond=%q checkPanicCall=%v", exprToString(breaklet.Cond), breaklet.checkPanicCall))
 		case StackResizeBreakpoint:
 			r = append(r, fmt.Sprintf("StackResizeBreakpoint Cond=%q", exprToString(breaklet.Cond)))
+		case PluginOpenBreakpoint:
+			r = append(r, "PluginOpenBreakpoint")
 		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:
+	case StackResizeBreakpoint, PluginOpenBreakpoint:
 		// no further checks
 
 	default:

pkg/proc/stackwatch.go

@@ -96,27 +96,15 @@ func (t *Target) setStackWatchBreakpoints(scope *EvalScope, watchpoint *Breakpoi
 
 	// Stack Resize Sentinel
 
-	fn := t.BinInfo().LookupFunc["runtime.copystack"]
-	if fn == nil {
-		return errors.New("could not find runtime.copystack")
-	}
-	text, err := Disassemble(t.Memory(), nil, t.Breakpoints(), t.BinInfo(), fn.Entry, fn.End)
+	retpcs, err := findRetPC(t, "runtime.copystack")
 	if err != nil {
 		return err
 	}
-	var retpc uint64
-	for _, instr := range text {
-		if instr.IsRet() {
-			if retpc != 0 {
-				return errors.New("runtime.copystack has too many return instructions")
-			}
-			retpc = instr.Loc.PC
-		}
+	if len(retpcs) > 1 {
+		return errors.New("runtime.copystack has too many return instructions")
 	}
-	if retpc == 0 {
-		return errors.New("could not find return instruction in runtime.copystack")
-	}
-	rszbp, err := t.SetBreakpoint(0, retpc, StackResizeBreakpoint, sameGCond)
+
+	rszbp, err := t.SetBreakpoint(0, retpcs[0], StackResizeBreakpoint, sameGCond)
 	if err != nil {
 		return err
 	}

pkg/proc/target.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/go-delve/delve/pkg/dwarf/op"
 	"github.com/go-delve/delve/pkg/goversion"
+	"github.com/go-delve/delve/pkg/logflags"
 	"github.com/go-delve/delve/pkg/proc/internal/ebpf"
 )
 
@@ -213,6 +214,7 @@ func NewTarget(p ProcessInternal, pid int, currentThread Thread, cfg NewTargetCo
 
 	t.createUnrecoveredPanicBreakpoint()
 	t.createFatalThrowBreakpoint()
+	t.createPluginOpenBreakpoint()
 
 	t.gcache.init(p.BinInfo())
 	t.fakeMemoryRegistryMap = make(map[string]*compositeMemory)
@@ -426,6 +428,21 @@ func (t *Target) createFatalThrowBreakpoint() {
 	}
 }
 
+// createPluginOpenBreakpoint creates a breakpoint at the return instruction
+// of plugin.Open (if it exists) that will try to enable suspended
+// breakpoints.
+func (t *Target) createPluginOpenBreakpoint() {
+	retpcs, _ := findRetPC(t, "plugin.Open")
+	for _, retpc := range retpcs {
+		bp, err := t.SetBreakpoint(0, retpc, PluginOpenBreakpoint, nil)
+		if err != nil {
+			t.BinInfo().logger.Errorf("could not set plugin.Open breakpoint: %v", err)
+		} else {
+			bp.Breaklets[len(bp.Breaklets)-1].callback = t.pluginOpenCallback
+		}
+	}
+}
+
 // CurrentThread returns the currently selected thread which will be used
 // for next/step/stepout and for reading variables, unless a goroutine is
 // selected.
@@ -588,6 +605,30 @@ func (t *Target) dwrapUnwrap(fn *Function) *Function {
 	return fn
 }
 
+func (t *Target) pluginOpenCallback(Thread) bool {
+	logger := logflags.DebuggerLogger()
+	for _, lbp := range t.Breakpoints().Logical {
+		if isSuspended(t, lbp) {
+			err := enableBreakpointOnTarget(t, lbp)
+			if err != nil {
+				logger.Debugf("could not enable breakpoint %d: %v", lbp.LogicalID, err)
+			} else {
+				logger.Debugf("suspended breakpoint %d enabled", lbp.LogicalID)
+			}
+		}
+	}
+	return false
+}
+
+func isSuspended(t *Target, lbp *LogicalBreakpoint) bool {
+	for _, bp := range t.Breakpoints().M {
+		if bp.LogicalID() == lbp.LogicalID {
+			return false
+		}
+	}
+	return true
+}
+
 type dummyRecordingManipulation struct {
 }
 

pkg/terminal/command.go

@@ -1741,19 +1741,37 @@ func setBreakpoint(t *Term, ctx callContext, tracepoint bool, argstr string) ([]
 	}
 
 	requestedBp.Tracepoint = tracepoint
-	locs, err := t.client.FindLocation(ctx.Scope, spec, true, t.substitutePathRules())
-	if err != nil {
-		if requestedBp.Name == "" {
-			return nil, err
-		}
+	locs, findLocErr := t.client.FindLocation(ctx.Scope, spec, true, t.substitutePathRules())
+	if findLocErr != nil && requestedBp.Name != "" {
 		requestedBp.Name = ""
 		spec = argstr
 		var err2 error
 		locs, err2 = t.client.FindLocation(ctx.Scope, spec, true, t.substitutePathRules())
-		if err2 != nil {
+		if err2 == nil {
+			findLocErr = nil
+		}
+	}
+	if findLocErr != nil && shouldAskToSuspendBreakpoint(t) {
+		fmt.Fprintf(os.Stderr, "Command failed: %s\n", findLocErr.Error())
+		findLocErr = nil
+		answer, err := yesno(t.line, "Set a suspended breakpoint (Delve will try to set this breakpoint when a plugin is loaded) [Y/n]?")
+		if err != nil {
 			return nil, err
 		}
+		if !answer {
+			return nil, nil
+		}
+		bp, err := t.client.CreateBreakpointWithExpr(requestedBp, spec, t.substitutePathRules(), true)
+		if err != nil {
+			return nil, err
+		}
+		fmt.Fprintf(t.stdout, "%s set at %s\n", formatBreakpointName(bp, true), t.formatBreakpointLocation(bp))
+		return nil, nil
+	}
+	if findLocErr != nil {
+		return nil, findLocErr
 	}
+
 	created := []*api.Breakpoint{}
 	for _, loc := range locs {
 		requestedBp.Addr = loc.PC
@@ -1763,7 +1781,7 @@ func setBreakpoint(t *Term, ctx callContext, tracepoint bool, argstr string) ([]
 			requestedBp.LoadArgs = &ShortLoadConfig
 		}
 
-		bp, err := t.client.CreateBreakpointWithExpr(requestedBp, spec, t.substitutePathRules())
+		bp, err := t.client.CreateBreakpointWithExpr(requestedBp, spec, t.substitutePathRules(), false)
 		if err != nil {
 			return nil, err
 		}
@@ -3199,3 +3217,8 @@ func (t *Term) formatBreakpointLocation(bp *api.Breakpoint) string {
 	}
 	return out.String()
 }
+
+func shouldAskToSuspendBreakpoint(t *Term) bool {
+	fns, _ := t.client.ListFunctions(`^plugin\.Open$`)
+	return len(fns) > 0
+}

pkg/terminal/starbind/starlark_mapping.go

@@ -319,6 +319,12 @@ func (env *Env) starlarkPredeclare() starlark.StringDict {
 				return starlark.None, decorateError(thread, err)
 			}
 		}
+		if len(args) > 3 && args[3] != starlark.None {
+			err := unmarshalStarlarkValue(args[3], &rpcArgs.Suspended, "Suspended")
+			if err != nil {
+				return starlark.None, decorateError(thread, err)
+			}
+		}
 		for _, kv := range kwargs {
 			var err error
 			switch kv[0].(starlark.String) {
@@ -328,6 +334,8 @@ func (env *Env) starlarkPredeclare() starlark.StringDict {
 				err = unmarshalStarlarkValue(kv[1], &rpcArgs.LocExpr, "LocExpr")
 			case "SubstitutePathRules":
 				err = unmarshalStarlarkValue(kv[1], &rpcArgs.SubstitutePathRules, "SubstitutePathRules")
+			case "Suspended":
+				err = unmarshalStarlarkValue(kv[1], &rpcArgs.Suspended, "Suspended")
 			default:
 				err = fmt.Errorf("unknown argument %q", kv[0])
 			}

service/client.go

@@ -70,7 +70,7 @@ type Client interface {
 	// CreateBreakpoint creates a new breakpoint.
 	CreateBreakpoint(*api.Breakpoint) (*api.Breakpoint, error)
 	// CreateBreakpointWithExpr creates a new breakpoint and sets an expression to restore it after it is disabled.
-	CreateBreakpointWithExpr(*api.Breakpoint, string, [][2]string) (*api.Breakpoint, error)
+	CreateBreakpointWithExpr(*api.Breakpoint, string, [][2]string, bool) (*api.Breakpoint, error)
 	// CreateWatchpoint creates a new watchpoint.
 	CreateWatchpoint(api.EvalScope, string, api.WatchType) (*api.Breakpoint, error)
 	// ListBreakpoints gets all breakpoints.

service/dap/server.go

@@ -1381,7 +1381,7 @@ func (s *Session) setBreakpoints(prefix string, totalBps int, metadataFunc func(
 				err = setLogMessage(bp, want.logMessage)
 				if err == nil {
 					// Create new breakpoints.
-					got, err = s.debugger.CreateBreakpoint(bp, "", nil)
+					got, err = s.debugger.CreateBreakpoint(bp, "", nil, false)
 				}
 			}
 		}

service/debugger/debugger.go

@@ -641,7 +641,11 @@ func (d *Debugger) state(retLoadCfg *proc.LoadConfig, withBreakpointInfo bool) (
 //
 // If LocExpr is specified it will be used, along with substitutePathRules,
 // to re-enable the breakpoint after it is disabled.
-func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint, locExpr string, substitutePathRules [][2]string) (*api.Breakpoint, error) {
+//
+// If suspended is true a logical breakpoint will be created even if the
+// location can not be found, the backend will attempt to enable the
+// breakpoint every time a new plugin is loaded.
+func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint, locExpr string, substitutePathRules [][2]string, suspended bool) (*api.Breakpoint, error) {
 	d.targetMutex.Lock()
 	defer d.targetMutex.Unlock()
 
@@ -721,12 +725,13 @@ func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint, locExpr string,
 		setbp.Expr = func(t *proc.Target) []uint64 {
 			locs, err := loc.Find(t, d.processArgs, nil, locExpr, false, substitutePathRules)
 			if err != nil || len(locs) != 1 {
+				logflags.DebuggerLogger().Debugf("could not evaluate breakpoint expression %q: %v (number of results %d)", locExpr, err, len(locs))
 				return nil
 			}
 			return locs[0].PCs
 		}
 	}
-	createdBp, err := createLogicalBreakpoint(d, requestedBp, &setbp)
+	createdBp, err := createLogicalBreakpoint(d, requestedBp, &setbp, suspended)
 
 	if err != nil {
 		return nil, err
@@ -761,7 +766,7 @@ func (d *Debugger) ConvertThreadBreakpoint(thread proc.Thread) *api.Breakpoint {
 
 // createLogicalBreakpoint creates one physical breakpoint for each address
 // in addrs and associates all of them with the same logical breakpoint.
-func createLogicalBreakpoint(d *Debugger, requestedBp *api.Breakpoint, setbp *proc.SetBreakpoint) (*api.Breakpoint, error) {
+func createLogicalBreakpoint(d *Debugger, requestedBp *api.Breakpoint, setbp *proc.SetBreakpoint, suspended bool) (*api.Breakpoint, error) {
 	id := requestedBp.ID
 
 	var lbp *proc.LogicalBreakpoint
@@ -781,8 +786,12 @@ func createLogicalBreakpoint(d *Debugger, requestedBp *api.Breakpoint, setbp *pr
 
 	err = d.target.EnableBreakpoint(lbp)
 	if err != nil {
-		delete(d.target.LogicalBreakpoints, lbp.LogicalID)
-		return nil, err
+		if suspended {
+			logflags.DebuggerLogger().Debugf("could not enable new breakpoint: %v (breakpoint will be suspended)", err)
+		} else {
+			delete(d.target.LogicalBreakpoints, lbp.LogicalID)
+			return nil, err
+		}
 	}
 
 	return d.convertBreakpoint(lbp), nil

service/rpc1/server.go

@@ -106,7 +106,7 @@ func (s *RPCServer) CreateBreakpoint(bp, newBreakpoint *api.Breakpoint) error {
 	if err := api.ValidBreakpointName(bp.Name); err != nil {
 		return err
 	}
-	createdbp, err := s.debugger.CreateBreakpoint(bp, "", nil)
+	createdbp, err := s.debugger.CreateBreakpoint(bp, "", nil, false)
 	if err != nil {
 		return err
 	}

service/rpc2/client.go

@@ -245,16 +245,16 @@ func (c *RPCClient) GetBreakpointByName(name string) (*api.Breakpoint, error) {
 // https://pkg.go.dev/github.com/go-delve/delve/service/debugger#Debugger.CreateBreakpoint
 func (c *RPCClient) CreateBreakpoint(breakPoint *api.Breakpoint) (*api.Breakpoint, error) {
 	var out CreateBreakpointOut
-	err := c.call("CreateBreakpoint", CreateBreakpointIn{*breakPoint, "", nil}, &out)
+	err := c.call("CreateBreakpoint", CreateBreakpointIn{*breakPoint, "", nil, false}, &out)
 	return &out.Breakpoint, err
 }
 
 // CreateBreakpointWithExpr is like CreateBreakpoint but will also set a
 // location expression to be used to restore the breakpoint after it is
 // disabled.
-func (c *RPCClient) CreateBreakpointWithExpr(breakPoint *api.Breakpoint, locExpr string, substitutePathRules [][2]string) (*api.Breakpoint, error) {
+func (c *RPCClient) CreateBreakpointWithExpr(breakPoint *api.Breakpoint, locExpr string, substitutePathRules [][2]string, suspended bool) (*api.Breakpoint, error) {
 	var out CreateBreakpointOut
-	err := c.call("CreateBreakpoint", CreateBreakpointIn{*breakPoint, locExpr, substitutePathRules}, &out)
+	err := c.call("CreateBreakpoint", CreateBreakpointIn{*breakPoint, locExpr, substitutePathRules, suspended}, &out)
 	return &out.Breakpoint, err
 }
 

service/rpc2/server.go

@@ -245,6 +245,7 @@ type CreateBreakpointIn struct {
 
 	LocExpr             string
 	SubstitutePathRules [][2]string
+	Suspended           bool
 }
 
 type CreateBreakpointOut struct {
@@ -259,7 +260,7 @@ func (s *RPCServer) CreateBreakpoint(arg CreateBreakpointIn, out *CreateBreakpoi
 	if err := api.ValidBreakpointName(arg.Breakpoint.Name); err != nil {
 		return err
 	}
-	createdbp, err := s.debugger.CreateBreakpoint(&arg.Breakpoint, arg.LocExpr, arg.SubstitutePathRules)
+	createdbp, err := s.debugger.CreateBreakpoint(&arg.Breakpoint, arg.LocExpr, arg.SubstitutePathRules, arg.Suspended)
 	if err != nil {
 		return err
 	}