Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

terminal: restore breakpoints set with line offset on restart #3425

Merged
merged 1 commit into from
Jul 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 42 additions & 36 deletions pkg/locspec/locations.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const maxFindLocationCandidates = 5
// LocationSpec is an interface that represents a parsed location spec string.
type LocationSpec interface {
// Find returns all locations that match the location spec.
Find(t *proc.Target, processArgs []string, scope *proc.EvalScope, locStr string, includeNonExecutableLines bool, substitutePathRules [][2]string) ([]api.Location, error)
Find(t *proc.Target, processArgs []string, scope *proc.EvalScope, locStr string, includeNonExecutableLines bool, substitutePathRules [][2]string) ([]api.Location, string, error)
}

// NormalLocationSpec represents a basic location spec.
Expand Down Expand Up @@ -269,15 +269,15 @@ func packageMatch(specPkg, symPkg string, packageMap map[string][]string) bool {

// Find will search all functions in the target program and filter them via the
// regex location spec. Only functions matching the regex will be returned.
func (loc *RegexLocationSpec) Find(t *proc.Target, _ []string, scope *proc.EvalScope, locStr string, includeNonExecutableLines bool, _ [][2]string) ([]api.Location, error) {
func (loc *RegexLocationSpec) Find(t *proc.Target, _ []string, scope *proc.EvalScope, locStr string, includeNonExecutableLines bool, _ [][2]string) ([]api.Location, string, error) {
if scope == nil {
//TODO(aarzilli): this needs only the list of function we should make it work
return nil, fmt.Errorf("could not determine location (scope is nil)")
return nil, "", fmt.Errorf("could not determine location (scope is nil)")
}
funcs := scope.BinInfo.Functions
matches, err := regexFilterFuncs(loc.FuncRegex, funcs)
if err != nil {
return nil, err
return nil, "", err
}
r := make([]api.Location, 0, len(matches))
for i := range matches {
Expand All @@ -286,39 +286,39 @@ func (loc *RegexLocationSpec) Find(t *proc.Target, _ []string, scope *proc.EvalS
r = append(r, addressesToLocation(addrs))
}
}
return r, nil
return r, "", nil
}

// Find returns the locations specified via the address location spec.
func (loc *AddrLocationSpec) Find(t *proc.Target, _ []string, scope *proc.EvalScope, locStr string, includeNonExecutableLines bool, _ [][2]string) ([]api.Location, error) {
func (loc *AddrLocationSpec) Find(t *proc.Target, _ []string, scope *proc.EvalScope, locStr string, includeNonExecutableLines bool, _ [][2]string) ([]api.Location, string, error) {
if scope == nil {
addr, err := strconv.ParseInt(loc.AddrExpr, 0, 64)
if err != nil {
return nil, fmt.Errorf("could not determine current location (scope is nil)")
return nil, "", fmt.Errorf("could not determine current location (scope is nil)")
}
return []api.Location{{PC: uint64(addr)}}, nil
return []api.Location{{PC: uint64(addr)}}, "", nil
}

v, err := scope.EvalExpression(loc.AddrExpr, proc.LoadConfig{FollowPointers: true})
if err != nil {
return nil, err
return nil, "", err
}
if v.Unreadable != nil {
return nil, v.Unreadable
return nil, "", v.Unreadable
}
switch v.Kind {
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
addr, _ := constant.Uint64Val(v.Value)
return []api.Location{{PC: addr}}, nil
return []api.Location{{PC: addr}}, "", nil
case reflect.Func:
fn := scope.BinInfo.PCToFunc(uint64(v.Base))
pc, err := proc.FirstPCAfterPrologue(t, fn, false)
if err != nil {
return nil, err
return nil, "", err
}
return []api.Location{{PC: pc}}, nil
return []api.Location{{PC: pc}}, v.Name, nil
default:
return nil, fmt.Errorf("wrong expression kind: %v", v.Kind)
return nil, "", fmt.Errorf("wrong expression kind: %v", v.Kind)
}
}

Expand Down Expand Up @@ -371,7 +371,7 @@ func (ale AmbiguousLocationError) Error() string {
// Find will return a list of locations that match the given location spec.
// This matches each other location spec that does not already have its own spec
// implemented (such as regex, or addr).
func (loc *NormalLocationSpec) Find(t *proc.Target, processArgs []string, scope *proc.EvalScope, locStr string, includeNonExecutableLines bool, substitutePathRules [][2]string) ([]api.Location, error) {
func (loc *NormalLocationSpec) Find(t *proc.Target, processArgs []string, scope *proc.EvalScope, locStr string, includeNonExecutableLines bool, substitutePathRules [][2]string) ([]api.Location, string, error) {
limit := maxFindLocationCandidates
var candidateFiles []string
for _, sourceFile := range t.BinInfo().Sources {
Expand All @@ -396,19 +396,19 @@ func (loc *NormalLocationSpec) Find(t *proc.Target, processArgs []string, scope

if matching := len(candidateFiles) + len(candidateFuncs); matching == 0 {
if scope == nil {
return nil, fmt.Errorf("location %q not found", locStr)
return nil, "", fmt.Errorf("location %q not found", locStr)
}
// if no result was found this locations string could be an
// expression that the user forgot to prefix with '*', try treating it as
// such.
addrSpec := &AddrLocationSpec{AddrExpr: locStr}
locs, err := addrSpec.Find(t, processArgs, scope, locStr, includeNonExecutableLines, nil)
locs, subst, err := addrSpec.Find(t, processArgs, scope, locStr, includeNonExecutableLines, nil)
if err != nil {
return nil, fmt.Errorf("location %q not found", locStr)
return nil, "", fmt.Errorf("location %q not found", locStr)
}
return locs, nil
return locs, subst, nil
} else if matching > 1 {
return nil, AmbiguousLocationError{Location: locStr, CandidatesString: append(candidateFiles, candidateFuncs...)}
return nil, "", AmbiguousLocationError{Location: locStr, CandidatesString: append(candidateFiles, candidateFuncs...)}
}

// len(candidateFiles) + len(candidateFuncs) == 1
Expand All @@ -417,22 +417,22 @@ func (loc *NormalLocationSpec) Find(t *proc.Target, processArgs []string, scope
if len(candidateFiles) == 1 {
if loc.LineOffset < 0 {
//lint:ignore ST1005 backwards compatibility
return nil, fmt.Errorf("Malformed breakpoint location, no line offset specified")
return nil, "", fmt.Errorf("Malformed breakpoint location, no line offset specified")
}
addrs, err = proc.FindFileLocation(t, candidateFiles[0], loc.LineOffset)
if includeNonExecutableLines {
if _, isCouldNotFindLine := err.(*proc.ErrCouldNotFindLine); isCouldNotFindLine {
return []api.Location{{File: candidateFiles[0], Line: loc.LineOffset}}, nil
return []api.Location{{File: candidateFiles[0], Line: loc.LineOffset}}, "", nil
}
}
} else { // len(candidateFuncs) == 1
addrs, err = proc.FindFunctionLocation(t, candidateFuncs[0], loc.LineOffset)
}

if err != nil {
return nil, err
return nil, "", err
}
return []api.Location{addressesToLocation(addrs)}, nil
return []api.Location{addressesToLocation(addrs)}, "", nil
}

func (loc *NormalLocationSpec) findFuncCandidates(bi *proc.BinaryInfo, limit int) []string {
Expand Down Expand Up @@ -585,42 +585,48 @@ func addressesToLocation(addrs []uint64) api.Location {
}

// Find returns the location after adding the offset amount to the current line number.
func (loc *OffsetLocationSpec) Find(t *proc.Target, _ []string, scope *proc.EvalScope, _ string, includeNonExecutableLines bool, _ [][2]string) ([]api.Location, error) {
func (loc *OffsetLocationSpec) Find(t *proc.Target, _ []string, scope *proc.EvalScope, _ string, includeNonExecutableLines bool, _ [][2]string) ([]api.Location, string, error) {
if scope == nil {
return nil, fmt.Errorf("could not determine current location (scope is nil)")
return nil, "", fmt.Errorf("could not determine current location (scope is nil)")
}
file, line, fn := scope.BinInfo.PCToLine(scope.PC)
if loc.Offset == 0 {
return []api.Location{{PC: scope.PC}}, nil
subst := ""
if fn != nil {
subst = fmt.Sprintf("%s:%d", file, line)
}
return []api.Location{{PC: scope.PC}}, subst, nil
}
file, line, fn := scope.BinInfo.PCToLine(scope.PC)
if fn == nil {
return nil, fmt.Errorf("could not determine current location")
return nil, "", fmt.Errorf("could not determine current location")
}
subst := fmt.Sprintf("%s:%d", file, line+loc.Offset)
addrs, err := proc.FindFileLocation(t, file, line+loc.Offset)
if includeNonExecutableLines {
if _, isCouldNotFindLine := err.(*proc.ErrCouldNotFindLine); isCouldNotFindLine {
return []api.Location{{File: file, Line: line + loc.Offset}}, nil
return []api.Location{{File: file, Line: line + loc.Offset}}, subst, nil
}
}
return []api.Location{addressesToLocation(addrs)}, err
return []api.Location{addressesToLocation(addrs)}, subst, err
}

// Find will return the location at the given line in the current file.
func (loc *LineLocationSpec) Find(t *proc.Target, _ []string, scope *proc.EvalScope, _ string, includeNonExecutableLines bool, _ [][2]string) ([]api.Location, error) {
func (loc *LineLocationSpec) Find(t *proc.Target, _ []string, scope *proc.EvalScope, _ string, includeNonExecutableLines bool, _ [][2]string) ([]api.Location, string, error) {
if scope == nil {
return nil, fmt.Errorf("could not determine current location (scope is nil)")
return nil, "", fmt.Errorf("could not determine current location (scope is nil)")
}
file, _, fn := scope.BinInfo.PCToLine(scope.PC)
if fn == nil {
return nil, fmt.Errorf("could not determine current location")
return nil, "", fmt.Errorf("could not determine current location")
}
subst := fmt.Sprintf("%s:%d", file, loc.Line)
addrs, err := proc.FindFileLocation(t, file, loc.Line)
if includeNonExecutableLines {
if _, isCouldNotFindLine := err.(*proc.ErrCouldNotFindLine); isCouldNotFindLine {
return []api.Location{{File: file, Line: loc.Line}}, nil
return []api.Location{{File: file, Line: loc.Line}}, subst, nil
}
}
return []api.Location{addressesToLocation(addrs)}, err
return []api.Location{addressesToLocation(addrs)}, subst, err
}

func regexFilterFuncs(filter string, allFuncs []proc.Function) ([]string, error) {
Expand Down
17 changes: 11 additions & 6 deletions pkg/terminal/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -1634,7 +1634,7 @@ func clearAll(t *Term, ctx callContext, args string) error {

var locPCs map[uint64]struct{}
if args != "" {
locs, err := t.client.FindLocation(api.EvalScope{GoroutineID: -1, Frame: 0}, args, true, t.substitutePathRules())
locs, _, err := t.client.FindLocation(api.EvalScope{GoroutineID: -1, Frame: 0}, args, true, t.substitutePathRules())
if err != nil {
return err
}
Expand Down Expand Up @@ -1790,14 +1790,16 @@ func setBreakpoint(t *Term, ctx callContext, tracepoint bool, argstr string) ([]
}

requestedBp.Tracepoint = tracepoint
locs, findLocErr := t.client.FindLocation(ctx.Scope, spec, true, t.substitutePathRules())
locs, substSpec, findLocErr := t.client.FindLocation(ctx.Scope, spec, true, t.substitutePathRules())
if findLocErr != nil && requestedBp.Name != "" {
requestedBp.Name = ""
spec = argstr
var err2 error
locs, err2 = t.client.FindLocation(ctx.Scope, spec, true, t.substitutePathRules())
var substSpec2 string
locs, substSpec2, err2 = t.client.FindLocation(ctx.Scope, spec, true, t.substitutePathRules())
if err2 == nil {
findLocErr = nil
substSpec = substSpec2
}
}
if findLocErr != nil && shouldAskToSuspendBreakpoint(t) {
Expand All @@ -1824,6 +1826,9 @@ func setBreakpoint(t *Term, ctx callContext, tracepoint bool, argstr string) ([]
if findLocErr != nil {
return nil, findLocErr
}
if substSpec != "" {
spec = substSpec
}

created := []*api.Breakpoint{}
for _, loc := range locs {
Expand Down Expand Up @@ -2431,7 +2436,7 @@ func getLocation(t *Term, ctx callContext, args string, showContext bool) (file
return loc.File, loc.Line, true, nil

default:
locs, err := t.client.FindLocation(ctx.Scope, args, false, t.substitutePathRules())
locs, _, err := t.client.FindLocation(ctx.Scope, args, false, t.substitutePathRules())
if err != nil {
return "", 0, false, err
}
Expand Down Expand Up @@ -2505,7 +2510,7 @@ func disassCommand(t *Term, ctx callContext, args string) error {

switch cmd {
case "":
locs, err := t.client.FindLocation(ctx.Scope, "+0", true, t.substitutePathRules())
locs, _, err := t.client.FindLocation(ctx.Scope, "+0", true, t.substitutePathRules())
if err != nil {
return err
}
Expand All @@ -2525,7 +2530,7 @@ func disassCommand(t *Term, ctx callContext, args string) error {
}
disasm, disasmErr = t.client.DisassembleRange(ctx.Scope, uint64(startpc), uint64(endpc), flavor)
case "-l":
locs, err := t.client.FindLocation(ctx.Scope, rest, true, t.substitutePathRules())
locs, _, err := t.client.FindLocation(ctx.Scope, rest, true, t.substitutePathRules())
if err != nil {
return err
}
Expand Down
26 changes: 26 additions & 0 deletions pkg/terminal/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1419,3 +1419,29 @@ func TestCreateBreakpointByLocExpr(t *testing.T) {
}
})
}

func TestRestartBreakpoints(t *testing.T) {
// Tests that breakpoints set using just a line number and with a line
// offset are preserved after restart. See issue #3423.
withTestTerminal("continuetestprog", t, func(term *FakeTerminal) {
term.MustExec("break main.main")
term.MustExec("continue")
term.MustExec("break 9")
term.MustExec("break +1")
out := term.MustExec("breakpoints")
t.Log("breakpoints before:\n", out)
term.MustExec("restart")
out = term.MustExec("breakpoints")
t.Log("breakpoints after:\n", out)
bps, err := term.client.ListBreakpoints(false)
assertNoError(t, err, "ListBreakpoints")
for _, bp := range bps {
if bp.ID < 0 {
continue
}
if bp.Addr == 0 {
t.Fatalf("breakpoint %d has address 0", bp.ID)
}
}
})
}
2 changes: 1 addition & 1 deletion service/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ type Client interface {
// * *<address> returns the location corresponding to the specified address
// NOTE: this function does not actually set breakpoints.
// If findInstruction is true FindLocation will only return locations that correspond to instructions.
FindLocation(scope api.EvalScope, loc string, findInstruction bool, substitutePathRules [][2]string) ([]api.Location, error)
FindLocation(scope api.EvalScope, loc string, findInstruction bool, substitutePathRules [][2]string) ([]api.Location, string, error)

// DisassembleRange disassemble code between startPC and endPC
DisassembleRange(scope api.EvalScope, startPC, endPC uint64, flavour api.AssemblyFlavour) (api.AsmInstructions, error)
Expand Down
23 changes: 14 additions & 9 deletions service/debugger/debugger.go
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@ func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint, locExpr string,
return nil, err
}
setbp.Expr = func(t *proc.Target) []uint64 {
locs, err := loc.Find(t, d.processArgs, nil, locExpr, false, substitutePathRules)
locs, _, err := loc.Find(t, d.processArgs, nil, locExpr, false, substitutePathRules)
if err != nil || len(locs) != 1 {
logflags.DebuggerLogger().Debugf("could not evaluate breakpoint expression %q: %v (number of results %d)", locExpr, err, len(locs))
return nil
Expand Down Expand Up @@ -1921,17 +1921,17 @@ func (d *Debugger) CurrentPackage() (string, error) {
}

// FindLocation will find the location specified by 'locStr'.
func (d *Debugger) FindLocation(goid int64, frame, deferredCall int, locStr string, includeNonExecutableLines bool, substitutePathRules [][2]string) ([]api.Location, error) {
func (d *Debugger) FindLocation(goid int64, frame, deferredCall int, locStr string, includeNonExecutableLines bool, substitutePathRules [][2]string) ([]api.Location, string, error) {
d.targetMutex.Lock()
defer d.targetMutex.Unlock()

if _, err := d.target.Valid(); err != nil {
return nil, err
return nil, "", err
}

loc, err := locspec.Parse(locStr)
if err != nil {
return nil, err
return nil, "", err
}

return d.findLocation(goid, frame, deferredCall, locStr, loc, includeNonExecutableLines, substitutePathRules)
Expand All @@ -1949,18 +1949,23 @@ func (d *Debugger) FindLocationSpec(goid int64, frame, deferredCall int, locStr
return nil, err
}

return d.findLocation(goid, frame, deferredCall, locStr, locSpec, includeNonExecutableLines, substitutePathRules)
locs, _, err := d.findLocation(goid, frame, deferredCall, locStr, locSpec, includeNonExecutableLines, substitutePathRules)
return locs, err
}

func (d *Debugger) findLocation(goid int64, frame, deferredCall int, locStr string, locSpec locspec.LocationSpec, includeNonExecutableLines bool, substitutePathRules [][2]string) ([]api.Location, error) {
func (d *Debugger) findLocation(goid int64, frame, deferredCall int, locStr string, locSpec locspec.LocationSpec, includeNonExecutableLines bool, substitutePathRules [][2]string) ([]api.Location, string, error) {
locations := []api.Location{}
t := proc.ValidTargets{Group: d.target}
subst := ""
for t.Next() {
pid := t.Pid()
s, _ := proc.ConvertEvalScope(t.Target, goid, frame, deferredCall)
locs, err := locSpec.Find(t.Target, d.processArgs, s, locStr, includeNonExecutableLines, substitutePathRules)
locs, s1, err := locSpec.Find(t.Target, d.processArgs, s, locStr, includeNonExecutableLines, substitutePathRules)
if s1 != "" {
subst = s1
}
if err != nil {
return nil, err
return nil, "", err
}
for i := range locs {
if locs[i].PC == 0 {
Expand All @@ -1977,7 +1982,7 @@ func (d *Debugger) findLocation(goid int64, frame, deferredCall int, locStr stri
}
locations = append(locations, locs...)
}
return locations, nil
return locations, subst, nil
}

// Disassemble code between startPC and endPC.
Expand Down
2 changes: 1 addition & 1 deletion service/rpc1/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ type FindLocationArgs struct {

func (c *RPCServer) FindLocation(args FindLocationArgs, answer *[]api.Location) error {
var err error
*answer, err = c.debugger.FindLocation(args.Scope.GoroutineID, args.Scope.Frame, args.Scope.DeferredCall, args.Loc, false, nil)
*answer, _, err = c.debugger.FindLocation(args.Scope.GoroutineID, args.Scope.Frame, args.Scope.DeferredCall, args.Loc, false, nil)
return err
}

Expand Down
4 changes: 2 additions & 2 deletions service/rpc2/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,10 +414,10 @@ func (c *RPCClient) AttachedToExistingProcess() bool {
return out.Answer
}

func (c *RPCClient) FindLocation(scope api.EvalScope, loc string, findInstructions bool, substitutePathRules [][2]string) ([]api.Location, error) {
func (c *RPCClient) FindLocation(scope api.EvalScope, loc string, findInstructions bool, substitutePathRules [][2]string) ([]api.Location, string, error) {
var out FindLocationOut
err := c.call("FindLocation", FindLocationIn{scope, loc, !findInstructions, substitutePathRules}, &out)
return out.Locations, err
return out.Locations, out.SubstituteLocExpr, err
}

// DisassembleRange disassembles code between startPC and endPC
Expand Down
Loading