diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index e7acf601..798aefc5 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -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) } }) } @@ -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) } } }) @@ -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() @@ -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) } } @@ -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) } } diff --git a/service/dap/server.go b/service/dap/server.go index e8be53f3..0ca8d605 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -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) diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index d5a721b1..17c562ba 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -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 { @@ -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 @@ -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 { @@ -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() @@ -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 } diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index ce386910..26fc6480 100644 --- a/service/test/integration2_test.go +++ b/service/test/integration2_test.go @@ -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) {