diff --git a/_fixtures/goroutinebreak.go b/_fixtures/goroutinebreak.go new file mode 100644 index 00000000..438d1ed8 --- /dev/null +++ b/_fixtures/goroutinebreak.go @@ -0,0 +1,26 @@ +package main + +import "runtime" + +const N = 10 + +func agoroutine(started chan<- struct{}, done chan<- struct{}, i int) { + started <- struct{}{} + done <- struct{}{} +} + +func main() { + done := make(chan struct{}) + started := make(chan struct{}) + for i := 0; i < N; i++ { + runtime.Breakpoint() + go agoroutine(started, done, i) + } + for i := 0; i < N; i++ { + <-started + } + runtime.Gosched() + for i := 0; i < N; i++ { + <-done + } +} diff --git a/service/dap/server.go b/service/dap/server.go index 2b8d8ced..3bf160fa 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -1819,25 +1819,24 @@ func stoppedGoroutineID(state *api.DebuggerState) (id int) { // stoppedOnBreakpointGoroutineID gets the goroutine id of the first goroutine // that is stopped on a real breakpoint, starting with the selected goroutine. func (s *Session) stoppedOnBreakpointGoroutineID(state *api.DebuggerState) (int, *api.Breakpoint) { - // Check if the selected goroutine is stopped on a real breakpoint - // since we would prefer to use that one. - goid := stoppedGoroutineID(state) - if g, _ := s.debugger.FindGoroutine(goid); g != nil && g.Thread != nil { - if bp := g.Thread.Breakpoint(); bp != nil && bp.Breakpoint != nil && !bp.Breakpoint.Tracepoint { - return goid, api.ConvertBreakpoint(bp.Breakpoint) - } + // Use the first goroutine that is stopped on a breakpoint. + gs := s.stoppedGs(state) + if len(gs) == 0 { + return 0, nil } - - // Some of the breakpoints may be log points, choose the goroutine - // that is not stopped on a tracepoint. - for _, th := range state.Threads { - if bp := th.Breakpoint; bp != nil { - if !bp.Tracepoint { - return th.GoroutineID, bp - } - } + goid := gs[0] + if goid == 0 { + return goid, state.CurrentThread.Breakpoint } - return 0, nil + g, _ := s.debugger.FindGoroutine(goid) + if g == nil || g.Thread == nil { + return goid, nil + } + bp := g.Thread.Breakpoint() + if bp == nil || bp.Breakpoint == nil { + return goid, nil + } + return goid, api.ConvertBreakpoint(bp.Breakpoint) } // stepUntilStopAndNotify is a wrapper around runUntilStopAndNotify that @@ -3539,12 +3538,13 @@ func (s *Session) resumeOnceAndCheckStop(command string, allowNextStateChange ch return state, err } - foundRealBreakpoint := s.handleLogPoints(state) + s.handleLogPoints(state) + gsOnBp := s.stoppedGs(state) switch s.debugger.StopReason() { case proc.StopBreakpoint, proc.StopManual: // Make sure a real manual stop was requested or a real breakpoint was hit. - if foundRealBreakpoint || s.checkHaltRequested() { + if len(gsOnBp) > 0 || s.checkHaltRequested() { s.setRunningCmd(false) } default: @@ -3559,17 +3559,42 @@ func (s *Session) resumeOnceAndCheckStop(command string, allowNextStateChange ch return state, err } -func (s *Session) handleLogPoints(state *api.DebuggerState) bool { - foundRealBreakpoint := false +func (s *Session) handleLogPoints(state *api.DebuggerState) { for _, th := range state.Threads { if bp := th.Breakpoint; bp != nil { - logged := s.logBreakpointMessage(bp, th.GoroutineID) - if !logged { - foundRealBreakpoint = true + s.logBreakpointMessage(bp, th.GoroutineID) + } + } +} + +func (s *Session) stoppedGs(state *api.DebuggerState) (gs []int) { + // Check the current thread first. There may be no selected goroutine. + if state.CurrentThread.Breakpoint != nil && !state.CurrentThread.Breakpoint.Tracepoint { + gs = append(gs, state.CurrentThread.GoroutineID) + } + if s.debugger.StopReason() == proc.StopHardcodedBreakpoint { + gs = append(gs, stoppedGoroutineID(state)) + } + for _, th := range state.Threads { + // Some threads may be stopped on a hardcoded breakpoint. + // TODO(suzmue): This is a workaround for detecting hard coded breakpoints, + // though this check is likely not sufficient. It would be better to resolve + // this in the debugger layer instead. + if th.Function.Name() == "runtime.breakpoint" { + gs = append(gs, th.GoroutineID) + continue + } + // We already added the current thread if it had a breakpoint. + if th.ID == state.CurrentThread.ID { + continue + } + if bp := th.Breakpoint; bp != nil { + if !th.Breakpoint.Tracepoint { + gs = append(gs, th.GoroutineID) } } } - return foundRealBreakpoint + return gs } func (s *Session) logBreakpointMessage(bp *api.Breakpoint, goid int) bool { diff --git a/service/dap/server_test.go b/service/dap/server_test.go index b8b20fd5..d1d8e508 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -3258,62 +3258,76 @@ func TestHaltPreventsAutoResume(t *testing.T) { }) } -// TestConcurrentBreakpointsLogPoints executes to a breakpoint and then tests -// that a breakpoint set in the main goroutine is hit the correct number of times -// and log points set in the children goroutines produce the correct number of -// output events. +// TestConcurrentBreakpointsLogPoints tests that a breakpoint set in the main +// goroutine is hit the correct number of times and log points set in the +// children goroutines produce the correct number of output events. func TestConcurrentBreakpointsLogPoints(t *testing.T) { if runtime.GOOS == "freebsd" { t.SkipNow() } - runTest(t, "goroutinestackprog", func(client *daptest.Client, fixture protest.Fixture) { - runDebugSessionWithBPs(t, client, "launch", - // Launch - func() { + tests := []struct { + name string + fixture string + start int + breakpoints []int + }{ + { + name: "source breakpoints", + fixture: "goroutinestackprog", + breakpoints: []int{23}, + }, + { + name: "hardcoded breakpoint", + fixture: "goroutinebreak", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + runTest(t, tt.fixture, func(client *daptest.Client, fixture protest.Fixture) { + client.InitializeRequest() + client.ExpectInitializeResponseAndCapabilities(t) + client.LaunchRequest("exec", fixture.Path, !stopOnEntry) - }, - // Set breakpoints - fixture.Source, []int{20}, - []onBreakpoint{{ - // Stop at line 20 - execute: func() { - checkStop(t, client, 1, "main.main", 20) - bps := []int{8, 23} - logMessages := map[int]string{8: "hello"} - client.SetBreakpointsRequestWithArgs(fixture.Source, bps, nil, nil, logMessages) - client.ExpectSetBreakpointsResponse(t) + client.ExpectInitializedEvent(t) + client.ExpectLaunchResponse(t) - client.ContinueRequest(1) - client.ExpectContinueResponse(t) + bps := append([]int{8}, tt.breakpoints...) + logMessages := map[int]string{8: "hello"} + client.SetBreakpointsRequestWithArgs(fixture.Source, bps, nil, nil, logMessages) + client.ExpectSetBreakpointsResponse(t) - // There may be up to 1 breakpoint and any number of log points that are - // hit concurrently. We should get a stopped event everytime the breakpoint - // is hit and an output event for each log point hit. - var oeCount, seCount int - for oeCount < 10 || seCount < 10 { - switch m := client.ExpectMessage(t).(type) { - case *dap.StoppedEvent: - if m.Body.Reason != "breakpoint" || !m.Body.AllThreadsStopped || m.Body.ThreadId != 1 { - t.Errorf("\ngot %#v\nwant Reason='breakpoint' AllThreadsStopped=true ThreadId=1", m) - } - checkStop(t, client, 1, "main.main", 23) - seCount++ - client.ContinueRequest(1) - case *dap.OutputEvent: - checkLogMessage(t, m, -1, "hello", fixture.Source, 8) - oeCount++ - case *dap.ContinueResponse: - case *dap.TerminatedEvent: - t.Fatalf("\nexpected 10 output events and 10 stopped events, got %d output events and %d stopped events", oeCount, seCount) - default: - t.Fatalf("Unexpected message type: expect StoppedEvent, OutputEvent, or ContinueResponse, got %#v", m) + client.ConfigurationDoneRequest() + client.ExpectConfigurationDoneResponse(t) + + // There may be up to 1 breakpoint and any number of log points that are + // hit concurrently. We should get a stopped event everytime the breakpoint + // is hit and an output event for each log point hit. + var oeCount, seCount int + for oeCount < 10 || seCount < 10 { + switch m := client.ExpectMessage(t).(type) { + case *dap.StoppedEvent: + if m.Body.Reason != "breakpoint" || !m.Body.AllThreadsStopped || m.Body.ThreadId != 1 { + t.Errorf("\ngot %#v\nwant Reason='breakpoint' AllThreadsStopped=true ThreadId=1", m) } + seCount++ + client.ContinueRequest(1) + case *dap.OutputEvent: + checkLogMessage(t, m, -1, "hello", fixture.Source, 8) + oeCount++ + case *dap.ContinueResponse: + case *dap.TerminatedEvent: + t.Fatalf("\nexpected 10 output events and 10 stopped events, got %d output events and %d stopped events", oeCount, seCount) + default: + t.Fatalf("Unexpected message type: expect StoppedEvent, OutputEvent, or ContinueResponse, got %#v", m) } - client.ExpectTerminatedEvent(t) - }, - disconnect: false, - }}) - }) + } + // TODO(suzmue): The dap server may identify some false + // positives for hard coded breakpoints, so there may still + // be more stopped events. + client.DisconnectRequestWithKillOption(true) + }) + }) + } } func TestSetBreakpointWhileRunning(t *testing.T) { @@ -4272,6 +4286,30 @@ func TestNextAndStep(t *testing.T) { }) } +func TestHardCodedBreakpoints(t *testing.T) { + runTest(t, "consts", func(client *daptest.Client, fixture protest.Fixture) { + runDebugSessionWithBPs(t, client, "launch", + // Launch + func() { + client.LaunchRequest("exec", fixture.Path, !stopOnEntry) + }, + fixture.Source, []int{28}, + []onBreakpoint{{ // Stop at line 28 + execute: func() { + checkStop(t, client, 1, "main.main", 28) + + client.ContinueRequest(1) + client.ExpectContinueResponse(t) + se := client.ExpectStoppedEvent(t) + if se.Body.ThreadId != 1 || se.Body.Reason != "breakpoint" { + t.Errorf("\ngot %#v\nwant ThreadId=1 Reason=\"breakpoint\"", se) + } + }, + disconnect: false, + }}) + }) +} + // TestStepInstruction executes to a breakpoint and tests stepping // a single instruction func TestStepInstruction(t *testing.T) {