diff --git a/pkg/proc/breakpoints.go b/pkg/proc/breakpoints.go index 0b5dafea..3c264c0c 100644 --- a/pkg/proc/breakpoints.go +++ b/pkg/proc/breakpoints.go @@ -6,6 +6,7 @@ import ( "go/ast" "go/constant" "go/parser" + "go/token" "reflect" ) @@ -71,6 +72,12 @@ type Breakpoint struct { Cond ast.Expr // internalCond is the same as Cond but used for the condition of internal breakpoints internalCond ast.Expr + // HitCond: if not nil the breakpoint will be triggered only if the evaluated HitCond returns + // true with the TotalHitCount. + HitCond *struct { + Op token.Token + Val int + } // ReturnInfo describes how to collect return variables when this // breakpoint is hit as a return breakpoint. @@ -164,36 +171,72 @@ type returnBreakpointInfo struct { // CheckCondition evaluates bp's condition on thread. func (bp *Breakpoint) CheckCondition(thread Thread) BreakpointState { bpstate := BreakpointState{Breakpoint: bp, Active: false, Internal: false, CondError: nil} - if bp.Cond == nil && bp.internalCond == nil { + bpstate.checkCond(thread) + // Update the breakpoint hit counts. + if bpstate.Breakpoint != nil && bpstate.Active { + if g, err := GetG(thread); err == nil { + bpstate.HitCount[g.ID]++ + } + bpstate.TotalHitCount++ + } + bpstate.checkHitCond(thread) + return bpstate +} + +func (bpstate *BreakpointState) checkCond(thread Thread) { + if bpstate.Cond == nil && bpstate.internalCond == nil { bpstate.Active = true - bpstate.Internal = bp.IsInternal() - return bpstate + bpstate.Internal = bpstate.IsInternal() + return } nextDeferOk := true - if bp.Kind&NextDeferBreakpoint != 0 { + if bpstate.Kind&NextDeferBreakpoint != 0 { var err error frames, err := ThreadStacktrace(thread, 2) if err == nil { nextDeferOk = isPanicCall(frames) if !nextDeferOk { - nextDeferOk, _ = isDeferReturnCall(frames, bp.DeferReturns) + nextDeferOk, _ = isDeferReturnCall(frames, bpstate.DeferReturns) } } } - if bp.IsInternal() { + if bpstate.IsInternal() { // Check internalCondition if this is also an internal breakpoint - bpstate.Active, bpstate.CondError = evalBreakpointCondition(thread, bp.internalCond) + bpstate.Active, bpstate.CondError = evalBreakpointCondition(thread, bpstate.internalCond) bpstate.Active = bpstate.Active && nextDeferOk if bpstate.Active || bpstate.CondError != nil { bpstate.Internal = true - return bpstate + return } } - if bp.IsUser() { + if bpstate.IsUser() { // Check normal condition if this is also a user breakpoint - bpstate.Active, bpstate.CondError = evalBreakpointCondition(thread, bp.Cond) + bpstate.Active, bpstate.CondError = evalBreakpointCondition(thread, bpstate.Cond) + } +} + +// checkHitCond evaluates bp's hit condition on thread. +func (bpstate *BreakpointState) checkHitCond(thread Thread) { + if bpstate.HitCond == nil || !bpstate.Active || bpstate.Internal { + return + } + // Evaluate the breakpoint condition. + switch bpstate.HitCond.Op { + case token.EQL: + bpstate.Active = int(bpstate.TotalHitCount) == bpstate.HitCond.Val + case token.NEQ: + bpstate.Active = int(bpstate.TotalHitCount) != bpstate.HitCond.Val + case token.GTR: + bpstate.Active = int(bpstate.TotalHitCount) > bpstate.HitCond.Val + case token.LSS: + bpstate.Active = int(bpstate.TotalHitCount) < bpstate.HitCond.Val + case token.GEQ: + bpstate.Active = int(bpstate.TotalHitCount) >= bpstate.HitCond.Val + case token.LEQ: + bpstate.Active = int(bpstate.TotalHitCount) <= bpstate.HitCond.Val + case token.REM: + bpstate.Active = int(bpstate.TotalHitCount)%bpstate.HitCond.Val == 0 } - return bpstate } func isPanicCall(frames []Stackframe) bool { diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index e324b034..12123a63 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -1787,12 +1787,6 @@ func (t *gdbThread) SetCurrentBreakpoint(adjustPC bool) error { } } t.CurrentBreakpoint = bp.CheckCondition(t) - if t.CurrentBreakpoint.Breakpoint != nil && t.CurrentBreakpoint.Active { - if g, err := proc.GetG(t); err == nil { - t.CurrentBreakpoint.HitCount[g.ID]++ - } - t.CurrentBreakpoint.TotalHitCount++ - } } return nil } diff --git a/pkg/proc/native/threads.go b/pkg/proc/native/threads.go index 62aa55ba..5ae8abc0 100644 --- a/pkg/proc/native/threads.go +++ b/pkg/proc/native/threads.go @@ -155,12 +155,6 @@ func (t *nativeThread) SetCurrentBreakpoint(adjustPC bool) error { if bp != nil { t.CurrentBreakpoint = bp.CheckCondition(t) - if t.CurrentBreakpoint.Breakpoint != nil && t.CurrentBreakpoint.Active { - if g, err := proc.GetG(t); err == nil { - t.CurrentBreakpoint.HitCount[g.ID]++ - } - t.CurrentBreakpoint.TotalHitCount++ - } } return nil } diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index ed9aa16c..d2cafaf1 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -1685,6 +1685,78 @@ func TestCondBreakpointError(t *testing.T) { }) } +func TestHitCondBreakpointEQ(t *testing.T) { + withTestProcess("break", t, func(p *proc.Target, fixture protest.Fixture) { + bp := setFileBreakpoint(p, t, fixture.Source, 7) + bp.HitCond = &struct { + Op token.Token + Val int + }{token.EQL, 3} + + assertNoError(p.Continue(), t, "Continue()") + ivar := evalVariable(p, t, "i") + + i, _ := constant.Int64Val(ivar.Value) + if i != 3 { + t.Fatalf("Stoppend on wrong hitcount %d\n", i) + } + + err := p.Continue() + if _, exited := err.(proc.ErrProcessExited); !exited { + t.Fatalf("Unexpected error on Continue(): %v", err) + } + }) +} + +func TestHitCondBreakpointGEQ(t *testing.T) { + protest.AllowRecording(t) + withTestProcess("break", t, func(p *proc.Target, fixture protest.Fixture) { + bp := setFileBreakpoint(p, t, fixture.Source, 7) + bp.HitCond = &struct { + Op token.Token + Val int + }{token.GEQ, 3} + + for it := 3; it <= 10; it++ { + assertNoError(p.Continue(), t, "Continue()") + ivar := evalVariable(p, t, "i") + + i, _ := constant.Int64Val(ivar.Value) + if int(i) != it { + t.Fatalf("Stoppend on wrong hitcount %d\n", i) + } + } + + assertNoError(p.Continue(), t, "Continue()") + }) +} + +func TestHitCondBreakpointREM(t *testing.T) { + protest.AllowRecording(t) + withTestProcess("break", t, func(p *proc.Target, fixture protest.Fixture) { + bp := setFileBreakpoint(p, t, fixture.Source, 7) + bp.HitCond = &struct { + Op token.Token + Val int + }{token.REM, 2} + + for it := 2; it <= 10; it += 2 { + assertNoError(p.Continue(), t, "Continue()") + ivar := evalVariable(p, t, "i") + + i, _ := constant.Int64Val(ivar.Value) + if int(i) != it { + t.Fatalf("Stoppend on wrong hitcount %d\n", i) + } + } + + err := p.Continue() + if _, exited := err.(proc.ErrProcessExited); !exited { + t.Fatalf("Unexpected error on Continue(): %v", err) + } + }) +} + func TestIssue356(t *testing.T) { // slice with a typedef does not get printed correctly protest.AllowRecording(t) diff --git a/service/api/conversions.go b/service/api/conversions.go index 08514b4b..7a2d7179 100644 --- a/service/api/conversions.go +++ b/service/api/conversions.go @@ -47,6 +47,9 @@ func ConvertBreakpoint(bp *proc.Breakpoint) *Breakpoint { var buf bytes.Buffer printer.Fprint(&buf, token.NewFileSet(), bp.Cond) b.Cond = buf.String() + if bp.HitCond != nil { + b.HitCond = fmt.Sprintf("%s %d", bp.HitCond.Op.String(), bp.HitCond.Val) + } return b } diff --git a/service/api/types.go b/service/api/types.go index 26b165d5..f9163237 100644 --- a/service/api/types.go +++ b/service/api/types.go @@ -67,6 +67,9 @@ type Breakpoint struct { // Breakpoint condition Cond string + // Breakpoint hit count condition. + // Supported hit count conditions are "NUMBER" and "OP NUMBER". + HitCond string // Tracepoint flag, signifying this is a tracepoint. Tracepoint bool `json:"continue"` diff --git a/service/dap/daptest/client.go b/service/dap/daptest/client.go index cb2036b7..2b897241 100644 --- a/service/dap/daptest/client.go +++ b/service/dap/daptest/client.go @@ -235,7 +235,6 @@ func (c *Client) SetConditionalBreakpointsRequest(file string, lines []int, cond Path: file, }, Breakpoints: make([]dap.SourceBreakpoint, len(lines)), - //sourceModified: false, } for i, l := range lines { request.Arguments.Breakpoints[i].Line = l @@ -247,6 +246,26 @@ func (c *Client) SetConditionalBreakpointsRequest(file string, lines []int, cond c.send(request) } +// SetBreakpointsRequest sends a 'setBreakpoints' request with conditions. +func (c *Client) SetHitConditionalBreakpointsRequest(file string, lines []int, conditions map[int]string) { + request := &dap.SetBreakpointsRequest{Request: *c.newRequest("setBreakpoints")} + request.Arguments = dap.SetBreakpointsArguments{ + Source: dap.Source{ + Name: filepath.Base(file), + Path: file, + }, + Breakpoints: make([]dap.SourceBreakpoint, len(lines)), + } + for i, l := range lines { + request.Arguments.Breakpoints[i].Line = l + cond, ok := conditions[l] + if ok { + request.Arguments.Breakpoints[i].HitCondition = cond + } + } + c.send(request) +} + // SetExceptionBreakpointsRequest sends a 'setExceptionBreakpoints' request. func (c *Client) SetExceptionBreakpointsRequest() { request := &dap.SetBreakpointsRequest{Request: *c.newRequest("setExceptionBreakpoints")} diff --git a/service/dap/server.go b/service/dap/server.go index 92de211f..6dcbd4e8 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -548,7 +548,6 @@ func (s *Server) handleRequest(request dap.Message) { s.onSetBreakpointsRequest(request) case *dap.SetFunctionBreakpointsRequest: // Optional (capability ‘supportsFunctionBreakpoints’) - // TODO: implement this request in V1 s.onSetFunctionBreakpointsRequest(request) case *dap.SetExceptionBreakpointsRequest: // Optional (capability ‘exceptionBreakpointFilters’) @@ -1030,38 +1029,239 @@ func (s *Server) onSetBreakpointsRequest(request *dap.SetBreakpointsRequest) { // According to the spec we should "set multiple breakpoints for a single source // and clear all previous breakpoints in that source." The simplest way is - // to clear all and then set all. + // to clear all and then set all. To maintain state (for hit count conditions) + // we want to amend existing breakpoints. // - // TODO(polina): should we optimize this as follows? // See https://github.com/golang/vscode-go/issues/163 for details. // If a breakpoint: // -- exists and not in request => ClearBreakpoint // -- exists and in request => AmendBreakpoint // -- doesn't exist and in request => SetBreakpoint - // Clear all existing breakpoints in the file that are not function breakpoints. - if err := s.clearMatchingBreakpoints(func(bp *api.Breakpoint) bool { return bp.File == serverPath && bp.Name == "" }); err != nil { + // Get all existing breakpoints that match for this source. + sourceRequestPrefix := fmt.Sprintf("sourceBp Path=%q ", request.Arguments.Source.Path) + existingBps := s.getMatchingBreakpoints(sourceRequestPrefix) + bpAdded := make(map[string]struct{}, len(existingBps)) + + // Amend existing breakpoints. + breakpoints := make([]dap.Breakpoint, len(request.Arguments.Breakpoints)) + for i, want := range request.Arguments.Breakpoints { + reqString := fmt.Sprintf("%s Line=%d Column=%d", sourceRequestPrefix, want.Line, want.Column) + var err error + got, ok := existingBps[reqString] + if !ok { + // Skip if the breakpoint does not already exist. + // These will be created after deleting existing + // breakpoints to avoid conflicts. + continue + } + if _, ok := bpAdded[reqString]; ok { + err = fmt.Errorf("Breakpoint exists at %q, line: %d, column: %d", request.Arguments.Source.Path, want.Line, want.Column) + } else { + got.Cond = want.Condition + got.HitCond = want.HitCondition + err = s.debugger.AmendBreakpoint(got) + bpAdded[reqString] = struct{}{} + } + + updateBreakpointsResponse(breakpoints, i, err, got, clientPath) + } + + // Clear existing breakpoints that were not added. + err := s.clearBreakpoints(existingBps, bpAdded) + if err != nil { s.sendErrorResponse(request.Request, UnableToSetBreakpoints, "Unable to set or clear breakpoints", err.Error()) return } - // Set all requested breakpoints. - response := &dap.SetBreakpointsResponse{Response: *newResponse(request.Request)} - response.Body.Breakpoints = make([]dap.Breakpoint, len(request.Arguments.Breakpoints)) for i, want := range request.Arguments.Breakpoints { - got, err := s.debugger.CreateBreakpoint( - &api.Breakpoint{File: serverPath, Line: want.Line, Cond: want.Condition}) - response.Body.Breakpoints[i].Verified = (err == nil) - if err != nil { - response.Body.Breakpoints[i].Line = want.Line - response.Body.Breakpoints[i].Message = err.Error() + reqString := fmt.Sprintf("%s Line=%d Column=%d", sourceRequestPrefix, want.Line, want.Column) + if _, ok := existingBps[reqString]; ok { + continue + } + + var got *api.Breakpoint + var err error + if _, ok := bpAdded[reqString]; ok { + err = fmt.Errorf("Breakpoint exists at %q, line: %d, column: %d", request.Arguments.Source.Path, want.Line, want.Column) } else { - response.Body.Breakpoints[i].Id = got.ID - response.Body.Breakpoints[i].Line = got.Line - response.Body.Breakpoints[i].Source = dap.Source{Name: request.Arguments.Source.Name, Path: clientPath} + // Create new breakpoints. + got, err = s.debugger.CreateBreakpoint( + &api.Breakpoint{File: serverPath, Line: want.Line, Cond: want.Condition, HitCond: want.HitCondition, Name: reqString}) + bpAdded[reqString] = struct{}{} + } + + updateBreakpointsResponse(breakpoints, i, err, got, clientPath) + } + response := &dap.SetBreakpointsResponse{Response: *newResponse(request.Request)} + response.Body.Breakpoints = breakpoints + + s.send(response) +} + +func updateBreakpointsResponse(breakpoints []dap.Breakpoint, i int, err error, got *api.Breakpoint, path string) { + breakpoints[i].Verified = (err == nil) + if err != nil { + breakpoints[i].Message = err.Error() + } else { + breakpoints[i].Id = got.ID + breakpoints[i].Line = got.Line + breakpoints[i].Source = dap.Source{Name: filepath.Base(path), Path: path} + } +} + +// functionBpPrefix is the prefix of bp.Name for every breakpoint bp set +// in this request. +const functionBpPrefix = "functionBreakpoint" + +func (s *Server) onSetFunctionBreakpointsRequest(request *dap.SetFunctionBreakpointsRequest) { + if s.noDebugProcess != nil { + s.sendErrorResponse(request.Request, UnableToSetBreakpoints, "Unable to set or clear breakpoints", "running in noDebug mode") + return + } + + // According to the spec, setFunctionBreakpoints "replaces all existing function + // breakpoints with new function breakpoints." The simplest way is + // to clear all and then set all. To maintain state (for hit count conditions) + // we want to amend existing breakpoints. + // + // See https://github.com/golang/vscode-go/issues/163 for details. + // If a breakpoint: + // -- exists and not in request => ClearBreakpoint + // -- exists and in request => AmendBreakpoint + // -- doesn't exist and in request => SetBreakpoint + + // Get all existing function breakpoints. + existingBps := s.getMatchingBreakpoints(functionBpPrefix) + bpAdded := make(map[string]struct{}, len(existingBps)) + for _, bp := range existingBps { + existingBps[bp.Name] = bp + } + + // Amend any existing breakpoints. + breakpoints := make([]dap.Breakpoint, len(request.Arguments.Breakpoints)) + for i, want := range request.Arguments.Breakpoints { + reqString := fmt.Sprintf("%s Name=%s", functionBpPrefix, want.Name) + var err error + got, ok := existingBps[reqString] + if !ok { + // Skip if the breakpoint does not already exist. + // These will be created after deleting existing + // breakpoints to avoid conflicts. + continue + } + if _, ok := bpAdded[reqString]; ok { + err = fmt.Errorf("Breakpoint exists at function %q", want.Name) + } else { + got.Cond = want.Condition + got.HitCond = want.HitCondition + err = s.debugger.AmendBreakpoint(got) + bpAdded[reqString] = struct{}{} + } + + var clientPath string + if got != nil { + clientPath = s.toClientPath(got.File) + } + updateBreakpointsResponse(breakpoints, i, err, got, clientPath) + } + + // Clear existing breakpoints that were not added. + err := s.clearBreakpoints(existingBps, bpAdded) + if err != nil { + s.sendErrorResponse(request.Request, UnableToSetBreakpoints, "Unable to set or clear breakpoints", err.Error()) + return + } + + // Create new breakpoints. + for i, want := range request.Arguments.Breakpoints { + reqString := fmt.Sprintf("%s Name=%s", functionBpPrefix, want.Name) + if _, ok := existingBps[reqString]; ok { + // Amend existing breakpoints. + continue + } + + // Set the function breakpoint breakpoint + spec, err := locspec.Parse(want.Name) + if err != nil { + breakpoints[i].Message = err.Error() + continue + } + if loc, ok := spec.(*locspec.NormalLocationSpec); !ok || loc.FuncBase == nil { + // Other locations do not make sense in the context of function breakpoints. + // Regex locations are likely to resolve to multiple places and offset locations + // are only meaningful at the time the breakpoint was created. + breakpoints[i].Message = fmt.Sprintf("breakpoint name %q could not be parsed as a function. name must be in the format 'funcName', 'funcName:line' or 'fileName:line'.", want.Name) + continue + } + + if want.Name[0] == '.' { + breakpoints[i].Message = "breakpoint names that are relative paths are not supported." + continue + } + // Find the location of the function name. CreateBreakpoint requires the name to include the base + // (e.g. main.functionName is supported but not functionName). + // We first find the location of the function, and then set breakpoints for that location. + var locs []api.Location + locs, err = s.debugger.FindLocationSpec(-1, 0, 0, want.Name, spec, true, s.args.substitutePathClientToServer) + if err != nil { + breakpoints[i].Message = err.Error() + continue + } + if len(locs) == 0 { + breakpoints[i].Message = fmt.Sprintf("no location found for %q", want.Name) + continue + } + if len(locs) > 0 { + s.log.Debugf("multiple locations found for %s", want.Name) + breakpoints[i].Message = fmt.Sprintf("multiple locations found for %s, function breakpoint is only set for the first location", want.Name) + } + + // Set breakpoint using the PCs that were found. + loc := locs[0] + got, err := s.debugger.CreateBreakpoint(&api.Breakpoint{Addr: loc.PC, Addrs: loc.PCs, Cond: want.Condition, Name: reqString}) + + var clientPath string + if got != nil { + clientPath = s.toClientPath(got.File) + } + updateBreakpointsResponse(breakpoints, i, err, got, clientPath) + } + + response := &dap.SetFunctionBreakpointsResponse{Response: *newResponse(request.Request)} + response.Body.Breakpoints = breakpoints + + s.send(response) +} + +func (s *Server) clearBreakpoints(existingBps map[string]*api.Breakpoint, bpAdded map[string]struct{}) error { + for req, bp := range existingBps { + if _, ok := bpAdded[req]; ok { + continue + } + _, err := s.debugger.ClearBreakpoint(bp) + if err != nil { + return err } } - s.send(response) + return nil +} + +func (s *Server) getMatchingBreakpoints(prefix string) map[string]*api.Breakpoint { + existing := s.debugger.Breakpoints() + matchingBps := make(map[string]*api.Breakpoint, len(existing)) + for _, bp := range existing { + // Skip special breakpoints such as for panic. + if bp.ID < 0 { + continue + } + // Skip breakpoints that do not meet the condition. + if !strings.HasPrefix(bp.Name, prefix) { + continue + } + matchingBps[bp.Name] = bp + } + return matchingBps } func (s *Server) onSetExceptionBreakpointsRequest(request *dap.SetExceptionBreakpointsRequest) { @@ -1957,102 +2157,6 @@ func (s *Server) onRestartRequest(request *dap.RestartRequest) { s.sendNotYetImplementedErrorResponse(request.Request) } -// functionBpPrefix is the prefix of bp.Name for every breakpoint bp set -// in this request. -const functionBpPrefix = "functionBreakpoint" - -func (s *Server) onSetFunctionBreakpointsRequest(request *dap.SetFunctionBreakpointsRequest) { - if s.noDebugProcess != nil { - s.sendErrorResponse(request.Request, UnableToSetBreakpoints, "Unable to set or clear breakpoints", "running in noDebug mode") - return - } - - // According to the spec, setFunctionBreakpoints "replaces all existing function - // breakpoints with new function breakpoints." The simplest way is - // to clear all and then set all. - - // Clear all existing function breakpoints in the file. - // Function breakpoints are set with the Name field set to be the - // function name. We cannot use FunctionName to determine this, because - // the debugger sets the function name for all breakpoints. - if err := s.clearMatchingBreakpoints(func(bp *api.Breakpoint) bool { return strings.HasPrefix(bp.Name, functionBpPrefix) }); err != nil { - s.sendErrorResponse(request.Request, UnableToSetBreakpoints, "Unable to set or clear breakpoints", err.Error()) - return - } - - // Set all requested breakpoints. - response := &dap.SetFunctionBreakpointsResponse{Response: *newResponse(request.Request)} - response.Body.Breakpoints = make([]dap.Breakpoint, len(request.Arguments.Breakpoints)) - for i, want := range request.Arguments.Breakpoints { - spec, err := locspec.Parse(want.Name) - if err != nil { - response.Body.Breakpoints[i].Message = err.Error() - continue - } - if loc, ok := spec.(*locspec.NormalLocationSpec); !ok || loc.FuncBase == nil { - // Other locations do not make sense in the context of function breakpoints. - // Regex locations are likely to resolve to multiple places and offset locations - // are only meaningful at the time the breakpoint was created. - response.Body.Breakpoints[i].Message = fmt.Sprintf("breakpoint name %q could not be parsed as a function. name must be in the format 'funcName', 'funcName:line' or 'fileName:line'.", want.Name) - continue - } - - if want.Name[0] == '.' { - response.Body.Breakpoints[i].Message = "breakpoint names that are relative paths are not supported." - continue - } - // Find the location of the function name. CreateBreakpoint requires the name to include the base - // (e.g. main.functionName is supported but not functionName). - // We first find the location of the function, and then set breakpoints for that location. - locs, err := s.debugger.FindLocationSpec(-1, 0, 0, want.Name, spec, true, s.args.substitutePathClientToServer) - if err != nil { - response.Body.Breakpoints[i].Message = err.Error() - continue - } - if len(locs) == 0 { - response.Body.Breakpoints[i].Message = fmt.Sprintf("no location found for %q", want.Name) - continue - } - if len(locs) > 0 { - s.log.Debugf("multiple locations found for %s", want.Name) - response.Body.Breakpoints[i].Message = fmt.Sprintf("multiple locations found for %s, function breakpoint is only set for the first location", want.Name) - } - - // Set breakpoint using the PCs that were found. - loc := locs[0] - got, err := s.debugger.CreateBreakpoint( - &api.Breakpoint{Name: fmt.Sprintf("%s%d", functionBpPrefix, i), Addr: loc.PC, Addrs: loc.PCs, Cond: want.Condition}) - response.Body.Breakpoints[i].Verified = (err == nil) - if err != nil { - response.Body.Breakpoints[i].Message = err.Error() - } else { - response.Body.Breakpoints[i].Id = got.ID - response.Body.Breakpoints[i].Line = got.Line - response.Body.Breakpoints[i].Source = dap.Source{Name: filepath.Base(got.File), Path: got.File} - } - } - s.send(response) -} - -func (s *Server) clearMatchingBreakpoints(matchCondition func(*api.Breakpoint) bool) error { - existing := s.debugger.Breakpoints() - for _, bp := range existing { - // Skip special breakpoints such as for panic. - if bp.ID < 0 { - continue - } - // Skip breakpoints that do not meet the condition. - if !matchCondition(bp) { - continue - } - _, err := s.debugger.ClearBreakpoint(bp) - if err != nil { - return err - } - } - return nil -} - // onStepBackRequest sends a not-yet-implemented error response. // Capability 'supportsStepBack' is not set 'initialize' response. func (s *Server) onStepBackRequest(request *dap.StepBackRequest) { diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 21fac004..75f5c6dd 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -1865,6 +1865,12 @@ func checkBreakpoints(t *testing.T, client *daptest.Client, bps []Breakpoint, br return } for i, bp := range breakpoints { + if bps[i].line < 0 && !bps[i].verified { + if bp.Verified != bps[i].verified || !strings.Contains(bp.Message, bps[i].msgPrefix) { + t.Errorf("got breakpoints[%d] = %#v, \nwant %#v", i, bp, bps[i]) + } + continue + } 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]) @@ -1903,7 +1909,7 @@ func TestSetBreakpoint(t *testing.T) { // Set another breakpoint inside the loop in loop(), twice to trigger error client.SetBreakpointsRequest(fixture.Source, []int{8, 8}) - expectSetBreakpointsResponse(t, client, []Breakpoint{{8, fixture.Source, true, ""}, {8, "", false, "Breakpoint exists"}}) + expectSetBreakpointsResponse(t, client, []Breakpoint{{8, fixture.Source, true, ""}, {-1, "", false, "Breakpoint exists"}}) // Continue into the loop client.ContinueRequest(1) @@ -1942,7 +1948,7 @@ func TestSetBreakpoint(t *testing.T) { // Set at a line without a statement client.SetBreakpointsRequest(fixture.Source, []int{1000}) - expectSetBreakpointsResponse(t, client, []Breakpoint{{1000, "", false, "could not find statement"}}) // all cleared, none set + expectSetBreakpointsResponse(t, client, []Breakpoint{{-1, "", false, "could not find statement"}}) // all cleared, none set }, // The program has an infinite loop, so we must kill it by disconnecting. disconnect: true, @@ -1978,7 +1984,7 @@ func TestSetFunctionBreakpoints(t *testing.T) { return } for i, bp := range got.Body.Breakpoints { - if bps[i].line < 0 { + if bps[i].line < 0 && !bps[i].verified { if bp.Verified != bps[i].verified || !strings.Contains(bp.Message, bps[i].errMsg) { t.Errorf("got breakpoints[%d] = %#v, \nwant %#v", i, bp, bps[i]) } @@ -2361,6 +2367,71 @@ func TestSetFunctionBreakpointWhileRunning(t *testing.T) { }) } +func TestHitConditionBreakpoints(t *testing.T) { + runTest(t, "break", func(client *daptest.Client, fixture protest.Fixture) { + runDebugSessionWithBPs(t, client, "launch", + // Launch + func() { + client.LaunchRequest("exec", fixture.Path, !stopOnEntry) + }, + // Set breakpoints + fixture.Source, []int{4}, + []onBreakpoint{{ + execute: func() { + client.SetHitConditionalBreakpointsRequest(fixture.Source, []int{7}, map[int]string{7: "4"}) + expectSetBreakpointsResponse(t, client, []Breakpoint{{7, fixture.Source, true, ""}}) + + client.ContinueRequest(1) + client.ExpectContinueResponse(t) + client.ExpectStoppedEvent(t) + checkStop(t, client, 1, "main.main", 7) + + // Check that we are stopped at the correct value of i. + client.VariablesRequest(1001) + locals := client.ExpectVariablesResponse(t) + checkVarExact(t, locals, 0, "i", "i", "4", "int", noChildren) + + // Change the hit condition. + client.SetHitConditionalBreakpointsRequest(fixture.Source, []int{7}, map[int]string{7: "% 2"}) + expectSetBreakpointsResponse(t, client, []Breakpoint{{7, fixture.Source, true, ""}}) + + client.ContinueRequest(1) + client.ExpectContinueResponse(t) + client.ExpectStoppedEvent(t) + checkStop(t, client, 1, "main.main", 7) + + // Check that we are stopped at the correct value of i. + client.VariablesRequest(1001) + locals = client.ExpectVariablesResponse(t) + checkVarExact(t, locals, 0, "i", "i", "6", "int", noChildren) + + // Expect an error if an assignment is passed. + client.SetHitConditionalBreakpointsRequest(fixture.Source, []int{7}, map[int]string{7: "= 2"}) + expectSetBreakpointsResponse(t, client, []Breakpoint{{-1, "", false, ""}}) + + // Change the hit condition. + client.SetHitConditionalBreakpointsRequest(fixture.Source, []int{7}, map[int]string{7: "< 8"}) + expectSetBreakpointsResponse(t, client, []Breakpoint{{7, fixture.Source, true, ""}}) + client.ContinueRequest(1) + client.ExpectContinueResponse(t) + client.ExpectStoppedEvent(t) + checkStop(t, client, 1, "main.main", 7) + + // Check that we are stopped at the correct value of i. + client.VariablesRequest(1001) + locals = client.ExpectVariablesResponse(t) + checkVarExact(t, locals, 0, "i", "i", "7", "int", noChildren) + + client.ContinueRequest(1) + client.ExpectContinueResponse(t) + + client.ExpectTerminatedEvent(t) + }, + disconnect: false, + }}) + }) +} + // 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. diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 00ff9333..e4505b31 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -6,11 +6,13 @@ import ( "errors" "fmt" "go/parser" + "go/token" "os" "path/filepath" "regexp" "runtime" "sort" + "strconv" "strings" "sync" "time" @@ -629,9 +631,6 @@ func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoin ) if requestedBp.Name != "" { - if err = api.ValidBreakpointName(requestedBp.Name); err != nil { - return nil, err - } if (d.findBreakpointByName(requestedBp.Name) != nil) || (d.findDisabledBreakpointByName(requestedBp.Name) != nil) { return nil, errors.New("breakpoint name already exists") } @@ -742,9 +741,6 @@ func (d *Debugger) AmendBreakpoint(amend *api.Breakpoint) error { if originals == nil && !disabled { return fmt.Errorf("no breakpoint with ID %d", amend.ID) } - if err := api.ValidBreakpointName(amend.Name); err != nil { - return err - } if !amend.Disabled && disabled { // enable the breakpoint bp, err := d.target.SetBreakpointWithID(amend.ID, amend.Addr) if err != nil { @@ -789,9 +785,63 @@ func copyBreakpointInfo(bp *proc.Breakpoint, requested *api.Breakpoint) (err err if requested.Cond != "" { bp.Cond, err = parser.ParseExpr(requested.Cond) } + bp.HitCond = nil + if requested.HitCond != "" { + opTok, val, parseErr := parseHitCondition(requested.HitCond) + if err == nil { + err = parseErr + } + if parseErr == nil { + bp.HitCond = &struct { + Op token.Token + Val int + }{opTok, val} + } + } return err } +func parseHitCondition(hitCond string) (token.Token, int, error) { + // A hit condition can be in the following formats: + // - "number" + // - "OP number" + hitConditionRegex := regexp.MustCompile(`((=|>|<|%|!)+|)( |)((\d|_)+)`) + + match := hitConditionRegex.FindStringSubmatch(strings.TrimSpace(hitCond)) + if match == nil || len(match) != 6 { + return 0, 0, fmt.Errorf("unable to parse breakpoint hit condition: %q\nhit conditions should be of the form \"number\" or \"OP number\"", hitCond) + } + + opStr := match[1] + var opTok token.Token + switch opStr { + case "==", "": + opTok = token.EQL + case ">=": + opTok = token.GEQ + case "<=": + opTok = token.LEQ + case ">": + opTok = token.GTR + case "<": + opTok = token.LSS + case "%": + opTok = token.REM + case "!=": + opTok = token.NEQ + default: + return 0, 0, fmt.Errorf("unable to parse breakpoint hit condition: %q\ninvalid operator: %q", hitCond, opStr) + } + + numStr := match[4] + val, parseErr := strconv.Atoi(numStr) + if parseErr != nil { + return 0, 0, fmt.Errorf("unable to parse breakpoint hit condition: %q\ninvalid number: %q", hitCond, numStr) + } + + return opTok, val, nil +} + // ClearBreakpoint clears a breakpoint. func (d *Debugger) ClearBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoint, error) { d.targetMutex.Lock() @@ -956,7 +1006,7 @@ func (d *Debugger) CreateWatchpoint(goid, frame, deferredCall int, expr string, if err != nil { return nil, err } - if api.ValidBreakpointName(expr) == nil && d.findBreakpointByName(expr) == nil { + if d.findBreakpointByName(expr) == nil { bp.Name = expr } return api.ConvertBreakpoint(bp), nil diff --git a/service/rpc1/server.go b/service/rpc1/server.go index 558594dd..2844dc82 100644 --- a/service/rpc1/server.go +++ b/service/rpc1/server.go @@ -103,6 +103,9 @@ func (s *RPCServer) ListBreakpoints(arg interface{}, breakpoints *[]*api.Breakpo } func (s *RPCServer) CreateBreakpoint(bp, newBreakpoint *api.Breakpoint) error { + if err := api.ValidBreakpointName(bp.Name); err != nil { + return err + } createdbp, err := s.debugger.CreateBreakpoint(bp) if err != nil { return err @@ -139,6 +142,9 @@ func (s *RPCServer) ClearBreakpointByName(name string, breakpoint *api.Breakpoin func (s *RPCServer) AmendBreakpoint(amend *api.Breakpoint, unused *int) error { *unused = 0 + if err := api.ValidBreakpointName(amend.Name); err != nil { + return err + } return s.debugger.AmendBreakpoint(amend) } diff --git a/service/rpc2/server.go b/service/rpc2/server.go index 3d5edb1c..05dbfbda 100644 --- a/service/rpc2/server.go +++ b/service/rpc2/server.go @@ -249,6 +249,9 @@ type CreateBreakpointOut struct { // // - Otherwise the value specified by arg.Breakpoint.Addr will be used. func (s *RPCServer) CreateBreakpoint(arg CreateBreakpointIn, out *CreateBreakpointOut) error { + if err := api.ValidBreakpointName(arg.Breakpoint.Name); err != nil { + return err + } createdbp, err := s.debugger.CreateBreakpoint(&arg.Breakpoint) if err != nil { return err @@ -314,6 +317,9 @@ func (s *RPCServer) ToggleBreakpoint(arg ToggleBreakpointIn, out *ToggleBreakpoi } } bp.Disabled = !bp.Disabled + if err := api.ValidBreakpointName(bp.Name); err != nil { + return err + } if err := s.debugger.AmendBreakpoint(bp); err != nil { return err } @@ -334,6 +340,9 @@ type AmendBreakpointOut struct { // // arg.Breakpoint.ID must be a valid breakpoint ID func (s *RPCServer) AmendBreakpoint(arg AmendBreakpointIn, out *AmendBreakpointOut) error { + if err := api.ValidBreakpointName(arg.Breakpoint.Name); err != nil { + return err + } return s.debugger.AmendBreakpoint(&arg.Breakpoint) }