proc,service/debugger: track how breakpoints were originally set (#3148)

Adds field to breakpoint struct to track how a breakpoint was
originally set, moves the logic for disabling and enabling a breakpoint
to proc.
This will allow creating suspended breakpoints that are automatically
enabled when a plugin is loaded. When follow exec mode is implemented
it will also be possible to automatically enable breakpoints (whether
or not they were suspended) on new child processes, as they are
spawned.

It also improves breakpoint restore after a restart, before this after
a restart breakpoints would be re-enabled using their file:line
position, for breakpoints set using a function name or a location
expression this could be the wrong location after a recompile.

Updates #1653
Updates #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) | Equivalent to API call [CreateBreakpoint](https://godoc.org/github.com/go-delve/delve/service/rpc2#RPCServer.CreateBreakpoint)
+create_breakpoint(Breakpoint, LocExpr, SubstitutePathRules) | 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)

_fixtures/testfnpos1.go

@@ -0,0 +1,16 @@
+package main
+
+import "fmt"
+
+func f1() {
+	fmt.Printf("f1\n")
+}
+
+func f2() {
+	fmt.Printf("f2\n")
+}
+
+func main() {
+	f1()
+	f2()
+}

_fixtures/testfnpos2.go

@@ -0,0 +1,16 @@
+package main
+
+import "fmt"
+
+func f2() {
+	fmt.Printf("f2\n")
+}
+
+func f1() {
+	fmt.Printf("f1\n")
+}
+
+func main() {
+	f1()
+	f2()
+}

pkg/proc/breakpoints.go

@@ -966,6 +966,8 @@ type LogicalBreakpoint struct {
 	Line         int
 	Enabled      bool
 
+	Set SetBreakpoint
+
 	Tracepoint  bool // Tracepoint flag
 	TraceReturn bool
 	Goroutine   bool     // Retrieve goroutine information
@@ -990,3 +992,17 @@ type LogicalBreakpoint struct {
 
 	UserData interface{} // Any additional information about the breakpoint
 }
+
+// SetBreakpoint describes how a breakpoint should be set.
+type SetBreakpoint struct {
+	FunctionName string
+	File         string
+	Line         int
+	Expr         func(*Target) []uint64
+	PidAddrs     []PidAddr
+}
+
+type PidAddr struct {
+	Pid  int
+	Addr uint64
+}

pkg/proc/target_group.go

@@ -1,6 +1,7 @@
 package proc
 
 import (
+	"bytes"
 	"fmt"
 	"strings"
 )
@@ -47,6 +48,32 @@ func NewGroup(t *Target) *TargetGroup {
 	}
 }
 
