Skip to content

Commit

Permalink
Automatically disable breakpoints with hitcount conditions that will …
Browse files Browse the repository at this point in the history 
…never be satisfied again (go-delve#2833)

* service/debugger: disable breakpoints with hitcond not satisfiable

To avoid slowing down the debugged process unnecessarily, we disable
breakpoints with a hit condition that can no longer be hit again.

* test: add integration tests for hit conditions no more satisfiable

* proc/test: fix typo in breakpoints related tests

* test: use the new API for hitcond integration tests
  • Loading branch information
@pippolo84
pippolo84 authored Jan 6, 2022
1 parent 7afe640 commit 79d5db2
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 10 deletions.
10 changes: 5 additions & 5 deletions pkg/proc/proc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1679,7 +1679,7 @@ func TestCondBreakpoint(t *testing.T) {

n, _ := constant.Int64Val(nvar.Value)
if n != 7 {
t.Fatalf("Stoppend on wrong goroutine %d\n", n)
t.Fatalf("Stopped on wrong goroutine %d\n", n)
}
})
}
Expand Down Expand Up @@ -1720,7 +1720,7 @@ func TestCondBreakpointError(t *testing.T) {

n, _ := constant.Int64Val(nvar.Value)
if n != 7 {
t.Fatalf("Stoppend on wrong goroutine %d\n", n)
t.Fatalf("Stopped on wrong goroutine %d\n", n)
}
}
})
Expand All @@ -1739,7 +1739,7 @@ func TestHitCondBreakpointEQ(t *testing.T) {

i, _ := constant.Int64Val(ivar.Value)
if i != 3 {
t.Fatalf("Stoppend on wrong hitcount %d\n", i)
t.Fatalf("Stopped on wrong hitcount %d\n", i)
}

err := p.Continue()
Expand All @@ -1764,7 +1764,7 @@ func TestHitCondBreakpointGEQ(t *testing.T) {

i, _ := constant.Int64Val(ivar.Value)
if int(i) != it {
t.Fatalf("Stoppend on wrong hitcount %d\n", i)
t.Fatalf("Stopped on wrong hitcount %d\n", i)
}
}

Expand All @@ -1787,7 +1787,7 @@ func TestHitCondBreakpointREM(t *testing.T) {

i, _ := constant.Int64Val(ivar.Value)
if int(i) != it {
t.Fatalf("Stoppend on wrong hitcount %d\n", i)
t.Fatalf("Stopped on wrong hitcount %d\n", i)
}
}

Expand Down
1 change: 1 addition & 0 deletions service/dap/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1346,6 +1346,7 @@ func (s *Session) setBreakpoints(prefix string, totalBps int, metadataFunc func(
if _, ok := createdBps[want.name]; ok {
err = fmt.Errorf("breakpoint already exists")
} else {
got.Disabled = false
got.Cond = want.condition
got.HitCond = want.hitCondition
err = setLogMessage(got, want.logMessage)
Expand Down
59 changes: 54 additions & 5 deletions service/debugger/debugger.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -770,12 +770,10 @@ func (d *Debugger) CreateEBPFTracepoint(fnName string) error {
return d.target.SetEBPFTracepoint(fnName)
}

// AmendBreakpoint will update the breakpoint with the matching ID.
// amendBreakpoint will update the breakpoint with the matching ID.
// It also enables or disables the breakpoint.
func (d *Debugger) AmendBreakpoint(amend *api.Breakpoint) error {
d.targetMutex.Lock()
defer d.targetMutex.Unlock()

// We can consume this function to avoid locking a goroutine.
func (d *Debugger) amendBreakpoint(amend *api.Breakpoint) error {
originals := d.findBreakpoint(amend.ID)

if len(originals) > 0 && originals[0].WatchExpr != "" && amend.Disabled {
Expand All @@ -792,6 +790,17 @@ func (d *Debugger) AmendBreakpoint(amend *api.Breakpoint) error {
return err
}
copyBreakpointInfo(bp, amend)
if breaklet := bp.UserBreaklet(); breaklet != nil {
breaklet.TotalHitCount = amend.TotalHitCount
breaklet.HitCount = map[int]uint64{}
for idx := range amend.HitCount {
i, err := strconv.Atoi(idx)
if err != nil {
return fmt.Errorf("can't convert goroutine ID: %w", err)
}
breaklet.HitCount[i] = amend.HitCount[idx]
}
}
delete(d.disabledBreakpoints, amend.ID)
}
if amend.Disabled && !disabled { // disable the breakpoint
Expand All @@ -809,6 +818,15 @@ func (d *Debugger) AmendBreakpoint(amend *api.Breakpoint) error {
return nil
}

// AmendBreakpoint will update the breakpoint with the matching ID.
// It also enables or disables the breakpoint.
func (d *Debugger) AmendBreakpoint(amend *api.Breakpoint) error {
d.targetMutex.Lock()
defer d.targetMutex.Unlock()

return d.amendBreakpoint(amend)
}

// CancelNext will clear internal breakpoints, thus cancelling the 'next',
// 'step' or 'stepout' operation.
func (d *Debugger) CancelNext() error {
Expand Down Expand Up @@ -957,6 +975,33 @@ func (d *Debugger) clearBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoint
return clearedBp, nil
}

// isBpHitCondNotSatisfiable returns true if the breakpoint bp has a hit
// condition that is no more satisfiable.
// The hit condition is considered no more satisfiable if it can no longer be
// hit again, for example with {Op: "==", Val: 1} and TotalHitCount == 1.
func isBpHitCondNotSatisfiable(bp *api.Breakpoint) bool {
if bp.HitCond == "" {
return false
}

tok, val, err := parseHitCondition(bp.HitCond)
if err != nil {
return false
}
switch tok {
case token.EQL, token.LEQ:
if int(bp.TotalHitCount) >= val {
return true
}
case token.LSS:
if int(bp.TotalHitCount) >= val-1 {
return true
}
}

return false
}

// Breakpoints returns the list of current breakpoints.
func (d *Debugger) Breakpoints(all bool) []*api.Breakpoint {
d.targetMutex.Lock()
Expand Down Expand Up @@ -1284,6 +1329,10 @@ func (d *Debugger) Command(command *api.DebuggerCommand, resumeNotify chan struc
}
}
}
if bp := state.CurrentThread.Breakpoint; bp != nil && isBpHitCondNotSatisfiable(bp) {
bp.Disabled = true
d.amendBreakpoint(bp)
}
return state, err
}

Expand Down
115 changes: 115 additions & 0 deletions service/test/integration2_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,121 @@ func TestClientServer_toggleAmendedBreakpoint(t *testing.T) {
})
}

func TestClientServer_disableHitCondLSSBreakpoint(t *testing.T) {
withTestClient2("break", t, func(c service.Client) {
fp := testProgPath(t, "break")
hitCondBp, err := c.CreateBreakpoint(&api.Breakpoint{
File: fp,
Line: 7,
HitCond: "< 3",
})
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

state := <-c.Continue()
if state.Err != nil {
t.Fatalf("Unexpected error: %v, state: %#v", state.Err, state)
}

f, l := state.CurrentThread.File, state.CurrentThread.Line
if f != "break.go" && l != 7 {
t.Fatal("Program did not hit breakpoint")
}

ivar, err := c.EvalVariable(api.EvalScope{GoroutineID: -1}, "i", normalLoadConfig)
assertNoError(err, t, "EvalVariable")

t.Logf("ivar: %s", ivar.SinglelineString())

if ivar.Value != "1" {
t.Fatalf("Wrong variable value: %s", ivar.Value)
}

bp, err := c.GetBreakpoint(hitCondBp.ID)
assertNoError(err, t, "GetBreakpoint()")

if bp.Disabled {
t.Fatalf(
"Hit condition %s is still satisfiable but breakpoint has been disabled",
bp.HitCond,
)
}

state = <-c.Continue()
if state.Err != nil {
t.Fatalf("Unexpected error: %v, state: %#v", state.Err, state)
}

f, l = state.CurrentThread.File, state.CurrentThread.Line
if f != "break.go" && l != 7 {
t.Fatal("Program did not hit breakpoint")
}

ivar, err = c.EvalVariable(api.EvalScope{GoroutineID: -1}, "i", normalLoadConfig)
assertNoError(err, t, "EvalVariable")

t.Logf("ivar: %s", ivar.SinglelineString())

if ivar.Value != "2" {
t.Fatalf("Wrong variable value: %s", ivar.Value)
}

bp, err = c.GetBreakpoint(hitCondBp.ID)
assertNoError(err, t, "GetBreakpoint()")

if !bp.Disabled {
t.Fatalf(
"Hit condition %s is no more satisfiable but breakpoint has not been disabled",
bp.HitCond,
)
}
})
}

func TestClientServer_disableHitEQLCondBreakpoint(t *testing.T) {
withTestClient2("break", t, func(c service.Client) {
fp := testProgPath(t, "break")
hitCondBp, err := c.CreateBreakpoint(&api.Breakpoint{
File: fp,
Line: 7,
HitCond: "== 3",
})
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

state := <-c.Continue()
if state.Err != nil {
t.Fatalf("Unexpected error: %v, state: %#v", state.Err, state)
}

f, l := state.CurrentThread.File, state.CurrentThread.Line
if f != "break.go" && l != 7 {
t.Fatal("Program did not hit breakpoint")
}

ivar, err := c.EvalVariable(api.EvalScope{GoroutineID: -1}, "i", normalLoadConfig)
assertNoError(err, t, "EvalVariable")

t.Logf("ivar: %s", ivar.SinglelineString())

if ivar.Value != "3" {
t.Fatalf("Wrong variable value: %s", ivar.Value)
}

bp, err := c.GetBreakpoint(hitCondBp.ID)
assertNoError(err, t, "GetBreakpoint()")

if !bp.Disabled {
t.Fatalf(
"Hit condition %s is no more satisfiable but breakpoint has not been disabled",
bp.HitCond,
)
}
})
}

func TestClientServer_switchThread(t *testing.T) {
protest.AllowRecording(t)
withTestClient2("testnextprog", t, func(c service.Client) {
Expand Down

0 comments on commit 79d5db2

Please sign in to comment.