diff --git a/_fixtures/nextcond.go b/_fixtures/nextcond.go new file mode 100644 index 0000000000000000000000000000000000000000..e1986c707b6ec9e36435b5cc2aedae528b8b8ee7 --- /dev/null +++ b/_fixtures/nextcond.go @@ -0,0 +1,12 @@ +package main + +var n = 10 + +func f1(x int) { +} + +func main() { + f1(n) + f1(n) + f1(n) +} diff --git a/pkg/proc/breakpoints.go b/pkg/proc/breakpoints.go index 202f1e53d114351a3a3469cf685b57116fbde69e..3f2c9fd4a664d726bed894c224fb3dfe52717004 100644 --- a/pkg/proc/breakpoints.go +++ b/pkg/proc/breakpoints.go @@ -17,11 +17,17 @@ type Breakpoint struct { File string Line int - Addr uint64 // Address breakpoint is set for. - OriginalData []byte // If software breakpoint, the data we replace with breakpoint instruction. - Name string // User defined name of the breakpoint - ID int // Monotonically increasing ID. - Kind BreakpointKind // Whether this is an internal breakpoint (for next'ing or stepping). + Addr uint64 // Address breakpoint is set for. + OriginalData []byte // If software breakpoint, the data we replace with breakpoint instruction. + Name string // User defined name of the breakpoint + ID int // Monotonically increasing ID. + + // Kind describes whether this is an internal breakpoint (for next'ing or + // stepping). + // A single breakpoint can be both a UserBreakpoint and some kind of + // internal breakpoint, but it can not be two different kinds of internal + // breakpoint. + Kind BreakpointKind // Breakpoint information Tracepoint bool // Tracepoint flag @@ -45,15 +51,17 @@ type Breakpoint struct { DeferReturns []uint64 // Cond: if not nil the breakpoint will be triggered only if evaluating Cond returns true Cond ast.Expr + // internalCond is the same as Cond but used for the condition of internal breakpoints + internalCond ast.Expr } // Breakpoint Kind determines the behavior of delve when the // breakpoint is reached. -type BreakpointKind int +type BreakpointKind uint16 const ( // UserBreakpoint is a user set breakpoint - UserBreakpoint BreakpointKind = iota + UserBreakpoint BreakpointKind = (1 << iota) // NextBreakpoint is a breakpoint set by Next, Continue // will stop on it and delete it NextBreakpoint @@ -95,11 +103,15 @@ func (iae InvalidAddressError) Error() string { } // CheckCondition evaluates bp's condition on thread. -func (bp *Breakpoint) CheckCondition(thread Thread) (bool, error) { - if bp.Cond == nil { - return true, nil +func (bp *Breakpoint) CheckCondition(thread Thread) BreakpointState { + bpstate := BreakpointState{Breakpoint: bp, Active: false, Internal: false, CondError: nil} + if bp.Cond == nil && bp.internalCond == nil { + bpstate.Active = true + bpstate.Internal = bp.Kind != UserBreakpoint + return bpstate } - if bp.Kind == NextDeferBreakpoint { + nextDeferOk := true + if bp.Kind&NextDeferBreakpoint != 0 { frames, err := ThreadStacktrace(thread, 2) if err == nil { ispanic := len(frames) >= 3 && frames[2].Current.Fn != nil && frames[2].Current.Fn.Name == "runtime.gopanic" @@ -112,15 +124,43 @@ func (bp *Breakpoint) CheckCondition(thread Thread) (bool, error) { } } } - if !ispanic && !isdeferreturn { - return false, nil - } + nextDeferOk = ispanic || isdeferreturn + } + } + if bp.Kind != UserBreakpoint { + // Check internalCondition if this is also an internal breakpoint + bpstate.Active, bpstate.CondError = evalBreakpointCondition(thread, bp.internalCond) + bpstate.Active = bpstate.Active && nextDeferOk + if bpstate.Active || bpstate.CondError != nil { + bpstate.Internal = true + return bpstate } } - return evalBreakpointCondition(thread, bp.Cond) + if bp.Kind&UserBreakpoint != 0 { + // Check normal condition if this is also a user breakpoint + bpstate.Active, bpstate.CondError = evalBreakpointCondition(thread, bp.Cond) + } + return bpstate +} + +// IsInternal returns true if bp is an internal breakpoint. +// User-set breakpoints can overlap with internal breakpoints, in that case +// both IsUser and IsInternal will be true. +func (bp *Breakpoint) IsInternal() bool { + return bp.Kind != UserBreakpoint +} + +// IsUser returns true if bp is a user-set breakpoint. +// User-set breakpoints can overlap with internal breakpoints, in that case +// both IsUser and IsInternal will be true. +func (bp *Breakpoint) IsUser() bool { + return bp.Kind&UserBreakpoint != 0 } func evalBreakpointCondition(thread Thread, cond ast.Expr) (bool, error) { + if cond == nil { + return true, nil + } scope, err := GoroutineScope(thread) if err != nil { return true, err @@ -138,11 +178,6 @@ func evalBreakpointCondition(thread Thread, cond ast.Expr) (bool, error) { return constant.BoolVal(v.Value), nil } -// Internal returns true for breakpoints not set directly by the user. -func (bp *Breakpoint) Internal() bool { - return bp.Kind != UserBreakpoint -} - // NoBreakpointError is returned when trying to // clear a breakpoint that does not exist. type NoBreakpointError struct { @@ -153,6 +188,7 @@ func (nbp NoBreakpointError) Error() string { return fmt.Sprintf("no breakpoint at %#v", nbp.Addr) } +// BreakpointMap represents an (address, breakpoint) map. type BreakpointMap struct { M map[uint64]*Breakpoint @@ -160,12 +196,14 @@ type BreakpointMap struct { internalBreakpointIDCounter int } +// NewBreakpointMap creates a new BreakpointMap. func NewBreakpointMap() BreakpointMap { return BreakpointMap{ M: make(map[uint64]*Breakpoint), } } +// ResetBreakpointIDCounter resets the breakpoint ID counter of bpmap. func (bpmap *BreakpointMap) ResetBreakpointIDCounter() { bpmap.breakpointIDCounter = 0 } @@ -178,7 +216,19 @@ type clearBreakpointFn func(*Breakpoint) error // to implement proc.Process.SetBreakpoint. func (bpmap *BreakpointMap) Set(addr uint64, kind BreakpointKind, cond ast.Expr, writeBreakpoint writeBreakpointFn) (*Breakpoint, error) { if bp, ok := bpmap.M[addr]; ok { - return bp, BreakpointExistsError{bp.File, bp.Line, bp.Addr} + // We can overlap one internal breakpoint with one user breakpoint, we + // need to support this otherwise a conditional breakpoint can mask a + // breakpoint set by next or step. + if (kind != UserBreakpoint && bp.Kind != UserBreakpoint) || (kind == UserBreakpoint && bp.Kind&UserBreakpoint != 0) { + return bp, BreakpointExistsError{bp.File, bp.Line, bp.Addr} + } + bp.Kind |= kind + if kind != UserBreakpoint { + bp.internalCond = cond + } else { + bp.Cond = cond + } + return bp, nil } f, l, fn, originalData, err := writeBreakpoint(addr) @@ -192,7 +242,6 @@ func (bpmap *BreakpointMap) Set(addr uint64, kind BreakpointKind, cond ast.Expr, Line: l, Addr: addr, Kind: kind, - Cond: cond, OriginalData: originalData, HitCount: map[int]uint64{}, } @@ -200,9 +249,11 @@ func (bpmap *BreakpointMap) Set(addr uint64, kind BreakpointKind, cond ast.Expr, if kind != UserBreakpoint { bpmap.internalBreakpointIDCounter++ newBreakpoint.ID = bpmap.internalBreakpointIDCounter + newBreakpoint.internalCond = cond } else { bpmap.breakpointIDCounter++ newBreakpoint.ID = bpmap.breakpointIDCounter + newBreakpoint.Cond = cond } bpmap.M[addr] = newBreakpoint @@ -228,6 +279,12 @@ func (bpmap *BreakpointMap) Clear(addr uint64, clearBreakpoint clearBreakpointFn return nil, NoBreakpointError{Addr: addr} } + bp.Kind &= ^UserBreakpoint + bp.Cond = nil + if bp.Kind != 0 { + return bp, nil + } + if err := clearBreakpoint(bp); err != nil { return nil, err } @@ -243,7 +300,9 @@ func (bpmap *BreakpointMap) Clear(addr uint64, clearBreakpoint clearBreakpointFn // instead, this function is used to implement that. func (bpmap *BreakpointMap) ClearInternalBreakpoints(clearBreakpoint clearBreakpointFn) error { for addr, bp := range bpmap.M { - if !bp.Internal() { + bp.Kind = bp.Kind & UserBreakpoint + bp.internalCond = nil + if bp.Kind != 0 { continue } if err := clearBreakpoint(bp); err != nil { @@ -253,3 +312,45 @@ func (bpmap *BreakpointMap) ClearInternalBreakpoints(clearBreakpoint clearBreakp } return nil } + +// HasInternalBreakpoints returns true if bpmap has at least one internal +// breakpoint set. +func (bpmap *BreakpointMap) HasInternalBreakpoints() bool { + for _, bp := range bpmap.M { + if bp.Kind != UserBreakpoint { + return true + } + } + return false +} + +// BreakpointState describes the state of a breakpoint in a thread. +type BreakpointState struct { + *Breakpoint + // Active is true if the breakpoint condition was met. + Active bool + // Internal is true if the breakpoint was matched as an internal + // breakpoint. + Internal bool + // CondError contains any error encountered while evaluating the + // breakpoint's condition. + CondError error +} + +func (bpstate *BreakpointState) Clear() { + bpstate.Breakpoint = nil + bpstate.Active = false + bpstate.Internal = false + bpstate.CondError = nil +} + +func (bpstate *BreakpointState) String() string { + s := bpstate.Breakpoint.String() + if bpstate.Active { + s += " active" + } + if bpstate.Internal { + s += " internal" + } + return s +} diff --git a/pkg/proc/core/core.go b/pkg/proc/core/core.go index 0c92dcb907393bec423cca68d855e6e5ed37590e..2222efda9be80e4d418e8a3cade8729c5ac5ba00 100644 --- a/pkg/proc/core/core.go +++ b/pkg/proc/core/core.go @@ -222,8 +222,8 @@ func (t *Thread) Location() (*proc.Location, error) { return &proc.Location{PC: t.th.Reg.Rip, File: f, Line: l, Fn: fn}, nil } -func (t *Thread) Breakpoint() (*proc.Breakpoint, bool, error) { - return nil, false, nil +func (t *Thread) Breakpoint() proc.BreakpointState { + return proc.BreakpointState{} } func (t *Thread) ThreadID() int { diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index 0eddd4dd18a253ff05b9fab201ddb575c6c6ba52..ec6f0ef11a73ff719646d16b3bcf800ee2b34045 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -127,14 +127,12 @@ type Process struct { // Thread is a thread. type Thread struct { - ID int - strID string - regs gdbRegisters - CurrentBreakpoint *proc.Breakpoint - BreakpointConditionMet bool - BreakpointConditionError error - p *Process - setbp bool // thread was stopped because of a breakpoint + ID int + strID string + regs gdbRegisters + CurrentBreakpoint proc.BreakpointState + p *Process + setbp bool // thread was stopped because of a breakpoint } // gdbRegisters represents the current value of the registers of a thread. @@ -557,7 +555,7 @@ func (p *Process) ContinueOnce() (proc.Thread, error) { if p.conn.direction == proc.Forward { // step threads stopped at any breakpoint over their breakpoint for _, thread := range p.threads { - if thread.CurrentBreakpoint != nil { + if thread.CurrentBreakpoint.Breakpoint != nil { if err := thread.stepInstruction(&threadUpdater{p: p}); err != nil { return nil, err } @@ -914,10 +912,8 @@ func (p *Process) Direction(dir proc.Direction) error { if p.conn.direction == dir { return nil } - for _, bp := range p.Breakpoints().M { - if bp.Internal() { - return ErrDirChange - } + if p.Breakpoints().HasInternalBreakpoints() { + return ErrDirChange } p.conn.direction = dir return nil @@ -971,8 +967,8 @@ func (p *Process) ClearInternalBreakpoints() error { return err } for _, thread := range p.threads { - if thread.CurrentBreakpoint == bp { - thread.CurrentBreakpoint = nil + if thread.CurrentBreakpoint.Breakpoint == bp { + thread.clearBreakpointState() } } return nil @@ -1098,7 +1094,7 @@ func (p *Process) setCurrentBreakpoints() error { } if !p.threadStopInfo { for _, th := range p.threads { - if th.CurrentBreakpoint == nil { + if th.CurrentBreakpoint.Breakpoint == nil { err := th.SetCurrentBreakpoint() if err != nil { return err @@ -1131,8 +1127,8 @@ func (t *Thread) Location() (*proc.Location, error) { return &proc.Location{PC: pc, File: f, Line: l, Fn: fn}, nil } -func (t *Thread) Breakpoint() (breakpoint *proc.Breakpoint, active bool, condErr error) { - return t.CurrentBreakpoint, (t.CurrentBreakpoint != nil && t.BreakpointConditionMet), t.BreakpointConditionError +func (t *Thread) Breakpoint() proc.BreakpointState { + return t.CurrentBreakpoint } func (t *Thread) ThreadID() int { @@ -1431,13 +1427,11 @@ func (t *Thread) reloadGAlloc() error { func (t *Thread) clearBreakpointState() { t.setbp = false - t.CurrentBreakpoint = nil - t.BreakpointConditionMet = false - t.BreakpointConditionError = nil + t.CurrentBreakpoint.Clear() } func (thread *Thread) SetCurrentBreakpoint() error { - thread.CurrentBreakpoint = nil + thread.clearBreakpointState() regs, err := thread.Registers(false) if err != nil { return err @@ -1449,9 +1443,8 @@ func (thread *Thread) SetCurrentBreakpoint() error { return err } } - thread.CurrentBreakpoint = bp - thread.BreakpointConditionMet, thread.BreakpointConditionError = bp.CheckCondition(thread) - if thread.CurrentBreakpoint != nil && thread.BreakpointConditionMet { + thread.CurrentBreakpoint = bp.CheckCondition(thread) + if thread.CurrentBreakpoint.Breakpoint != nil && thread.CurrentBreakpoint.Active { if g, err := proc.GetG(thread); err == nil { thread.CurrentBreakpoint.HitCount[g.ID]++ } diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index 3118aa7973e8143ca18921ef8851a20c2fb4846f..d8b2c44bc5a4ace70c02528c9d2a17b1552dbc13 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -210,8 +210,7 @@ func (dbp *Process) writeBreakpoint(addr uint64) (string, int, *proc.Function, [ } // SetBreakpoint sets a breakpoint at addr, and stores it in the process wide -// break point table. Setting a break point must be thread specific due to -// ptrace actions needing the thread to be in a signal-delivery-stop. +// break point table. func (dbp *Process) SetBreakpoint(addr uint64, kind proc.BreakpointKind, cond ast.Expr) (*proc.Breakpoint, error) { return dbp.breakpoints.Set(addr, kind, cond, dbp.writeBreakpoint) } @@ -235,7 +234,7 @@ func (dbp *Process) ContinueOnce() (proc.Thread, error) { dbp.allGCache = nil for _, th := range dbp.threads { - th.clearBreakpointState() + th.CurrentBreakpoint.Clear() } if dbp.resumeChan != nil { @@ -276,7 +275,7 @@ func (dbp *Process) StepInstruction() (err error) { if dbp.exited { return &proc.ProcessExitedError{Pid: dbp.Pid()} } - thread.clearBreakpointState() + thread.CurrentBreakpoint.Clear() err = thread.StepInstruction() if err != nil { return err @@ -386,8 +385,8 @@ func (dbp *Process) ClearInternalBreakpoints() error { return err } for _, thread := range dbp.threads { - if thread.CurrentBreakpoint == bp { - thread.CurrentBreakpoint = nil + if thread.CurrentBreakpoint.Breakpoint == bp { + thread.CurrentBreakpoint.Clear() } } return nil diff --git a/pkg/proc/native/proc_darwin.go b/pkg/proc/native/proc_darwin.go index 190f62e2d99ecc8866b38705709edacebb9ce2a4..488f2325075c7deb4f6aa3d479231240bdd6060f 100644 --- a/pkg/proc/native/proc_darwin.go +++ b/pkg/proc/native/proc_darwin.go @@ -99,7 +99,7 @@ func Launch(cmd []string, wd string) (*Process, error) { dbp.allGCache = nil for _, th := range dbp.threads { - th.clearBreakpointState() + th.CurrentBreakpoint.Clear() } trapthread, err := dbp.trapWait(-1) @@ -425,11 +425,11 @@ func (dbp *Process) exitGuard(err error) error { func (dbp *Process) resume() error { // all threads stopped over a breakpoint are made to step over it for _, thread := range dbp.threads { - if thread.CurrentBreakpoint != nil { + if thread.CurrentBreakpoint.Breakpoint != nil { if err := thread.StepInstruction(); err != nil { return err } - thread.CurrentBreakpoint = nil + thread.CurrentBreakpoint.Clear() } } // everything is resumed diff --git a/pkg/proc/native/proc_linux.go b/pkg/proc/native/proc_linux.go index 36df14adf28eb846e8416c408cc147868bfd876d..3d76820c72f5d9cb57880c19d9772b5d06f45ea5 100644 --- a/pkg/proc/native/proc_linux.go +++ b/pkg/proc/native/proc_linux.go @@ -369,7 +369,7 @@ func (dbp *Process) wait(pid, options int) (int, *sys.WaitStatus, error) { func (dbp *Process) setCurrentBreakpoints(trapthread *Thread) error { for _, th := range dbp.threads { - if th.CurrentBreakpoint == nil { + if th.CurrentBreakpoint.Breakpoint == nil { if err := th.SetCurrentBreakpoint(); err != nil { if err == sys.ESRCH { // This thread quit between the point where we received the breakpoint and @@ -399,11 +399,11 @@ func (dbp *Process) exitGuard(err error) error { func (dbp *Process) resume() error { // all threads stopped over a breakpoint are made to step over it for _, thread := range dbp.threads { - if thread.CurrentBreakpoint != nil { + if thread.CurrentBreakpoint.Breakpoint != nil { if err := thread.StepInstruction(); err != nil { return err } - thread.CurrentBreakpoint = nil + thread.CurrentBreakpoint.Clear() } } // everything is resumed diff --git a/pkg/proc/native/proc_windows.go b/pkg/proc/native/proc_windows.go index e3afebe5e5e11ee15889916a7fa3cf3772ced523..53d500b7b5b9f1ea3da6926d4c5cc3c41e5f3d0d 100644 --- a/pkg/proc/native/proc_windows.go +++ b/pkg/proc/native/proc_windows.go @@ -447,11 +447,11 @@ func (dbp *Process) exitGuard(err error) error { func (dbp *Process) resume() error { for _, thread := range dbp.threads { - if thread.CurrentBreakpoint != nil { + if thread.CurrentBreakpoint.Breakpoint != nil { if err := thread.StepInstruction(); err != nil { return err } - thread.CurrentBreakpoint = nil + thread.CurrentBreakpoint.Clear() } } diff --git a/pkg/proc/native/threads.go b/pkg/proc/native/threads.go index d520f7f7a6a07cc55eb4ce9a136451163cdd9778..ef51bca9c15dda9671b6dc8556b5489a5e46c50d 100644 --- a/pkg/proc/native/threads.go +++ b/pkg/proc/native/threads.go @@ -12,11 +12,9 @@ import ( // a whole, and Status represents the last result of a `wait` call // on this thread. type Thread struct { - ID int // Thread ID or mach port - Status *WaitStatus // Status returned from last wait call - CurrentBreakpoint *proc.Breakpoint // Breakpoint thread is currently stopped at - BreakpointConditionMet bool // Output of evaluating the breakpoint's condition - BreakpointConditionError error // Error evaluating the breakpoint's condition + ID int // Thread ID or mach port + Status *WaitStatus // Status returned from last wait call + CurrentBreakpoint proc.BreakpointState // Breakpoint thread is currently stopped at dbp *Process singleStepping bool @@ -141,18 +139,17 @@ func (thread *Thread) Halt() (err error) { // SetCurrentBreakpoint sets the current breakpoint that this // thread is stopped at as CurrentBreakpoint on the thread struct. func (thread *Thread) SetCurrentBreakpoint() error { - thread.CurrentBreakpoint = nil + thread.CurrentBreakpoint.Clear() pc, err := thread.PC() if err != nil { return err } if bp, ok := thread.dbp.FindBreakpoint(pc); ok { - thread.CurrentBreakpoint = bp if err = thread.SetPC(bp.Addr); err != nil { return err } - thread.BreakpointConditionMet, thread.BreakpointConditionError = bp.CheckCondition(thread) - if thread.CurrentBreakpoint != nil && thread.BreakpointConditionMet { + thread.CurrentBreakpoint = bp.CheckCondition(thread) + if thread.CurrentBreakpoint.Breakpoint != nil && thread.CurrentBreakpoint.Active { if g, err := proc.GetG(thread); err == nil { thread.CurrentBreakpoint.HitCount[g.ID]++ } @@ -162,14 +159,8 @@ func (thread *Thread) SetCurrentBreakpoint() error { return nil } -func (thread *Thread) clearBreakpointState() { - thread.CurrentBreakpoint = nil - thread.BreakpointConditionMet = false - thread.BreakpointConditionError = nil -} - -func (th *Thread) Breakpoint() (*proc.Breakpoint, bool, error) { - return th.CurrentBreakpoint, th.CurrentBreakpoint != nil && th.BreakpointConditionMet, th.BreakpointConditionError +func (th *Thread) Breakpoint() proc.BreakpointState { + return th.CurrentBreakpoint } func (th *Thread) ThreadID() int { diff --git a/pkg/proc/proc.go b/pkg/proc/proc.go index b6ac15868bd18a259f6dda57e0ac8e5dc035e33b..2a836657857c6a17931fc8808426330c31021c09 100644 --- a/pkg/proc/proc.go +++ b/pkg/proc/proc.go @@ -68,10 +68,8 @@ func Next(dbp Process) (err error) { if dbp.Exited() { return &ProcessExitedError{Pid: dbp.Pid()} } - for _, bp := range dbp.Breakpoints().M { - if bp.Internal() { - return fmt.Errorf("next while nexting") - } + if dbp.Breakpoints().HasInternalBreakpoints() { + return fmt.Errorf("next while nexting") } if err = next(dbp, false); err != nil { @@ -106,10 +104,10 @@ func Continue(dbp Process) error { } curthread := dbp.CurrentThread() - curbp, curbpActive, _ := curthread.Breakpoint() + curbp := curthread.Breakpoint() switch { - case curbp == nil: + case curbp.Breakpoint == nil: // runtime.Breakpoint or manual stop if recorded, _ := dbp.Recorded(); onRuntimeBreakpoint(curthread) && !recorded { // Single-step current thread until we exit runtime.breakpoint and @@ -127,7 +125,7 @@ func Continue(dbp Process) error { } } return conditionErrors(threads) - case curbpActive && curbp.Internal(): + case curbp.Active && curbp.Internal: if curbp.Kind == StepBreakpoint { // See description of proc.(*Process).next for the meaning of StepBreakpoints if err := conditionErrors(threads); err != nil { @@ -154,7 +152,7 @@ func Continue(dbp Process) error { } return conditionErrors(threads) } - case curbpActive: + case curbp.Active: onNextGoroutine, err := onNextGoroutine(curthread, dbp.Breakpoints()) if err != nil { return err @@ -178,9 +176,9 @@ func Continue(dbp Process) error { func conditionErrors(threads []Thread) error { var condErr error for _, th := range threads { - if bp, _, bperr := th.Breakpoint(); bp != nil && bperr != nil { + if bp := th.Breakpoint(); bp.Breakpoint != nil && bp.CondError != nil { if condErr == nil { - condErr = bperr + condErr = bp.CondError } else { return fmt.Errorf("multiple errors evaluating conditions") } @@ -195,15 +193,15 @@ func conditionErrors(threads []Thread) error { // - trapthread func pickCurrentThread(dbp Process, trapthread Thread, threads []Thread) error { for _, th := range threads { - if bp, active, _ := th.Breakpoint(); active && bp.Internal() { + if bp := th.Breakpoint(); bp.Active && bp.Internal { return dbp.SwitchThread(th.ThreadID()) } } - if _, active, _ := trapthread.Breakpoint(); active { + if bp := trapthread.Breakpoint(); bp.Active { return dbp.SwitchThread(trapthread.ThreadID()) } for _, th := range threads { - if _, active, _ := th.Breakpoint(); active { + if bp := th.Breakpoint(); bp.Active { return dbp.SwitchThread(th.ThreadID()) } } @@ -216,10 +214,8 @@ func Step(dbp Process) (err error) { if dbp.Exited() { return &ProcessExitedError{Pid: dbp.Pid()} } - for _, bp := range dbp.Breakpoints().M { - if bp.Internal() { - return fmt.Errorf("next while nexting") - } + if dbp.Breakpoints().HasInternalBreakpoints() { + return fmt.Errorf("next while nexting") } if err = next(dbp, true); err != nil { @@ -336,7 +332,7 @@ func StepOut(dbp Process) error { } } - if bp, _, _ := curthread.Breakpoint(); bp == nil { + if bp := curthread.Breakpoint(); bp.Breakpoint == nil { curthread.SetCurrentBreakpoint() } diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 10c6a2f6fc463d8abaf6efdf41f37e02700ac610..33f0808e0371c77d706652998ab0b4cd88800a9c 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -550,7 +550,7 @@ func TestNextConcurrentVariant2(t *testing.T) { } assertNoError(err, t, "EvalVariable") vval, _ = constant.Int64Val(v.Value) - if bp, _, _ := p.CurrentThread().Breakpoint(); bp == nil { + if bpstate := p.CurrentThread().Breakpoint(); bpstate.Breakpoint == nil { if vval != initVval { t.Fatal("Did not end up on same goroutine") } @@ -1038,11 +1038,11 @@ func TestContinueMulti(t *testing.T) { } assertNoError(err, t, "Continue()") - if bp, _, _ := p.CurrentThread().Breakpoint(); bp.ID == bp1.ID { + if bp := p.CurrentThread().Breakpoint(); bp.ID == bp1.ID { mainCount++ } - if bp, _, _ := p.CurrentThread().Breakpoint(); bp.ID == bp2.ID { + if bp := p.CurrentThread().Breakpoint(); bp.ID == bp2.ID { sayhiCount++ } } @@ -1447,7 +1447,7 @@ func TestBreakpointCountsWithDetection(t *testing.T) { assertNoError(err, t, "Continue()") } for _, th := range p.ThreadList() { - if bp, _, _ := th.Breakpoint(); bp == nil { + if bp := th.Breakpoint(); bp.Breakpoint == nil { continue } scope, err := proc.GoroutineScope(th) @@ -1864,8 +1864,8 @@ func TestPanicBreakpoint(t *testing.T) { protest.AllowRecording(t) withTestProcess("panic", t, func(p proc.Process, fixture protest.Fixture) { assertNoError(proc.Continue(p), t, "Continue()") - bp, _, _ := p.CurrentThread().Breakpoint() - if bp == nil || bp.Name != proc.UnrecoveredPanic { + bp := p.CurrentThread().Breakpoint() + if bp.Breakpoint == nil || bp.Name != proc.UnrecoveredPanic { t.Fatalf("not on unrecovered-panic breakpoint: %v", bp) } }) @@ -1874,8 +1874,8 @@ func TestPanicBreakpoint(t *testing.T) { func TestCmdLineArgs(t *testing.T) { expectSuccess := func(p proc.Process, fixture protest.Fixture) { err := proc.Continue(p) - bp, _, _ := p.CurrentThread().Breakpoint() - if bp != nil && bp.Name == proc.UnrecoveredPanic { + bp := p.CurrentThread().Breakpoint() + if bp.Breakpoint != nil && bp.Name == proc.UnrecoveredPanic { t.Fatalf("testing args failed on unrecovered-panic breakpoint: %v", bp) } exit, exited := err.(proc.ProcessExitedError) @@ -1890,8 +1890,8 @@ func TestCmdLineArgs(t *testing.T) { expectPanic := func(p proc.Process, fixture protest.Fixture) { proc.Continue(p) - bp, _, _ := p.CurrentThread().Breakpoint() - if bp == nil || bp.Name != proc.UnrecoveredPanic { + bp := p.CurrentThread().Breakpoint() + if bp.Breakpoint == nil || bp.Name != proc.UnrecoveredPanic { t.Fatalf("not on unrecovered-panic breakpoint: %v", bp) } } @@ -2326,15 +2326,6 @@ func TestStepConcurrentDirect(t *testing.T) { }) } -func nextInProgress(p proc.Process) bool { - for _, bp := range p.Breakpoints().M { - if bp.Internal() { - return true - } - } - return false -} - func TestStepConcurrentPtr(t *testing.T) { protest.AllowRecording(t) withTestProcess("teststepconcurrent", t, func(p proc.Process, fixture protest.Fixture) { @@ -2364,10 +2355,9 @@ func TestStepConcurrentPtr(t *testing.T) { f, ln := currentLineNumber(p, t) if ln != 24 { for _, th := range p.ThreadList() { - bp, bpactive, bperr := th.Breakpoint() - t.Logf("thread %d stopped on breakpoint %v %v %v", th.ThreadID(), bp, bpactive, bperr) + t.Logf("thread %d stopped on breakpoint %v", th.ThreadID(), th.Breakpoint()) } - curbp, _, _ := p.CurrentThread().Breakpoint() + curbp := p.CurrentThread().Breakpoint() t.Fatalf("Program did not continue at expected location (24): %s:%d %#x [%v] (gid %d count %d)", f, ln, currentPC(p, t), curbp, p.SelectedGoroutine().ID, count) } @@ -2385,7 +2375,7 @@ func TestStepConcurrentPtr(t *testing.T) { kvals[gid] = k assertNoError(proc.Step(p), t, "Step()") - for nextInProgress(p) { + for p.Breakpoints().HasInternalBreakpoints() { if p.SelectedGoroutine().ID == gid { t.Fatalf("step did not step into function call (but internal breakpoints still active?) (%d %d)", gid, p.SelectedGoroutine().ID) } @@ -2705,7 +2695,7 @@ func TestStacktraceWithBarriers(t *testing.T) { gs, err := proc.GoroutinesInfo(p) assertNoError(err, t, "GoroutinesInfo()") for _, th := range p.ThreadList() { - if bp, _, _ := th.Breakpoint(); bp == nil { + if bp := th.Breakpoint(); bp.Breakpoint == nil { continue } @@ -3140,3 +3130,23 @@ func TestAttachStripped(t *testing.T) { } os.Remove(fixture.Path) } + +func TestIssue844(t *testing.T) { + // Conditional breakpoints should not prevent next from working if their + // condition isn't met. + withTestProcess("nextcond", t, func(p proc.Process, fixture protest.Fixture) { + setFileBreakpoint(p, t, fixture, 9) + condbp := setFileBreakpoint(p, t, fixture, 10) + condbp.Cond = &ast.BinaryExpr{ + Op: token.EQL, + X: &ast.Ident{Name: "n"}, + Y: &ast.BasicLit{Kind: token.INT, Value: "11"}, + } + assertNoError(proc.Continue(p), t, "Continue") + assertNoError(proc.Next(p), t, "Next") + f, l := currentLineNumber(p, t) + if l != 10 { + t.Fatalf("continued to wrong location %s:%d (expected 10)", f, l) + } + }) +} diff --git a/pkg/proc/scope_test.go b/pkg/proc/scope_test.go index f09a40a3a186187e39de7d2e8f37234fcc28b539..d29a5ca6276d4f0c998506cbd387cf42d26c0a79 100644 --- a/pkg/proc/scope_test.go +++ b/pkg/proc/scope_test.go @@ -86,7 +86,7 @@ func TestScope(t *testing.T) { } assertNoError(err, t, "Continue()") } - bp, _, _ := p.CurrentThread().Breakpoint() + bp := p.CurrentThread().Breakpoint() scopeCheck := findScopeCheck(scopeChecks, bp.Line) if scopeCheck == nil { diff --git a/pkg/proc/threads.go b/pkg/proc/threads.go index b6d55d3de65ceba5fe112932fc061aa0693025f5..948f756092685f069f6b9f4aac14a27b53014f44 100644 --- a/pkg/proc/threads.go +++ b/pkg/proc/threads.go @@ -19,11 +19,7 @@ type Thread interface { Location() (*Location, error) // Breakpoint will return the breakpoint that this thread is stopped at or // nil if the thread is not stopped at any breakpoint. - // Active will be true if the thread is stopped at a breakpoint and the - // breakpoint's condition is met. - // If there was an error evaluating the breakpoint's condition it will be - // returned as condErr - Breakpoint() (breakpoint *Breakpoint, active bool, condErr error) + Breakpoint() BreakpointState ThreadID() int Registers(floatingPoint bool) (Registers, error) Arch() Arch @@ -261,7 +257,7 @@ func next(dbp Process, stepInto bool) error { // there it's ok. } - if bp, _, _ := curthread.Breakpoint(); bp == nil { + if bp := curthread.Breakpoint(); bp.Breakpoint == nil { curthread.SetCurrentBreakpoint() } success = true @@ -414,7 +410,7 @@ func onRuntimeBreakpoint(thread Thread) bool { func onNextGoroutine(thread Thread, breakpoints *BreakpointMap) (bool, error) { var bp *Breakpoint for i := range breakpoints.M { - if breakpoints.M[i].Internal() && breakpoints.M[i].Cond != nil { + if breakpoints.M[i].Kind != UserBreakpoint && breakpoints.M[i].internalCond != nil { bp = breakpoints.M[i] break } @@ -432,7 +428,7 @@ func onNextGoroutine(thread Thread, breakpoints *BreakpointMap) (bool, error) { // runtime.curg.goid == X && (runtime.frameoff == Y || runtime.frameoff == Z) // Here we are only interested in testing the runtime.curg.goid clause. w := onNextGoroutineWalker{thread: thread} - ast.Walk(&w, bp.Cond) + ast.Walk(&w, bp.internalCond) return w.ret, w.err } diff --git a/service/api/conversions.go b/service/api/conversions.go index 3d7a7bbfbe7737b153ec5cf679572008f90a1937..dc053d5766cb7b316ae54552b393ffd30373d818 100644 --- a/service/api/conversions.go +++ b/service/api/conversions.go @@ -64,8 +64,8 @@ func ConvertThread(th proc.Thread) *Thread { var bp *Breakpoint - if b, active, _ := th.Breakpoint(); active { - bp = ConvertBreakpoint(b) + if b := th.Breakpoint(); b.Active { + bp = ConvertBreakpoint(b.Breakpoint) } if g, _ := proc.GetG(th); g != nil { diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 1e4981648c4ef9919316c5a1d9520f4519288775..13dd10be29f1331cfb64312e54b946b1c058f5be 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -264,12 +264,7 @@ func (d *Debugger) state() (*api.DebuggerState, error) { } } - for _, bp := range d.target.Breakpoints().M { - if bp.Internal() { - state.NextInProgress = true - break - } - } + state.NextInProgress = d.target.Breakpoints().HasInternalBreakpoints() if recorded, _ := d.target.Recorded(); recorded { state.When, _ = d.target.When() @@ -399,10 +394,9 @@ func (d *Debugger) Breakpoints() []*api.Breakpoint { func (d *Debugger) breakpoints() []*api.Breakpoint { bps := []*api.Breakpoint{} for _, bp := range d.target.Breakpoints().M { - if bp.Internal() { - continue + if bp.IsUser() { + bps = append(bps, api.ConvertBreakpoint(bp)) } - bps = append(bps, api.ConvertBreakpoint(bp)) } return bps }