+// NewGroupRestart creates a new group of targets containing t and
+// sets breakpoints and other attributes from oldgrp.
+// Breakpoints that can not be set will be discarded, if discard is not nil
+// it will be called for each discarded breakpoint.
+func NewGroupRestart(t *Target, oldgrp *TargetGroup, discard func(*LogicalBreakpoint, error)) *TargetGroup {
+	grp := NewGroup(t)
+	grp.LogicalBreakpoints = oldgrp.LogicalBreakpoints
+	t.Breakpoints().Logical = grp.LogicalBreakpoints
+	for _, bp := range grp.LogicalBreakpoints {
+		if bp.LogicalID < 0 || !bp.Enabled {
+			continue
+		}
+		bp.TotalHitCount = 0
+		bp.HitCount = make(map[int64]uint64)
+		bp.Set.PidAddrs = nil // breakpoints set through a list of addresses can not be restored after a restart
+		err := grp.EnableBreakpoint(bp)
+		if err != nil {
+			if discard != nil {
+				discard(bp, err)
+			}
+			delete(grp.LogicalBreakpoints, bp.LogicalID)
+		}
+	}
+	return grp
+}
+
 // Targets returns a slice of all targets in the group, including the
 // ones that are no longer valid.
 func (grp *TargetGroup) Targets() []*Target {
@@ -128,6 +155,125 @@ func (grp *TargetGroup) TargetForThread(thread Thread) *Target {
 	return nil
 }
 
+// EnableBreakpoint re-enables a disabled logical breakpoint.
+func (grp *TargetGroup) EnableBreakpoint(lbp *LogicalBreakpoint) error {
+	var err0, errNotFound, errExists error
+	didSet := false
+targetLoop:
+	for _, p := range grp.targets {
+		err := enableBreakpointOnTarget(p, lbp)
+
+		switch err.(type) {
+		case nil:
+			didSet = true
+		case *ErrFunctionNotFound, *ErrCouldNotFindLine:
+			errNotFound = err
+		case BreakpointExistsError:
+			errExists = err
+		default:
+			err0 = err
+			break targetLoop
+		}
+	}
+	if errNotFound != nil && !didSet {
+		return errNotFound
+	}
+	if errExists != nil && !didSet {
+		return errExists
+	}
+	if !didSet {
+		if _, err := grp.Valid(); err != nil {
+			return err
+		}
+	}
+	if err0 != nil {
+		it := ValidTargets{Group: grp}
+		for it.Next() {
+			for _, bp := range it.Breakpoints().M {
+				if bp.LogicalID() == lbp.LogicalID {
+					if err1 := it.ClearBreakpoint(bp.Addr); err1 != nil {
+						return fmt.Errorf("error while creating breakpoint: %v, additionally the breakpoint could not be properly rolled back: %v", err0, err1)
+					}
+				}
+			}
+		}
+		return err0
+	}
+	lbp.Enabled = true
+	return nil
+}
+
+func enableBreakpointOnTarget(p *Target, lbp *LogicalBreakpoint) error {
+	var err error
+	var addrs []uint64
+	switch {
+	case lbp.Set.File != "":
+		addrs, err = FindFileLocation(p, lbp.Set.File, lbp.Set.Line)
+	case lbp.Set.FunctionName != "":
+		addrs, err = FindFunctionLocation(p, lbp.Set.FunctionName, lbp.Set.Line)
+	case lbp.Set.Expr != nil:
+		addrs = lbp.Set.Expr(p)
+	case len(lbp.Set.PidAddrs) > 0:
+		for _, pidAddr := range lbp.Set.PidAddrs {
+			if pidAddr.Pid == p.Pid() {
+				addrs = append(addrs, pidAddr.Addr)
+			}
+		}
+	default:
+		return fmt.Errorf("breakpoint %d can not be enabled", lbp.LogicalID)
+	}
+
+	if err != nil {
+		return err
+	}
+
+	for _, addr := range addrs {
+		_, err = p.SetBreakpoint(lbp.LogicalID, addr, UserBreakpoint, nil)
+		if err != nil {
+			if _, isexists := err.(BreakpointExistsError); isexists {
+				continue
+			}
+			return err
+		}
+	}
+
+	return err
+}
+
+// DisableBreakpoint disables a logical breakpoint.
+func (grp *TargetGroup) DisableBreakpoint(lbp *LogicalBreakpoint) error {
+	var errs []error
+	n := 0
+	it := ValidTargets{Group: grp}
+	for it.Next() {
+		for _, bp := range it.Breakpoints().M {
+			if bp.LogicalID() == lbp.LogicalID {
+				n++
+				err := it.ClearBreakpoint(bp.Addr)
+				if err != nil {
+					errs = append(errs, err)
+				}
+			}
+		}
+	}
+	if len(errs) > 0 {
+		buf := new(bytes.Buffer)
+		for i, err := range errs {
+			fmt.Fprintf(buf, "%s", err)
+			if i != len(errs)-1 {
+				fmt.Fprintf(buf, ", ")
+			}
+		}
+
+		if len(errs) == n {
+			return fmt.Errorf("unable to clear breakpoint %d: %v", lbp.LogicalID, buf.String())
+		}
+		return fmt.Errorf("unable to clear breakpoint %d (partial): %s", lbp.LogicalID, buf.String())
+	}
+	lbp.Enabled = false
+	return nil
+}
+
 // ValidTargets iterates through all valid targets in Group.
 type ValidTargets struct {
 	*Target

pkg/terminal/command.go

@@ -1763,7 +1763,7 @@ func setBreakpoint(t *Term, ctx callContext, tracepoint bool, argstr string) ([]
 			requestedBp.LoadArgs = &ShortLoadConfig
 		}
 
-		bp, err := t.client.CreateBreakpoint(requestedBp)
+		bp, err := t.client.CreateBreakpointWithExpr(requestedBp, spec, t.substitutePathRules())
 		if err != nil {
 			return nil, err
 		}

pkg/terminal/starbind/starlark_mapping.go

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

service/client.go

@@ -69,6 +69,8 @@ type Client interface {
 	GetBreakpointByName(name string) (*api.Breakpoint, error)
 	// 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)
 	// 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)
+					got, err = s.debugger.CreateBreakpoint(bp, "", nil)
 				}
 			}
 		}

