diff --git a/pkg/proc/breakpoints.go b/pkg/proc/breakpoints.go index 3e01bea5..b331dbf0 100644 --- a/pkg/proc/breakpoints.go +++ b/pkg/proc/breakpoints.go @@ -237,21 +237,13 @@ func NewBreakpointMap() BreakpointMap { } } -// ResetBreakpointIDCounter resets the breakpoint ID counter of bpmap. -func (bpmap *BreakpointMap) ResetBreakpointIDCounter() { - bpmap.breakpointIDCounter = 0 -} - -// WriteBreakpointFn is a type that represents a function to be used for -// writting breakpoings into the target. -type WriteBreakpointFn func(addr uint64) (file string, line int, fn *Function, originalData []byte, err error) - -type clearBreakpointFn func(*Breakpoint) error - -// Set creates a breakpoint at addr calling writeBreakpoint. Do not call this -// function, call proc.Process.SetBreakpoint instead, this function exists -// to implement proc.Process.SetBreakpoint. -func (bpmap *BreakpointMap) Set(addr uint64, kind BreakpointKind, cond ast.Expr, writeBreakpoint WriteBreakpointFn) (*Breakpoint, error) { +// SetBreakpoint sets a breakpoint at addr, and stores it in the process wide +// break point table. +func (t *Target) SetBreakpoint(addr uint64, kind BreakpointKind, cond ast.Expr) (*Breakpoint, error) { + if valid, err := t.Valid(); !valid { + return nil, err + } + bpmap := t.Breakpoints() if bp, ok := bpmap.M[addr]; ok { // We can overlap one internal breakpoint with one user breakpoint, we // need to support this otherwise a conditional breakpoint can mask a @@ -268,7 +260,7 @@ func (bpmap *BreakpointMap) Set(addr uint64, kind BreakpointKind, cond ast.Expr, return bp, nil } - f, l, fn, originalData, err := writeBreakpoint(addr) + f, l, fn, originalData, err := t.proc.WriteBreakpoint(addr) if err != nil { return nil, err } @@ -303,9 +295,10 @@ func (bpmap *BreakpointMap) Set(addr uint64, kind BreakpointKind, cond ast.Expr, return newBreakpoint, nil } -// SetWithID creates a breakpoint at addr, with the specified logical ID. -func (bpmap *BreakpointMap) SetWithID(id int, addr uint64, writeBreakpoint WriteBreakpointFn) (*Breakpoint, error) { - bp, err := bpmap.Set(addr, UserBreakpoint, nil, writeBreakpoint) +// setBreakpointWithID creates a breakpoint at addr, with the specified logical ID. +func (t *Target) setBreakpointWithID(id int, addr uint64) (*Breakpoint, error) { + bpmap := t.Breakpoints() + bp, err := t.SetBreakpoint(addr, UserBreakpoint, nil) if err == nil { bp.LogicalID = id bpmap.breakpointIDCounter-- @@ -313,9 +306,12 @@ func (bpmap *BreakpointMap) SetWithID(id int, addr uint64, writeBreakpoint Write return bp, err } -// Clear clears the breakpoint at addr. -// Do not call this function call proc.Process.ClearBreakpoint instead. -func (bpmap *BreakpointMap) Clear(addr uint64, clearBreakpoint clearBreakpointFn) (*Breakpoint, error) { +// ClearBreakpoint clears the breakpoint at addr. +func (t *Target) ClearBreakpoint(addr uint64) (*Breakpoint, error) { + if valid, err := t.Valid(); !valid { + return nil, err + } + bpmap := t.Breakpoints() bp, ok := bpmap.M[addr] if !ok { return nil, NoBreakpointError{Addr: addr} @@ -327,7 +323,7 @@ func (bpmap *BreakpointMap) Clear(addr uint64, clearBreakpoint clearBreakpointFn return bp, nil } - if err := clearBreakpoint(bp); err != nil { + if err := t.proc.EraseBreakpoint(bp); err != nil { return nil, err } @@ -338,9 +334,9 @@ func (bpmap *BreakpointMap) Clear(addr uint64, clearBreakpoint clearBreakpointFn // ClearInternalBreakpoints removes all internal breakpoints from the map, // calling clearBreakpoint on each one. -// Do not call this function, call proc.Process.ClearInternalBreakpoints -// instead, this function is used to implement that. -func (bpmap *BreakpointMap) ClearInternalBreakpoints(clearBreakpoint clearBreakpointFn) error { +func (t *Target) ClearInternalBreakpoints() error { + bpmap := t.Breakpoints() + threads := t.ThreadList() for addr, bp := range bpmap.M { bp.Kind = bp.Kind & UserBreakpoint bp.internalCond = nil @@ -348,9 +344,14 @@ func (bpmap *BreakpointMap) ClearInternalBreakpoints(clearBreakpoint clearBreakp if bp.Kind != 0 { continue } - if err := clearBreakpoint(bp); err != nil { + if err := t.proc.EraseBreakpoint(bp); err != nil { return err } + for _, thread := range threads { + if thread.Breakpoint().Breakpoint == bp { + thread.Breakpoint().Clear() + } + } delete(bpmap.M, addr) } return nil diff --git a/pkg/proc/core/core.go b/pkg/proc/core/core.go index 41c7df79..984ffdf3 100644 --- a/pkg/proc/core/core.go +++ b/pkg/proc/core/core.go @@ -3,7 +3,6 @@ package core import ( "errors" "fmt" - "go/ast" "io" "github.com/go-delve/delve/pkg/proc" @@ -216,7 +215,6 @@ func OpenCore(corePath, exePath string, debugInfoDirs []string) (*proc.Target, e return proc.NewTarget(p, proc.NewTargetConfig{ Path: exePath, DebugInfoDirs: debugInfoDirs, - WriteBreakpoint: p.writeBreakpoint, DisableAsyncPreempt: false, StopReason: proc.StopAttached}) } @@ -231,9 +229,9 @@ func (p *process) EntryPoint() (uint64, error) { return p.entryPoint, nil } -// writeBreakpoint is a noop function since you +// WriteBreakpoint is a noop function since you // cannot write breakpoints into core files. -func (p *process) writeBreakpoint(addr uint64) (file string, line int, fn *proc.Function, originalData []byte, err error) { +func (p *process) WriteBreakpoint(addr uint64) (file string, line int, fn *proc.Function, originalData []byte, err error) { return "", 0, nil, nil, errors.New("cannot write a breakpoint to a core file") } @@ -365,10 +363,10 @@ func (p *process) Breakpoints() *proc.BreakpointMap { return &p.breakpoints } -// ClearBreakpoint will always return an error as you cannot set or clear +// EraseBreakpoint will always return an error as you cannot set or clear // breakpoints on core files. -func (p *process) ClearBreakpoint(addr uint64) (*proc.Breakpoint, error) { - return nil, proc.NoBreakpointError{Addr: addr} +func (p *process) EraseBreakpoint(bp *proc.Breakpoint) error { + return proc.NoBreakpointError{Addr: bp.Addr} } // ClearInternalBreakpoints will always return nil and have no @@ -430,11 +428,6 @@ func (p *process) Pid() int { func (p *process) ResumeNotify(chan<- struct{}) { } -// SetBreakpoint will always return an error for core files as you cannot write memory or control execution. -func (p *process) SetBreakpoint(addr uint64, kind proc.BreakpointKind, cond ast.Expr) (*proc.Breakpoint, error) { - return nil, ErrWriteCore -} - // ThreadList will return a list of all threads currently in the process. func (p *process) ThreadList() []proc.Thread { r := make([]proc.Thread, 0, len(p.Threads)) diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index c88ed334..dbfb82c8 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -67,7 +67,6 @@ import ( "encoding/binary" "errors" "fmt" - "go/ast" "net" "os" "os/exec" @@ -555,7 +554,6 @@ func (p *gdbProcess) initialize(path string, debugInfoDirs []string, stopReason tgt, err := proc.NewTarget(p, proc.NewTargetConfig{ Path: path, DebugInfoDirs: debugInfoDirs, - WriteBreakpoint: p.writeBreakpoint, DisableAsyncPreempt: runtime.GOOS == "darwin", StopReason: stopReason}) if err != nil { @@ -1083,7 +1081,7 @@ func (p *gdbProcess) FindBreakpoint(pc uint64) (*proc.Breakpoint, bool) { return nil, false } -func (p *gdbProcess) writeBreakpoint(addr uint64) (string, int, *proc.Function, []byte, error) { +func (p *gdbProcess) WriteBreakpoint(addr uint64) (string, int, *proc.Function, []byte, error) { f, l, fn := p.bi.PCToLine(uint64(addr)) if err := p.conn.setBreakpoint(addr); err != nil { @@ -1093,37 +1091,8 @@ func (p *gdbProcess) writeBreakpoint(addr uint64) (string, int, *proc.Function, return f, l, fn, nil, nil } -// SetBreakpoint creates a new breakpoint. -func (p *gdbProcess) SetBreakpoint(addr uint64, kind proc.BreakpointKind, cond ast.Expr) (*proc.Breakpoint, error) { - if p.exited { - return nil, &proc.ErrProcessExited{Pid: p.conn.pid} - } - return p.breakpoints.Set(addr, kind, cond, p.writeBreakpoint) -} - -// ClearBreakpoint clears a breakpoint at the given address. -func (p *gdbProcess) ClearBreakpoint(addr uint64) (*proc.Breakpoint, error) { - if p.exited { - return nil, &proc.ErrProcessExited{Pid: p.conn.pid} - } - return p.breakpoints.Clear(addr, func(bp *proc.Breakpoint) error { - return p.conn.clearBreakpoint(bp.Addr) - }) -} - -// ClearInternalBreakpoints clear all internal use breakpoints like those set by 'next'. -func (p *gdbProcess) ClearInternalBreakpoints() error { - return p.breakpoints.ClearInternalBreakpoints(func(bp *proc.Breakpoint) error { - if err := p.conn.clearBreakpoint(bp.Addr); err != nil { - return err - } - for _, thread := range p.threads { - if thread.CurrentBreakpoint.Breakpoint == bp { - thread.clearBreakpointState() - } - } - return nil - }) +func (p *gdbProcess) EraseBreakpoint(bp *proc.Breakpoint) error { + return p.conn.clearBreakpoint(bp.Addr) } type threadUpdater struct { diff --git a/pkg/proc/gdbserial/rr_test.go b/pkg/proc/gdbserial/rr_test.go index e949b67a..eed5ec54 100644 --- a/pkg/proc/gdbserial/rr_test.go +++ b/pkg/proc/gdbserial/rr_test.go @@ -49,7 +49,7 @@ func assertNoError(err error, t testing.TB, s string) { } } -func setFunctionBreakpoint(p proc.Process, t *testing.T, fname string) *proc.Breakpoint { +func setFunctionBreakpoint(p *proc.Target, t *testing.T, fname string) *proc.Breakpoint { _, f, l, _ := runtime.Caller(1) f = filepath.Base(f) @@ -117,7 +117,7 @@ func TestRestartDuringStop(t *testing.T) { }) } -func setFileBreakpoint(p proc.Process, t *testing.T, fixture protest.Fixture, lineno int) *proc.Breakpoint { +func setFileBreakpoint(p *proc.Target, t *testing.T, fixture protest.Fixture, lineno int) *proc.Breakpoint { _, f, l, _ := runtime.Caller(1) f = filepath.Base(f) diff --git a/pkg/proc/interface.go b/pkg/proc/interface.go index d5f7a729..cd1ad0c8 100644 --- a/pkg/proc/interface.go +++ b/pkg/proc/interface.go @@ -1,9 +1,5 @@ package proc -import ( - "go/ast" -) - // Process represents the target of the debugger. This // target could be a system process, core file, etc. // @@ -14,8 +10,9 @@ import ( type Process interface { Info ProcessManipulation - BreakpointManipulation RecordingManipulation + + Breakpoints() *BreakpointMap } // ProcessInternal holds a set of methods that are not meant to be called by @@ -32,6 +29,9 @@ type ProcessInternal interface { Restart(pos string) error Detach(bool) error ContinueOnce() (trapthread Thread, stopReason StopReason, err error) + + WriteBreakpoint(addr uint64) (file string, line int, fn *Function, originalData []byte, err error) + EraseBreakpoint(*Breakpoint) error } // RecordingManipulation is an interface for manipulating process recordings. @@ -101,11 +101,3 @@ type ProcessManipulation interface { // after a call to RequestManualStop. CheckAndClearManualStopRequest() bool } - -// BreakpointManipulation is an interface for managing breakpoints. -type BreakpointManipulation interface { - Breakpoints() *BreakpointMap - SetBreakpoint(addr uint64, kind BreakpointKind, cond ast.Expr) (*Breakpoint, error) - ClearBreakpoint(addr uint64) (*Breakpoint, error) - ClearInternalBreakpoints() error -} diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index 5381c506..9e4aeb61 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -1,7 +1,6 @@ package native import ( - "go/ast" "os" "runtime" "sync" @@ -115,15 +114,6 @@ func (dbp *nativeProcess) Detach(kill bool) (err error) { dbp.bi.Close() return nil } - // Clean up any breakpoints we've set. - for _, bp := range dbp.breakpoints.M { - if bp != nil { - _, err := dbp.ClearBreakpoint(bp.Addr) - if err != nil { - return err - } - } - } dbp.execPtraceFunc(func() { err = dbp.detach(kill) if err != nil { @@ -215,7 +205,7 @@ func (dbp *nativeProcess) CheckAndClearManualStopRequest() bool { return msr } -func (dbp *nativeProcess) writeBreakpoint(addr uint64) (string, int, *proc.Function, []byte, error) { +func (dbp *nativeProcess) WriteBreakpoint(addr uint64) (string, int, *proc.Function, []byte, error) { f, l, fn := dbp.bi.PCToLine(uint64(addr)) originalData := make([]byte, dbp.bi.Arch.BreakpointSize()) @@ -230,18 +220,8 @@ func (dbp *nativeProcess) writeBreakpoint(addr uint64) (string, int, *proc.Funct return f, l, fn, originalData, nil } -// SetBreakpoint sets a breakpoint at addr, and stores it in the process wide -// break point table. -func (dbp *nativeProcess) SetBreakpoint(addr uint64, kind proc.BreakpointKind, cond ast.Expr) (*proc.Breakpoint, error) { - return dbp.breakpoints.Set(addr, kind, cond, dbp.writeBreakpoint) -} - -// ClearBreakpoint clears the breakpoint at addr. -func (dbp *nativeProcess) ClearBreakpoint(addr uint64) (*proc.Breakpoint, error) { - if dbp.exited { - return nil, &proc.ErrProcessExited{Pid: dbp.Pid()} - } - return dbp.breakpoints.Clear(addr, dbp.currentThread.ClearBreakpoint) +func (dbp *nativeProcess) EraseBreakpoint(bp *proc.Breakpoint) error { + return dbp.currentThread.ClearBreakpoint(bp) } // ContinueOnce will continue the target until it stops. @@ -305,27 +285,10 @@ func (dbp *nativeProcess) initialize(path string, debugInfoDirs []string) (*proc return proc.NewTarget(dbp, proc.NewTargetConfig{ Path: path, DebugInfoDirs: debugInfoDirs, - WriteBreakpoint: dbp.writeBreakpoint, DisableAsyncPreempt: runtime.GOOS == "windows" || runtime.GOOS == "freebsd", StopReason: stopReason}) } -// ClearInternalBreakpoints will clear all non-user set breakpoints. These -// breakpoints are set for internal operations such as 'next'. -func (dbp *nativeProcess) ClearInternalBreakpoints() error { - return dbp.breakpoints.ClearInternalBreakpoints(func(bp *proc.Breakpoint) error { - if err := dbp.currentThread.ClearBreakpoint(bp); err != nil { - return err - } - for _, thread := range dbp.threads { - if thread.CurrentBreakpoint.Breakpoint == bp { - thread.CurrentBreakpoint.Clear() - } - } - return nil - }) -} - func (dbp *nativeProcess) handlePtraceFuncs() { // We must ensure here that we are running on the same thread during // while invoking the ptrace(2) syscall. This is due to the fact that ptrace(2) expects diff --git a/pkg/proc/target.go b/pkg/proc/target.go index 5aedcf5e..9711275e 100644 --- a/pkg/proc/target.go +++ b/pkg/proc/target.go @@ -80,11 +80,10 @@ const ( // NewTargetConfig contains the configuration for a new Target object, type NewTargetConfig struct { - Path string // path of the main executable - DebugInfoDirs []string // Directories to search for split debug info - WriteBreakpoint WriteBreakpointFn // Function to write a breakpoint to the target process - DisableAsyncPreempt bool // Go 1.14 asynchronous preemption should be disabled - StopReason StopReason // Initial stop reason + Path string // path of the main executable + DebugInfoDirs []string // Directories to search for split debug info + DisableAsyncPreempt bool // Go 1.14 asynchronous preemption should be disabled + StopReason StopReason // Initial stop reason } // DisableAsyncPreemptEnv returns a process environment (like os.Environ) @@ -128,8 +127,8 @@ func NewTarget(p Process, cfg NewTargetConfig) (*Target, error) { g, _ := GetG(p.CurrentThread()) t.selectedGoroutine = g - createUnrecoveredPanicBreakpoint(p, cfg.WriteBreakpoint) - createFatalThrowBreakpoint(p, cfg.WriteBreakpoint) + t.createUnrecoveredPanicBreakpoint() + t.createFatalThrowBreakpoint() t.gcache.init(p.BinInfo()) @@ -216,8 +215,18 @@ func (p *Target) SwitchThread(tid int) error { // we were previously debugging. // If kill is true then the process will be killed when we detach. func (t *Target) Detach(kill bool) error { - if !kill && t.asyncPreemptChanged { - setAsyncPreemptOff(t, t.asyncPreemptOff) + if !kill { + if t.asyncPreemptChanged { + setAsyncPreemptOff(t, t.asyncPreemptOff) + } + for _, bp := range t.Breakpoints().M { + if bp != nil { + _, err := t.ClearBreakpoint(bp.Addr) + if err != nil { + return err + } + } + } } t.StopReason = StopUnknown return t.proc.Detach(kill) @@ -255,13 +264,13 @@ func setAsyncPreemptOff(p *Target, v int64) { } // createUnrecoveredPanicBreakpoint creates the unrecoverable-panic breakpoint. -func createUnrecoveredPanicBreakpoint(p Process, writeBreakpoint WriteBreakpointFn) { - panicpcs, err := FindFunctionLocation(p, "runtime.startpanic", 0) +func (t *Target) createUnrecoveredPanicBreakpoint() { + panicpcs, err := FindFunctionLocation(t.Process, "runtime.startpanic", 0) if _, isFnNotFound := err.(*ErrFunctionNotFound); isFnNotFound { - panicpcs, err = FindFunctionLocation(p, "runtime.fatalpanic", 0) + panicpcs, err = FindFunctionLocation(t.Process, "runtime.fatalpanic", 0) } if err == nil { - bp, err := p.Breakpoints().SetWithID(unrecoveredPanicID, panicpcs[0], writeBreakpoint) + bp, err := t.setBreakpointWithID(unrecoveredPanicID, panicpcs[0]) if err == nil { bp.Name = UnrecoveredPanic bp.Variables = []string{"runtime.curg._panic.arg"} @@ -270,10 +279,10 @@ func createUnrecoveredPanicBreakpoint(p Process, writeBreakpoint WriteBreakpoint } // createFatalThrowBreakpoint creates the a breakpoint as runtime.fatalthrow. -func createFatalThrowBreakpoint(p Process, writeBreakpoint WriteBreakpointFn) { - fatalpcs, err := FindFunctionLocation(p, "runtime.fatalthrow", 0) +func (t *Target) createFatalThrowBreakpoint() { + fatalpcs, err := FindFunctionLocation(t.Process, "runtime.fatalthrow", 0) if err == nil { - bp, err := p.Breakpoints().SetWithID(fatalThrowID, fatalpcs[0], writeBreakpoint) + bp, err := t.setBreakpointWithID(fatalThrowID, fatalpcs[0]) if err == nil { bp.Name = FatalThrow } diff --git a/pkg/proc/target_exec.go b/pkg/proc/target_exec.go index 792a66ca..8ac620b5 100644 --- a/pkg/proc/target_exec.go +++ b/pkg/proc/target_exec.go @@ -625,7 +625,7 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error { return nil } -func setStepIntoBreakpoints(dbp Process, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr) error { +func setStepIntoBreakpoints(dbp *Target, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr) error { for _, instr := range text { if instr.Loc.File != topframe.Current.File || instr.Loc.Line != topframe.Current.Line || !instr.IsCall() { continue @@ -645,7 +645,7 @@ func setStepIntoBreakpoints(dbp Process, text []AsmInstruction, topframe Stackfr return nil } -func setStepIntoBreakpointsReverse(dbp Process, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr) error { +func setStepIntoBreakpointsReverse(dbp *Target, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr) error { // Set a breakpoint after every CALL instruction for i, instr := range text { if instr.Loc.File != topframe.Current.File || !instr.IsCall() || instr.DestLoc == nil || instr.DestLoc.Fn == nil { @@ -708,7 +708,7 @@ func removePCsBetween(pcs []uint64, start, end uint64) []uint64 { return out } -func setStepIntoBreakpoint(dbp Process, text []AsmInstruction, cond ast.Expr) error { +func setStepIntoBreakpoint(dbp *Target, text []AsmInstruction, cond ast.Expr) error { if len(text) <= 0 { return nil } @@ -873,7 +873,7 @@ func skipAutogeneratedWrappersOut(g *G, thread Thread, startTopframe, startRetfr // setDeferBreakpoint is a helper function used by next and StepOut to set a // breakpoint on the first deferred function. -func setDeferBreakpoint(p Process, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr, stepInto bool) (uint64, error) { +func setDeferBreakpoint(p *Target, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr, stepInto bool) (uint64, error) { // Set breakpoint on the most recently deferred function (if any) var deferpc uint64 if topframe.TopmostDefer != nil && topframe.TopmostDefer.DeferredPC != 0 { diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 2bdcb8ca..c83eedad 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -581,7 +581,7 @@ func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoin // createLogicalBreakpoint creates one physical breakpoint for each address // in addrs and associates all of them with the same logical breakpoint. -func createLogicalBreakpoint(p proc.Process, addrs []uint64, requestedBp *api.Breakpoint) (*api.Breakpoint, error) { +func createLogicalBreakpoint(p *proc.Target, addrs []uint64, requestedBp *api.Breakpoint) (*api.Breakpoint, error) { bps := make([]*proc.Breakpoint, len(addrs)) var err error for i := range addrs {