From 38c8068237061e1b85d0eb61ba18d6534efef249 Mon Sep 17 00:00:00 2001 From: Polina Sokolova Date: Wed, 5 May 2021 12:55:26 -0700 Subject: [PATCH 1/5] service/dap: support pause request --- service/dap/daptest/client.go | 11 +++- service/dap/error_ids.go | 1 + service/dap/server.go | 29 ++++++--- service/dap/server_test.go | 112 ++++++++++++++++++++++++++-------- 4 files changed, 116 insertions(+), 37 deletions(-) diff --git a/service/dap/daptest/client.go b/service/dap/daptest/client.go index e2390c8ffd..b5eafdd9e6 100644 --- a/service/dap/daptest/client.go +++ b/service/dap/daptest/client.go @@ -107,6 +107,11 @@ func (c *Client) ExpectContinueResponse(t *testing.T) *dap.ContinueResponse { return c.ExpectMessage(t).(*dap.ContinueResponse) } +func (c *Client) ExpectPauseResponse(t *testing.T) *dap.PauseResponse { + t.Helper() + return c.ExpectMessage(t).(*dap.PauseResponse) +} + func (c *Client) ExpectNextResponse(t *testing.T) *dap.NextResponse { t.Helper() return c.ExpectMessage(t).(*dap.NextResponse) @@ -456,9 +461,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 68a4d3de2e..57f437a8e8 100644 --- a/service/dap/error_ids.go +++ b/service/dap/error_ids.go @@ -20,6 +20,7 @@ const ( UnableToListGlobals = 2007 UnableToLookupVariable = 2008 UnableToEvaluateExpression = 2009 + UnableToHalt = 2010 DebuggeeIsRunning = 4000 DisconnectError = 5000 diff --git a/service/dap/server.go b/service/dap/server.go index d786a35a77..7e137c6861 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -360,7 +360,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 @@ -368,7 +368,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: @@ -1204,10 +1203,20 @@ func (s *Server) doStepCommand(command string, threadId int, asyncSetupDone chan s.doCommand(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 @@ -1896,7 +1905,7 @@ func (s *Server) resetHandlesForStoppedEvent() { // asynchornous command has completed setup or was interrupted // due to an error, so the server is ready to receive new requests. func (s *Server) doCommand(command string, asyncSetupDone chan struct{}) { - // TODO(polina): it appears that debugger.Command doesn't close + // TODO(polina): it appears that debugger.Command doesn't always close // asyncSetupDone (e.g. when having an error next while nexting). // So we should always close it ourselves just in case. defer s.asyncCommandDone(asyncSetupDone) @@ -1913,14 +1922,18 @@ func (s *Server) doCommand(command string, asyncSetupDone chan struct{}) { if err == nil { stopped.Body.ThreadId = stoppedGoroutineID(state) + file, line := "?", -1 + if state.CurrentThread != nil { + file, line = state.CurrentThread.File, state.CurrentThread.Line + } sr := s.debugger.StopReason() - s.log.Debugf("%q command stopped - reason %q", command, sr) + s.log.Debugf("%q command stopped - reason %q, location %s:%d", command, sr, file, line) switch sr { case proc.StopNextFinished: 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" default: stopped.Body.Reason = "breakpoint" diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 65e011adec..bd3ca703c5 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -576,6 +576,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) @@ -2660,15 +2674,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) @@ -2682,6 +2689,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) @@ -2979,6 +2999,64 @@ 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) + + // Advance past the breakpoint line, so breakpoint and pause don't compete + // Even though StopReason "pause" is supposed to trump "breakpoint", + // sometimes that is not the case. + client.NextRequest(1) + client.ExpectNextResponse(t) + client.ExpectStoppedEvent(t) + verifyStopLocation(t, client, 1, "main.loop", 7) + + // Pause will be a no-op at a breakpoint: there will be no additional stopped events + client.PauseRequest(1) + client.ExpectPauseResponse(t) + verifyStopLocation(t, client, 1, "main.loop", 7) + + for i := 0; i < 10; i++ { + // Continue resumes all goroutines, so thread id is ignored + client.ContinueRequest(-i) + client.ExpectContinueResponse(t) + + // Halt pauses all goroutines, so thread id is ignored + client.PauseRequest(-i) + // 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) { @@ -3030,24 +3108,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) { From 185e1ba91b7976cccec3f5549699bea3890feb13 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Thu, 6 May 2021 03:56:29 -0400 Subject: [PATCH 2/5] 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. --- service/dap/daptest/client.go | 7 ++++ service/dap/error_ids.go | 1 + service/dap/server.go | 16 ++++++++ service/dap/server_test.go | 76 ++++++++++++++++++++++++++++++++--- 4 files changed, 95 insertions(+), 5 deletions(-) diff --git a/service/dap/daptest/client.go b/service/dap/daptest/client.go index b5eafdd9e6..0a94e0d953 100644 --- a/service/dap/daptest/client.go +++ b/service/dap/daptest/client.go @@ -351,6 +351,13 @@ func (c *Client) InitializeRequest() { c.send(request) } +// InitializeRequestWithArgs sends an 'initialize' request with specified arguments. +func (c *Client) InitializeRequestWithArgs(args dap.InitializeRequestArguments) { + request := &dap.InitializeRequest{Request: *c.newRequest("initialize")} + request.Arguments = args + c.send(request) +} + // LaunchRequest sends a 'launch' request with the specified args. func (c *Client) LaunchRequest(mode, program string, stopOnEntry bool) { request := &dap.LaunchRequest{Request: *c.newRequest("launch")} diff --git a/service/dap/error_ids.go b/service/dap/error_ids.go index 57f437a8e8..925ab5dbb4 100644 --- a/service/dap/error_ids.go +++ b/service/dap/error_ids.go @@ -12,6 +12,7 @@ const ( // values below are inspired the original vscode-go debug adaptor. FailedToLaunch = 3000 FailedToAttach = 3001 + FailedToInitialize = 3002 UnableToSetBreakpoints = 2002 UnableToDisplayThreads = 2003 UnableToProduceStackTrace = 2004 diff --git a/service/dap/server.go b/service/dap/server.go index 7e137c6861..8c94273695 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -607,6 +607,22 @@ func (s *Server) logToConsole(msg string) { } func (s *Server) onInitializeRequest(request *dap.InitializeRequest) { + if request.Arguments.PathFormat != "path" { + s.sendErrorResponse(request.Request, FailedToInitialize, "Failed to initialize", + fmt.Sprintf("Unsupported 'pathFormat' value '%s'.", request.Arguments.PathFormat)) + return + } + if !request.Arguments.LinesStartAt1 { + s.sendErrorResponse(request.Request, FailedToInitialize, "Failed to initialize", + "Only 1-based line numbers are supported.") + return + } + if !request.Arguments.ColumnsStartAt1 { + s.sendErrorResponse(request.Request, FailedToInitialize, "Failed to initialize", + "Only 1-based column numbers are supported.") + return + } + // TODO(polina): Respond with an error if debug session is in progress? response := &dap.InitializeResponse{Response: *newResponse(request.Request)} response.Body.SupportsConfigurationDoneRequest = true diff --git a/service/dap/server_test.go b/service/dap/server_test.go index bd3ca703c5..4f8e2cc171 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -50,6 +50,16 @@ func runTest(t *testing.T, name string, test func(c *daptest.Client, f protest.F var buildFlags protest.BuildFlags = protest.AllNonOptimized fixture := protest.BuildFixture(name, buildFlags) + // Start the DAP server. + client := startDapServer(t) + // client.Close will close the client connectinon, which will cause a connection error + // on the server side and signal disconnect to unblock Stop() above. + defer client.Close() + + test(client, fixture) +} + +func startDapServer(t *testing.T) *daptest.Client { // Start the DAP server. listener, err := net.Listen("tcp", ":0") if err != nil { @@ -76,11 +86,7 @@ func runTest(t *testing.T, name string, test func(c *daptest.Client, f protest.F }() client := daptest.NewClient(listener.Addr().String()) - // This will close the client connectinon, which will cause a connection error - // on the server side and signal disconnect to unblock Stop() above. - defer client.Close() - - test(client, fixture) + return client } // TestLaunchStopOnEntry emulates the message exchange that can be observed with @@ -3397,6 +3403,66 @@ func TestBadAttachRequest(t *testing.T) { }) } +func TestBadInitializeRequest(t *testing.T) { + runInitializeTest := func(args dap.InitializeRequestArguments, err string) { + t.Helper() + // Only one initialize request is allowed, so use a new server + // for each test. + client := startDapServer(t) + // client.Close will close the client connectinon, which will cause a connection error + // on the server side and signal disconnect to unblock Stop() above. + defer client.Close() + + client.InitializeRequestWithArgs(args) + response := client.ExpectErrorResponse(t) + if response.Command != "initialize" { + t.Errorf("Command got %q, want \"launch\"", response.Command) + } + if response.Message != "Failed to initialize" { + t.Errorf("Message got %q, want \"Failed to launch\"", response.Message) + } + if response.Body.Error.Id != 3002 { + t.Errorf("Id got %d, want 3002", response.Body.Error.Id) + } + if response.Body.Error.Format != err { + t.Errorf("\ngot %q\nwant %q", response.Body.Error.Format, err) + } + } + + // Bad path format. + runInitializeTest(dap.InitializeRequestArguments{ + AdapterID: "go", + PathFormat: "url", // unsupported 'pathFormat' + LinesStartAt1: true, + ColumnsStartAt1: true, + Locale: "en-us", + }, + "Failed to initialize: Unsupported 'pathFormat' value 'url'.", + ) + + // LinesStartAt1 must be true. + runInitializeTest(dap.InitializeRequestArguments{ + AdapterID: "go", + PathFormat: "path", + LinesStartAt1: false, // only 1-based line numbers are supported + ColumnsStartAt1: true, + Locale: "en-us", + }, + "Failed to initialize: Only 1-based line numbers are supported.", + ) + + // ColumnsStartAt1 must be true. + runInitializeTest(dap.InitializeRequestArguments{ + AdapterID: "go", + PathFormat: "path", + LinesStartAt1: true, + ColumnsStartAt1: false, // only 1-based column numbers are supported + Locale: "en-us", + }, + "Failed to initialize: Only 1-based column numbers are supported.", + ) +} + func TestBadlyFormattedMessageToServer(t *testing.T) { runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) { // Send a badly formatted message to the server, and expect it to close the From 6c8f0ada9fc09d1dd730c3b7620881f6bb99d22c Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Thu, 6 May 2021 19:05:17 +0200 Subject: [PATCH 3/5] TeamCity: fix Windows builds (#2467) Bintray is shutting down and the URL we used to install mingw is no longer available. Use chocolatey instead. --- _scripts/test_windows.ps1 | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/_scripts/test_windows.ps1 b/_scripts/test_windows.ps1 index 2011a70585..7d814b676b 100644 --- a/_scripts/test_windows.ps1 +++ b/_scripts/test_windows.ps1 @@ -3,14 +3,11 @@ param ( [Parameter(Mandatory = $true)][string]$arch ) +# Install Chocolatey +#Set-ExecutionPolicy Bypass -Scope Process -Force; [System.Net.ServicePointManager]::SecurityProtocol = [System.Net.ServicePointManager]::SecurityProtocol -bor 3072; iex ((New-Object System.Net.WebClient).DownloadString('https://chocolatey.org/install.ps1')) + # Install MinGW. -if (-Not(Test-Path "C:\mingw64")) -{ - $file = "x86_64-4.9.2-release-win32-seh-rt_v4-rev3.7z" - $url = "https://bintray.com/artifact/download/drewwells/generic/$file" - Invoke-WebRequest -UserAgent wget -Uri $url -OutFile $file - &7z x -oC:\ $file > $null -} +choco install -y mingw # Install Procdump if (-Not(Test-Path "C:\procdump")) @@ -66,4 +63,4 @@ Write-Host $env:GOPATH go version go env -mingw32-make test +go run _scripts/make.go test From c62d09f2dbb47e3bdb39ffeca59469b8a1c1ccac Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Thu, 6 May 2021 19:33:56 +0200 Subject: [PATCH 4/5] 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 --- Documentation/backend_test_health.md | 25 +++-- _fixtures/databpcountstest.go | 31 ++++++ _fixtures/databpeasy.go | 34 +++++++ pkg/proc/amd64util/debugregs.go | 130 +++++++++++++++++++++++++ pkg/proc/breakpoints.go | 104 ++++++++++++++++++++ pkg/proc/gdbserial/gdbserver.go | 3 + pkg/proc/native/nonative_darwin.go | 12 +++ pkg/proc/native/proc.go | 22 ++++- pkg/proc/native/proc_linux.go | 8 ++ pkg/proc/native/threads.go | 60 +++++++++--- pkg/proc/native/threads_darwin.go | 12 +++ pkg/proc/native/threads_freebsd.go | 12 +++ pkg/proc/native/threads_linux_386.go | 12 +++ pkg/proc/native/threads_linux_amd64.go | 74 ++++++++++++++ pkg/proc/native/threads_linux_arm64.go | 12 +++ pkg/proc/native/threads_windows.go | 12 +++ pkg/proc/proc_test.go | 91 +++++++++++++++++ pkg/proc/target.go | 3 + pkg/proc/target_exec.go | 5 +- service/dap/server.go | 2 + 20 files changed, 637 insertions(+), 27 deletions(-) create mode 100644 _fixtures/databpcountstest.go create mode 100644 _fixtures/databpeasy.go create mode 100644 pkg/proc/amd64util/debugregs.go diff --git a/Documentation/backend_test_health.md b/Documentation/backend_test_health.md index e49f5b202b..0aa52306b8 100644 --- a/Documentation/backend_test_health.md +++ b/Documentation/backend_test_health.md @@ -1,23 +1,30 @@ Tests skipped by each supported backend: -* 386 skipped = 2.7% (4/147) +* 386 skipped = 4% (6/149) * 1 broken * 3 broken - cgo stacktraces -* arm64 skipped = 2% (3/147) + * 2 not implemented +* arm64 skipped = 3.4% (5/149) * 1 broken * 1 broken - cgo stacktraces * 1 broken - global variable symbolication -* darwin/arm64 skipped = 0.68% (1/147) + * 2 not implemented +* darwin skipped = 1.3% (2/149) + * 2 not implemented +* darwin/arm64 skipped = 0.67% (1/149) * 1 broken - cgo stacktraces -* darwin/lldb skipped = 0.68% (1/147) +* darwin/lldb skipped = 0.67% (1/149) * 1 upstream issue -* freebsd skipped = 8.2% (12/147) +* freebsd skipped = 9.4% (14/149) * 11 broken - * 1 not implemented -* linux/386/pie skipped = 0.68% (1/147) + * 3 not implemented +* linux/386/pie skipped = 0.67% (1/149) * 1 broken -* pie skipped = 0.68% (1/147) +* pie skipped = 0.67% (1/149) * 1 upstream issue - https://github.com/golang/go/issues/29322 -* windows skipped = 1.4% (2/147) +* rr skipped = 1.3% (2/149) + * 2 not implemented +* windows skipped = 2.7% (4/149) * 1 broken + * 2 not implemented * 1 upstream issue diff --git a/_fixtures/databpcountstest.go b/_fixtures/databpcountstest.go new file mode 100644 index 0000000000..6a79a28b8b --- /dev/null +++ b/_fixtures/databpcountstest.go @@ -0,0 +1,31 @@ +package main + +import ( + "fmt" + "math/rand" + "sync" + "time" +) + +var globalvar1 int + +func demo(id int, wait *sync.WaitGroup) { + for i := 0; i < 100; i++ { + sleep := rand.Intn(10) + 1 + fmt.Printf("id: %d step: %d sleeping %d\n", id, i, sleep) + globalvar1 = globalvar1 + 1 + time.Sleep(time.Duration(sleep) * time.Millisecond) + } + + wait.Done() +} + +func main() { + wait := new(sync.WaitGroup) + wait.Add(1) + wait.Add(1) + go demo(1, wait) + go demo(2, wait) + + wait.Wait() +} diff --git a/_fixtures/databpeasy.go b/_fixtures/databpeasy.go new file mode 100644 index 0000000000..c5aa8e51cf --- /dev/null +++ b/_fixtures/databpeasy.go @@ -0,0 +1,34 @@ +package main + +import ( + "fmt" + "runtime" +) + +var globalvar1 = 0 +var globalvar2 = 0 + +func main() { // Position 0 + runtime.LockOSThread() + globalvar2 = 1 + fmt.Printf("%d %d\n", globalvar1, globalvar2) + globalvar2 = globalvar1 + 1 + globalvar1 = globalvar2 + 1 + fmt.Printf("%d %d\n", globalvar1, globalvar2) // Position 1 + runtime.Breakpoint() + globalvar2 = globalvar2 + 1 // Position 2 + globalvar2 = globalvar1 + globalvar2 // Position 3 + fmt.Printf("%d %d\n", globalvar1, globalvar2) + globalvar1 = globalvar2 + 1 + fmt.Printf("%d %d\n", globalvar1, globalvar2) + runtime.Breakpoint() + done := make(chan struct{}) // Position 4 + go f(done) + <-done +} + +func f(done chan struct{}) { + runtime.LockOSThread() + globalvar1 = globalvar2 + 1 + close(done) // Position 5 +} diff --git a/pkg/proc/amd64util/debugregs.go b/pkg/proc/amd64util/debugregs.go new file mode 100644 index 0000000000..23de122bbb --- /dev/null +++ b/pkg/proc/amd64util/debugregs.go @@ -0,0 +1,130 @@ +package amd64util + +import ( + "errors" + "fmt" +) + +// DebugRegisters represents x86 debug registers described in the Intel 64 +// and IA-32 Architectures Software Developer's Manual, Vol. 3B, section +// 17.2 +type DebugRegisters struct { + pAddrs [4]*uint64 + pDR6, pDR7 *uint64 + Dirty bool +} + +func NewDebugRegisters(pDR0, pDR1, pDR2, pDR3, pDR6, pDR7 *uint64) *DebugRegisters { + return &DebugRegisters{ + pAddrs: [4]*uint64{pDR0, pDR1, pDR2, pDR3}, + pDR6: pDR6, + pDR7: pDR7, + Dirty: false, + } +} + +func lenrwBitsOffset(idx uint8) uint8 { + return 16 + idx*4 +} + +func enableBitOffset(idx uint8) uint8 { + return idx * 2 +} + +func (drs *DebugRegisters) breakpoint(idx uint8) (addr uint64, read, write bool, sz int) { + enable := *(drs.pDR7) & (1 << enableBitOffset(idx)) + if enable == 0 { + return 0, false, false, 0 + } + + addr = *(drs.pAddrs[idx]) + lenrw := (*(drs.pDR7) >> lenrwBitsOffset(idx)) & 0xf + write = (lenrw & 0x1) != 0 + read = (lenrw & 0x2) != 0 + switch lenrw >> 2 { + case 0x0: + sz = 1 + case 0x1: + sz = 2 + case 0x2: + sz = 8 // sic + case 0x3: + sz = 4 + } + return addr, read, write, sz +} + +// SetBreakpoint sets hardware breakpoint at index 'idx' to the specified +// address, read/write flags and size. +// If the breakpoint is already in use but the parameters match it does +// nothing. +func (drs *DebugRegisters) SetBreakpoint(idx uint8, addr uint64, read, write bool, sz int) error { + if int(idx) >= len(drs.pAddrs) { + return fmt.Errorf("hardware breakpoints exhausted") + } + curaddr, curread, curwrite, cursz := drs.breakpoint(idx) + if curaddr != 0 { + if (curaddr != addr) || (curread != read) || (curwrite != write) || (cursz != sz) { + return fmt.Errorf("hardware breakpoint %d already in use (address %#x)", idx, curaddr) + } + // hardware breakpoint already set + return nil + } + + if read && !write { + return errors.New("break on read only not supported") + } + + *(drs.pAddrs[idx]) = addr + var lenrw uint64 + if write { + lenrw |= 0x1 + } + if read { + lenrw |= 0x2 + } + switch sz { + case 1: + // already ok + case 2: + lenrw |= 0x1 << 2 + case 4: + lenrw |= 0x3 << 2 + case 8: + lenrw |= 0x2 << 2 + default: + return fmt.Errorf("data breakpoint of size %d not supported", sz) + } + *(drs.pDR7) &^= (0xf << lenrwBitsOffset(idx)) // clear old settings + *(drs.pDR7) |= lenrw << lenrwBitsOffset(idx) + *(drs.pDR7) |= 1 << enableBitOffset(idx) // enable + drs.Dirty = true + return nil +} + +// ClearBreakpoint disables the hardware breakpoint at index 'idx'. If the +// breakpoint was already disabled it does nothing. +func (drs *DebugRegisters) ClearBreakpoint(idx uint8) { + if *(drs.pDR7)&(1<> 4) +} + +// withSize returns a new HWBreakType with the size set to the specified value +func (wtype WatchType) withSize(sz uint8) WatchType { + return WatchType((sz << 4) | uint8(wtype&0xf)) +} + +var ErrHWBreakUnsupported = errors.New("hardware breakpoints not implemented") + func (bp *Breakpoint) String() string { return fmt.Sprintf("Breakpoint %d at %#v %s:%d (%d)", bp.LogicalID, bp.Addr, bp.File, bp.Line, bp.TotalHitCount) } @@ -243,6 +277,49 @@ func NewBreakpointMap() BreakpointMap { // SetBreakpoint sets a breakpoint at addr, and stores it in the process wide // break point table. func (t *Target) SetBreakpoint(addr uint64, kind BreakpointKind, cond ast.Expr) (*Breakpoint, error) { + return t.setBreakpointInternal(addr, kind, 0, cond) +} + +// SetWatchpoint sets a data breakpoint at addr and stores it in the +// process wide break point table. +func (t *Target) SetWatchpoint(scope *EvalScope, expr string, wtype WatchType, cond ast.Expr) (*Breakpoint, error) { + if (wtype&WatchWrite == 0) && (wtype&WatchRead == 0) { + return nil, errors.New("at least one of read and write must be set for watchpoint") + } + + n, err := parser.ParseExpr(expr) + if err != nil { + return nil, err + } + xv, err := scope.evalAST(n) + if err != nil { + return nil, err + } + if xv.Addr == 0 || xv.Flags&VariableFakeAddress != 0 || xv.DwarfType == nil { + return nil, fmt.Errorf("can not watch %q", expr) + } + if xv.Unreadable != nil { + return nil, fmt.Errorf("expression %q is unreadable: %v", expr, xv.Unreadable) + } + if xv.Kind == reflect.UnsafePointer || xv.Kind == reflect.Invalid { + return nil, fmt.Errorf("can not watch variable of type %s", xv.Kind.String()) + } + sz := xv.DwarfType.Size() + if sz <= 0 || sz > int64(t.BinInfo().Arch.PtrSize()) { + //TODO(aarzilli): it is reasonable to expect to be able to watch string + //and interface variables and we could support it by watching certain + //member fields here. + return nil, fmt.Errorf("can not watch variable of type %s", xv.DwarfType.String()) + } + if xv.Addr >= scope.g.stack.lo && xv.Addr < scope.g.stack.hi { + //TODO(aarzilli): support watching stack variables + return nil, errors.New("can not watch stack allocated variable") + } + + return t.setBreakpointInternal(xv.Addr, UserBreakpoint, wtype.withSize(uint8(sz)), cond) +} + +func (t *Target) setBreakpointInternal(addr uint64, kind BreakpointKind, wtype WatchType, cond ast.Expr) (*Breakpoint, error) { if valid, err := t.Valid(); !valid { return nil, err } @@ -270,8 +347,25 @@ func (t *Target) SetBreakpoint(addr uint64, kind BreakpointKind, cond ast.Expr) fnName = fn.Name } + hwidx := uint8(0) + if wtype != 0 { + m := make(map[uint8]bool) + for _, bp := range bpmap.M { + if bp.WatchType != 0 { + m[bp.HWBreakIndex] = true + } + } + for hwidx = 0; true; hwidx++ { + if !m[hwidx] { + break + } + } + } + newBreakpoint := &Breakpoint{ FunctionName: fnName, + WatchType: wtype, + HWBreakIndex: hwidx, File: f, Line: l, Addr: addr, @@ -372,6 +466,16 @@ func (bpmap *BreakpointMap) HasInternalBreakpoints() bool { return false } +// HasHWBreakpoints returns true if there are hardware breakpoints. +func (bpmap *BreakpointMap) HasHWBreakpoints() bool { + for _, bp := range bpmap.M { + if bp.WatchType != 0 { + return true + } + } + return false +} + // BreakpointState describes the state of a breakpoint in a thread. type BreakpointState struct { *Breakpoint diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index d35da689a3..176ccf9ff1 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -1209,6 +1209,9 @@ func (p *gdbProcess) FindBreakpoint(pc uint64) (*proc.Breakpoint, bool) { } func (p *gdbProcess) WriteBreakpoint(bp *proc.Breakpoint) error { + if bp.WatchType != 0 { + return errors.New("hardware breakpoints not supported") + } return p.conn.setBreakpoint(bp.Addr, p.breakpointKind) } diff --git a/pkg/proc/native/nonative_darwin.go b/pkg/proc/native/nonative_darwin.go index 95614cc384..ab42dd3248 100644 --- a/pkg/proc/native/nonative_darwin.go +++ b/pkg/proc/native/nonative_darwin.go @@ -114,6 +114,18 @@ func (t *nativeThread) restoreRegisters(sr proc.Registers) error { panic(ErrNativeBackendDisabled) } +func (t *nativeThread) findHardwareBreakpoint() (*proc.Breakpoint, error) { + panic(ErrNativeBackendDisabled) +} + +func (t *nativeThread) writeHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + panic(ErrNativeBackendDisabled) +} + +func (t *nativeThread) clearHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + panic(ErrNativeBackendDisabled) +} + // Stopped returns whether the thread is stopped at // the operating system level. func (t *nativeThread) Stopped() bool { diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go index 1e0d463fcd..ef54644f8b 100644 --- a/pkg/proc/native/proc.go +++ b/pkg/proc/native/proc.go @@ -206,6 +206,16 @@ func (dbp *nativeProcess) CheckAndClearManualStopRequest() bool { } func (dbp *nativeProcess) WriteBreakpoint(bp *proc.Breakpoint) error { + if bp.WatchType != 0 { + for _, thread := range dbp.threads { + err := thread.writeHardwareBreakpoint(bp.Addr, bp.WatchType, bp.HWBreakIndex) + if err != nil { + return err + } + } + return nil + } + bp.OriginalData = make([]byte, dbp.bi.Arch.BreakpointSize()) _, err := dbp.memthread.ReadMemory(bp.OriginalData, bp.Addr) if err != nil { @@ -215,7 +225,17 @@ func (dbp *nativeProcess) WriteBreakpoint(bp *proc.Breakpoint) error { } func (dbp *nativeProcess) EraseBreakpoint(bp *proc.Breakpoint) error { - return dbp.memthread.ClearBreakpoint(bp) + if bp.WatchType != 0 { + for _, thread := range dbp.threads { + err := thread.clearHardwareBreakpoint(bp.Addr, bp.WatchType, bp.HWBreakIndex) + if err != nil { + return err + } + } + return nil + } + + return dbp.memthread.clearSoftwareBreakpoint(bp) } // ContinueOnce will continue the target until it stops. diff --git a/pkg/proc/native/proc_linux.go b/pkg/proc/native/proc_linux.go index a4376891dd..652990f18a 100644 --- a/pkg/proc/native/proc_linux.go +++ b/pkg/proc/native/proc_linux.go @@ -268,6 +268,14 @@ func (dbp *nativeProcess) addThread(tid int, attach bool) (*nativeThread, error) if dbp.memthread == nil { dbp.memthread = dbp.threads[tid] } + for _, bp := range dbp.Breakpoints().M { + if bp.WatchType != 0 { + err := dbp.threads[tid].writeHardwareBreakpoint(bp.Addr, bp.WatchType, bp.HWBreakIndex) + if err != nil { + return nil, err + } + } + } return dbp.threads[tid], nil } diff --git a/pkg/proc/native/threads.go b/pkg/proc/native/threads.go index 286318cddc..62aa55bafc 100644 --- a/pkg/proc/native/threads.go +++ b/pkg/proc/native/threads.go @@ -53,6 +53,17 @@ func (t *nativeThread) StepInstruction() (err error) { defer func() { t.singleStepping = false }() + + if bp := t.CurrentBreakpoint.Breakpoint; bp != nil && bp.WatchType != 0 && t.dbp.Breakpoints().M[bp.Addr] == bp { + err = t.clearHardwareBreakpoint(bp.Addr, bp.WatchType, bp.HWBreakIndex) + if err != nil { + return err + } + defer func() { + err = t.writeHardwareBreakpoint(bp.Addr, bp.WatchType, bp.HWBreakIndex) + }() + } + pc, err := t.PC() if err != nil { return err @@ -61,7 +72,7 @@ func (t *nativeThread) StepInstruction() (err error) { bp, ok := t.dbp.FindBreakpoint(pc, false) if ok { // Clear the breakpoint so that we can continue execution. - err = t.ClearBreakpoint(bp) + err = t.clearSoftwareBreakpoint(bp) if err != nil { return err } @@ -109,23 +120,40 @@ func (t *nativeThread) Common() *proc.CommonThread { // thread is stopped at as CurrentBreakpoint on the thread struct. func (t *nativeThread) SetCurrentBreakpoint(adjustPC bool) error { t.CurrentBreakpoint.Clear() - pc, err := t.PC() - if err != nil { - return err - } - // If the breakpoint instruction does not change the value - // of PC after being executed we should look for breakpoints - // with bp.Addr == PC and there is no need to call SetPC - // after finding one. - adjustPC = adjustPC && t.BinInfo().Arch.BreakInstrMovesPC() + var bp *proc.Breakpoint - if bp, ok := t.dbp.FindBreakpoint(pc, adjustPC); ok { - if adjustPC { - if err = t.setPC(bp.Addr); err != nil { - return err + if t.dbp.Breakpoints().HasHWBreakpoints() { + var err error + bp, err = t.findHardwareBreakpoint() + if err != nil { + return err + } + } + if bp == nil { + pc, err := t.PC() + if err != nil { + return err + } + + // If the breakpoint instruction does not change the value + // of PC after being executed we should look for breakpoints + // with bp.Addr == PC and there is no need to call SetPC + // after finding one. + adjustPC = adjustPC && t.BinInfo().Arch.BreakInstrMovesPC() + + var ok bool + bp, ok = t.dbp.FindBreakpoint(pc, adjustPC) + if ok { + if adjustPC { + if err = t.setPC(bp.Addr); err != nil { + return err + } } } + } + + if bp != nil { t.CurrentBreakpoint = bp.CheckCondition(t) if t.CurrentBreakpoint.Breakpoint != nil && t.CurrentBreakpoint.Active { if g, err := proc.GetG(t); err == nil { @@ -148,8 +176,8 @@ func (t *nativeThread) ThreadID() int { return t.ID } -// ClearBreakpoint clears the specified breakpoint. -func (t *nativeThread) ClearBreakpoint(bp *proc.Breakpoint) error { +// clearSoftwareBreakpoint clears the specified breakpoint. +func (t *nativeThread) clearSoftwareBreakpoint(bp *proc.Breakpoint) error { if _, err := t.WriteMemory(bp.Addr, bp.OriginalData); err != nil { return fmt.Errorf("could not clear breakpoint %s", err) } diff --git a/pkg/proc/native/threads_darwin.go b/pkg/proc/native/threads_darwin.go index fe12bede82..b30f6398a5 100644 --- a/pkg/proc/native/threads_darwin.go +++ b/pkg/proc/native/threads_darwin.go @@ -132,3 +132,15 @@ func (t *nativeThread) ReadMemory(buf []byte, addr uint64) (int, error) { func (t *nativeThread) restoreRegisters(sr proc.Registers) error { return errors.New("not implemented") } + +func (t *nativeThread) writeHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + return proc.ErrHWBreakUnsupported +} + +func (t *nativeThread) clearHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + return proc.ErrHWBreakUnsupported +} + +func (t *nativeThread) findHardwareBreakpoint() (*proc.Breakpoint, error) { + return nil, nil +} diff --git a/pkg/proc/native/threads_freebsd.go b/pkg/proc/native/threads_freebsd.go index 4af26e1b6a..4405d95236 100644 --- a/pkg/proc/native/threads_freebsd.go +++ b/pkg/proc/native/threads_freebsd.go @@ -120,3 +120,15 @@ func (t *nativeThread) ReadMemory(data []byte, addr uint64) (n int, err error) { t.dbp.execPtraceFunc(func() { n, err = ptraceReadData(t.ID, uintptr(addr), data) }) return n, err } + +func (t *nativeThread) writeHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + return proc.ErrHWBreakUnsupported +} + +func (t *nativeThread) clearHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + return proc.ErrHWBreakUnsupported +} + +func (t *nativeThread) findHardwareBreakpoint() (*proc.Breakpoint, error) { + return nil, nil +} diff --git a/pkg/proc/native/threads_linux_386.go b/pkg/proc/native/threads_linux_386.go index 338a166bad..9320c9f230 100644 --- a/pkg/proc/native/threads_linux_386.go +++ b/pkg/proc/native/threads_linux_386.go @@ -8,3 +8,15 @@ import ( func (t *nativeThread) restoreRegisters(savedRegs proc.Registers) error { return fmt.Errorf("restore regs not supported on i386") } + +func (t *nativeThread) writeHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + return proc.ErrHWBreakUnsupported +} + +func (t *nativeThread) clearHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + return proc.ErrHWBreakUnsupported +} + +func (t *nativeThread) findHardwareBreakpoint() (*proc.Breakpoint, error) { + return nil, nil +} diff --git a/pkg/proc/native/threads_linux_amd64.go b/pkg/proc/native/threads_linux_amd64.go index 049195adc7..35681a4aaa 100644 --- a/pkg/proc/native/threads_linux_amd64.go +++ b/pkg/proc/native/threads_linux_amd64.go @@ -7,6 +7,7 @@ import ( sys "golang.org/x/sys/unix" "github.com/go-delve/delve/pkg/proc" + "github.com/go-delve/delve/pkg/proc/amd64util" "github.com/go-delve/delve/pkg/proc/linutil" ) @@ -45,3 +46,76 @@ func (t *nativeThread) restoreRegisters(savedRegs proc.Registers) error { } return restoreRegistersErr } + +const debugRegUserOffset = 848 // offset of debug registers in the user struct, see source/arch/x86/kernel/ptrace.c + +func (t *nativeThread) withDebugRegisters(f func(*amd64util.DebugRegisters) error) error { + var err error + t.dbp.execPtraceFunc(func() { + debugregs := make([]uint64, 8) + + for i := range debugregs { + if i == 4 || i == 5 { + continue + } + _, _, err = sys.Syscall6(sys.SYS_PTRACE, sys.PTRACE_PEEKUSR, uintptr(t.ID), uintptr(debugRegUserOffset+uintptr(i)*unsafe.Sizeof(debugregs[0])), uintptr(unsafe.Pointer(&debugregs[i])), 0, 0) + if err != nil && err != syscall.Errno(0) { + return + } + } + + drs := amd64util.NewDebugRegisters(&debugregs[0], &debugregs[1], &debugregs[2], &debugregs[3], &debugregs[6], &debugregs[7]) + + err = f(drs) + + if drs.Dirty { + for i := range debugregs { + if i == 4 || i == 5 { + // Linux will return EIO for DR4 and DR5 + continue + } + _, _, err = sys.Syscall6(sys.SYS_PTRACE, sys.PTRACE_POKEUSR, uintptr(t.ID), uintptr(debugRegUserOffset+uintptr(i)*unsafe.Sizeof(debugregs[0])), uintptr(debugregs[i]), 0, 0) + if err != nil && err != syscall.Errno(0) { + return + } + } + } + }) + if err == syscall.Errno(0) || err == sys.ESRCH { + err = nil + } + return err +} + +func (t *nativeThread) writeHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + return t.withDebugRegisters(func(drs *amd64util.DebugRegisters) error { + return drs.SetBreakpoint(idx, addr, wtype.Read(), wtype.Write(), wtype.Size()) + }) +} + +func (t *nativeThread) clearHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + return t.withDebugRegisters(func(drs *amd64util.DebugRegisters) error { + drs.ClearBreakpoint(idx) + return nil + }) +} + +func (t *nativeThread) findHardwareBreakpoint() (*proc.Breakpoint, error) { + var retbp *proc.Breakpoint + err := t.withDebugRegisters(func(drs *amd64util.DebugRegisters) error { + ok, idx := drs.GetActiveBreakpoint() + if ok { + for _, bp := range t.dbp.Breakpoints().M { + if bp.WatchType != 0 && bp.HWBreakIndex == idx { + retbp = bp + break + } + } + } + return nil + }) + if err != nil { + return nil, err + } + return retbp, nil +} diff --git a/pkg/proc/native/threads_linux_arm64.go b/pkg/proc/native/threads_linux_arm64.go index f2f4d29084..b9835ee2c9 100644 --- a/pkg/proc/native/threads_linux_arm64.go +++ b/pkg/proc/native/threads_linux_arm64.go @@ -42,3 +42,15 @@ func (t *nativeThread) restoreRegisters(savedRegs proc.Registers) error { } return restoreRegistersErr } + +func (t *nativeThread) writeHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + return proc.ErrHWBreakUnsupported +} + +func (t *nativeThread) clearHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + return proc.ErrHWBreakUnsupported +} + +func (t *nativeThread) findHardwareBreakpoint() (*proc.Breakpoint, error) { + return nil, nil +} diff --git a/pkg/proc/native/threads_windows.go b/pkg/proc/native/threads_windows.go index 52662c7ea7..030a987be3 100644 --- a/pkg/proc/native/threads_windows.go +++ b/pkg/proc/native/threads_windows.go @@ -155,3 +155,15 @@ func (t *nativeThread) ReadMemory(buf []byte, addr uint64) (int, error) { func (t *nativeThread) restoreRegisters(savedRegs proc.Registers) error { return _SetThreadContext(t.os.hThread, savedRegs.(*winutil.AMD64Registers).Context) } + +func (t *nativeThread) writeHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + return proc.ErrHWBreakUnsupported +} + +func (t *nativeThread) clearHardwareBreakpoint(addr uint64, wtype proc.WatchType, idx uint8) error { + return proc.ErrHWBreakUnsupported +} + +func (t *nativeThread) findHardwareBreakpoint() (*proc.Breakpoint, error) { + return nil, nil +} diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 361441157f..6a3fb32cc3 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -5216,3 +5216,94 @@ func TestVariablesWithExternalLinking(t *testing.T) { } }) } + +func TestWatchpointsBasic(t *testing.T) { + skipOn(t, "not implemented", "windows") + skipOn(t, "not implemented", "freebsd") + skipOn(t, "not implemented", "darwin") + skipOn(t, "not implemented", "386") + skipOn(t, "not implemented", "arm64") + skipOn(t, "not implemented", "rr") + + withTestProcess("databpeasy", t, func(p *proc.Target, fixture protest.Fixture) { + setFunctionBreakpoint(p, t, "main.main") + assertNoError(p.Continue(), t, "Continue 0") + assertLineNumber(p, t, 11, "Continue 0") // Position 0 + + scope, err := proc.GoroutineScope(p.CurrentThread()) + assertNoError(err, t, "GoroutineScope") + + bp, err := p.SetWatchpoint(scope, "globalvar1", proc.WatchWrite, nil) + assertNoError(err, t, "SetDataBreakpoint(write-only)") + + assertNoError(p.Continue(), t, "Continue 1") + assertLineNumber(p, t, 17, "Continue 1") // Position 1 + + p.ClearBreakpoint(bp.Addr) + + assertNoError(p.Continue(), t, "Continue 2") + assertLineNumber(p, t, 19, "Continue 2") // Position 2 + + _, err = p.SetWatchpoint(scope, "globalvar1", proc.WatchWrite|proc.WatchRead, nil) + assertNoError(err, t, "SetDataBreakpoint(read-write)") + + assertNoError(p.Continue(), t, "Continue 3") + assertLineNumber(p, t, 20, "Continue 3") // Position 3 + + p.ClearBreakpoint(bp.Addr) + + assertNoError(p.Continue(), t, "Continue 4") + assertLineNumber(p, t, 25, "Continue 4") // Position 4 + + _, err = p.SetWatchpoint(scope, "globalvar1", proc.WatchWrite, nil) + assertNoError(err, t, "SetDataBreakpoint(write-only, again)") + + assertNoError(p.Continue(), t, "Continue 5") + assertLineNumber(p, t, 33, "Continue 5") // Position 5 + }) +} + +func TestWatchpointCounts(t *testing.T) { + skipOn(t, "not implemented", "windows") + skipOn(t, "not implemented", "freebsd") + skipOn(t, "not implemented", "darwin") + skipOn(t, "not implemented", "386") + skipOn(t, "not implemented", "arm64") + skipOn(t, "not implemented", "rr") + + protest.AllowRecording(t) + withTestProcess("databpcountstest", t, func(p *proc.Target, fixture protest.Fixture) { + setFunctionBreakpoint(p, t, "main.main") + assertNoError(p.Continue(), t, "Continue 0") + + scope, err := proc.GoroutineScope(p.CurrentThread()) + assertNoError(err, t, "GoroutineScope") + + bp, err := p.SetWatchpoint(scope, "globalvar1", proc.WatchWrite, nil) + assertNoError(err, t, "SetWatchpoint(write-only)") + + for { + if err := p.Continue(); err != nil { + if _, exited := err.(proc.ErrProcessExited); exited { + break + } + assertNoError(err, t, "Continue()") + } + } + + t.Logf("TotalHitCount: %d", bp.TotalHitCount) + if bp.TotalHitCount != 200 { + t.Fatalf("Wrong TotalHitCount for the breakpoint (%d)", bp.TotalHitCount) + } + + if len(bp.HitCount) != 2 { + t.Fatalf("Wrong number of goroutines for breakpoint (%d)", len(bp.HitCount)) + } + + for _, v := range bp.HitCount { + if v != 100 { + t.Fatalf("Wrong HitCount for breakpoint (%v)", bp.HitCount) + } + } + }) +} diff --git a/pkg/proc/target.go b/pkg/proc/target.go index 9bc0e59cdc..b873200719 100644 --- a/pkg/proc/target.go +++ b/pkg/proc/target.go @@ -101,6 +101,8 @@ func (sr StopReason) String() string { return "next finished" case StopCallReturned: return "call returned" + case StopWatchpoint: + return "watchpoint" default: return "" } @@ -116,6 +118,7 @@ const ( StopManual // A manual stop was requested StopNextFinished // The next/step/stepout command terminated StopCallReturned // An injected call completed + StopWatchpoint // The target process hit one or more watchpoints ) // NewTargetConfig contains the configuration for a new Target object, diff --git a/pkg/proc/target_exec.go b/pkg/proc/target_exec.go index 385ce9e25c..068dee9a66 100644 --- a/pkg/proc/target_exec.go +++ b/pkg/proc/target_exec.go @@ -201,6 +201,9 @@ func (dbp *Target) Continue() error { dbp.ClearInternalBreakpoints() } dbp.StopReason = StopBreakpoint + if curbp.Breakpoint.WatchType != 0 { + dbp.StopReason = StopWatchpoint + } return conditionErrors(threads) default: // not a manual stop, not on runtime.Breakpoint, not on a breakpoint, just repeat @@ -413,11 +416,11 @@ func (dbp *Target) StepInstruction() (err error) { if ok, err := dbp.Valid(); !ok { return err } - thread.Breakpoint().Clear() err = thread.StepInstruction() if err != nil { return err } + thread.Breakpoint().Clear() err = thread.SetCurrentBreakpoint(true) if err != nil { return err diff --git a/service/dap/server.go b/service/dap/server.go index 8c94273695..b498754e00 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1951,6 +1951,8 @@ func (s *Server) doCommand(command string, asyncSetupDone chan struct{}) { stopped.Body.Reason = "pause" case proc.StopUnknown: // can happen while terminating stopped.Body.Reason = "unknown" + case proc.StopWatchpoint: + stopped.Body.Reason = "data breakpoint" default: stopped.Body.Reason = "breakpoint" } From e45e21056933fbc4c326b7e4c87f35b09581797e Mon Sep 17 00:00:00 2001 From: Polina Sokolova Date: Fri, 7 May 2021 00:02:53 -0700 Subject: [PATCH 5/5] simplify pause test --- service/dap/server_test.go | 47 ++++++++++++++------------------------ 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/service/dap/server_test.go b/service/dap/server_test.go index b595354a85..a96c5711f3 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -3018,38 +3018,25 @@ func TestPauseAndContinue(t *testing.T) { execute: func() { verifyStopLocation(t, client, 1, "main.loop", 6) - // Advance past the breakpoint line, so breakpoint and pause don't compete - // Even though StopReason "pause" is supposed to trump "breakpoint", - // sometimes that is not the case. - client.NextRequest(1) - client.ExpectNextResponse(t) - client.ExpectStoppedEvent(t) - verifyStopLocation(t, client, 1, "main.loop", 7) + // Continue resumes all goroutines, so thread id is ignored + client.ContinueRequest(12345) + client.ExpectContinueResponse(t) - // Pause will be a no-op at a breakpoint: there will be no additional stopped events - client.PauseRequest(1) - client.ExpectPauseResponse(t) - verifyStopLocation(t, client, 1, "main.loop", 7) - - for i := 0; i < 10; i++ { - // Continue resumes all goroutines, so thread id is ignored - client.ContinueRequest(-i) - client.ExpectContinueResponse(t) - - // Halt pauses all goroutines, so thread id is ignored - client.PauseRequest(-i) - // 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) + 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) } }