service/debugger/debugger.go

@@ -1,7 +1,6 @@
 package debugger
 
 import (
-	"bytes"
 	"debug/dwarf"
 	"debug/elf"
 	"debug/macho"
@@ -519,24 +518,9 @@ func (d *Debugger) Restart(rerecord bool, pos string, resetArgs bool, newArgs []
 	}
 
 	discarded := []api.DiscardedBreakpoint{}
-	p.Breakpoints().Logical = d.target.LogicalBreakpoints
+	d.target = proc.NewGroupRestart(p, d.target, func(oldBp *proc.LogicalBreakpoint, err error) {
-	d.target = proc.NewGroup(p)
+		discarded = append(discarded, api.DiscardedBreakpoint{Breakpoint: api.ConvertLogicalBreakpoint(oldBp), Reason: err.Error()})
-	for _, oldBp := range d.target.LogicalBreakpoints {
+	})
-		if oldBp.LogicalID < 0 || !oldBp.Enabled {
-			continue
-		}
-		if len(oldBp.File) > 0 {
-			oldBp.TotalHitCount = 0
-			oldBp.HitCount = make(map[int64]uint64)
-			err := d.createPhysicalBreakpoints(oldBp)
-			if err != nil {
-				discarded = append(discarded, api.DiscardedBreakpoint{Breakpoint: api.ConvertLogicalBreakpoint(oldBp), Reason: err.Error()})
-				continue
-			}
-		} else {
-			discarded = append(discarded, api.DiscardedBreakpoint{Breakpoint: api.ConvertLogicalBreakpoint(oldBp), Reason: "can not recreate watchpoint on restart"})
-		}
-	}
 	return discarded, nil
 }
 
@@ -654,21 +638,15 @@ func (d *Debugger) state(retLoadCfg *proc.LoadConfig, withBreakpointInfo bool) (
 // Note that this method will use the first successful method in order to
 // create a breakpoint, so mixing different fields will not result is multiple
 // breakpoints being set.
-func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoint, error) {
+//
+// 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) {
 	d.targetMutex.Lock()
 	defer d.targetMutex.Unlock()
 
-	if len(d.target.Targets()) != 1 {
-		//TODO(aarzilli):
-		// - the calls to FindFileLocation and FindFunctionLocation need to be done on all targets
-		// - the Addrs slice and the Addr field need to be converted to a format
-		// that can specify to which target an address belongs when there are
-		// multiple targets (but this must happen in a backwards compatible way)
-		panic("multiple targets not implemented")
-	}
-
 	var (
-		addrs []uint64
+		setbp proc.SetBreakpoint
 		err   error
 	)
 
@@ -678,9 +656,17 @@ func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoin
 		}
 	}
 
+	if lbp := d.target.LogicalBreakpoints[requestedBp.ID]; lbp != nil {
+		abp := d.convertBreakpoint(lbp)
+		return abp, proc.BreakpointExistsError{File: lbp.File, Line: lbp.Line}
+	}
+
 	switch {
 	case requestedBp.TraceReturn:
-		addrs = []uint64{requestedBp.Addr}
+		if len(d.target.Targets()) != 1 {
+			return nil, ErrNotImplementedWithMultitarget
+		}
+		setbp.PidAddrs = []proc.PidAddr{{Pid: d.target.Selected.Pid(), Addr: requestedBp.Addr}}
 	case len(requestedBp.File) > 0:
 		fileName := requestedBp.File
 		if runtime.GOOS == "windows" {
@@ -697,21 +683,51 @@ func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoin
 				}
 			}
 		}
