1. change ExpectOutputEventProcessExited to accept -1 exit status, this
is a real problem on linux but we don't know how to fix it and we
already have a test in proc for it.
2. change TestNoDebug_AcceptNoRequestButDisconnect to be less picky
about message ordering, all message orderings seem to be fine, there
is no reason to insist on a particular one, since the DAP server is
unable to actually produce it deterministically.
Fixes#2860
* dap: support 'Env' attribute for launch requests
Env is applied in addition to the delve process environment
variables. The env setting is done by calling os.Setenv
as early as possible when a Launch request is received.
Prior discussion is in https://github.com/go-delve/delve/pull/2582
In Visual Studio Code, setting null for an environment variable
in launch.json or tasks.json indicates users want to unset
the environment variable. Support the behavior by accepting
nil value.
* dap: Env field itself can be omitempty
* edit comment
* service/debugger: disable breakpoints with hitcond not satisfiable
To avoid slowing down the debugged process unnecessarily, we disable
breakpoints with a hit condition that can no longer be hit again.
* test: add integration tests for hit conditions no more satisfiable
* proc/test: fix typo in breakpoints related tests
* test: use the new API for hitcond integration tests
* go.mod: update golang.org/x/tools to v0.1.8
Fixes TestGeneratedDoc on go1.18
* TeamCity: bump test matrix
Add 1.18 to test matrix. Remove 1.15 from test matrix and from support range.
* proc,tests: update for regabi on arm64 and 386
Make sure that stacktrace registers always contain the PC register of
the current frame, even though the debug_frame rules might not specify
it on architectures that use a link register.
The PC register is needed to look up loclist entries for variable
evaluation.
* goversion: bump maximum supported Go version to 1.18
* proc: disable asyncpreempt on linux/arm64
Asyncpreempt on linux/arm64 can sometimes restart a sequence of
instructions which will make breakpoint appear to be hit twice in some
cases.
From the DAP spec:
If this attribute exists and is non-empty, the backend must not 'break' (stop) but log the message instead.
Expressions within {} are interpolated.
This change parses the log messages and stores the parsed values as a format string and list of expressions to evaluate and get the string value of.
Updates golang/vscode-go#123
* service/dap: fix goroutine id selection for hardcoded breakpoints
Determining the stopped goroutine id on a breakpoint required
checking for breakpoints since some may be tracepoints. However,
there may be goroutines stopped on hardcoded breakpoints with
no breakpoint. We fix this by checking for runtime.breakpoint or
StopReason=proc.StopHardcodedBreakpoint.
We want to provide more flexibility for users to make changes to their configurations while the debug session is running. This could be accomplished by creating a custom request, but that were require a new UI as well, and every client of dlv dap to provide its own UI for this. By using the evaluate context, users can use the already existing debug console to change their configurations.
This change includes a refactor of the terminal code in order to share the code with the dap package.
This change provides a very similar to UI as the terminal package, but there are different configuration options that are DAP specific. We plan to use this same mechanism to expose a few other commands including "sources" to help users debug an ineffective substitutePath configuration.
This high-level goal is to push more shutdown logic into Session and not rely Server.Stop() because dap's version of Stop() is different from rpccommon. That means triggering the same shutdown steps in multiple places, so additional safeguards are added to make some of the duplicate steps no-ops to avoid errors, extra logging, etc.
This includes the following changes:
close client conn when request loop in serveDAPCodec exits (to match rpccommon)
exit request loop after processing a disconnect request (instead of relying on next read to fail because conn got closed by triggered Stop(), which is skipped in case of accept-multiclient and is not closed in Stop() in rpccommon)
reset debugger to nil to avoid shutting it down more than once and causing duplicate logging (in case stopDebugSession is called from onDisconnectRequest and Server.Stop or Session.Close as things shut down)
reset binaryToRemove to "" upon removal to avoid duplicate error (in case Close() is called more than once outside of Stop(), which technically is not the case right now)
expand testing for all possible server shutdown triggers
testing for Session-only shutdown as it will be integrated into rpccommon without dap.Server wrapper
updates golang/vscode-go#1830
updates #2328
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
* service/dap: remove deadlock in TestLaunchDebugRequest
Fixes#2746
* terminal: fix TestScopePrefix flakiness
When there are more than 10 frames api.PrintStack will prefix the
output with spaces to right justify the frame number, which confuses
TestScopePrefix.
* _scripts: pass -buildvcs for Go 1.18 and later on TeamCity
Go 1.18 will try to stamp builds with the VCS version, this doesn't
work on TeamCity because the checkout isn't a valid repository (but
looks like it).
Pass -buildvcs=false to disable this feature.
* proc: switch to goroutine stack if first frame's func can not be found
If the first frame on the system stack can not be resolved to a
function switch directly to the goroutine stack.
This adds a new `--client-addr=host:port` flag to `dlv dap`.
If it is supplied, the dap process will dial into the tcp port where
a DAP client is waiting, and work with only the DAP client.
The DAP client is supposed to start the normal DAP message
exchange starting with the 'initialize' request after the dlv dap
process dials in and the connection is set up.
VS Code Go extension plans to use this mode for
* reliably detecting `dlv dap` readiness. Currently it depends on
watching the log stream. After this PR, it can listen on a network port.
* running `dlv dap` from any terminal (part of RunInTerminal workflow
implementation).
A set breakpoints request could come in while the program is running. For a seamless user experience, the server should set the breakpoint and then continue program execution.
Updates golang/vscode-go#1676
* service/dap: refactor the server into two layers
* Add delay before setting debugger in remote tests
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
Log points are special kinds of breakpoints that do not 'break' but instead log a message and then continue. This change implements basic log points that simply log the provided message, without any interpolation.
In order to resume execution after hitting a breakpoint, I added a new lock resumeMu and tracked the running state within the DAP server. resumeMu must be held in order to issue a debugger request that would start execution. This means it can be used to make sure that another goroutine does not resume execution while you are holding the lock.
Most of the synchronization logic is taken from PR #2530
Updates golang/vscode-go#123
When we set a limit on the number of threads that would be
returned, it was possible that the selected thread was not
included in the list of threads. This could cause issues
because the stopped reason is associated with the selected
goroutine, so users could be missing out on important info.
This change makes sure that the selected goroutine is included
by adding it to the end of the list.
Formally define these types and document their meaning.
We will auto-generate the dlv-dap documentation from these Go type doc.
mapToStruct is a helper that sets the given struct's fields with the
info in map[string]interface{} (launch/attach's Arguments). We achieve
this by reencoding map[string]interface{} to json and decoding back to
the target struct. If go-dap left the implementation-specific arguments
as json.RawMessage and let the implementation decode as needed, this
reencoding could've been avoided.
encoding/json itself does not have mean to enforce required fields.
There was a test case that checks substitutePath elements must set
both from/to fields. Path.UnmarshalJSON implements the check.
I am not yet sure about the need for distinction between missing
'from/to' and empty strings yet. (empty value is useful when dealing with
a binary built with trimpath, right?)
A minor behavior change - previously, if noDebug is not a boolean type,
we ignored the attribute silently. Since we use json decoding, any
mismatched types will cause an error and this non-boolean type noDebug
attribute will result in launch failure.
* service/dap: add test verifying handling of relative program path
* Add exec test, log build dir and document in --help
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
* 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
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.
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
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.
* 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.
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.
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
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
- add 'clipboard' capability
- apply a larger string limit for 'hover' and 'clipboard' context
- truncate the string representation of compound (or pointer of compound)
type variable
* pkg/proc: implement support for hit count breakpoints
* update comment
* udpate hitcount comment
* update HitCond description
* add test for hit condition error
* respond to review
* service/dap: add support for hit count breakpoints
* use amendbps to preserve hit counts
* update test health doc
* fix failing test
* simplify hit conditions
* REmove RequestString, use name instead
* update backend_test_health.md
* document hit count cond
* fix tests
If the output binary name does not end with .exe, it can't run on Windows
in noDebug mode. If user-provided output name doesn't have file extension
name (.exe), append it.
Fixesgolang/vscode-go#1473
