proc,service: represent logical breakpoints explicitly

Adds a LogicalBreakpoint type to represent logical breakpoints
explicitly. Until now logical breakpoints were constructed implicitly
by grouping physical breakpoints together by their LogicalID.

Having logical breakpoints represented explicitly allows for a simpler
implementation of disabled breakpoints, as well as allowing a simple
implementation of delayed breakpoints (go-delve#1653, go-delve#2551) and in general of
breakpoints spanning multiple processes if we implement debugging
process trees (go-delve#2551).

Updates go-delve#1653
Updates go-delve#2551

Author: aarzilli

pkg/proc/breakpoints.go

@@ -47,7 +47,6 @@ type Breakpoint struct {
 
 	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
 
 	WatchExpr     string
 	WatchType     WatchType
@@ -59,14 +58,7 @@ type Breakpoint struct {
 	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
-	UserData    interface{} // Any additional information about the breakpoint
+	Logical *LogicalBreakpoint
 
 	// ReturnInfo describes how to collect return variables when this
 	// breakpoint is hit as a return breakpoint.
@@ -85,9 +77,6 @@ type Breaklet struct {
 	// 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
-
 	// DeferReturns: when kind == NextDeferBreakpoint this breakpoint
 	// will also check if the caller is runtime.gopanic or if the return
 	// address is in the DeferReturns array.
@@ -99,13 +88,6 @@ type Breaklet struct {
 	// the function, not when the function is called directly
 	DeferReturns []uint64
 
-	// 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
-	}
-
 	// checkPanicCall checks that the breakpoint happened while the function was
 	// called by a panic. It is only checked for WatchOutOfScopeBreakpoint Kind.
 	checkPanicCall bool
@@ -206,10 +188,12 @@ func (bp *Breakpoint) VerboseDescr() []string {
 		r = append(r, fmt.Sprintf("HWBreakIndex=%#x watchStackOff=%#x", bp.HWBreakIndex, bp.watchStackOff))
 	}
 
+	lbp := bp.Logical
+
 	for _, breaklet := range bp.Breaklets {
 		switch breaklet.Kind {
 		case UserBreakpoint:
-			r = append(r, fmt.Sprintf("User Cond=%q HitCond=%v", exprToString(breaklet.Cond), breaklet.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:
@@ -280,11 +264,14 @@ func (bpstate *BreakpointState) checkCond(tgt *Target, breaklet *Breaklet, threa
 
 	switch breaklet.Kind {
 	case UserBreakpoint:
-		if g, err := GetG(thread); err == nil {
-			breaklet.HitCount[g.ID]++
+		lbp := bpstate.Breakpoint.Logical
+		if lbp != nil {
+			if g, err := GetG(thread); err == nil {
+				lbp.HitCount[g.ID]++
+			}
+			lbp.TotalHitCount++
 		}
-		breaklet.TotalHitCount++
-		active = checkHitCond(breaklet)
+		active = checkHitCond(lbp)
 
 	case StepBreakpoint, NextBreakpoint, NextDeferBreakpoint:
 		nextDeferOk := true
@@ -331,26 +318,26 @@ func (bpstate *BreakpointState) checkCond(tgt *Target, breaklet *Breaklet, threa
 }
 
 // checkHitCond evaluates bp's hit condition on thread.
-func checkHitCond(breaklet *Breaklet) bool {
-	if breaklet.HitCond == nil {
+func checkHitCond(lbp *LogicalBreakpoint) bool {
+	if lbp == nil || lbp.HitCond == nil {
 		return true
 	}
 	// Evaluate the breakpoint condition.
-	switch breaklet.HitCond.Op {
+	switch lbp.HitCond.Op {
 	case token.EQL:
-		return int(breaklet.TotalHitCount) == breaklet.HitCond.Val
+		return int(lbp.TotalHitCount) == lbp.HitCond.Val
 	case token.NEQ:
-		return int(breaklet.TotalHitCount) != breaklet.HitCond.Val
+		return int(lbp.TotalHitCount) != lbp.HitCond.Val
 	case token.GTR:
-		return int(breaklet.TotalHitCount) > breaklet.HitCond.Val
+		return int(lbp.TotalHitCount) > lbp.HitCond.Val
 	case token.LSS:
-		return int(breaklet.TotalHitCount) < breaklet.HitCond.Val
+		return int(lbp.TotalHitCount) < lbp.HitCond.Val
 	case token.GEQ:
-		return int(breaklet.TotalHitCount) >= breaklet.HitCond.Val
+		return int(lbp.TotalHitCount) >= lbp.HitCond.Val
 	case token.LEQ:
-		return int(breaklet.TotalHitCount) <= breaklet.HitCond.Val
+		return int(lbp.TotalHitCount) <= lbp.HitCond.Val
 	case token.REM:
-		return int(breaklet.TotalHitCount)%breaklet.HitCond.Val == 0
+		return int(lbp.TotalHitCount)%lbp.HitCond.Val == 0
 	}
 	return false
 }
@@ -468,6 +455,9 @@ func (nbp NoBreakpointError) Error() string {
 type BreakpointMap struct {
 	M map[uint64]*Breakpoint
 
+	// Logical is a map of logical breakpoints.
+	Logical map[int]*LogicalBreakpoint
+
 	// WatchOutOfScope is the list of watchpoints that went out of scope during
 	// the last resume operation
 	WatchOutOfScope []*Breakpoint
@@ -651,23 +641,42 @@ func (t *Target) setBreakpointInternal(logicalID int, addr uint64, kind Breakpoi
 	bpmap := t.Breakpoints()
 	newBreaklet := &Breaklet{Kind: kind, Cond: cond}
 	if kind == UserBreakpoint {
-		newBreaklet.HitCount = map[int]uint64{}
 		newBreaklet.LogicalID = logicalID
 	}
+
+	setLogicalBreakpoint := func(bp *Breakpoint) {
+		if kind != UserBreakpoint || bp.Logical != nil {
+			return
+		}
+		if bpmap.Logical == nil {
+			bpmap.Logical = make(map[int]*LogicalBreakpoint)
+		}
+		lbp := bpmap.Logical[logicalID]
+		if lbp == nil {
+			lbp = &LogicalBreakpoint{LogicalID: logicalID}
+			lbp.HitCount = make(map[int]uint64)
+			lbp.Enabled = true
+			bpmap.Logical[logicalID] = lbp
+		}
+		bp.Logical = lbp
+		breaklet := bp.UserBreaklet()
+		if breaklet != nil && breaklet.Cond == nil {
+			breaklet.Cond = lbp.Cond
+		}
+		lbp.File = bp.File
+		lbp.Line = bp.Line
+		fn := t.BinInfo().PCToFunc(bp.Addr)
+		if fn != nil {
+			lbp.FunctionName = fn.NameWithoutTypeParams()
+		}
+	}
+
 	if bp, ok := bpmap.M[addr]; ok {
 		if !bp.canOverlap(kind) {
 			return bp, BreakpointExistsError{bp.File, bp.Line, bp.Addr}
 		}
-		if kind == UserBreakpoint {
-			bp.Tracepoint = false
-			bp.TraceReturn = false
-			bp.Goroutine = false
-			bp.Stacktrace = 0
-			bp.Variables = nil
-			bp.LoadArgs = nil
-			bp.LoadLocals = nil
-		}
 		bp.Breaklets = append(bp.Breaklets, newBreaklet)
+		setLogicalBreakpoint(bp)
 		return bp, nil
 	}
 
@@ -708,6 +717,7 @@ func (t *Target) setBreakpointInternal(logicalID int, addr uint64, kind Breakpoi
 	}
 
 	newBreakpoint.Breaklets = append(newBreakpoint.Breaklets, newBreaklet)
+	setLogicalBreakpoint(newBreakpoint)
 
 	bpmap.M[addr] = newBreakpoint
 
@@ -741,6 +751,9 @@ func (t *Target) ClearBreakpoint(addr uint64) error {
 	for i := range bp.Breaklets {
 		if bp.Breaklets[i].Kind == UserBreakpoint {
 			bp.Breaklets[i] = nil
+			if bp.WatchExpr == "" {
+				bp.Logical = nil
+			}
 		}
 	}
 
@@ -805,6 +818,9 @@ func (t *Target) finishClearBreakpoint(bp *Breakpoint) (bool, error) {
 	}
 
 	delete(t.Breakpoints().M, bp.Addr)
+	if bp.WatchExpr != "" && bp.Logical != nil {
+		delete(t.Breakpoints().Logical, bp.Logical.LogicalID)
+	}
 	return true, nil
 }
 
@@ -925,3 +941,36 @@ func returnInfoError(descr string, err error, mem MemoryReadWriter) []*Variable
 	v.Name = "return value read error"
 	return []*Variable{v}
 }
+
+// LogicalBreakpoint represents a breakpoint set by a user.
+type LogicalBreakpoint struct {
+	LogicalID    int
+	Name         string
+	FunctionName string
+	File         string
+	Line         int
+	Enabled      bool
+
+	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
+
+	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
+
+	// 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
+	}
+
+	// Cond: if not nil the breakpoint will be triggered only if evaluating Cond returns true
+	Cond ast.Expr
+
+	UserData interface{} // Any additional information about the breakpoint
+}

pkg/proc/gdbserial/rr_test.go

@@ -128,7 +128,7 @@ func setFileBreakpoint(p *proc.Target, t *testing.T, fixture protest.Fixture, li
 	if len(addrs) != 1 {
 		t.Fatalf("%s:%d: setFileLineBreakpoint(%s, %d): too many results %v", f, l, fixture.Source, lineno, addrs)
 	}
-	bp, err := p.SetBreakpoint(0, addrs[0], proc.UserBreakpoint, nil)
+	bp, err := p.SetBreakpoint(int(addrs[0]), addrs[0], proc.UserBreakpoint, nil)
 	if err != nil {
 		t.Fatalf("%s:%d: SetBreakpoint: %v", f, l, err)
 	}
@@ -164,18 +164,18 @@ func TestReverseBreakpointCounts(t *testing.T) {
 			}
 		}
 
-		t.Logf("TotalHitCount: %d", bp.UserBreaklet().TotalHitCount)
-		if bp.UserBreaklet().TotalHitCount != 200 {
-			t.Fatalf("Wrong TotalHitCount for the breakpoint (%d)", bp.UserBreaklet().TotalHitCount)
+		t.Logf("TotalHitCount: %d", bp.Logical.TotalHitCount)
+		if bp.Logical.TotalHitCount != 200 {
+			t.Fatalf("Wrong TotalHitCount for the breakpoint (%d)", bp.Logical.TotalHitCount)
 		}
 
-		if len(bp.UserBreaklet().HitCount) != 2 {
-			t.Fatalf("Wrong number of goroutines for breakpoint (%d)", len(bp.UserBreaklet().HitCount))
+		if len(bp.Logical.HitCount) != 2 {
+			t.Fatalf("Wrong number of goroutines for breakpoint (%d)", len(bp.Logical.HitCount))
 		}
 
-		for _, v := range bp.UserBreaklet().HitCount {
+		for _, v := range bp.Logical.HitCount {
 			if v != 100 {
-				t.Fatalf("Wrong HitCount for breakpoint (%v)", bp.UserBreaklet().HitCount)
+				t.Fatalf("Wrong HitCount for breakpoint (%v)", bp.Logical.HitCount)
 			}
 		}
 	})