-		addrs, err = proc.FindFileLocation(d.target.Selected, fileName, requestedBp.Line)
+		setbp.File = fileName
+		setbp.Line = requestedBp.Line
 	case len(requestedBp.FunctionName) > 0:
-		addrs, err = proc.FindFunctionLocation(d.target.Selected, requestedBp.FunctionName, requestedBp.Line)
+		setbp.FunctionName = requestedBp.FunctionName
+		setbp.Line = requestedBp.Line
 	case len(requestedBp.Addrs) > 0:
-		addrs = requestedBp.Addrs
+		setbp.PidAddrs = make([]proc.PidAddr, len(requestedBp.Addrs))
-		//TODO(aarzilli): read requestedBp.AddrPid
+		if len(d.target.Targets()) == 1 {
+			pid := d.target.Selected.Pid()
+			for i, addr := range requestedBp.Addrs {
+				setbp.PidAddrs[i] = proc.PidAddr{Pid: pid, Addr: addr}
+			}
+		} else {
+			if len(requestedBp.Addrs) != len(requestedBp.AddrPid) {
+				return nil, errors.New("mismatched length in addrs and addrpid")
+			}
+			for i, addr := range requestedBp.Addrs {
+				setbp.PidAddrs[i] = proc.PidAddr{Pid: requestedBp.AddrPid[i], Addr: addr}
+			}
+		}
 	default:
-		addrs = []uint64{requestedBp.Addr}
+		if requestedBp.Addr != 0 {
+			setbp.PidAddrs = []proc.PidAddr{{Pid: d.target.Selected.Pid(), Addr: requestedBp.Addr}}
+		}
 	}
 
 	if err != nil {
 		return nil, err
 	}
 
-	createdBp, err := createLogicalBreakpoint(d, addrs, requestedBp, 0)
+	if locExpr != "" {
+		loc, err := locspec.Parse(locExpr)
+		if err != nil {
+			return nil, err
+		}
+		setbp.Expr = func(t *proc.Target) []uint64 {
+			locs, err := loc.Find(t, d.processArgs, nil, locExpr, false, substitutePathRules)
+			if err != nil || len(locs) != 1 {
+				return nil
+			}
+			return locs[0].PCs
+		}
+	}
+	createdBp, err := createLogicalBreakpoint(d, requestedBp, &setbp)
+
 	if err != nil {
 		return nil, err
 	}
