Skip to content

Commit

Permalink
terminal: restore breakpoints set with line offset on restart
Browse files Browse the repository at this point in the history 
Change FindLocation so it can return a substitute location expression
and propagate it to pkg/terminal/command.
When breakpoints are set using the syntax :<lineno> or +<lineno>
produce a substitute location expression that doesn't depend on having
a valid scope and can be used to restore the breakpoint.

Fixes #3423
  • Loading branch information
@aarzilli
aarzilli committed Jun 20, 2023
1 parent 656c4f1 commit 6b25770
Show file tree
Hide file tree
Showing 10 changed files with 109 additions and 66 deletions.
78 changes: 42 additions & 36 deletions pkg/locspec/locations.go
View file
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 \"%s\" not found", locStr)
return nil, "", fmt.Errorf("location \"%s\" 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 \"%s\" not found", locStr)
return nil, "", fmt.Errorf("location \"%s\" 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
View file
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 @@ -1783,14 +1783,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 @@ -1817,6 +1819,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 @@ -2424,7 +2429,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 @@ -2498,7 +2503,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 @@ -2518,7 +2523,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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1404,3 +1404,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
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,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 @@ -1907,17 +1907,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 @@ -1935,18 +1935,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 @@ -1963,7 +1968,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
View file
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
View file
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

0 comments on commit 6b25770

Please sign in to comment.