service/dap: set function breakpoints while running (#2499)

service/dap/server.go

@@ -460,6 +460,14 @@ func (s *Server) handleRequest(request dap.Message) {
 			// For this to apply in cases other than api.Continue, we would also need to
 			// introduce a new version of halt that skips ClearInternalBreakpoints
 			// in proc.(*Target).Continue, leaving NextInProgress as true.
+		case *dap.SetFunctionBreakpointsRequest:
+			s.log.Debug("halting execution to set breakpoints")
+			_, err := s.debugger.Command(&api.DebuggerCommand{Name: api.Halt}, nil)
+			if err != nil {
+				s.sendErrorResponse(request.Request, UnableToSetBreakpoints, "Unable to set or clear breakpoints", err.Error())
+				return
+			}
+			s.onSetFunctionBreakpointsRequest(request)
 		default:
 			r := request.(dap.RequestMessage).GetRequest()
 			s.sendErrorResponse(*r, DebuggeeIsRunning, fmt.Sprintf("Unable to process `%s`", r.Command), "debuggee is running")
@@ -1888,7 +1896,6 @@ func (s *Server) onSetFunctionBreakpointsRequest(request *dap.SetFunctionBreakpo
 		s.sendErrorResponse(request.Request, UnableToSetBreakpoints, "Unable to set or clear breakpoints", "running in noDebug mode")
 		return
 	}
-	// TODO(polina): handle this while running by halting first.
 
 	// According to the spec, setFunctionBreakpoints "replaces all existing function
 	// breakpoints with new function breakpoints." The simplest way is

service/dap/server_test.go

@@ -1859,11 +1859,16 @@ func expectSetBreakpointsResponse(t *testing.T, client *daptest.Client, bps []Br
 
 func checkSetBreakpointsResponse(t *testing.T, client *daptest.Client, bps []Breakpoint, got *dap.SetBreakpointsResponse) {
 	t.Helper()
-	if len(got.Body.Breakpoints) != len(bps) {
+	checkBreakpoints(t, client, bps, got.Body.Breakpoints)
-		t.Errorf("got %#v,\nwant len(Breakpoints)=%d", got, len(bps))
+}
+
+func checkBreakpoints(t *testing.T, client *daptest.Client, bps []Breakpoint, breakpoints []dap.Breakpoint) {
+	t.Helper()
+	if len(breakpoints) != len(bps) {
+		t.Errorf("got %#v,\nwant len(Breakpoints)=%d", breakpoints, len(bps))
 		return
 	}
-	for i, bp := range got.Body.Breakpoints {
+	for i, bp := range breakpoints {
 		if bp.Line != bps[i].line || bp.Verified != bps[i].verified || bp.Source.Path != bps[i].path ||
 			!strings.HasPrefix(bp.Message, bps[i].msgPrefix) {
 			t.Errorf("got breakpoints[%d] = %#v, \nwant %#v", i, bp, bps[i])
@@ -2219,6 +2224,23 @@ func expectSetBreakpointsResponseAndStoppedEvent(t *testing.T, client *daptest.C
 	return se, br
 }
 
+func expectSetFunctionBreakpointsResponseAndStoppedEvent(t *testing.T, client *daptest.Client) (se *dap.StoppedEvent, br *dap.SetFunctionBreakpointsResponse) {
+	for i := 0; i < 2; i++ {
+		switch m := client.ExpectMessage(t).(type) {
+		case *dap.StoppedEvent:
+			se = m
+		case *dap.SetFunctionBreakpointsResponse:
+			br = m
+		default:
+			t.Fatalf("Unexpected message type: expect StoppedEvent or SetFunctionBreakpointsResponse, got %#v", m)
+		}
+	}
+	if se == nil || br == nil {
+		t.Fatal("Expected StoppedEvent and SetFunctionBreakpointsResponse")
+	}
+	return se, br
+}
+
 func TestSetBreakpointWhileRunning(t *testing.T) {
 	runTest(t, "integrationprog", func(client *daptest.Client, fixture protest.Fixture) {
 		runDebugSessionWithBPs(t, client, "launch",
@@ -2275,6 +2297,74 @@ func TestSetBreakpointWhileRunning(t *testing.T) {
 	})
 }
 
+func TestSetFunctionBreakpointWhileRunning(t *testing.T) {
+	runTest(t, "integrationprog", func(client *daptest.Client, fixture protest.Fixture) {
+		runDebugSessionWithBPs(t, client, "launch",
+			// Launch
+			func() {
+				client.LaunchRequest("exec", fixture.Path, !stopOnEntry)
+			},
+			// Set breakpoints
+			fixture.Source, []int{16},
+			[]onBreakpoint{{
+				execute: func() {
+					// The program loops 3 times over lines 14-15-8-9-10-16
+					handleStop(t, client, 1, "main.main", 16) // Line that sleeps for 1 second
+
+					// We can set breakpoints while nexting
+					client.NextRequest(1)
+					client.ExpectNextResponse(t)
+					client.ExpectContinuedEvent(t)
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{{Name: "main.sayhi"}}) // [16,] => [16, 8]
+					se, br := expectSetFunctionBreakpointsResponseAndStoppedEvent(t, client)
+					if se.Body.Reason != "pause" || !se.Body.AllThreadsStopped || se.Body.ThreadId != 0 && se.Body.ThreadId != 1 {
+						t.Errorf("\ngot  %#v\nwant Reason='pause' AllThreadsStopped=true ThreadId=0/1", se)
+					}
+					checkBreakpoints(t, client, []Breakpoint{{8, fixture.Source, true, ""}}, br.Body.Breakpoints)
+
+					client.SetBreakpointsRequest(fixture.Source, []int{}) // [16,8] => [8]
+					expectSetBreakpointsResponse(t, client, []Breakpoint{})
+
+					// Halt cancelled next, so if we continue we will not stop at line 14.
+					client.ContinueRequest(1)
+					client.ExpectContinueResponse(t)
+					se = client.ExpectStoppedEvent(t)
+					if se.Body.Reason != "function breakpoint" || !se.Body.AllThreadsStopped || se.Body.ThreadId != 1 {
+						t.Errorf("\ngot  %#v\nwant Reason='breakpoint' AllThreadsStopped=true ThreadId=1", se)
+					}
+					handleStop(t, client, 1, "main.sayhi", 8)
+
+					// We can set breakpoints while continuing
+					client.ContinueRequest(1)
+					client.ExpectContinueResponse(t)
+					client.SetFunctionBreakpointsRequest([]dap.FunctionBreakpoint{}) // [8,] => []
+					se, br = expectSetFunctionBreakpointsResponseAndStoppedEvent(t, client)
+					if se.Body.Reason != "pause" || !se.Body.AllThreadsStopped || se.Body.ThreadId != 0 && se.Body.ThreadId != 1 {
+						t.Errorf("\ngot  %#v\nwant Reason='pause' AllThreadsStopped=true ThreadId=0/1", se)
+					}
+					checkBreakpoints(t, client, []Breakpoint{}, br.Body.Breakpoints)
+					if se.Body.Reason != "pause" || !se.Body.AllThreadsStopped || se.Body.ThreadId != 0 && se.Body.ThreadId != 1 {
+						t.Errorf("\ngot  %#v\nwant Reason='pause' AllThreadsStopped=true ThreadId=0/1", se)
+					}
+					checkBreakpoints(t, client, []Breakpoint{}, br.Body.Breakpoints)
+
+					client.SetBreakpointsRequest(fixture.Source, []int{16}) // [] => [16]
+					expectSetBreakpointsResponse(t, client, []Breakpoint{{16, fixture.Source, true, ""}})
+
+					client.ContinueRequest(1)
+					client.ExpectContinueResponse(t)
+					se = client.ExpectStoppedEvent(t)
+					if se.Body.Reason != "breakpoint" || !se.Body.AllThreadsStopped || se.Body.ThreadId != 1 {
+						t.Errorf("\ngot  %#v\nwant Reason='breakpoint' AllThreadsStopped=true ThreadId=1", se)
+					}
+					handleStop(t, client, 1, "main.main", 16)
+
+				},
+				disconnect: true,
+			}})
+	})
+}
+
 // TestLaunchSubstitutePath sets a breakpoint using a path
 // that does not exist and expects the substitutePath attribute
 // in the launch configuration to take care of the mapping.