From c10b222bad3970ecff635cd33a82c33c13593f39 Mon Sep 17 00:00:00 2001 From: polinasok <51177946+polinasok@users.noreply.github.com> Date: Mon, 17 May 2021 10:37:15 -0700 Subject: [PATCH] service/dap: support pause request (#2466) * service/dap: support pause request * service/dap: validate the client configurations in initialize request (#2435) The client can specify certain configurations in the initialize request. For example, pathFormat determines the pathFormat. We do not currently support configuring pathFormat, linesStartAt1, or columnsStartAt1, so we report an error if the client attempts to set these to an unsupported value. * TeamCity: fix Windows builds (#2467) Bintray is shutting down and the URL we used to install mingw is no longer available. Use chocolatey instead. * proc/native: low level support for watchpoints in linux/amd64 (#2301) Adds the low-level support for watchpoints (aka data breakpoints) to the native linux/amd64 backend. Does not add user interface or functioning support for watchpoints on stack variables. Updates #279 * simplify pause test Co-authored-by: Polina Sokolova Co-authored-by: Suzy Mueller Co-authored-by: Alessandro Arzilli --- service/dap/daptest/client.go | 6 +-- service/dap/error_ids.go | 3 +- service/dap/server.go | 27 +++++++--- service/dap/server_test.go | 99 ++++++++++++++++++++++++++--------- 4 files changed, 98 insertions(+), 37 deletions(-) diff --git a/service/dap/daptest/client.go b/service/dap/daptest/client.go index b58bfad7..52221adc 100644 --- a/service/dap/daptest/client.go +++ b/service/dap/daptest/client.go @@ -285,9 +285,9 @@ func (c *Client) StepOutRequest(thread int) { } // PauseRequest sends a 'pause' request. -func (c *Client) PauseRequest() { - request := &dap.NextRequest{Request: *c.newRequest("pause")} - // TODO(polina): arguments +func (c *Client) PauseRequest(threadId int) { + request := &dap.PauseRequest{Request: *c.newRequest("pause")} + request.Arguments.ThreadId = threadId c.send(request) } diff --git a/service/dap/error_ids.go b/service/dap/error_ids.go index 1a5344df..2ae0f330 100644 --- a/service/dap/error_ids.go +++ b/service/dap/error_ids.go @@ -21,7 +21,8 @@ const ( UnableToListGlobals = 2007 UnableToLookupVariable = 2008 UnableToEvaluateExpression = 2009 - UnableToGetExceptionInfo = 2010 + UnableToHalt = 2010 + UnableToGetExceptionInfo = 2011 // Add more codes as we support more requests DebuggeeIsRunning = 4000 DisconnectError = 5000 diff --git a/service/dap/server.go b/service/dap/server.go index 0d808558..a7802ea8 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -377,7 +377,7 @@ func (s *Server) handleRequest(request dap.Message) { return } - // These requests, can be handeled regardless of whether the targret is running + // These requests, can be handled regardless of whether the targret is running switch request := request.(type) { case *dap.DisconnectRequest: // Required @@ -385,7 +385,6 @@ func (s *Server) handleRequest(request dap.Message) { return case *dap.PauseRequest: // Required - // TODO: implement this request in V0 s.onPauseRequest(request) return case *dap.TerminateRequest: @@ -1275,10 +1274,20 @@ func (s *Server) doStepCommand(command string, threadId int, asyncSetupDone chan s.doRunCommand(command, asyncSetupDone) } -// onPauseRequest sends a not-yet-implemented error response. +// onPauseRequest handles 'pause' request. // This is a mandatory request to support. -func (s *Server) onPauseRequest(request *dap.PauseRequest) { // TODO V0 - s.sendNotYetImplementedErrorResponse(request.Request) +func (s *Server) onPauseRequest(request *dap.PauseRequest) { + _, err := s.debugger.Command(&api.DebuggerCommand{Name: api.Halt}, nil) + if err != nil { + s.sendErrorResponse(request.Request, UnableToHalt, "Unable to halt execution", err.Error()) + return + } + s.send(&dap.PauseResponse{Response: *newResponse(request.Request)}) + // No need to send any event here. + // If we received this request while stopped, there already was an event for the stop. + // If we received this while running, then doCommand will unblock and trigger the right + // event, using debugger.StopReason because manual stop reason always wins even if we + // simultaneously receive a manual stop request and hit a breakpoint. } // stackFrame represents the index of a frame within @@ -2089,7 +2098,11 @@ func (s *Server) doRunCommand(command string, asyncSetupDone chan struct{}) { } stopReason := s.debugger.StopReason() - s.log.Debugf("%q command stopped - reason %q", command, stopReason) + file, line := "?", -1 + if state != nil && state.CurrentThread != nil { + file, line = state.CurrentThread.File, state.CurrentThread.Line + } + s.log.Debugf("%q command stopped - reason %q, location %s:%d", command, stopReason, file, line) s.resetHandlesForStoppedEvent() stopped := &dap.StoppedEvent{Event: *newEvent("stopped")} @@ -2106,7 +2119,7 @@ func (s *Server) doRunCommand(command string, asyncSetupDone chan struct{}) { stopped.Body.Reason = "step" case proc.StopManual: // triggered by halt stopped.Body.Reason = "pause" - case proc.StopUnknown: // can happen while stopping + case proc.StopUnknown: // can happen while terminating stopped.Body.Reason = "unknown" case proc.StopWatchpoint: stopped.Body.Reason = "data breakpoint" diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 64a2ed75..93637cb0 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -572,6 +572,20 @@ func TestPreSetBreakpoint(t *testing.T) { // "Continue" is triggered after the response is sent client.ExpectTerminatedEvent(t) + + // Pause request after termination should result in an error. + // But in certain cases this request actually succeeds. + client.PauseRequest(1) + switch r := client.ExpectMessage(t).(type) { + case *dap.ErrorResponse: + if r.Message != "Unable to halt execution" { + t.Errorf("\ngot %#v\nwant Message='Unable to halt execution'", r) + } + case *dap.PauseResponse: + default: + t.Fatalf("Unexpected response type: expect error or pause, got %#v", r) + } + client.DisconnectRequest() client.ExpectOutputEventProcessExited(t, 0) client.ExpectOutputEventDetaching(t) @@ -2821,15 +2835,8 @@ func TestFatalThrowBreakpoint(t *testing.T) { }) } -// handleStop covers the standard sequence of reqeusts issued by -// a client at a breakpoint or another non-terminal stop event. -// The details have been tested by other tests, -// so this is just a sanity check. -// Skips line check if line is -1. -func handleStop(t *testing.T, client *daptest.Client, thread int, name string, line int) { +func verifyStopLocation(t *testing.T, client *daptest.Client, thread int, name string, line int) { t.Helper() - client.ThreadsRequest() - client.ExpectThreadsResponse(t) client.StackTraceRequest(thread, 0, 20) st := client.ExpectStackTraceResponse(t) @@ -2843,6 +2850,19 @@ func handleStop(t *testing.T, client *daptest.Client, thread int, name string, l t.Errorf("\ngot %#v\nwant Name=%q", st, name) } } +} + +// handleStop covers the standard sequence of requests issued by +// a client at a breakpoint or another non-terminal stop event. +// The details have been tested by other tests, +// so this is just a sanity check. +// Skips line check if line is -1. +func handleStop(t *testing.T, client *daptest.Client, thread int, name string, line int) { + t.Helper() + client.ThreadsRequest() + client.ExpectThreadsResponse(t) + + verifyStopLocation(t, client, thread, name, line) client.ScopesRequest(1000) client.ExpectScopesResponse(t) @@ -3140,6 +3160,51 @@ func TestAttachRequest(t *testing.T) { }) } +func TestPauseAndContinue(t *testing.T) { + runTest(t, "loopprog", func(client *daptest.Client, fixture protest.Fixture) { + runDebugSessionWithBPs(t, client, "launch", + // Launch + func() { + client.LaunchRequest("exec", fixture.Path, !stopOnEntry) + }, + // Set breakpoints + fixture.Source, []int{6}, + []onBreakpoint{{ + execute: func() { + verifyStopLocation(t, client, 1, "main.loop", 6) + + // Continue resumes all goroutines, so thread id is ignored + client.ContinueRequest(12345) + client.ExpectContinueResponse(t) + + time.Sleep(time.Second) + + // Halt pauses all goroutines, so thread id is ignored + client.PauseRequest(56789) + // Since we are in async mode while running, we might receive next two messages in either order. + for i := 0; i < 2; i++ { + msg := client.ExpectMessage(t) + switch m := msg.(type) { + case *dap.StoppedEvent: + if m.Body.Reason != "pause" || m.Body.ThreadId != 0 && m.Body.ThreadId != 1 { + t.Errorf("\ngot %#v\nwant ThreadId=0/1 Reason='pause'", m) + } + case *dap.PauseResponse: + default: + t.Fatalf("got %#v, want StoppedEvent or PauseResponse", m) + } + } + + // Pause will be a no-op at a pause: there will be no additional stopped events + client.PauseRequest(1) + client.ExpectPauseResponse(t) + }, + // The program has an infinite loop, so we must kill it by disconnecting. + disconnect: true, + }}) + }) +} + func TestUnupportedCommandResponses(t *testing.T) { var got *dap.ErrorResponse runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) { @@ -3188,24 +3253,6 @@ func TestUnupportedCommandResponses(t *testing.T) { }) } -func TestRequiredNotYetImplementedResponses(t *testing.T) { - var got *dap.ErrorResponse - runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) { - seqCnt := 1 - expectNotYetImplemented := func(cmd string) { - t.Helper() - got = client.ExpectNotYetImplementedErrorResponse(t) - if got.RequestSeq != seqCnt || got.Command != cmd { - t.Errorf("\ngot %#v\nwant RequestSeq=%d Command=%s", got, seqCnt, cmd) - } - seqCnt++ - } - - client.PauseRequest() - expectNotYetImplemented("pause") - }) -} - func TestOptionalNotYetImplementedResponses(t *testing.T) { var got *dap.ErrorResponse runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {