* service/dap: add test that verifies output path is relative to wd
* Use cleanExeName to get the right output name on Win
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
Change the socket search to check both the remote and local fields of the
socket match the socket we want to find.
Sockets are identified by the 4-uple
local_addr, local_port, remote_addr, remote_port
Two socket can differ by a single one of this four elements.
It is possible for the same local_port to be used by two different sockets,
as long as they are connecting to different remote addresses (or remote
ports).
An example of this bug in action can be seen at:
https://github.com/golang/vscode-go/runs/3141270564?check_suite_focus=true
There the server starts listening on 127.0.0.1:46011 and rejects a valid
client connection by finding the following socket:
60: 0100007F:DD82 0100007F:962D 06 00000000:00000000 03:00000133 00000000 0 0 0 3 0000000000000000
the local address of this socket is 0100007F:DD82 (127.0.0.1:56706), and the
remote address is 0100007F:962D (127.0.0.1:38445).
The reported error is:
closing connection from different user (127.0.0.1:56706): connections to localhost are only accepted from the same UNIX user for security reasons
note how the local port does match the socket line (56706) but the remote
port is wrong (38445 instead of 46011).
Note also that the state of this socket is 06, or TIME_WAIT, which would be
impossible if this was the right socket, since the right socket would still
be open.
Fixes https://github.com/golang/vscode-go/issues/1555
In order for DAP to support halting the program (either manually or on a breakpoint) performing some action and then resuming execution, there needs to be a way to stop the program without clearing the internal breakpoints. This is necessary for log points and stopping the program to set breakpoints.
The debugging UI makes it seem like a user should be able to set or clear a breakpoint at any time. Adding this ability to complete synchronous requests while the program is running is thus important to create a seamless user experience.
This change just adds a configuration to determine whether the target should clear the stepping breakpoints, and changes the server to use this new mode. Using the new mode means that the DAP server must determine when it expect the next to be canceled and do this manually.
* terminal,service: add way to see internal breakpoints
Now that Delve has internal breakpoints that survive for long periods
of time it will be useful to have an option to display them.
* proc,terminal,service: support stack watchpoints
Adds support for watchpoints on stack allocated variables.
When a stack variable is watched, in addition to the normal watchpoint
some support breakpoints are created:
- one breakpoint inside runtime.copystack, used to adjust the address
of the watchpoint when the stack is resized
- one or more breakpoints used to detect when the stack variable goes
out of scope, those are similar to the breakpoints set by StepOut.
Implements #279
* proc: move breakpoint condition evaluation out of backends
Moves breakpoint condition evaluation from the point where breakpoints
are set, inside ContinueOnce, to (*Target).Continue.
This accomplishes three things:
1. the breakpoint evaluation method needs not be exported anymore
2. breakpoint condition evaluation can be done with a full scope,
containing a Target object, something that wasn't possible before
because ContinueOnce doesn't have access to the Target object.
3. moves breakpoint condition evaluation out of the critical section
where some of the threads of the target process might be still
running.
* proc/native: handle process death during stop() on Windows
It is possible that the thread dies while we are inside the stop()
function. This results in an Access is denied error being returned by
SuspendThread being called on threads that no longer exist.
Delay the reporting the error from SuspendThread until the end of
stop() and only report it if the thread still exists at that point.
Fixes flakyness with TestIssue1101 that was exacerbated by moving
breakpoint condition evaluation outside of the backends.
The Threads request sent immediately after the ConfigurationDone
request can be processed in two possible states:
- while the program is being executed, when it will return Id=1,
Name=Dummy
- after the program terminates, when it will return Id=-1, Name=Current
additionally the response could be received in any order with the
Terminate event.
Remove the problematic Threads request from the test.
There is already a lock on the actual buffered tracepoints collection
within proc, and this method call doesn't do anything to mutate Target
otherwise so we shouldn't be opening ourselves up for a race condition
error or any other kind of parallelism problem.
Additionally, with this lock we essentially can never get the data until
the process has exited becuase `continue` will lock the target. This
change allows us to get the buffered tracepoint information immediately
and display it as the program is running.
The loop searching for a suitable goroutine is not guaranteed to ever
find it, and failing to find one is not an error.
Changes testStepParkedHelper to match the behavior of TestNextParked in
proc_test.go.
Deletes TestStepInParked because it does not test anything meaningful
beyond what's already tested by TestNextParked.
Returning monotonically increasing totalFrames values for subsequent requests can be used to enforce paging in the client. If we are not at the end of the stackFrames that are returned, we return a higher total frames to suggest to the client that they should request more frames.
To make it more clear that the user can resume the program when they encounter a next while nexting error, make the exception information have instructions for resuming the program. This implements the suggestions outlined by @polinasok in #2443.
* pkg/proc: Prefer throw instead of fatalthrow
Currently there is a breakpoint set at runtime.fatalthrow to catch any
situation where the runtime crashes (e.g. deadlock).
When we do this, we go up a frame in order to parse the crash reason.
The problem is that there is no guarentee the "s" variable we attempt to
parse will still be considered "live".
Since runtime.fatalthrow is never called directly, set a breakpoint on
runtime.throw instead and prevent having
to search up a stack frame in order to
get the throw reason.
Fixes#2602
* service/dap: Fix TestFatalThrowBreakpoint
* Reenable TestFatalThrow DAP test
* service/dap: Don't skip test on < 1.17
* service/dap: Update test constraint for 1.16
* pkg/proc: Reinstate runtime.fatalthrow as switchstack exception
Using issue419.go, I observed that the continue command fails with an error when debugger receives and forwards an interrupt. In spite of the stopped event, vscode still shows the state as RUNNING because the threads request is unable to retrieve any threads, but at least one dummy thread is always expected.
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
This adds a cap and a log message if there are many goroutines. This will help
prevent the debugger from freezing, but does not yet address making sure the
interesting goroutines are the ones that are returned.
Updates golang/vscode-go#129
Changes Breakpoint to allow multiple overlapping internal breakpoints
on the same instruction address.
This is done by changing the Breakpoint structure to contain a list of
"breaklets", each breaklet has a BreakpointKind and a condition
expression, independent of the other.
A breakpoint is considered active if any of its breaklets are active.
A breakpoint is removed when all its breaklets are removed.
We also change the terminology "internal breakpoint" to "stepping
breakpoint":
HasInternalBreakpoints -> HasSteppingBreakpoints
IsInternal -> IsStepping
etc...
The motivation for this change is implementing watchpoints on stack
variables.
Watching a stack variable requires also setting a special breakpoint to
find out when the variable goes out of scope. These breakpoints can not
be UserBreakpoints because only one user breakpoint is allowed on the
same instruction and they can not be internal breakpoints because they
should not be cleared when a next operation is completed (they should
be cleared when the variable watch is cleared).
Updates #279
This PR aims to add support for rr replay and core actions from the DAP layer. This basically encloses the following:
New launch modes: replay and core
The following modes are added:
replay: Replays an rr trace, allowing backwards flows (reverse continue and stepback). Requires a traceDirPath property on launch.json pointing to a valid rr trace directory.
Equivalent to dlv replay <tracedir> command.
core: Replays a core dump file, showing its callstack and the file matching the callsite. Requires a coreFilePath property on launch.json pointing to a valid coredump file.
Equivalent to dlv core <exe> <corefile> command.
Dependencies
To achieve this the following additional changes were made:
Implement the onStepBackRequest and onReverseContinueRequest methods on service/dap
Adapt onLaunchRequest with the requried validations and logic for these new modes
Use CapabilitiesEvent responses to enable the StepBack controls on the supported scenarios (see dicussion here)
Add the corresponding launch.json support on vs code:
Support for replay and core modes golang/vscode-go#1268
* service/dap: send continued event before step response
Send the continued event before the step response to make sure that there is no time where the client believes that only a single thread is running.
Updates golang/vscode-go#1617
* move to helper
* service/dap: send terminated event when disconnecting
If the program terminates while disconnecting, either because it
was killed or otherwise, send a terminated event.
The slice `out.List` is created with a length of 0 but a capacity of
`len(imgs)`. This causes the line below to panic as we try to index into
the `out.List` slice instead of append to it. This patch resolves the
issue by appending instead of indexing into the slice.
Co-authored-by: Derek Parker <deparker@redhat.com>
* service/dap: add panic and throw text to stopped event
We can add more information to the stopped events on errors using
the `Text` field in the stopped event. We already use this to display
the runtime errors. Adding this information to the stopped reason will
also help to show the user additional info when a stopped event is not
associated with a particular goroutine.
* proc: support new Go 1.17 panic/defer mechanism
Go 1.17 will create wrappers for deferred calls that take arguments.
Change defer reading code so that wrappers are automatically unwrapped.
Also the deferred function is called directly by runtime.gopanic, without going through runtime.callN which means that sometimes when a panic happens the stack is either:
0. deferred function call
1. deferred call wrapper
2. runtime.gopanic
or:
0. deferred function call
1. runtime.gopanic
instead of always being:
0. deferred function call
1. runtime.callN
2. runtime.gopanic
the isPanicCall check is changed accordingly.
* test: miscellaneous minor test fixes for Go 1.17
* proc: resolve inlined calls when stepping out of runtime.breakpoint
Calls to runtime.Breakpoint are inlined in Go 1.17 when inlining is
enabled, resolve inlined calls in stepInstructionOut.
* proc: add support for debugCallV2 with regabi
This change adds support for the new debug call protocol which had to
change for the new register ABI introduced in Go 1.17.
Summary of changes:
- Abstracts over the debug call version depending on the Go version
found in the binary.
- Uses R12 instead of RAX as the debug protocol register when the binary
is from Go 1.17 or later.
- Creates a variable directly from the DWARF entry for function
arguments to support passing arguments however the ABI expects.
- Computes a very conservative stack frame size for the call when
injecting a call into a Go process whose version is >=1.17.
Co-authored-by: Michael Anthony Knyszek <mknyszek@google.com>
Co-authored-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
* TeamCity: enable tests on go-tip
* goversion: version compatibility bump
* TeamCity: fix go-tip builds on macOS/arm64
Co-authored-by: Michael Anthony Knyszek <mknyszek@google.com>
The code previously expected the server to close when receiving
a message that it could not decode. However, there may be changes to
the spec that make new requests that we have not handled yet. In
that case, it would be preferable to return an error and continue handling
incoming requests.
We told clients that further loading of variables can be done by
specifying a type cast using the address of a variable that we
returned.
This does not work for registerized variables (or, in general,
variables that have a complex location expression) because we don't
give them unique addresses and we throw away the compositeMemory object
we made to read them.
This commit changes proc so that:
1. variables with location expression divided in pieces do get a unique
memory address
2. the compositeMemory object is saved somewhere
3. when an integer is cast back into a pointer type we look through our
saved compositeMemory objects to see if there is one that covers the
specified address and use it.
The unique memory addresses we generate have the MSB set to 1, as
specified by the Intel 86x64 manual addresses in this form are reserved
for kernel memory (which we can not read anyway) so we are guaranteed
to never generate a fake memory address that overlaps a real memory
address of the application.
The unfortunate side effect of this is that it will break clients that
do not deserialize the address to a 64bit integer. This practice is
contrary to how we defined our types and contrary to the specification
of the JSON format, as of json.org, however it is also fairly common,
due to javascript itself having only 53bit integers.
We could come up with a new mechanism but then even more old clients
would have to be changed.
Adds filtering and grouping to the goroutines command.
The current implementation of the goroutines command is modeled after
the threads command of gdb. It works well for programs that have up to
a couple dozen goroutines but becomes unusable quickly after that.
This commit adds the ability to filter and group goroutines by several
different properties, allowing a better debugging experience on
programs that have hundreds or thousands of goroutines.
The existing documentation was a little light on details. This patch
gives more insight into how the argument to this function is used and
interpreted.
* service/rpc2,service/debugger: more doc updates
Combine debugger.CreateBreakpoint documentation with the documentation
that already existed for the rpc2 server CreateBreakpoint method.
Also adds more contextual information for both the server and client
with links to debugger.CreateBreakpoint documentation.
We can get the throw reason by looking at the argument "s" in runtime.throw. This is not currently working in Go 1.16 or Go 1.17 (see golang/go#46425), but does work in Go 1.15 and Go 1.14
Ensure that any command executed after the process we are trying to
debug prints a correct and consistent exit status.
Previously the exit code was being lost after the first time we printed
that a process has exited. Additionally, certain commands would print
the PID of the process and other would not. This change makes everything
more correct and consistent.
Commit 30cdedae6910f5e9af6739845bacfd5b8778e745 introduced a dependency
from service/dap to pkg/terminal to call a stack printing function,
it's weird to have code that implements the DAP protocol depend on the
code for the JSON-RPC client.
Move PrintStack to a different package that can be called by both.
On hovers, including the full value for complex types is unnecessary, since the user will likely need to expand the children to inspect the values. Additionally, this can really slow down getting the hover values. This change will allow truncating the value in hover if the variable has a variables reference.
Updates golang/vscode-go#1435
Apply a presentation hint to the internal runtime stack frames, so that these can be deemphasized in the UI. This should allow
users to more easily inspect their own code, and will keep the option to view those frames if they choose to.
If the client supports paging, we allow them to fetch array and slice items in chunks that we assume will be of a reasonable size. For example, VS Code requests indexed variables in chunks of 100.
Fixesgolang/vscode-go#1518
