From d7b3019fe06dd5909a20dfd2ed63e7388b2bad09 Mon Sep 17 00:00:00 2001 From: aarzilli Date: Fri, 12 Aug 2022 15:18:21 +0200 Subject: [PATCH] proc,service/debugger: track how breakpoints were originally set 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 | 2 +- _fixtures/testfnpos1.go | 16 ++ _fixtures/testfnpos2.go | 16 ++ pkg/proc/breakpoints.go | 16 ++ pkg/proc/target_group.go | 146 +++++++++++ pkg/terminal/command.go | 2 +- pkg/terminal/starbind/starlark_mapping.go | 16 ++ service/client.go | 2 + service/dap/server.go | 2 +- service/debugger/debugger.go | 286 ++++++++-------------- service/rpc1/server.go | 2 +- service/rpc2/client.go | 11 +- service/rpc2/server.go | 5 +- service/test/integration2_test.go | 39 +++ 14 files changed, 372 insertions(+), 189 deletions(-) create mode 100644 _fixtures/testfnpos1.go create mode 100644 _fixtures/testfnpos2.go diff --git a/Documentation/cli/starlark.md b/Documentation/cli/starlark.md index ffe7224464..d1c2cfd75d 100644 --- a/Documentation/cli/starlark.md +++ b/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) diff --git a/_fixtures/testfnpos1.go b/_fixtures/testfnpos1.go new file mode 100644 index 0000000000..b6afc7a94a --- /dev/null +++ b/_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() +} diff --git a/_fixtures/testfnpos2.go b/_fixtures/testfnpos2.go new file mode 100644 index 0000000000..9572603fc8 --- /dev/null +++ b/_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() +} diff --git a/pkg/proc/breakpoints.go b/pkg/proc/breakpoints.go index 2b59692135..5e4ca5bc73 100644 --- a/pkg/proc/breakpoints.go +++ b/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 +} diff --git a/pkg/proc/target_group.go b/pkg/proc/target_group.go index 71f9bd9400..07a464625a 100644 --- a/pkg/proc/target_group.go +++ b/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 diff --git a/pkg/terminal/command.go b/pkg/terminal/command.go index 74164bfc6a..5060d3faa7 100644 --- a/pkg/terminal/command.go +++ b/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 } diff --git a/pkg/terminal/starbind/starlark_mapping.go b/pkg/terminal/starbind/starlark_mapping.go index daf5f1ee7a..c3c502491b 100644 --- a/pkg/terminal/starbind/starlark_mapping.go +++ b/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]) } diff --git a/service/client.go b/service/client.go index cccddd503e..71694f8c79 100644 --- a/service/client.go +++ b/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. diff --git a/service/dap/server.go b/service/dap/server.go index acc6a28340..94f41e667a 100644 --- a/service/dap/server.go +++ b/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) } } } diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index ba9d3a2bdb..5b88ef3e05 100644 --- a/service/debugger/debugger.go +++ b/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.NewGroup(p) - 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"}) - } - } + d.target = proc.NewGroupRestart(p, d.target, func(oldBp *proc.LogicalBreakpoint, err error) { + discarded = append(discarded, api.DiscardedBreakpoint{Breakpoint: api.ConvertLogicalBreakpoint(oldBp), Reason: err.Error()}) + }) 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 - //TODO(aarzilli): read requestedBp.AddrPid + setbp.PidAddrs = make([]proc.PidAddr, len(requestedBp.Addrs)) + 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) { - if len(d.target.Targets()) != 1 { - 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} - } +func createLogicalBreakpoint(d *Debugger, requestedBp *api.Breakpoint, setbp *proc.SetBreakpoint) (*api.Breakpoint, error) { + id := requestedBp.ID 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)) - for i := range addrs { - bps[i], err = p.SetBreakpoint(id, addrs[i], proc.UserBreakpoint, nil) - if err != nil { - break - } - } - if err != nil { - delete(p.Breakpoints().Logical, id) - 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 -} + lbp.Set = *setbp -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) + err = d.target.EnableBreakpoint(lbp) 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()) + delete(d.target.LogicalBreakpoints, lbp.LogicalID) + return nil, err } - return nil -} -func isBreakpointExistsErr(err error) bool { - _, r := err.(proc.BreakpointExistsError) - return r + return d.convertBreakpoint(lbp), nil } 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 { - original.Enabled = false - err := copyLogicalBreakpointInfo(original, amend) - if err != nil { - return err - } - return d.clearPhysicalBreakpoints(amend.ID) + enabledBefore := original.Enabled + err := copyLogicalBreakpointInfo(original, amend) + if err != nil { + return err } + original.Enabled = !amend.Disabled - if !amend.Disabled && !original.Enabled { - original.Enabled = true - copyLogicalBreakpointInfo(original, amend) - return d.createPhysicalBreakpoints(original) + 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) } - - err := copyLogicalBreakpointInfo(original, amend) if err != nil { return err } - for _, bp := range d.findBreakpoint(amend.ID) { - bp.UserBreaklet().Cond = original.Cond + + 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 { - panic("multiple targets not implemented") + if _, err := d.target.Valid(); err != nil { + 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.clearPhysicalBreakpoints(requestedBp.ID) + err := d.target.DisableBreakpoint(lbp) 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) diff --git a/service/rpc1/server.go b/service/rpc1/server.go index 57d46cacaa..a7de538407 100644 --- a/service/rpc1/server.go +++ b/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 } diff --git a/service/rpc2/client.go b/service/rpc2/client.go index d31f1cee04..45b08cb84d 100644 --- a/service/rpc2/client.go +++ b/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 } diff --git a/service/rpc2/server.go b/service/rpc2/server.go index 495fdd2939..a798e3b486 100644 --- a/service/rpc2/server.go +++ b/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 } diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index 39cb6683e3..e30b61f241 100644 --- a/service/test/integration2_test.go +++ b/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) + } + }) +}