PenaDevops/delve
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

proc: allow multiple overlapping internal breakpoints (#2519)

Browse Source 
Changes Breakpoint to allow multiple overlapping internal breakpoints
on the same instruction address.
This is done by changing the Breakpoint structure to contain a list of
"breaklets", each breaklet has a BreakpointKind and a condition
expression, independent of the other.
A breakpoint is considered active if any of its breaklets are active.
A breakpoint is removed when all its breaklets are removed.
We also change the terminology "internal breakpoint" to "stepping
breakpoint":

HasInternalBreakpoints -> HasSteppingBreakpoints
IsInternal -> IsStepping
etc...

The motivation for this change is implementing watchpoints on stack
variables.
Watching a stack variable requires also setting a special breakpoint to
find out when the variable goes out of scope. These breakpoints can not
be UserBreakpoints because only one user breakpoint is allowed on the
same instruction and they can not be internal breakpoints because they
should not be cleared when a next operation is completed (they should
be cleared when the variable watch is cleared).

Updates #279
This commit is contained in:
Alessandro Arzilli 2021-07-21 17:24:19 +02:00 committed by GitHub
parent c5e533b131
commit 658d36cb19
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 331 additions and 254 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

316
pkg/proc/breakpoints.go
View File

@ -40,21 +40,34 @@ type Breakpoint struct {
WatchType WatchType
HWBreakIndex uint8 // hardware breakpoint index
// 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
// Breaklets is the list of overlapping breakpoints on this physical breakpoint.
// There can be at most one UserBreakpoint in this list but multiple internal breakpoints are allowed.
Breaklets []*Breaklet
// Breakpoint information
Tracepoint bool // Tracepoint flag
TraceReturn bool
Goroutine bool // Retrieve goroutine information
Stacktrace int // Number of stack frames to retrieve
Variables []string // Variables to evaluate
LoadArgs *LoadConfig
LoadLocals *LoadConfig
Tracepoint bool // Tracepoint flag
TraceReturn bool
Goroutine bool // Retrieve goroutine information
Stacktrace int // Number of stack frames to retrieve
Variables []string // Variables to evaluate
LoadArgs *LoadConfig
LoadLocals *LoadConfig
// ReturnInfo describes how to collect return variables when this
// breakpoint is hit as a return breakpoint.
returnInfo *returnBreakpointInfo
}
// Breaklet represents one of multiple breakpoints that can overlap on a
// single physical breakpoint.
type Breaklet struct {
// Kind describes whether this is a stepping breakpoint (for next'ing or
// stepping).
Kind BreakpointKind
// Cond: if not nil the breakpoint will be triggered only if evaluating Cond returns true
Cond ast.Expr
HitCount map[int]uint64 // Number of times a breakpoint has been reached in a certain goroutine
TotalHitCount uint64 // Number of times a breakpoint has been reached
@ -68,20 +81,13 @@ type Breakpoint struct {
// function only triggers on panic or on the defer call to
// the function, not when the function is called directly
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
// HitCond: if not nil the breakpoint will be triggered only if the evaluated HitCond returns
// true with the TotalHitCount.
HitCond *struct {
Op token.Token
Val int
}
// ReturnInfo describes how to collect return variables when this
// breakpoint is hit as a return breakpoint.
returnInfo *returnBreakpointInfo
}
// BreakpointKind determines the behavior of delve when the
@ -103,6 +109,8 @@ const (
// Continue will set a new breakpoint (of NextBreakpoint kind) on the
// destination of CALL, delete this breakpoint and then continue again
StepBreakpoint
steppingMask = NextBreakpoint | NextDeferBreakpoint | StepBreakpoint
)
// WatchType is the watchpoint type
@ -136,7 +144,7 @@ func (wtype WatchType) withSize(sz uint8) WatchType {
var ErrHWBreakUnsupported = errors.New("hardware breakpoints not implemented")
func (bp *Breakpoint) String() string {
return fmt.Sprintf("Breakpoint %d at %#v %s:%d (%d)", bp.LogicalID, bp.Addr, bp.File, bp.Line, bp.TotalHitCount)
return fmt.Sprintf("Breakpoint %d at %#v %s:%d", bp.LogicalID, bp.Addr, bp.File, bp.Line)
}
// BreakpointExistsError is returned when trying to set a breakpoint at
@ -170,73 +178,87 @@ type returnBreakpointInfo struct {
// CheckCondition evaluates bp's condition on thread.
func (bp *Breakpoint) CheckCondition(thread Thread) BreakpointState {
bpstate := BreakpointState{Breakpoint: bp, Active: false, Internal: false, CondError: nil}
bpstate.checkCond(thread)
// Update the breakpoint hit counts.
if bpstate.Breakpoint != nil && bpstate.Active {
if g, err := GetG(thread); err == nil {
bpstate.HitCount[g.ID]++
}
bpstate.TotalHitCount++
bpstate := BreakpointState{Breakpoint: bp, Active: false, Stepping: false, SteppingInto: false, CondError: nil}
for _, breaklet := range bp.Breaklets {
bpstate.checkCond(breaklet, thread)
}
bpstate.checkHitCond(thread)
return bpstate
}
func (bpstate *BreakpointState) checkCond(thread Thread) {
if bpstate.Cond == nil && bpstate.internalCond == nil {
bpstate.Active = true
bpstate.Internal = bpstate.IsInternal()
func (bpstate *BreakpointState) checkCond(breaklet *Breaklet, thread Thread) {
var condErr error
active := true
if breaklet.Cond != nil {
active, condErr = evalBreakpointCondition(thread, breaklet.Cond)
}
if condErr != nil && bpstate.CondError == nil {
bpstate.CondError = condErr
}
if !active {
return
}
nextDeferOk := true
if bpstate.Kind&NextDeferBreakpoint != 0 {
var err error
frames, err := ThreadStacktrace(thread, 2)
if err == nil {
nextDeferOk, _ = isPanicCall(frames)
if !nextDeferOk {
nextDeferOk, _ = isDeferReturnCall(frames, bpstate.DeferReturns)
switch breaklet.Kind {
case UserBreakpoint:
if g, err := GetG(thread); err == nil {
breaklet.HitCount[g.ID]++
}
breaklet.TotalHitCount++
active = checkHitCond(breaklet)
case StepBreakpoint, NextBreakpoint, NextDeferBreakpoint:
nextDeferOk := true
if breaklet.Kind&NextDeferBreakpoint != 0 {
var err error
frames, err := ThreadStacktrace(thread, 2)
if err == nil {
nextDeferOk, _ = isPanicCall(frames)
if !nextDeferOk {
nextDeferOk, _ = isDeferReturnCall(frames, breaklet.DeferReturns)
}
}
}
}
if bpstate.IsInternal() {
// Check internalCondition if this is also an internal breakpoint
bpstate.Active, bpstate.CondError = evalBreakpointCondition(thread, bpstate.internalCond)
bpstate.Active = bpstate.Active && nextDeferOk
if bpstate.Active || bpstate.CondError != nil {
bpstate.Internal = true
return
active = active && nextDeferOk
if active {
bpstate.Stepping = true
if breaklet.Kind == StepBreakpoint {
bpstate.SteppingInto = true
}
}
default:
bpstate.CondError = fmt.Errorf("internal error unknown breakpoint kind %v", breaklet.Kind)
}
if bpstate.IsUser() {
// Check normal condition if this is also a user breakpoint
bpstate.Active, bpstate.CondError = evalBreakpointCondition(thread, bpstate.Cond)
if active {
bpstate.Active = true
}
}
// checkHitCond evaluates bp's hit condition on thread.
func (bpstate *BreakpointState) checkHitCond(thread Thread) {
if bpstate.HitCond == nil || !bpstate.Active || bpstate.Internal {
return
func checkHitCond(breaklet *Breaklet) bool {
if breaklet.HitCond == nil {
return true
}
// Evaluate the breakpoint condition.
switch bpstate.HitCond.Op {
switch breaklet.HitCond.Op {
case token.EQL:
bpstate.Active = int(bpstate.TotalHitCount) == bpstate.HitCond.Val
return int(breaklet.TotalHitCount) == breaklet.HitCond.Val
case token.NEQ:
bpstate.Active = int(bpstate.TotalHitCount) != bpstate.HitCond.Val
return int(breaklet.TotalHitCount) != breaklet.HitCond.Val
case token.GTR:
bpstate.Active = int(bpstate.TotalHitCount) > bpstate.HitCond.Val
return int(breaklet.TotalHitCount) > breaklet.HitCond.Val
case token.LSS:
bpstate.Active = int(bpstate.TotalHitCount) < bpstate.HitCond.Val
return int(breaklet.TotalHitCount) < breaklet.HitCond.Val
case token.GEQ:
bpstate.Active = int(bpstate.TotalHitCount) >= bpstate.HitCond.Val
return int(breaklet.TotalHitCount) >= breaklet.HitCond.Val
case token.LEQ:
bpstate.Active = int(bpstate.TotalHitCount) <= bpstate.HitCond.Val
return int(breaklet.TotalHitCount) <= breaklet.HitCond.Val
case token.REM:
bpstate.Active = int(bpstate.TotalHitCount)%bpstate.HitCond.Val == 0
return int(breaklet.TotalHitCount)%breaklet.HitCond.Val == 0
}
return false
}
func isPanicCall(frames []Stackframe) (bool, int) {
@ -271,18 +293,39 @@ func isDeferReturnCall(frames []Stackframe, deferReturns []uint64) (bool, uint64
return false, 0
}
// 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
// IsStepping returns true if bp is an stepping breakpoint.
// User-set breakpoints can overlap with stepping breakpoints, in that case
// both IsUser and IsStepping will be true.
func (bp *Breakpoint) IsStepping() bool {
for _, breaklet := range bp.Breaklets {
if breaklet.Kind&steppingMask != 0 {
return true
}
}
return false
}
// 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.
// User-set breakpoints can overlap with stepping breakpoints, in that case
// both IsUser and IsStepping will be true.
func (bp *Breakpoint) IsUser() bool {
return bp.Kind&UserBreakpoint != 0
for _, breaklet := range bp.Breaklets {
if breaklet.Kind == UserBreakpoint {
return true
}
}
return false
}
// UserBreaklet returns the user breaklet for this breakpoint, or nil if
// none exist.
func (bp *Breakpoint) UserBreaklet() *Breaklet {
for _, breaklet := range bp.Breaklets {
if breaklet.Kind == UserBreakpoint {
return breaklet
}
}
return nil
}
func evalBreakpointCondition(thread Thread, cond ast.Expr) (bool, error) {
@ -389,19 +432,15 @@ func (t *Target) setBreakpointInternal(addr uint64, kind BreakpointKind, wtype W
return nil, err
}
bpmap := t.Breakpoints()
newBreaklet := &Breaklet{Kind: kind, Cond: cond}
if kind == UserBreakpoint {
newBreaklet.HitCount = map[int]uint64{}
}
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
// breakpoint set by next or step.
if (kind != UserBreakpoint && bp.Kind != UserBreakpoint) || (kind == UserBreakpoint && bp.IsUser()) {
if !bp.canOverlap(kind) {
return bp, BreakpointExistsError{bp.File, bp.Line, bp.Addr}
}
bp.Kind |= kind
if kind != UserBreakpoint {
bp.internalCond = cond
} else {
bp.Cond = cond
}
bp.Breaklets = append(bp.Breaklets, newBreaklet)
return bp, nil
}
@ -434,8 +473,6 @@ func (t *Target) setBreakpointInternal(addr uint64, kind BreakpointKind, wtype W
File: f,
Line: l,
Addr: addr,
Kind: kind,
HitCount: map[int]uint64{},
}
err := t.proc.WriteBreakpoint(newBreakpoint)
@ -446,13 +483,13 @@ func (t *Target) setBreakpointInternal(addr uint64, kind BreakpointKind, wtype W
if kind != UserBreakpoint {
bpmap.internalBreakpointIDCounter++
newBreakpoint.LogicalID = bpmap.internalBreakpointIDCounter
newBreakpoint.internalCond = cond
} else {
bpmap.breakpointIDCounter++
newBreakpoint.LogicalID = bpmap.breakpointIDCounter
newBreakpoint.Cond = cond
}
newBreakpoint.Breaklets = append(newBreakpoint.Breaklets, newBreaklet)
bpmap.M[addr] = newBreakpoint
return newBreakpoint, nil
@ -469,62 +506,93 @@ func (t *Target) SetBreakpointWithID(id int, addr uint64) (*Breakpoint, error) {
return bp, err
}
// canOverlap returns true if a breakpoint of kind can be overlapped to the
// already existing breaklets in bp.
// At most one user breakpoint can be set but multiple internal breakpoints are allowed.
// All other internal breakpoints are allowed to overlap freely.
func (bp *Breakpoint) canOverlap(kind BreakpointKind) bool {
if kind == UserBreakpoint {
return !bp.IsUser()
}
return true
}
// 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]
bp, ok := t.Breakpoints().M[addr]
if !ok {
return nil, NoBreakpointError{Addr: addr}
}
bp.Kind &= ^UserBreakpoint
bp.Cond = nil
if bp.Kind != 0 {
return bp, nil
for i := range bp.Breaklets {
if bp.Breaklets[i].Kind == UserBreakpoint {
bp.Breaklets[i] = nil
}
}
if err := t.proc.EraseBreakpoint(bp); err != nil {
_, err := t.finishClearBreakpoint(bp)
if err != nil {
return nil, err
}
delete(bpmap.M, addr)
return bp, nil
}
// ClearInternalBreakpoints removes all internal breakpoints from the map,
// ClearInternalBreakpoints removes all stepping breakpoints from the map,
// calling clearBreakpoint on each one.
func (t *Target) ClearInternalBreakpoints() error {
func (t *Target) ClearSteppingBreakpoints() error {
bpmap := t.Breakpoints()
threads := t.ThreadList()
for addr, bp := range bpmap.M {
bp.Kind = bp.Kind & UserBreakpoint
bp.internalCond = nil
bp.returnInfo = nil
if bp.Kind != 0 {
continue
}
if err := t.proc.EraseBreakpoint(bp); err != nil {
return err
}
for _, thread := range threads {
if thread.Breakpoint().Breakpoint == bp {
thread.Breakpoint().Clear()
for _, bp := range bpmap.M {
for i := range bp.Breaklets {
if bp.Breaklets[i].Kind&steppingMask != 0 {
bp.Breaklets[i] = nil
}
}
cleared, err := t.finishClearBreakpoint(bp)
if err != nil {
return err
}
if cleared {
for _, thread := range threads {
if thread.Breakpoint().Breakpoint == bp {
thread.Breakpoint().Clear()
}
}
}
delete(bpmap.M, addr)
}
return nil
}
// HasInternalBreakpoints returns true if bpmap has at least one internal
// finishClearBreakpoint clears nil breaklets from the breaklet list of bp
// and if it is empty erases the breakpoint.
// Returns true if the breakpoint was deleted
func (t *Target) finishClearBreakpoint(bp *Breakpoint) (bool, error) {
oldBreaklets := bp.Breaklets
bp.Breaklets = bp.Breaklets[:0]
for _, breaklet := range oldBreaklets {
if breaklet != nil {
bp.Breaklets = append(bp.Breaklets, breaklet)
}
}
if len(bp.Breaklets) > 0 {
return false, nil
}
if err := t.proc.EraseBreakpoint(bp); err != nil {
return false, err
}
delete(t.Breakpoints().M, bp.Addr)
return true, nil
}
// HasSteppingBreakpoints returns true if bpmap has at least one stepping
// breakpoint set.
func (bpmap *BreakpointMap) HasInternalBreakpoints() bool {
func (bpmap *BreakpointMap) HasSteppingBreakpoints() bool {
for _, bp := range bpmap.M {
if bp.IsInternal() {
if bp.IsStepping() {
return true
}
}
@ -544,11 +612,14 @@ func (bpmap *BreakpointMap) HasHWBreakpoints() bool {
// BreakpointState describes the state of a breakpoint in a thread.
type BreakpointState struct {
*Breakpoint
// Active is true if the breakpoint condition was met.
// Active is true if the condition of any breaklet is met.
Active bool
// Internal is true if the breakpoint was matched as an internal
// Stepping is true if one of the active breaklets is a stepping
// breakpoint.
Internal bool
Stepping bool
// SteppingInto is true if one of the active stepping breaklets has Kind ==
// StepBreakpoint.
SteppingInto bool
// CondError contains any error encountered while evaluating the
// breakpoint's condition.
CondError error
@ -558,7 +629,8 @@ type BreakpointState struct {
func (bpstate *BreakpointState) Clear() {
bpstate.Breakpoint = nil
bpstate.Active = false
bpstate.Internal = false
bpstate.Stepping = false
bpstate.SteppingInto = false
bpstate.CondError = nil
}
@ -567,8 +639,8 @@ func (bpstate *BreakpointState) String() string {
if bpstate.Active {
s += " active"
}
if bpstate.Internal {
s += " internal"
if bpstate.Stepping {
s += " stepping"
}
return s
}

2
pkg/proc/gdbserial/gdbserver.go
View File

@ -1185,7 +1185,7 @@ func (p *gdbProcess) ChangeDirection(dir proc.Direction) error {
if p.conn.direction == dir {
return nil
}
if p.Breakpoints().HasInternalBreakpoints() {
if p.Breakpoints().HasSteppingBreakpoints() {
return ErrDirChange
}
p.conn.direction = dir

14
pkg/proc/gdbserial/rr_test.go
View File

@ -164,18 +164,18 @@ func TestReverseBreakpointCounts(t *testing.T) {
}
}
t.Logf("TotalHitCount: %d", bp.TotalHitCount)
if bp.TotalHitCount != 200 {
t.Fatalf("Wrong TotalHitCount for the breakpoint (%d)", bp.TotalHitCount)
t.Logf("TotalHitCount: %d", bp.UserBreaklet().TotalHitCount)
if bp.UserBreaklet().TotalHitCount != 200 {
t.Fatalf("Wrong TotalHitCount for the breakpoint (%d)", bp.UserBreaklet().TotalHitCount)
}
if len(bp.HitCount) != 2 {
t.Fatalf("Wrong number of goroutines for breakpoint (%d)", len(bp.HitCount))
if len(bp.UserBreaklet().HitCount) != 2 {
t.Fatalf("Wrong number of goroutines for breakpoint (%d)", len(bp.UserBreaklet().HitCount))
}
for _, v := range bp.HitCount {
for _, v := range bp.UserBreaklet().HitCount {
if v != 100 {
t.Fatalf("Wrong HitCount for breakpoint (%v)", bp.HitCount)
t.Fatalf("Wrong HitCount for breakpoint (%v)", bp.UserBreaklet().HitCount)
}
}
})

74
pkg/proc/proc_test.go
View File

@ -318,8 +318,8 @@ func TestBreakpoint(t *testing.T) {
assertNoError(err, t, "Registers")
pc := regs.PC()
if bp.TotalHitCount != 1 {
t.Fatalf("Breakpoint should be hit once, got %d\n", bp.TotalHitCount)
if bp.UserBreaklet().TotalHitCount != 1 {
t.Fatalf("Breakpoint should be hit once, got %d\n", bp.UserBreaklet().TotalHitCount)
}
if pc-1 != bp.Addr && pc != bp.Addr {
@ -1460,18 +1460,18 @@ func TestBreakpointCounts(t *testing.T) {
}
}
t.Logf("TotalHitCount: %d", bp.TotalHitCount)
if bp.TotalHitCount != 200 {
t.Fatalf("Wrong TotalHitCount for the breakpoint (%d)", bp.TotalHitCount)
t.Logf("TotalHitCount: %d", bp.UserBreaklet().TotalHitCount)
if bp.UserBreaklet().TotalHitCount != 200 {
t.Fatalf("Wrong TotalHitCount for the breakpoint (%d)", bp.UserBreaklet().TotalHitCount)
}
if len(bp.HitCount) != 2 {
t.Fatalf("Wrong number of goroutines for breakpoint (%d)", len(bp.HitCount))
if len(bp.UserBreaklet().HitCount) != 2 {
t.Fatalf("Wrong number of goroutines for breakpoint (%d)", len(bp.UserBreaklet().HitCount))
}
for _, v := range bp.HitCount {
for _, v := range bp.UserBreaklet().HitCount {
if v != 100 {
t.Fatalf("Wrong HitCount for breakpoint (%v)", bp.HitCount)
t.Fatalf("Wrong HitCount for breakpoint (%v)", bp.UserBreaklet().HitCount)
}
}
})
@ -1527,23 +1527,23 @@ func TestBreakpointCountsWithDetection(t *testing.T) {
total += m[i] + 1
}
if uint64(total) != bp.TotalHitCount {
t.Fatalf("Mismatched total count %d %d\n", total, bp.TotalHitCount)
if uint64(total) != bp.UserBreaklet().TotalHitCount {
t.Fatalf("Mismatched total count %d %d\n", total, bp.UserBreaklet().TotalHitCount)
}
}
t.Logf("TotalHitCount: %d", bp.TotalHitCount)
if bp.TotalHitCount != 200 {
t.Fatalf("Wrong TotalHitCount for the breakpoint (%d)", bp.TotalHitCount)
t.Logf("TotalHitCount: %d", bp.UserBreaklet().TotalHitCount)
if bp.UserBreaklet().TotalHitCount != 200 {
t.Fatalf("Wrong TotalHitCount for the breakpoint (%d)", bp.UserBreaklet().TotalHitCount)
}
if len(bp.HitCount) != 2 {
t.Fatalf("Wrong number of goroutines for breakpoint (%d)", len(bp.HitCount))
if len(bp.UserBreaklet().HitCount) != 2 {
t.Fatalf("Wrong number of goroutines for breakpoint (%d)", len(bp.UserBreaklet().HitCount))
}
for _, v := range bp.HitCount {
for _, v := range bp.UserBreaklet().HitCount {
if v != 100 {
t.Fatalf("Wrong HitCount for breakpoint (%v)", bp.HitCount)
t.Fatalf("Wrong HitCount for breakpoint (%v)", bp.UserBreaklet().HitCount)
}
}
})
@ -1656,7 +1656,7 @@ func TestCondBreakpoint(t *testing.T) {
protest.AllowRecording(t)
withTestProcess("parallel_next", t, func(p *proc.Target, fixture protest.Fixture) {
bp := setFileBreakpoint(p, t, fixture.Source, 9)
bp.Cond = &ast.BinaryExpr{
bp.UserBreaklet().Cond = &ast.BinaryExpr{
Op: token.EQL,
X: &ast.Ident{Name: "n"},
Y: &ast.BasicLit{Kind: token.INT, Value: "7"},
@ -1678,7 +1678,7 @@ func TestCondBreakpointError(t *testing.T) {
protest.AllowRecording(t)
withTestProcess("parallel_next", t, func(p *proc.Target, fixture protest.Fixture) {
bp := setFileBreakpoint(p, t, fixture.Source, 9)
bp.Cond = &ast.BinaryExpr{
bp.UserBreaklet().Cond = &ast.BinaryExpr{
Op: token.EQL,
X: &ast.Ident{Name: "nonexistentvariable"},
Y: &ast.BasicLit{Kind: token.INT, Value: "7"},
@ -1693,7 +1693,7 @@ func TestCondBreakpointError(t *testing.T) {
t.Fatalf("Unexpected error on first Continue(): %v", err)
}
bp.Cond = &ast.BinaryExpr{
bp.UserBreaklet().Cond = &ast.BinaryExpr{
Op: token.EQL,
X: &ast.Ident{Name: "n"},
Y: &ast.BasicLit{Kind: token.INT, Value: "7"},
@ -1718,7 +1718,7 @@ func TestCondBreakpointError(t *testing.T) {
func TestHitCondBreakpointEQ(t *testing.T) {
withTestProcess("break", t, func(p *proc.Target, fixture protest.Fixture) {
bp := setFileBreakpoint(p, t, fixture.Source, 7)
bp.HitCond = &struct {
bp.UserBreaklet().HitCond = &struct {
Op token.Token
Val int
}{token.EQL, 3}
@ -1742,7 +1742,7 @@ func TestHitCondBreakpointGEQ(t *testing.T) {
protest.AllowRecording(t)
withTestProcess("break", t, func(p *proc.Target, fixture protest.Fixture) {
bp := setFileBreakpoint(p, t, fixture.Source, 7)
bp.HitCond = &struct {
bp.UserBreaklet().HitCond = &struct {
Op token.Token
Val int
}{token.GEQ, 3}
@ -1765,7 +1765,7 @@ func TestHitCondBreakpointREM(t *testing.T) {
protest.AllowRecording(t)
withTestProcess("break", t, func(p *proc.Target, fixture protest.Fixture) {
bp := setFileBreakpoint(p, t, fixture.Source, 7)
bp.HitCond = &struct {
bp.UserBreaklet().HitCond = &struct {
Op token.Token
Val int
}{token.REM, 2}
@ -2525,7 +2525,7 @@ func TestStepConcurrentPtr(t *testing.T) {
kvals[gid] = k
assertNoError(p.Step(), t, "Step()")
for p.Breakpoints().HasInternalBreakpoints() {
for p.Breakpoints().HasSteppingBreakpoints() {
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)
}
@ -3155,7 +3155,7 @@ func TestIssue844(t *testing.T) {
withTestProcess("nextcond", t, func(p *proc.Target, fixture protest.Fixture) {
setFileBreakpoint(p, t, fixture.Source, 9)
condbp := setFileBreakpoint(p, t, fixture.Source, 10)
condbp.Cond = &ast.BinaryExpr{
condbp.UserBreaklet().Cond = &ast.BinaryExpr{
Op: token.EQL,
X: &ast.Ident{Name: "n"},
Y: &ast.BasicLit{Kind: token.INT, Value: "11"},
@ -3611,7 +3611,7 @@ func TestIssue1145(t *testing.T) {
}()
assertNoError(p.Next(), t, "Next()")
if p.Breakpoints().HasInternalBreakpoints() {
if p.Breakpoints().HasSteppingBreakpoints() {
t.Fatal("has internal breakpoints after manual stop request")
}
})
@ -4048,7 +4048,7 @@ func TestIssue1264(t *testing.T) {
// of evaluating a single boolean variable.
withTestProcess("issue1264", t, func(p *proc.Target, fixture protest.Fixture) {
bp := setFileBreakpoint(p, t, fixture.Source, 8)
bp.Cond = &ast.Ident{Name: "equalsTwo"}
bp.UserBreaklet().Cond = &ast.Ident{Name: "equalsTwo"}
assertNoError(p.Continue(), t, "Continue()")
assertLineNumber(p, t, 8, "after continue")
})
@ -4488,7 +4488,7 @@ func TestIssue1615(t *testing.T) {
withTestProcess("issue1615", t, func(p *proc.Target, fixture protest.Fixture) {
bp := setFileBreakpoint(p, t, fixture.Source, 19)
bp.Cond = &ast.BinaryExpr{
bp.UserBreaklet().Cond = &ast.BinaryExpr{
Op: token.EQL,
X: &ast.Ident{Name: "s"},
Y: &ast.BasicLit{Kind: token.STRING, Value: `"projects/my-gcp-project-id-string/locations/us-central1/queues/my-task-queue-name"`},
@ -4641,7 +4641,7 @@ func BenchmarkConditionalBreakpoints(b *testing.B) {
b.N = 1
withTestProcess("issue1549", b, func(p *proc.Target, fixture protest.Fixture) {
bp := setFileBreakpoint(p, b, fixture.Source, 12)
bp.Cond = &ast.BinaryExpr{
bp.UserBreaklet().Cond = &ast.BinaryExpr{
Op: token.EQL,
X: &ast.Ident{Name: "value"},
Y: &ast.BasicLit{Kind: token.INT, Value: "-1"},
@ -5327,18 +5327,18 @@ func TestWatchpointCounts(t *testing.T) {
}
}
t.Logf("TotalHitCount: %d", bp.TotalHitCount)
if bp.TotalHitCount != 200 {
t.Fatalf("Wrong TotalHitCount for the breakpoint (%d)", bp.TotalHitCount)
t.Logf("TotalHitCount: %d", bp.UserBreaklet().TotalHitCount)
if bp.UserBreaklet().TotalHitCount != 200 {
t.Fatalf("Wrong TotalHitCount for the breakpoint (%d)", bp.UserBreaklet().TotalHitCount)
}
if len(bp.HitCount) != 2 {
t.Fatalf("Wrong number of goroutines for breakpoint (%d)", len(bp.HitCount))
if len(bp.UserBreaklet().HitCount) != 2 {
t.Fatalf("Wrong number of goroutines for breakpoint (%d)", len(bp.UserBreaklet().HitCount))
}
for _, v := range bp.HitCount {
for _, v := range bp.UserBreaklet().HitCount {
if v != 100 {
t.Fatalf("Wrong HitCount for breakpoint (%v)", bp.HitCount)
t.Fatalf("Wrong HitCount for breakpoint (%v)", bp.UserBreaklet().HitCount)
}
}
})

88
pkg/proc/target_exec.go
View File

@ -31,12 +31,12 @@ func (dbp *Target) Next() (err error) {
if _, err := dbp.Valid(); err != nil {
return err
}
if dbp.Breakpoints().HasInternalBreakpoints() {
if dbp.Breakpoints().HasSteppingBreakpoints() {
return fmt.Errorf("next while nexting")
}
if err = next(dbp, false, false); err != nil {
dbp.ClearInternalBreakpoints()
dbp.ClearSteppingBreakpoints()
return
}
@ -60,13 +60,13 @@ func (dbp *Target) Continue() error {
// manual stop request and hit a breakpoint.
if dbp.CheckAndClearManualStopRequest() {
dbp.StopReason = StopManual
dbp.ClearInternalBreakpoints()
dbp.ClearSteppingBreakpoints()
}
}()
for {
if dbp.CheckAndClearManualStopRequest() {
dbp.StopReason = StopManual
dbp.ClearInternalBreakpoints()
dbp.ClearSteppingBreakpoints()
return nil
}
dbp.ClearCaches()
@ -90,7 +90,7 @@ func (dbp *Target) Continue() error {
return err
}
if dbp.StopReason == StopLaunched {
dbp.ClearInternalBreakpoints()
dbp.ClearSteppingBreakpoints()
}
threads := dbp.ThreadList()
@ -154,9 +154,8 @@ func (dbp *Target) Continue() error {
}
return conditionErrors(threads)
}
case curbp.Active && curbp.Internal:
switch curbp.Kind {
case StepBreakpoint:
case curbp.Active && curbp.Stepping:
if curbp.SteppingInto {
// See description of proc.(*Process).next for the meaning of StepBreakpoints
if err := conditionErrors(threads); err != nil {
return err
@ -177,14 +176,14 @@ func (dbp *Target) Continue() error {
return err
}
} else {
if err := dbp.ClearInternalBreakpoints(); err != nil {
if err := dbp.ClearSteppingBreakpoints(); err != nil {
return err
}
return dbp.StepInstruction()
}
default:
} else {
curthread.Common().returnValues = curbp.Breakpoint.returnInfo.Collect(dbp, curthread)
if err := dbp.ClearInternalBreakpoints(); err != nil {
if err := dbp.ClearSteppingBreakpoints(); err != nil {
return err
}
dbp.StopReason = StopNextFinished
@ -196,13 +195,13 @@ func (dbp *Target) Continue() error {
return err
}
if onNextGoroutine {
err := dbp.ClearInternalBreakpoints()
err := dbp.ClearSteppingBreakpoints()
if err != nil {
return err
}
}
if curbp.Name == UnrecoveredPanic {
dbp.ClearInternalBreakpoints()
dbp.ClearSteppingBreakpoints()
}
dbp.StopReason = StopBreakpoint
if curbp.Breakpoint.WatchType != 0 {
@ -241,7 +240,7 @@ func conditionErrors(threads []Thread) error {
// - trapthread
func pickCurrentThread(dbp *Target, trapthread Thread, threads []Thread) error {
for _, th := range threads {
if bp := th.Breakpoint(); bp.Active && bp.Internal {
if bp := th.Breakpoint(); bp.Active && bp.Stepping {
return dbp.SwitchThread(th.ThreadID())
}
}
@ -308,17 +307,17 @@ func (dbp *Target) Step() (err error) {
if _, err := dbp.Valid(); err != nil {
return err
}
if dbp.Breakpoints().HasInternalBreakpoints() {
if dbp.Breakpoints().HasSteppingBreakpoints() {
return fmt.Errorf("next while nexting")
}
if err = next(dbp, true, false); err != nil {
_ = dbp.ClearInternalBreakpoints()
_ = dbp.ClearSteppingBreakpoints()
return err
}
if bp := dbp.CurrentThread().Breakpoint().Breakpoint; bp != nil && bp.Kind == StepBreakpoint && dbp.GetDirection() == Backward {
dbp.ClearInternalBreakpoints()
if bpstate := dbp.CurrentThread().Breakpoint(); bpstate.Breakpoint != nil && bpstate.Active && bpstate.SteppingInto && dbp.GetDirection() == Backward {
dbp.ClearSteppingBreakpoints()
return dbp.StepInstruction()
}
@ -345,7 +344,7 @@ func (dbp *Target) StepOut() error {
if _, err := dbp.Valid(); err != nil {
return err
}
if dbp.Breakpoints().HasInternalBreakpoints() {
if dbp.Breakpoints().HasSteppingBreakpoints() {
return fmt.Errorf("next while nexting")
}
@ -360,7 +359,7 @@ func (dbp *Target) StepOut() error {
success := false
defer func() {
if !success {
dbp.ClearInternalBreakpoints()
dbp.ClearSteppingBreakpoints()
}
}()
@ -502,7 +501,7 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error {
success := false
defer func() {
if !success {
dbp.ClearInternalBreakpoints()
dbp.ClearSteppingBreakpoints()
}
}()
@ -616,7 +615,7 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error {
for _, pc := range pcs {
if _, err := allowDuplicateBreakpoint(dbp.SetBreakpoint(pc, NextBreakpoint, sameFrameCond)); err != nil {
dbp.ClearInternalBreakpoints()
dbp.ClearSteppingBreakpoints()
return err
}
}
@ -631,25 +630,12 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error {
if !topframe.Inlined {
topframe, retframe := skipAutogeneratedWrappersOut(selg, curthread, &topframe, &retframe)
retFrameCond := astutil.And(sameGCond, frameoffCondition(retframe))
var sameOrRetFrameCond ast.Expr
if sameGCond != nil {
sameOrRetFrameCond = astutil.And(sameGCond, astutil.Or(frameoffCondition(topframe), frameoffCondition(retframe)))
}
// Add a breakpoint on the return address for the current frame.
// For inlined functions there is no need to do this, the set of PCs
// returned by the AllPCsBetween call above already cover all instructions
// of the containing function.
bp, err := dbp.SetBreakpoint(retframe.Current.PC, NextBreakpoint, retFrameCond)
if _, isexists := err.(BreakpointExistsError); isexists {
if bp.Kind == NextBreakpoint {
// If the return address shares the same address with one of the lines
// of the function (because we are stepping through a recursive
// function) then the corresponding breakpoint should be active both on
// this frame and on the return frame.
bp.Cond = sameOrRetFrameCond
}
}
bp, _ := dbp.SetBreakpoint(retframe.Current.PC, NextBreakpoint, retFrameCond)
// Return address could be wrong, if we are unable to set a breakpoint
// there it's ok.
if bp != nil {
@ -685,6 +671,7 @@ func setStepIntoBreakpoints(dbp *Target, curfn *Function, text []AsmInstruction,
}
func setStepIntoBreakpointsReverse(dbp *Target, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr) error {
bpmap := dbp.Breakpoints()
// 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 {
@ -696,8 +683,11 @@ func setStepIntoBreakpointsReverse(dbp *Target, text []AsmInstruction, topframe
}
if nextIdx := i + 1; nextIdx < len(text) {
if _, err := allowDuplicateBreakpoint(dbp.SetBreakpoint(text[nextIdx].Loc.PC, StepBreakpoint, sameGCond)); err != nil {
return err
_, ok := bpmap.M[text[nextIdx].Loc.PC]
if !ok {
if _, err := allowDuplicateBreakpoint(dbp.SetBreakpoint(text[nextIdx].Loc.PC, StepBreakpoint, sameGCond)); err != nil {
return err
}
}
}
}
@ -935,7 +925,12 @@ func setDeferBreakpoint(p *Target, text []AsmInstruction, topframe Stackframe, s
// If DeferReturns is set then the breakpoint will also be triggered when
// called from runtime.deferreturn. We only do this for the step command,
// not for next or stepout.
bp.DeferReturns = FindDeferReturnCalls(text)
for _, breaklet := range bp.Breaklets {
if breaklet.Kind == NextDeferBreakpoint {
breaklet.DeferReturns = FindDeferReturnCalls(text)
break
}
}
}
}
@ -1021,14 +1016,17 @@ func stepOutReverse(p *Target, topframe, retframe Stackframe, sameGCond ast.Expr
// onNextGoroutine returns true if this thread is on the goroutine requested by the current 'next' command
func onNextGoroutine(thread Thread, breakpoints *BreakpointMap) (bool, error) {
var bp *Breakpoint
var breaklet *Breaklet
breakletSearch:
for i := range breakpoints.M {
if breakpoints.M[i].Kind != UserBreakpoint && breakpoints.M[i].internalCond != nil {
bp = breakpoints.M[i]
break
for _, blet := range breakpoints.M[i].Breaklets {
if blet.Kind&steppingMask != 0 && blet.Cond != nil {
breaklet = blet
break breakletSearch
}
}
}
if bp == nil {
if breaklet == nil {
return false, nil
}
// Internal breakpoint conditions can take multiple different forms:
@ -1041,7 +1039,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.internalCond)
ast.Walk(&w, breaklet.Cond)
return w.ret, w.err
}

55
service/api/conversions.go
View File

@ -20,35 +20,38 @@ import (
// an api.Breakpoint.
func ConvertBreakpoint(bp *proc.Breakpoint) *Breakpoint {
b := &Breakpoint{
Name: bp.Name,
ID: bp.LogicalID,
FunctionName: bp.FunctionName,
File: bp.File,
Line: bp.Line,
Addr: bp.Addr,
Tracepoint: bp.Tracepoint,
TraceReturn: bp.TraceReturn,
Stacktrace: bp.Stacktrace,
Goroutine: bp.Goroutine,
Variables: bp.Variables,
LoadArgs: LoadConfigFromProc(bp.LoadArgs),
LoadLocals: LoadConfigFromProc(bp.LoadLocals),
WatchExpr: bp.WatchExpr,
WatchType: WatchType(bp.WatchType),
TotalHitCount: bp.TotalHitCount,
Addrs: []uint64{bp.Addr},
Name: bp.Name,
ID: bp.LogicalID,
FunctionName: bp.FunctionName,
File: bp.File,
Line: bp.Line,
Addr: bp.Addr,
Tracepoint: bp.Tracepoint,
TraceReturn: bp.TraceReturn,
Stacktrace: bp.Stacktrace,
Goroutine: bp.Goroutine,
Variables: bp.Variables,
LoadArgs: LoadConfigFromProc(bp.LoadArgs),
LoadLocals: LoadConfigFromProc(bp.LoadLocals),
WatchExpr: bp.WatchExpr,
WatchType: WatchType(bp.WatchType),
Addrs: []uint64{bp.Addr},
}
b.HitCount = map[string]uint64{}
for idx := range bp.HitCount {
b.HitCount[strconv.Itoa(idx)] = bp.HitCount[idx]
}
breaklet := bp.UserBreaklet()
if breaklet != nil {
b.TotalHitCount = breaklet.TotalHitCount
b.HitCount = map[string]uint64{}
for idx := range breaklet.HitCount {
b.HitCount[strconv.Itoa(idx)] = breaklet.HitCount[idx]
}
var buf bytes.Buffer
printer.Fprint(&buf, token.NewFileSet(), bp.Cond)
b.Cond = buf.String()
if bp.HitCond != nil {
b.HitCond = fmt.Sprintf("%s %d", bp.HitCond.Op.String(), bp.HitCond.Val)
var buf bytes.Buffer
printer.Fprint(&buf, token.NewFileSet(), breaklet.Cond)
b.Cond = buf.String()
if breaklet.HitCond != nil {
b.HitCond = fmt.Sprintf("%s %d", breaklet.HitCond.Op.String(), breaklet.HitCond.Val)
}
}
return b

35
service/debugger/debugger.go
View File

@ -617,7 +617,7 @@ func (d *Debugger) state(retLoadCfg *proc.LoadConfig) (*api.DebuggerState, error
}
}
state.NextInProgress = d.target.Breakpoints().HasInternalBreakpoints()
state.NextInProgress = d.target.Breakpoints().HasSteppingBreakpoints()
if recorded, _ := d.target.Recorded(); recorded {
state.When, _ = d.target.When()
@ -801,7 +801,7 @@ func (d *Debugger) AmendBreakpoint(amend *api.Breakpoint) error {
func (d *Debugger) CancelNext() error {
d.targetMutex.Lock()
defer d.targetMutex.Unlock()
return d.target.ClearInternalBreakpoints()
return d.target.ClearSteppingBreakpoints()
}
func copyBreakpointInfo(bp *proc.Breakpoint, requested *api.Breakpoint) (err error) {
@ -813,21 +813,24 @@ func copyBreakpointInfo(bp *proc.Breakpoint, requested *api.Breakpoint) (err err
bp.Variables = requested.Variables
bp.LoadArgs = api.LoadConfigToProc(requested.LoadArgs)
bp.LoadLocals = api.LoadConfigToProc(requested.LoadLocals)
bp.Cond = nil
if requested.Cond != "" {
bp.Cond, err = parser.ParseExpr(requested.Cond)
}
bp.HitCond = nil
if requested.HitCond != "" {
opTok, val, parseErr := parseHitCondition(requested.HitCond)
if err == nil {
err = parseErr
breaklet := bp.UserBreaklet()
if breaklet != nil {
breaklet.Cond = nil
if requested.Cond != "" {
breaklet.Cond, err = parser.ParseExpr(requested.Cond)
}
if parseErr == nil {
bp.HitCond = &struct {
Op token.Token
Val int
}{opTok, val}
breaklet.HitCond = nil
if requested.HitCond != "" {
opTok, val, parseErr := parseHitCondition(requested.HitCond)
if err == nil {
err = parseErr
}
if parseErr == nil {
breaklet.HitCond = &struct {
Op token.Token
Val int
}{opTok, val}
}
}
}
return err

1
service/test/integration2_test.go
View File

@ -510,6 +510,7 @@ func TestClientServer_clearBreakpoint(t *testing.T) {
func TestClientServer_toggleBreakpoint(t *testing.T) {
withTestClient2("testtoggle", t, func(c service.Client) {
toggle := func(bp *api.Breakpoint) {
t.Helper()
dbp, err := c.ToggleBreakpoint(bp.ID)
if err != nil {
t.Fatalf("Unexpected error: %v", err)