Proposal: add logical breakpoints to handle inlined calls better

25 views
Skip to first unread message

Alessandro Arzilli

unread,
Nov 23, 2018, 6:24:36 AM11/23/18
to delve-dev
# Problem

Users think of breakpoints as applying to a line of source code. Delve
however represents breakpoints using a proc.Breakpoint object, these
objects are tightly associated with a single memory address, rather than
with a line of source code. This discrepancy doesn't usually matter
because, normally, a line of source code is compiled only once into an
executable file.

There are however cases where this isn't true and a line of source code
is compiled multiple times:

1. When calls to a function are inlined the entire body of the function
will be present at multiple points in the executable file.

2. When loading plugins in memory any function defined in a package
imported by multiple plugins will be present multiple times in memory.
For example if two plugins both import "fmt" and use fmt.Printf the
final process will contain two implementations of fmt.Printf.

3. C programs defining static functions inside header files.

4. If Go ever gets type-parametric functions ("generics") and implements
them by compile-time specialization

In all these cases we would like users to be able to treat a group of
physical breakpoints, at different memory addresses, as a single logical
breakpoint.

For example if the user types:

b someFunc

and someFunc has a concrete instance as well as several inlined calls we
would want delve to create one breakpoint for the concrete instance and
one breakpoint for each inlined call. But we would also want to
manipulate it (adding conditions, deleting it) as if it was a single
breakpoint.

We also probably want this to happen at the JSON-RPC API level, i.e. we
do not want clients to have to do special things to handle programs
containing inlined functions correctly. Ideally any change would happen
below the JSON-RPC API layer, so that we keep backwards compatibility.

# Proposal

1. We keep `proc.Breakpoint` more or less as it is. This will be our
internal representation of physical breakpoints, pkg/proc and its
sub-packages will not have to worry about logical breakpoints.

2. We change FindFunctionLocation and FindFileLocation to return a
[]uint64 representing all matching addresses for the file:line or
function:line expression. This will require changing the type of
LookupFunc to map[string][]*proc.Function and tracking the inlined calls
in a field of proc.Function.

3. In service/debugger we introduce some data structure (a map? a slice?
doesn't matter) that will map between physical breakpoints and logical
breakpoints and vice versa.

4. API calls CreateBreakpoint, ClearBreakpoint, GetBreakpoint,
AmendBreakpoint and ListBreakpoints will all work with logical
breakpoints with service/debugger doing the necessary de/muxing to
physical breakpoints (meaning to proc.Breakpoint objects).

This means that when a client calls:

CreateBreakpoint(CreateBreakpointIn{
Breakpoint: { File: "somefile", Line: 42 } })

what happens is that we (possibly) create multiple proc.Breakpoint
in response, all associated to a single logical breakpoint in
service/debugger and return a single api.Breakpoint object in response.

5. API call FindLocation gets deprecated, there's just no way for a
caller of FindLocation to distinguish between an ambiguous expression
and a non-ambiguous expression that maps to multiple memory addresses.
Instead CreateBreakpointIn gets a new field Expr which is evaluated
similarly to FindLocation but does the right thing with logical
breakpoints. Deprecation means: we warn users to use CreateBreakpoint
instead, change the client howto accordingly and leave it in place.
I think the command line client is the only thing calling it but leaving
it there doesn't hurt.

6. api.Breakpoint gets a new field "Addrs []uint64" containing all the
addresses, we can't remove the Addr field because of backwards
compatibility so it will contain a copy of Addrs[0] (which in most cases
is going to be the only addr anyway)

This set of changes adds support for logical breakpoints to delve
without revolutionizing everything and maintaining backwards
compatibility. I think it's the best way to go at it.

Given that the code that needs to change overlaps substantially with the
code changed by plugin support I'm going to wait to do this until after
plugin support is reviewed and merged.
Reply all
Reply to author
Forward
0 new messages