service/dap: Implement suspended breakpoints (#4075)

* service/dap: Implement suspended breakpoints

This enables support for 'suspended' breakpoints, for example for a
plugin that hasn't been loaded yet.

Fixes #4074

* Address comments

* Cleanup

pkg/proc/proc_test.go

@@ -4165,6 +4165,49 @@ func TestPluginStepping(t *testing.T) {
 		{contNext, "plugintest2.go:42"}})
 }
 
+func TestBreakpointMaterializedEvent(t *testing.T) {
+	protest.MustHaveCgo(t)
+	pluginFixtures := protest.WithPlugins(t, protest.AllNonOptimized, "plugin1/", "plugin2/")
+
+	withTestProcessArgs("plugintest2", t, ".", []string{pluginFixtures[0].Path, pluginFixtures[1].Path}, protest.AllNonOptimized, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) {
+		path := filepath.Join(fixture.BuildDir, "plugin2", "plugin2.go")
+		const lineno = 23
+
+		// Set a suspended breakpoint.
+		bp2 := &proc.LogicalBreakpoint{
+			LogicalID: 2,
+			HitCount:  make(map[int64]uint64),
+			Set:       proc.SetBreakpoint{File: path, Line: lineno},
+		}
+		grp.LogicalBreakpoints[2] = bp2
+		err := grp.SetBreakpointEnabled(bp2, true)
+		if !errors.As(err, new(*proc.ErrCouldNotFindLine)) {
+			if err == nil {
+				t.Fatal("Expected not to be able to find the breakpoint")
+			} else {
+				t.Fatal(err)
+			}
+		}
+
+		// Collect events
+		var events []*proc.Event
+		grp.SetEventsFn(func(e *proc.Event) { events = append(events, e) })
+
+		// Continue past the plugin load.
+		setFileBreakpoint(p, t, fixture.Source, 35)
+		assertNoError(grp.Continue(), t, "Continue")
+
+		// The breakpoint should be enabled now
+		if !bp2.Enabled() || len(events) == 0 {
+			t.Fatal("Breakpoint did not materialize")
+		}
+		e := events[0]
+		if e.Kind != proc.EventBreakpointMaterialized {
+			t.Fatalf("Wrong event kind: want breakpoint materialized (%v), got %v", proc.EventBreakpointMaterialized, e.Kind)
+		}
+	})
+}
+
 func TestIssue1601(t *testing.T) {
 	protest.MustHaveCgo(t)
 	// Tests that recursive types involving C qualifiers and typedefs are parsed correctly

pkg/proc/target.go

@@ -578,13 +578,27 @@ func (t *Target) dwrapUnwrap(fn *Function) *Function {
 func (t *Target) pluginOpenCallback(Thread, *Target) (bool, error) {
 	logger := logflags.DebuggerLogger()
 	for _, lbp := range t.Breakpoints().Logical {
-		if isSuspended(t, lbp) {
-			err := enableBreakpointOnTarget(t, lbp)
-			if err != nil {
-				logger.Debugf("could not enable breakpoint %d: %v", lbp.LogicalID, err)
-			} else {
-				logger.Debugf("suspended breakpoint %d enabled", lbp.LogicalID)
-			}
+		// If the breakpoint is suspended, materialize it.
+		if !isSuspended(t, lbp) {
+			continue
+		}
+
+		err := enableBreakpointOnTarget(t, lbp)
+		if err != nil {
+			logger.Debugf("could not enable breakpoint %d: %v", lbp.LogicalID, err)
+			continue
+		}
+
+		logger.Debugf("suspended breakpoint %d enabled", lbp.LogicalID)
+
+		// Notify the client.
+		if fn := t.BinInfo().eventsFn; fn != nil {
+			fn(&Event{
+				Kind: EventBreakpointMaterialized,
+				BreakpointMaterializedEventDetails: &BreakpointMaterializedEventDetails{
+					Breakpoint: lbp,
+				},
+			})
 		}
 	}
 	return false, nil

pkg/proc/target_group.go

@@ -575,6 +575,7 @@ func (it *ValidTargets) Reset() {
 type Event struct {
 	Kind EventKind
 	*BinaryInfoDownloadEventDetails
+	*BreakpointMaterializedEventDetails
 }
 
 type EventKind uint8
@@ -583,9 +584,15 @@ const (
 	EventResumed EventKind = iota
 	EventStopped
 	EventBinaryInfoDownload
+	EventBreakpointMaterialized
 )
 
-// BinaryInfoDownloadEventDetails details of a BinaryInfoDownload event.
+// BinaryInfoDownloadEventDetails describes the details of a BinaryInfoDownloadEvent
 type BinaryInfoDownloadEventDetails struct {
 	ImagePath, Progress string
 }
+
+// BreakpointMaterializedEventDetails describes the details of a BreakpointMaterializedEvent
+type BreakpointMaterializedEventDetails struct {
+	Breakpoint *LogicalBreakpoint
+}

pkg/terminal/terminal.go

@@ -137,6 +137,17 @@ func New(client service.Client, conf *config.Config) *Term {
 				if !firstEventBinaryInfoDownload {
 					fmt.Fprintf(t.stdout, "\n")
 				}
+			case api.EventBreakpointMaterialized:
+				bp := event.BreakpointMaterializedEventDetails.Breakpoint
+				file := t.formatPath(bp.File)
+
+				// Append the function name.
+				var extra string
+				if bp.FunctionName != "" {
+					extra = " (" + bp.FunctionName + ")"
+				}
+
+				fmt.Fprintf(t.stdout, "Breakpoint %d materialized at %s:%d%s\n", bp.ID, file, bp.Line, extra)
 			}
 		})
 	}

service/api/conversions.go

@@ -473,5 +473,11 @@ func ConvertEvent(event *proc.Event) *Event {
 		}
 	}
 
+	if event.BreakpointMaterializedEventDetails != nil {
+		r.BreakpointMaterializedEventDetails = &BreakpointMaterializedEventDetails{
+			Breakpoint: ConvertLogicalBreakpoint(event.BreakpointMaterializedEventDetails.Breakpoint),
+		}
+	}
+
 	return r
 }

