pkg/proc,service/debugger: do not disable unsatisfiable breakpoints (#3868)

Previously breakpoints with hitcount conditions that became
unsatisfiable
would become disabled, this was done as an optimization so that the
continue loop would no longer need to stop on them and evaluate their
conditions.
As a side effect this meant that on restart these breakpoints would
remain disabled, even though their hit condition returned satisfiable.

This commit changes Delve behavior so that breakpoints with
unsatisifiable hitcount conditions are no longer disabled but the
associated physical breakpoints are removed anyway, preserving the
optimization.

Some refactoring is done to the way conditions are represented and the
enable status is managed so that in the future it will be possible to
use hitcount conditions to implement "chained" breakpoints (also known
as dependet breakpoints), i.e. breakpoints that become active only
after a second breakpoint has been hit.

pkg/proc/breakpoints.go

@@ -1,12 +1,14 @@
 package proc
 
 import (
+	"bytes"
 	"debug/dwarf"
 	"errors"
 	"fmt"
 	"go/ast"
 	"go/constant"
 	"go/parser"
+	"go/printer"
 	"go/token"
 	"reflect"
 
@@ -211,7 +213,7 @@ func (bp *Breakpoint) VerboseDescr() []string {
 	for _, breaklet := range bp.Breaklets {
 		switch breaklet.Kind {
 		case UserBreakpoint:
-			r = append(r, fmt.Sprintf("User Cond=%q HitCond=%v", exprToString(breaklet.Cond), lbp.HitCond))
+			r = append(r, fmt.Sprintf("User Cond=%q HitCond=%v", exprToString(breaklet.Cond), lbp.hitCond))
 		case NextBreakpoint:
 			r = append(r, fmt.Sprintf("Next Cond=%q", exprToString(breaklet.Cond)))
 		case NextDeferBreakpoint:
@@ -360,7 +362,7 @@ func (bpstate *BreakpointState) checkCond(tgt *Target, breaklet *Breaklet, threa
 
 // checkHitCond evaluates bp's hit condition on thread.
 func checkHitCond(lbp *LogicalBreakpoint, goroutineID int64) bool {
-	if lbp == nil || lbp.HitCond == nil {
+	if lbp == nil || lbp.hitCond == nil {
 		return true
 	}
 	hitCount := int(lbp.TotalHitCount)
@@ -368,21 +370,21 @@ func checkHitCond(lbp *LogicalBreakpoint, goroutineID int64) bool {
 		hitCount = int(lbp.HitCount[goroutineID])
 	}
 	// Evaluate the breakpoint condition.
-	switch lbp.HitCond.Op {
+	switch lbp.hitCond.Op {
 	case token.EQL:
-		return hitCount == lbp.HitCond.Val
+		return hitCount == lbp.hitCond.Val
 	case token.NEQ:
-		return hitCount != lbp.HitCond.Val
+		return hitCount != lbp.hitCond.Val
 	case token.GTR:
-		return hitCount > lbp.HitCond.Val
+		return hitCount > lbp.hitCond.Val
 	case token.LSS:
-		return hitCount < lbp.HitCond.Val
+		return hitCount < lbp.hitCond.Val
 	case token.GEQ:
-		return hitCount >= lbp.HitCond.Val
+		return hitCount >= lbp.hitCond.Val
 	case token.LEQ:
-		return hitCount <= lbp.HitCond.Val
+		return hitCount <= lbp.hitCond.Val
 	case token.REM:
-		return hitCount%lbp.HitCond.Val == 0
+		return hitCount%lbp.hitCond.Val == 0
 	}
 	return false
 }
@@ -699,13 +701,14 @@ func (t *Target) setBreakpointInternal(logicalID int, addr uint64, kind Breakpoi
 		if lbp == nil {
 			lbp = &LogicalBreakpoint{LogicalID: logicalID}
 			lbp.HitCount = make(map[int64]uint64)
-			lbp.Enabled = true
+			lbp.enabled = true
+			lbp.condSatisfiable = true
 			bpmap.Logical[logicalID] = lbp
 		}
 		bp.Logical = lbp
 		breaklet := bp.UserBreaklet()
 		if breaklet != nil && breaklet.Cond == nil {
-			breaklet.Cond = lbp.Cond
+			breaklet.Cond = lbp.cond
 		}
 		if lbp.File == "" && lbp.Line == 0 {
 			lbp.File = bp.File
@@ -1032,7 +1035,7 @@ type LogicalBreakpoint struct {
 	FunctionName string
 	File         string
 	Line         int
-	Enabled      bool
+	enabled      bool
 
 	Set SetBreakpoint
 
@@ -1048,15 +1051,18 @@ type LogicalBreakpoint struct {
 	TotalHitCount uint64           // Number of times a breakpoint has been reached
 	HitCondPerG   bool             // Use per goroutine hitcount as HitCond operand, instead of total hitcount
 
-	// HitCond: if not nil the breakpoint will be triggered only if the evaluated HitCond returns
+	// hitCond: if not nil the breakpoint will be triggered only if the evaluated HitCond returns
 	// true with the TotalHitCount.
-	HitCond *struct {
+	hitCond *struct {
 		Op  token.Token
 		Val int
 	}
 
-	// Cond: if not nil the breakpoint will be triggered only if evaluating Cond returns true
+	// cond: if not nil the breakpoint will be triggered only if evaluating Cond returns true
-	Cond ast.Expr
+	cond ast.Expr
+
+	// condSatisfiable is true when 'cond && hitCond' can potentially be true.
+	condSatisfiable bool
 
 	UserData interface{} // Any additional information about the breakpoint
 	// Name of root function from where tracing needs to be done
@@ -1079,3 +1085,35 @@ type PidAddr struct {
 	Pid  int
 	Addr uint64
 }
+
+// Enabled returns true if the breakpoint is enabled.
+func (lbp *LogicalBreakpoint) Enabled() bool {
+	return lbp.enabled
+}
+
+// HitCond returns the hit condition.
+func (lbp *LogicalBreakpoint) HitCond() string {
+	if lbp.hitCond == nil {
+		return ""
+	}
+	return fmt.Sprintf("%s %d", lbp.hitCond.Op.String(), lbp.hitCond.Val)
+}
+
+func (lbp *LogicalBreakpoint) Cond() string {
+	var buf bytes.Buffer
+	printer.Fprint(&buf, token.NewFileSet(), lbp.cond)
+	return buf.String()
+}
+
+func breakpointConditionSatisfiable(lbp *LogicalBreakpoint) bool {
+	if lbp.hitCond == nil || lbp.HitCondPerG {
+		return true
+	}
+	switch lbp.hitCond.Op {
+	case token.EQL, token.LEQ:
+		return int(lbp.TotalHitCount) < lbp.hitCond.Val
+	case token.LSS:
+		return int(lbp.TotalHitCount) < lbp.hitCond.Val-1
+	}
+	return true
+}

pkg/proc/proc_test.go

@@ -1540,10 +1540,7 @@ func TestCondBreakpointError(t *testing.T) {
 func TestHitCondBreakpointEQ(t *testing.T) {
 	withTestProcess("break", t, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) {
 		bp := setFileBreakpoint(p, t, fixture.Source, 7)
-		bp.Logical.HitCond = &struct {
+		grp.ChangeBreakpointCondition(bp.Logical, "", "== 3", false)
-			Op  token.Token
-			Val int
-		}{token.EQL, 3}
 
 		assertNoError(grp.Continue(), t, "Continue()")
 		ivar := evalVariable(p, t, "i")
@@ -1564,10 +1561,7 @@ func TestHitCondBreakpointGEQ(t *testing.T) {
 	protest.AllowRecording(t)
 	withTestProcess("break", t, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) {
 		bp := setFileBreakpoint(p, t, fixture.Source, 7)
-		bp.Logical.HitCond = &struct {
+		grp.ChangeBreakpointCondition(bp.Logical, "", ">= 3", false)
-			Op  token.Token
-			Val int
-		}{token.GEQ, 3}
 
 		for it := 3; it <= 10; it++ {
 			assertNoError(grp.Continue(), t, "Continue()")
@@ -1587,10 +1581,7 @@ func TestHitCondBreakpointREM(t *testing.T) {
 	protest.AllowRecording(t)
 	withTestProcess("break", t, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) {
 		bp := setFileBreakpoint(p, t, fixture.Source, 7)
-		bp.Logical.HitCond = &struct {
+		grp.ChangeBreakpointCondition(bp.Logical, "", "% 2", false)
-			Op  token.Token
-			Val int
-		}{token.REM, 2}
 
 		for it := 2; it <= 10; it += 2 {
 			assertNoError(grp.Continue(), t, "Continue()")
@@ -5123,8 +5114,9 @@ func TestFollowExec(t *testing.T) {
 		grp.LogicalBreakpoints[2] = &proc.LogicalBreakpoint{LogicalID: 2, Set: proc.SetBreakpoint{FunctionName: "main.traceme2"}, HitCount: make(map[int64]uint64)}
 		grp.LogicalBreakpoints[3] = &proc.LogicalBreakpoint{LogicalID: 3, Set: proc.SetBreakpoint{FunctionName: "main.traceme3"}, HitCount: make(map[int64]uint64)}
 
-		assertNoError(grp.EnableBreakpoint(grp.LogicalBreakpoints[1]), t, "EnableBreakpoint(main.traceme1)")
+		assertNoError(grp.SetBreakpointEnabled(grp.LogicalBreakpoints[1], true), t, "EnableBreakpoint(main.traceme1)")
-		assertNoError(grp.EnableBreakpoint(grp.LogicalBreakpoints[3]), t, "EnableBreakpoint(main.traceme3)")
+		assertNoError(grp.SetBreakpointEnabled(grp.LogicalBreakpoints[2], true), t, "EnableBreakpoint(main.traceme2)")
+		assertNoError(grp.SetBreakpointEnabled(grp.LogicalBreakpoints[3], true), t, "EnableBreakpoint(main.traceme3)")
 
 		assertNoError(grp.FollowExec(true, ""), t, "FollowExec")
 
@@ -5297,8 +5289,9 @@ func TestFollowExecRegexFilter(t *testing.T) {
 		grp.LogicalBreakpoints[2] = &proc.LogicalBreakpoint{LogicalID: 2, Set: proc.SetBreakpoint{FunctionName: "main.traceme2"}, HitCount: make(map[int64]uint64)}
 		grp.LogicalBreakpoints[3] = &proc.LogicalBreakpoint{LogicalID: 3, Set: proc.SetBreakpoint{FunctionName: "main.traceme3"}, HitCount: make(map[int64]uint64)}
 
-		assertNoError(grp.EnableBreakpoint(grp.LogicalBreakpoints[1]), t, "EnableBreakpoint(main.traceme1)")
+		assertNoError(grp.SetBreakpointEnabled(grp.LogicalBreakpoints[1], true), t, "EnableBreakpoint(main.traceme1)")
-		assertNoError(grp.EnableBreakpoint(grp.LogicalBreakpoints[3]), t, "EnableBreakpoint(main.traceme3)")
+		assertNoError(grp.SetBreakpointEnabled(grp.LogicalBreakpoints[2], true), t, "EnableBreakpoint(main.traceme2)")
+		assertNoError(grp.SetBreakpointEnabled(grp.LogicalBreakpoints[3], true), t, "EnableBreakpoint(main.traceme3)")
 
 		assertNoError(grp.FollowExec(true, "spawn.* child C1"), t, "FollowExec")
 

pkg/proc/target_exec.go

@@ -78,6 +78,11 @@ func (grp *TargetGroup) Continue() error {
 		}
 	}()
 	for {
+		err := grp.manageUnsatisfiableBreakpoints()
+		if err != nil {
+			return err
+		}
+
 		if grp.cctx.CheckAndClearManualStopRequest() {
 			grp.finishManualStop()
 			return nil

pkg/proc/target_group.go

@@ -3,7 +3,10 @@ package proc
 import (
 	"bytes"
 	"fmt"
+	"go/parser"
+	"go/token"
 	"regexp"
+	"strconv"
 	"strings"
 
 	"github.com/go-delve/delve/pkg/logflags"
@@ -76,8 +79,9 @@ func Restart(grp, oldgrp *TargetGroup, discard func(*LogicalBreakpoint, error))
 		bp.TotalHitCount = 0
 		bp.HitCount = make(map[int64]uint64)
 		bp.Set.PidAddrs = nil // breakpoints set through a list of addresses can not be restored after a restart
-		if bp.Enabled {
+		if bp.enabled {
-			err := grp.EnableBreakpoint(bp)
+			bp.condSatisfiable = breakpointConditionSatisfiable(bp)
+			err := grp.enableBreakpoint(bp)
 			if err != nil {
 				if discard != nil {
 					discard(bp, err)
@@ -253,8 +257,22 @@ func (grp *TargetGroup) TargetForThread(tid int) *Target {
 	return nil
 }
 
-// EnableBreakpoint re-enables a disabled logical breakpoint.
+// SetBreakpointEnabled either enables or disabled the specified breakpoint based on the value of enabled.
-func (grp *TargetGroup) EnableBreakpoint(lbp *LogicalBreakpoint) error {
+func (grp *TargetGroup) SetBreakpointEnabled(lbp *LogicalBreakpoint, enabled bool) (err error) {
+	switch {
+	case lbp.enabled && !enabled:
+		lbp.enabled = false
+		err = grp.disableBreakpoint(lbp)
+	case !lbp.enabled && enabled:
+		lbp.enabled = true
+		lbp.condSatisfiable = breakpointConditionSatisfiable(lbp)
+		err = grp.enableBreakpoint(lbp)
+	}
+	return
+}
+
+// enableBreakpoint re-enables a disabled logical breakpoint.
+func (grp *TargetGroup) enableBreakpoint(lbp *LogicalBreakpoint) error {
 	var err0, errNotFound, errExists error
 	didSet := false
 targetLoop:
@@ -297,11 +315,13 @@ targetLoop:
 		}
 		return err0
 	}
-	lbp.Enabled = true
 	return nil
 }
 
 func enableBreakpointOnTarget(p *Target, lbp *LogicalBreakpoint) error {
+	if !lbp.enabled || !lbp.condSatisfiable {
+		return nil
+	}
 	var err error
 	var addrs []uint64
 	switch {
@@ -338,8 +358,8 @@ func enableBreakpointOnTarget(p *Target, lbp *LogicalBreakpoint) error {
 	return err
 }
 
-// DisableBreakpoint disables a logical breakpoint.
+// disableBreakpoint disables a logical breakpoint.
-func (grp *TargetGroup) DisableBreakpoint(lbp *LogicalBreakpoint) error {
+func (grp *TargetGroup) disableBreakpoint(lbp *LogicalBreakpoint) error {
 	var errs []error
 	n := 0
 	it := ValidTargets{Group: grp}
@@ -368,7 +388,110 @@ func (grp *TargetGroup) DisableBreakpoint(lbp *LogicalBreakpoint) error {
 		}
 		return fmt.Errorf("unable to clear breakpoint %d (partial): %s", lbp.LogicalID, buf.String())
 	}
-	lbp.Enabled = false
+	return nil
+}
+
+// ChangeBreakpointCondition changes the breakpoint condition of lbp.
+func (grp *TargetGroup) ChangeBreakpointCondition(lbp *LogicalBreakpoint, cond, hitCond string, hitCondPerG bool) error {
+	lbp.cond = nil
+	if cond != "" {
+		var err error
+		lbp.cond, err = parser.ParseExpr(cond)
+		if err != nil {
+			return err
+		}
+	}
+
+	t := ValidTargets{Group: grp}
+	for t.Next() {
+		for _, bp := range t.Breakpoints().M {
+			if bp.LogicalID() == lbp.LogicalID {
+				bp.UserBreaklet().Cond = lbp.cond
+			}
+		}
+	}
+
+	lbp.hitCond = nil
+	if hitCond != "" {
+		opTok, val, err := parseHitCondition(hitCond)
+		if err != nil {
+			return err
+		}
+		lbp.hitCond = &struct {
+			Op  token.Token
+			Val int
+		}{opTok, val}
+		lbp.HitCondPerG = hitCondPerG
+	}
+
+	if lbp.enabled {
+		switch {
+		case lbp.condSatisfiable && !breakpointConditionSatisfiable(lbp):
+			lbp.condSatisfiable = false
+			grp.disableBreakpoint(lbp)
+		case !lbp.condSatisfiable && breakpointConditionSatisfiable(lbp):
+			lbp.condSatisfiable = true
+			grp.enableBreakpoint(lbp)
+		}
+	}
+
+	return nil
+}
+
+func parseHitCondition(hitCond string) (token.Token, int, error) {
+	// A hit condition can be in the following formats:
+	// - "number"
+	// - "OP number"
+	hitConditionRegex := regexp.MustCompile(`(([=><%!])+|)( |)((\d|_)+)`)
+
+	match := hitConditionRegex.FindStringSubmatch(strings.TrimSpace(hitCond))
+	if match == nil || len(match) != 6 {
+		return 0, 0, fmt.Errorf("unable to parse breakpoint hit condition: %q\nhit conditions should be of the form \"number\" or \"OP number\"", hitCond)
+	}
+
+	opStr := match[1]
+	var opTok token.Token
+	switch opStr {
+	case "==", "":
+		opTok = token.EQL
+	case ">=":
+		opTok = token.GEQ
+	case "<=":
+		opTok = token.LEQ
+	case ">":
+		opTok = token.GTR
+	case "<":
+		opTok = token.LSS
+	case "%":
+		opTok = token.REM
+	case "!=":
+		opTok = token.NEQ
+	default:
+		return 0, 0, fmt.Errorf("unable to parse breakpoint hit condition: %q\ninvalid operator: %q", hitCond, opStr)
+	}
+
+	numStr := match[4]
+	val, parseErr := strconv.Atoi(numStr)
+	if parseErr != nil {
+		return 0, 0, fmt.Errorf("unable to parse breakpoint hit condition: %q\ninvalid number: %q", hitCond, numStr)
+	}
+
+	return opTok, val, nil
+}
+
+// manageUnsatisfiableBreakpoints automatically disables breakpoints with unsatisifiable hit conditions.
+func (grp *TargetGroup) manageUnsatisfiableBreakpoints() error {
+	for _, lbp := range grp.LogicalBreakpoints {
+		if lbp.enabled {
+			if lbp.condSatisfiable && !breakpointConditionSatisfiable(lbp) {
+				lbp.condSatisfiable = false
+				err := grp.disableBreakpoint(lbp)
+				if err != nil {
+					return err
+				}
+			}
+		}
+	}
 	return nil
 }
 

pkg/terminal/command_test.go

@@ -1231,7 +1231,6 @@ func TestHitCondBreakpoint(t *testing.T) {
 		if !strings.Contains(out, "2\n") {
 			t.Fatalf("wrong value of j")
 		}
-		term.MustExec("toggle bp1")
 		listIsAt(t, term, "continue", 16, -1, -1)
 		// second g hit
 		out = term.MustExec("print j")

service/api/conversions.go

@@ -1,11 +1,8 @@
 package api
 
 import (
-	"bytes"
 	"fmt"
 	"go/constant"
-	"go/printer"
-	"go/token"
 	"reflect"
 	"sort"
 	"strconv"
@@ -32,7 +29,7 @@ func ConvertLogicalBreakpoint(lbp *proc.LogicalBreakpoint) *Breakpoint {
 		LoadArgs:         LoadConfigFromProc(lbp.LoadArgs),
 		LoadLocals:       LoadConfigFromProc(lbp.LoadLocals),
 		TotalHitCount:    lbp.TotalHitCount,
-		Disabled:         !lbp.Enabled,
+		Disabled:         !lbp.Enabled(),
 		UserData:         lbp.UserData,
 		RootFuncName:     lbp.RootFuncName,
 		TraceFollowCalls: lbp.TraceFollowCalls,
@@ -43,14 +40,10 @@ func ConvertLogicalBreakpoint(lbp *proc.LogicalBreakpoint) *Breakpoint {
 		b.HitCount[strconv.FormatInt(idx, 10)] = lbp.HitCount[idx]
 	}
 
-	if lbp.HitCond != nil {
+	b.HitCond = lbp.HitCond()
-		b.HitCond = fmt.Sprintf("%s %d", lbp.HitCond.Op.String(), lbp.HitCond.Val)
 	b.HitCondPerG = lbp.HitCondPerG
-	}
 
-	var buf bytes.Buffer
+	b.Cond = lbp.Cond()
-	printer.Fprint(&buf, token.NewFileSet(), lbp.Cond)
-	b.Cond = buf.String()
 
 	return b
 }

service/debugger/debugger.go

@@ -7,8 +7,6 @@ import (
 	"debug/pe"
 	"errors"
 	"fmt"
-	"go/parser"
-	"go/token"
 	"io"
 	"os"
 	"os/exec"
@@ -18,7 +16,6 @@ import (
 	"runtime"
 	"slices"
 	"sort"
-	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -768,10 +765,10 @@ func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint, locExpr string,
 		d.breakpointIDCounter = id
 	}
 
-	lbp := &proc.LogicalBreakpoint{LogicalID: id, HitCount: make(map[int64]uint64), Enabled: true}
+	lbp := &proc.LogicalBreakpoint{LogicalID: id, HitCount: make(map[int64]uint64)}
 	d.target.LogicalBreakpoints[id] = lbp
 
-	err = copyLogicalBreakpointInfo(lbp, requestedBp)
+	err = d.copyLogicalBreakpointInfo(lbp, requestedBp)
 	if err != nil {
 		return nil, err
 	}
@@ -790,7 +787,7 @@ func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint, locExpr string,
 		}
 	}
 
-	err = d.target.EnableBreakpoint(lbp)
+	err = d.target.SetBreakpointEnabled(lbp, true)
 	if err != nil {
 		if suspended {
 			logflags.DebuggerLogger().Debugf("could not enable new breakpoint: %v (breakpoint will be suspended)", err)
@@ -847,34 +844,18 @@ func (d *Debugger) amendBreakpoint(amend *api.Breakpoint) error {
 	if original == nil {
 		return fmt.Errorf("no breakpoint with ID %d", amend.ID)
 	}
-	enabledBefore := original.Enabled
+	if d.isWatchpoint(original) && amend.Disabled {
-	err := copyLogicalBreakpointInfo(original, amend)
-	if err != nil {
-		return err
-	}
-	original.Enabled = !amend.Disabled
-
-	switch {
-	case enabledBefore && !original.Enabled:
-		if d.isWatchpoint(original) {
 		return errors.New("can not disable watchpoints")
 	}
-		err = d.target.DisableBreakpoint(original)
+	err := d.copyLogicalBreakpointInfo(original, amend)
-	case !enabledBefore && original.Enabled:
+	if err != nil {
-		err = d.target.EnableBreakpoint(original)
+		return err
 	}
+	err = d.target.SetBreakpointEnabled(original, !amend.Disabled)
 	if err != nil {
 		return err
 	}
 
-	t := proc.ValidTargets{Group: d.target}
-	for t.Next() {
-		for _, bp := range t.Breakpoints().M {
-			if bp.LogicalID() == amend.ID {
-				bp.UserBreaklet().Cond = original.Cond
-			}
-		}
-	}
 	return nil
 }
 
@@ -907,7 +888,7 @@ func (d *Debugger) CancelNext() error {
 	return d.target.ClearSteppingBreakpoints()
 }
 
-func copyLogicalBreakpointInfo(lbp *proc.LogicalBreakpoint, requested *api.Breakpoint) error {
+func (d *Debugger) copyLogicalBreakpointInfo(lbp *proc.LogicalBreakpoint, requested *api.Breakpoint) error {
 	lbp.Name = requested.Name
 	lbp.Tracepoint = requested.Tracepoint
 	lbp.TraceReturn = requested.TraceReturn
@@ -919,70 +900,8 @@ func copyLogicalBreakpointInfo(lbp *proc.LogicalBreakpoint, requested *api.Break
 	lbp.UserData = requested.UserData
 	lbp.RootFuncName = requested.RootFuncName
 	lbp.TraceFollowCalls = requested.TraceFollowCalls
-	lbp.Cond = nil
-	if requested.Cond != "" {
-		var err error
-		lbp.Cond, err = parser.ParseExpr(requested.Cond)
-		if err != nil {
-			return err
-		}
-	}
 
-	lbp.HitCond = nil
+	return d.target.ChangeBreakpointCondition(lbp, requested.Cond, requested.HitCond, requested.HitCondPerG)
-	if requested.HitCond != "" {
-		opTok, val, err := parseHitCondition(requested.HitCond)
-		if err != nil {
-			return err
-		}
-		lbp.HitCond = &struct {
-			Op  token.Token
-			Val int
-		}{opTok, val}
-		lbp.HitCondPerG = requested.HitCondPerG
-	}
-
-	return nil
-}
-
-func parseHitCondition(hitCond string) (token.Token, int, error) {
-	// A hit condition can be in the following formats:
-	// - "number"
-	// - "OP number"
-	hitConditionRegex := regexp.MustCompile(`(([=><%!])+|)( |)((\d|_)+)`)
-
-	match := hitConditionRegex.FindStringSubmatch(strings.TrimSpace(hitCond))
-	if match == nil || len(match) != 6 {
-		return 0, 0, fmt.Errorf("unable to parse breakpoint hit condition: %q\nhit conditions should be of the form \"number\" or \"OP number\"", hitCond)
-	}
-
-	opStr := match[1]
-	var opTok token.Token
-	switch opStr {
-	case "==", "":
-		opTok = token.EQL
-	case ">=":
-		opTok = token.GEQ
-	case "<=":
-		opTok = token.LEQ
-	case ">":
-		opTok = token.GTR
-	case "<":
-		opTok = token.LSS
-	case "%":
-		opTok = token.REM
-	case "!=":
-		opTok = token.NEQ
-	default:
-		return 0, 0, fmt.Errorf("unable to parse breakpoint hit condition: %q\ninvalid operator: %q", hitCond, opStr)
-	}
-
-	numStr := match[4]
-	val, parseErr := strconv.Atoi(numStr)
-	if parseErr != nil {
-		return 0, 0, fmt.Errorf("unable to parse breakpoint hit condition: %q\ninvalid number: %q", hitCond, numStr)
-	}
-
-	return opTok, val, nil
 }
 
 // ClearBreakpoint clears a breakpoint.
@@ -1000,7 +919,7 @@ func (d *Debugger) ClearBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoint
 	lbp := d.target.LogicalBreakpoints[requestedBp.ID]
 	clearedBp := d.convertBreakpoint(lbp)
 
-	err := d.target.DisableBreakpoint(lbp)
+	err := d.target.SetBreakpointEnabled(lbp, false)
 	if err != nil {
 		return nil, err
 	}
@@ -1011,33 +930,6 @@ 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()
@@ -1337,10 +1229,6 @@ 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)
-	}
 
 	d.maybePrintUnattendedStopWarning(d.target.Selected.StopReason, state.CurrentThread, clientStatusCh)
 	return state, err

service/test/integration2_test.go

@@ -625,18 +625,25 @@ func TestClientServer_disableHitCondLSSBreakpoint(t *testing.T) {
 			Line:    7,
 			HitCond: "< 3",
 		})
-		if err != nil {
+		assertNoError(err, t, "CreateBreakpoint")
-			t.Fatalf("Unexpected error: %v", err)
+		bp, err := c.CreateBreakpoint(&api.Breakpoint{File: fp, Line: 8})
+		assertNoError(err, t, "CreateBreakpoint")
+
+		if len(bp.Addrs) == 0 {
+			t.Fatalf("no addresses for breakpoint")
 		}
 
+		continueTo := func(ln int, ival string) {
 			state := <-c.Continue()
-		if state.Err != nil {
+			assertNoError(state.Err, t, fmt.Sprintf("Unexpected error: %v, state: %#v", state.Err, state))
-			t.Fatalf("Unexpected error: %v, state: %#v", state.Err, state)
-		}
 
 			f, l := state.CurrentThread.File, state.CurrentThread.Line
-		if f != "break.go" && l != 7 {
+			if f != fp || l != ln {
-			t.Fatal("Program did not hit breakpoint")
+				t.Fatalf("Program did not hit breakpoint %s:%d", f, l)
+			}
+
+			if ival == "" {
+				return
 			}
 
 			ivar, err := c.EvalVariable(api.EvalScope{GoroutineID: -1}, "i", normalLoadConfig)
@@ -644,47 +651,20 @@ func TestClientServer_disableHitCondLSSBreakpoint(t *testing.T) {
 
 			t.Logf("ivar: %s", ivar.SinglelineString())
 
-		if ivar.Value != "1" {
+			if ivar.Value != ival {
 				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()
+		continueTo(7, "1")
-		if state.Err != nil {
+		continueTo(7, "2")
-			t.Fatalf("Unexpected error: %v, state: %#v", state.Err, state)
+		continueTo(8, "")
-		}
-
-		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 {
+		if len(bp.Addrs) != 0 {
-			t.Fatalf(
+			t.Fatalf("Hit condition %s is no longer satisfiable but breakpoint has not been disabled", bp.HitCond)
-				"Hit condition %s is no more satisfiable but breakpoint has not been disabled",
-				bp.HitCond,
-			)
 		}
 	})
 }
@@ -697,17 +677,19 @@ func TestClientServer_disableHitEQLCondBreakpoint(t *testing.T) {
 			Line:    7,
 			HitCond: "== 3",
 		})
-		if err != nil {
+		assertNoError(err, t, "CreateBreakpoint")
-			t.Fatalf("Unexpected error: %v", err)
+		bp, err := c.CreateBreakpoint(&api.Breakpoint{File: fp, Line: 8})
+		assertNoError(err, t, "CreateBreakpoint")
+
+		if len(bp.Addrs) == 0 {
+			t.Fatalf("no addresses for breakpoint")
 		}
 
 		state := <-c.Continue()
-		if state.Err != nil {
+		assertNoError(state.Err, t, "Continue")
-			t.Fatalf("Unexpected error: %v, state: %#v", state.Err, state)
-		}
 
 		f, l := state.CurrentThread.File, state.CurrentThread.Line
-		if f != "break.go" && l != 7 {
+		if f != fp || l != 7 {
 			t.Fatal("Program did not hit breakpoint")
 		}
 
@@ -720,14 +702,18 @@ func TestClientServer_disableHitEQLCondBreakpoint(t *testing.T) {
 			t.Fatalf("Wrong variable value: %s", ivar.Value)
 		}
 
-		bp, err := c.GetBreakpoint(hitCondBp.ID)
+		state = <-c.Continue()
+		assertNoError(state.Err, t, "Continue")
+
+		if state.CurrentThread.File != fp || state.CurrentThread.Line != 8 {
+			t.Fatal("Program did not hit breakpoint")
+		}
+
+		bp, err = c.GetBreakpoint(hitCondBp.ID)
 		assertNoError(err, t, "GetBreakpoint()")
 
-		if !bp.Disabled {
+		if len(bp.Addrs) != 0 {
-			t.Fatalf(
+			t.Fatalf("Hit condition %s is no more satisfiable but breakpoint has not been disabled", bp.HitCond)
-				"Hit condition %s is no more satisfiable but breakpoint has not been disabled",
-				bp.HitCond,
-			)
 		}
 	})
 }