@@ -745,23 +761,15 @@ 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, addrs []uint64, requestedBp *api.Breakpoint, id int) (*api.Breakpoint, error) {
+func createLogicalBreakpoint(d *Debugger, requestedBp *api.Breakpoint, setbp *proc.SetBreakpoint) (*api.Breakpoint, error) {
-	if len(d.target.Targets()) != 1 {
+	id := requestedBp.ID
-		panic("multiple targets not implemented")
-	}
-	p := d.target.Selected
-
-	if lbp := p.Breakpoints().Logical[requestedBp.ID]; lbp != nil {
-		abp := d.convertBreakpoint(lbp)
-		return abp, proc.BreakpointExistsError{File: lbp.File, Line: lbp.Line}
-	}
 
 	var lbp *proc.LogicalBreakpoint
 	if id <= 0 {
 		d.breakpointIDCounter++
 		id = d.breakpointIDCounter
 		lbp = &proc.LogicalBreakpoint{LogicalID: id, HitCount: make(map[int64]uint64), Enabled: true}
-		p.Breakpoints().Logical[id] = lbp
+		d.target.LogicalBreakpoints[id] = lbp
 	}
 
 	err := copyLogicalBreakpointInfo(lbp, requestedBp)
@@ -769,102 +777,15 @@ func createLogicalBreakpoint(d *Debugger, addrs []uint64, requestedBp *api.Break
 		return nil, err
 	}
 
-	bps := make([]*proc.Breakpoint, len(addrs))
+	lbp.Set = *setbp
-	for i := range addrs {
+
-		bps[i], err = p.SetBreakpoint(id, addrs[i], proc.UserBreakpoint, nil)
+	err = d.target.EnableBreakpoint(lbp)
-		if err != nil {
-			break
-		}
-	}
 	if err != nil {
-		delete(p.Breakpoints().Logical, id)
+		delete(d.target.LogicalBreakpoints, lbp.LogicalID)
-		if isBreakpointExistsErr(err) {
-			return nil, err
-		}
-		for _, bp := range bps {
-			if bp == nil {
-				continue
-			}
-			if err1 := p.ClearBreakpoint(bp.Addr); err1 != nil {
-				err = fmt.Errorf("error while creating breakpoint: %v, additionally the breakpoint could not be properly rolled back: %v", err, err1)
-				return nil, err
-			}
-		}
 		return nil, err
 	}
 
-	return d.convertBreakpoint(bps[0].Logical), nil
+	return d.convertBreakpoint(lbp), nil
-}
-
-func (d *Debugger) createPhysicalBreakpoints(lbp *proc.LogicalBreakpoint) error {
-	if len(d.target.Targets()) != 1 {
-		panic("multiple targets not implemented")
-	}
-	p := d.target.Selected
-	addrs, err := proc.FindFileLocation(p, lbp.File, lbp.Line)
-	if err != nil {
-		return err
-	}
-	bps := make([]*proc.Breakpoint, len(addrs))
-	for i := range addrs {
-		bps[i], err = p.SetBreakpoint(lbp.LogicalID, addrs[i], proc.UserBreakpoint, nil)
-		if err != nil {
-			break
-		}
-	}
-	if err != nil {
-		if isBreakpointExistsErr(err) {
-			return err
-		}
-		for _, bp := range bps {
-			if bp == nil {
-				continue
-			}
-			if err1 := p.ClearBreakpoint(bp.Addr); err1 != nil {
-				return fmt.Errorf("error while creating breakpoint: %v, additionally the breakpoint could not be properly rolled back: %v", err, err1)
-			}
-		}
-		return err
-	}
-	return nil
-}
-
-func (d *Debugger) clearPhysicalBreakpoints(id int) error {
-	if len(d.target.Targets()) != 1 {
-		panic("multiple targets not implemented")
-	}
-	p := d.target.Selected
-	var errs []error
-	n := 0
-	for _, bp := range p.Breakpoints().M {
-		if bp.LogicalID() == id {
-			n++
-			err := p.ClearBreakpoint(bp.Addr)
-			if err != nil {
-				errs = append(errs, err)
-			}
-		}
-	}
-	if len(errs) > 0 {
-		buf := new(bytes.Buffer)
-		for i, err := range errs {
-			fmt.Fprintf(buf, "%s", err)
-			if i != len(errs)-1 {
-				fmt.Fprintf(buf, ", ")
-			}
-		}
-
-		if len(errs) == n {
-			return fmt.Errorf("unable to clear breakpoint %d: %v", id, buf.String())
-		}
-		return fmt.Errorf("unable to clear breakpoint %d (partial): %s", id, buf.String())
-	}
-	return nil
-}
-
-func isBreakpointExistsErr(err error) bool {
-	_, r := err.(proc.BreakpointExistsError)
-	return r
 }
 
 func (d *Debugger) CreateEBPFTracepoint(fnName string) error {
@@ -881,38 +802,53 @@ func (d *Debugger) CreateEBPFTracepoint(fnName string) error {
 // It also enables or disables the breakpoint.
 // We can consume this function to avoid locking a goroutine.
 func (d *Debugger) amendBreakpoint(amend *api.Breakpoint) error {
-	if len(d.target.Targets()) != 1 {
-		panic("multiple targets not implemented")
-	}
 	original := d.target.LogicalBreakpoints[amend.ID]
 	if original == nil {
 		return fmt.Errorf("no breakpoint with ID %d", amend.ID)
 	}
-	if amend.Disabled && original.Enabled {
+	enabledBefore := original.Enabled
-		original.Enabled = false
-		err := copyLogicalBreakpointInfo(original, amend)
-		if err != nil {
-			return err
-		}
-		return d.clearPhysicalBreakpoints(amend.ID)
-	}
-
-	if !amend.Disabled && !original.Enabled {
-		original.Enabled = true
-		copyLogicalBreakpointInfo(original, amend)
-		return d.createPhysicalBreakpoints(original)
-	}
-
 	err := copyLogicalBreakpointInfo(original, amend)
 	if err != nil {
 		return err
 	}
-	for _, bp := range d.findBreakpoint(amend.ID) {
+	original.Enabled = !amend.Disabled
-		bp.UserBreaklet().Cond = original.Cond
+
+	switch {
+	case enabledBefore && !original.Enabled:
+		if d.isWatchpoint(original) {
+			return errors.New("can not disable watchpoints")
+		}
+		err = d.target.DisableBreakpoint(original)
+	case !enabledBefore && original.Enabled:
+		err = d.target.EnableBreakpoint(original)
+	}
+	if err != nil {
+		return err
+	}
+
+	t := proc.ValidTargets{Group: d.target}
+	for t.Next() {
+		for _, bp := range t.Breakpoints().M {
+			if bp.LogicalID() == amend.ID {
+				bp.UserBreaklet().Cond = original.Cond
+			}
+		}
 	}
 	return nil
 }
 
+func (d *Debugger) isWatchpoint(lbp *proc.LogicalBreakpoint) bool {
+	t := proc.ValidTargets{Group: d.target}
+	for t.Next() {
+		for _, bp := range t.Breakpoints().M {
+			if bp.LogicalID() == lbp.LogicalID {
+				return bp.WatchType != 0
+			}
+		}
+	}
+	return false
+}
+
 // AmendBreakpoint will update the breakpoint with the matching ID.
 // It also enables or disables the breakpoint.
 func (d *Debugger) AmendBreakpoint(amend *api.Breakpoint) error {
@@ -1015,26 +951,28 @@ func (d *Debugger) ClearBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoint
 
 // clearBreakpoint clears a breakpoint, we can consume this function to avoid locking a goroutine
 func (d *Debugger) clearBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoint, error) {
-	if len(d.target.Targets()) != 1 {
+	if _, err := d.target.Valid(); err != nil {
-		panic("multiple targets not implemented")
+		return nil, err
 	}
-	p := d.target.Selected
 
 	if requestedBp.ID <= 0 {
-		bp := p.Breakpoints().M[requestedBp.Addr]
+		if len(d.target.Targets()) != 1 {
+			return nil, ErrNotImplementedWithMultitarget
+		}
+		bp := d.target.Selected.Breakpoints().M[requestedBp.Addr]
 		requestedBp.ID = bp.LogicalID()
 	}
 
 	lbp := d.target.LogicalBreakpoints[requestedBp.ID]
 	clearedBp := d.convertBreakpoint(lbp)
 
-	delete(d.target.LogicalBreakpoints, requestedBp.ID)
+	err := d.target.DisableBreakpoint(lbp)
-
-	err := d.clearPhysicalBreakpoints(requestedBp.ID)
 	if err != nil {
 		return nil, err
 	}
 
+	delete(d.target.LogicalBreakpoints, requestedBp.ID)
+
 	d.log.Infof("cleared breakpoint: %#v", clearedBp)
 	return clearedBp, nil
 }
@@ -1106,21 +1044,6 @@ func (d *Debugger) FindBreakpoint(id int) *api.Breakpoint {
 	return d.convertBreakpoint(lbp)
 }
 
-func (d *Debugger) findBreakpoint(id int) []*proc.Breakpoint {
-	if len(d.target.Targets()) != 1 {
-		panic("multiple targets not implemented")
-	}
-	p := d.target.Selected
-
-	var bps []*proc.Breakpoint
-	for _, bp := range p.Breakpoints().M {
-		if bp.LogicalID() == id {
-			bps = append(bps, bp)
-		}
-	}
-	return bps
-}
-
 // FindBreakpointByName returns the breakpoint specified by 'name'
 func (d *Debugger) FindBreakpointByName(name string) *api.Breakpoint {
 	d.targetMutex.Lock()
@@ -1139,9 +1062,6 @@ func (d *Debugger) findBreakpointByName(name string) *api.Breakpoint {
 
 // CreateWatchpoint creates a watchpoint on the specified expression.
 func (d *Debugger) CreateWatchpoint(goid int64, frame, deferredCall int, expr string, wtype api.WatchType) (*api.Breakpoint, error) {
-	if len(d.target.Targets()) != 1 {
-		panic("multiple targets not implemented")
-	}
 	p := d.target.Selected
 
 	s, err := proc.ConvertEvalScope(p, goid, frame, deferredCall)

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)
+	createdbp, err := s.debugger.CreateBreakpoint(bp, "", nil)
 	if err != nil {
 		return err
 	}

service/rpc2/client.go

@@ -245,7 +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}, &out)
+	err := c.call("CreateBreakpoint", CreateBreakpointIn{*breakPoint, "", nil}, &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) {
+	var out CreateBreakpointOut
+	err := c.call("CreateBreakpoint", CreateBreakpointIn{*breakPoint, locExpr, substitutePathRules}, &out)
 	return &out.Breakpoint, err
 }
 

service/rpc2/server.go

@@ -242,6 +242,9 @@ func (s *RPCServer) ListBreakpoints(arg ListBreakpointsIn, out *ListBreakpointsO
 
 type CreateBreakpointIn struct {
 	Breakpoint api.Breakpoint
+
+	LocExpr             string
+	SubstitutePathRules [][2]string
 }
 
 type CreateBreakpointOut struct {
@@ -256,7 +259,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)
+	createdbp, err := s.debugger.CreateBreakpoint(&arg.Breakpoint, arg.LocExpr, arg.SubstitutePathRules)
 	if err != nil {
 		return err
 	}

service/test/integration2_test.go

@@ -2856,3 +2856,42 @@ func TestNonGoDebug(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+func TestRestart_PreserveFunctionBreakpoint(t *testing.T) {
+	// Tests that function breakpoint get restored correctly, after a rebuild,
+	// even if the function changed position in the source file.
+
+	dir := protest.FindFixturesDir()
+
+	copy := func(inpath string) {
+		buf, err := ioutil.ReadFile(inpath)
+		assertNoError(err, t, fmt.Sprintf("Reading %q", inpath))
+		outpath := filepath.Join(dir, "testfnpos.go")
+		assertNoError(ioutil.WriteFile(outpath, buf, 0666), t, fmt.Sprintf("Creating %q", outpath))
+	}
+
+	copy(filepath.Join(dir, "testfnpos1.go"))
+
+	withTestClient2Extended("testfnpos", t, 0, [3]string{}, func(c service.Client, f protest.Fixture) {
+		_, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.f1"})
+		assertNoError(err, t, "CreateBreakpoint")
+		state := <-c.Continue()
+		assertNoError(state.Err, t, "Continue")
+		t.Logf("%s:%d", state.CurrentThread.File, state.CurrentThread.Line)
+		if state.CurrentThread.Line != 5 {
+			t.Fatalf("wrong location %s:%d", state.CurrentThread.File, state.CurrentThread.Line)
+		}
+
+		// rewrite test file and restart, rebuilding
+		copy(filepath.Join(dir, "testfnpos2.go"))
+		_, err = c.Restart(true)
+		assertNoError(err, t, "Restart(true)")
+
+		state = <-c.Continue()
+		assertNoError(state.Err, t, "Continue")
+		t.Logf("%s:%d", state.CurrentThread.File, state.CurrentThread.Line)
+		if state.CurrentThread.Line != 9 {
+			t.Fatalf("wrong location %s:%d", state.CurrentThread.File, state.CurrentThread.Line)
+		}
+	})
+}