service/api/types.go

@@ -693,6 +693,7 @@ type GuessSubstitutePathIn struct {
 type Event struct {
 	Kind EventKind
 	*BinaryInfoDownloadEventDetails
+	*BreakpointMaterializedEventDetails
 }
 
 type EventKind uint8
@@ -701,9 +702,15 @@ const (
 	EventResumed EventKind = iota
 	EventStopped
 	EventBinaryInfoDownload
+	EventBreakpointMaterialized
 )
 
 // BinaryInfoDownloadEventDetails describes the details of a BinaryInfoDownloadEvent
 type BinaryInfoDownloadEventDetails struct {
 	ImagePath, Progress string
 }
+
+// BreakpointMaterializedEventDetails describes the details of a BreakpointMaterializedEvent
+type BreakpointMaterializedEventDetails struct {
+	Breakpoint *Breakpoint
+}

service/dap/server.go

@@ -1512,6 +1512,15 @@ func (s *Session) setBreakpoints(prefix string, totalBps int, metadataFunc func(
 	// Any breakpoint that existed before this request but was not amended must be deleted.
 	s.clearBreakpoints(existingBps, createdBps)
 
+	// Check if the plugin package is present or follow-exec is enabled.
+	// Suspended breakpoints are only relevant when new executable code is
+	// loaded. That can happen when a new process is spawned - which requires
+	// follow-exec - or when new executable code is added to an existing binary.
+	// The only fully supported way of doing the latter is plugins, hence this
+	// check.
+	fns, _ := s.debugger.Functions(`^plugin\.Open$`, 0)
+	suspended := len(fns) > 0 || s.debugger.FollowExecEnabled()
+
 	// Add new breakpoints.
 	for i := range totalBps {
 		want := metadataFunc(i)
@@ -1537,7 +1546,7 @@ func (s *Session) setBreakpoints(prefix string, totalBps int, metadataFunc func(
 				err = setLogMessage(bp, want.logMessage)
 				if err == nil {
 					// Create new breakpoints.
-					got, err = s.debugger.CreateBreakpoint(bp, "", nil, false)
+					got, err = s.debugger.CreateBreakpoint(bp, "", nil, suspended)
 				}
 			}
 		}
@@ -1560,12 +1569,27 @@ func setLogMessage(bp *api.Breakpoint, msg string) error {
 }
 
 func (s *Session) updateBreakpointsResponse(breakpoints []dap.Breakpoint, i int, err error, got *api.Breakpoint) {
+	// TODO(@Lslightly): For DAP v1.68.0, Reason can be set to "pending" when a
+	// breakpoint is suspended. But it seems that nothing different happens.
+
+	// Is the breakpoint suspended?
+	if err == nil && len(got.Addrs) == 0 {
+		err = errors.New("unable to set breakpoint")
+	}
+
 	breakpoints[i].Verified = err == nil
 	if err != nil {
 		breakpoints[i].Message = err.Error()
-	} else {
-		path := s.toClientPath(got.File)
+	}
+
+	// If the error is connected to a specific breakpoint, tell the user.
+	if got != nil {
 		breakpoints[i].Id = got.ID
+	}
+
+	// If we have a file path, update the breakpoint.
+	if got != nil && got.File != "" {
+		path := s.toClientPath(got.File)
 		breakpoints[i].Line = got.Line
 		breakpoints[i].Source = &dap.Source{Name: filepath.Base(path), Path: path}
 	}
@@ -4063,6 +4087,21 @@ func (s *Session) convertDebuggerEvent(event *proc.Event) {
 				Category: "console",
 			},
 		})
+	case proc.EventBreakpointMaterialized:
+		bp := api.ConvertLogicalBreakpoint(event.Breakpoint)
+		path := s.toClientPath(bp.File)
+		s.send(&dap.BreakpointEvent{
+			Event: *newEvent("breakpoint"),
+			Body: dap.BreakpointEventBody{
+				Reason: "changed",
+				Breakpoint: dap.Breakpoint{
+					Verified: true,
+					Id:       bp.ID,
+					Line:     bp.Line,
+					Source:   &dap.Source{Name: filepath.Base(path), Path: path},
+				},
+			},
+		})
 	}
 }