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
* dap: use larger variable load limits in 'repl', 'variables' context
When evaluate requests are triggered in the context of 'repl'
(DEBUG CONSOLE in VSCode) or 'variables' (copy values from VARIABLES
section in VSCode), they are the result of human action and have
more rooms to display. So it is not too bad to apply longer limits.
Variable auto-loading for strings or arrays is nice but currently
it's unclear to me how this should be integrated in the DEBUG
CONSOLE or with the Copy Value feature. Until we have better ideas
and tools, let's go with these larger limits.
Unfortunately, the "Copy Value" from WATCH section triggers evaluate
requests with "watch" context and we don't want to load large data
automatically for "watch". So, users who want to query a large value
should first copy the expression to DEBUG CONSOLE and evaluate it.
Not ideal but not the end of the world either.
Updates golang/vscode-go#1318
* dap: apply large limit only to the string type result
* dap: move string reload logic to convertVariable* where other reload logic is
Currently we are thinking string reload for evaluation as a temporary
workaround until we figure out an intutitive way to present long strings.
So, I hope moving this logic near other reload logic may be better.
And, use the address based expression when reloading - when handling the
function return values, we may not have an expression to use.
* dap: make deep source check happy
* dap: move string reevaluation logic back to onEvaluateRequest
Reloading string variables is tricky if they are in registers.
We don't attempt to reload them but for clarity, move this up
to the onEvaluateRequest handler.
For function call, use a generous limit for string load
since the results are volatile.
* dap: check variable isn't affected by evaluate in other context
The Types field is only set in childrenToDAPVariables if the client
capability is supported. There is no need to check again after the
children have been calculated.
* dap: handle SetVariable requests
The handler invokes debugger.SetVariableInScope, except for
string type variables. For which, we rely on the `call` command.
Moved the call expression handling logic to the new `doCall`
function, so it can be reused by the SetVariable requenst
handler.
With this PR, every successful SetVariable request triggers
a StoppedEvent - that's a hack to reset the variablesHandle
map internally and notify the client of this change. It will
be nice if we can just update cached data corresponding to
the updated variable. But I cannot find an easy and safe way
to achieve this yet.
Also fixed a small bug in the call expression evaluation -
Previously, dlv dap returned an error "Unable to evaluate
expression: call stopped" if the call expression is for
variable assignment. (e.g. "call animal = "rabbit").
* dap: address comments from aarzilli
resetHandlesForStop & sendStoppedEvent unconditionally after
call command is left as a TODO - This is an existing code path
(just refactored) and an preexisting bug. Fixing it here
requires updates in TestEvaluateCallRequest and I prefer
addressing it in a separate cl.
Disabled call injection testing on arm64. Separated TestSetVariable
into two, one that doesn't involve call injection and another that
may involve call injection.
Fixed variableByName by removing unnecessary recursion.
* dap: address polina's comments
- removed the hard reset for every variable set
- added tests for various variable types
- added tests that involves interrupted function calls. (breakpoint/panic)
And,
- changed to utilize EvalVariableInScope to access the variable instead
of searching the children by name.
- changed to utilize evaluate requests when verifying whether the variable
is changed as expected in testing. Since now we avoid resetting the variable
handles after variable reset, either we need to trigger scope changes
explicitly, or stop depending on the variables request.
* dap: address comments
- Discuss the problem around the current doCall implementation
and the implication.
- Refine the description on how VS Code handles after setVariable
and evaluate request (there could be followup scopes/evaluate requests).
- Use the explicit line numbers for breakpoints in the SetVariable tests.
- Do not use errors.Is - we could've used golang.org/x/xerrors polyfill
but that's an additional dependency, and we will remove this check once
tests that depend on old behavior are fixed.
* dap: remove errTerminated and adjust the test
* dap: evaluate in the outer frame, instead of advancing to the next bp
On linux, delve RPC server allows only connections from the same user
if --only-same-user is set (true, by default). Do the same for DAP
server.
Moved the sameuser check logic to service/internal/sameuser.
Considered importing service/rpccommon from the dap server,
but when we eventually migrate to multiplex rpc and dap from one
port, I am afraid that can cause cyclic imports.
* service/dap: implement setFunctionBreakpoints request
* Fix the errors that would not allow func set
* use find locations instead of FindFunctionLocation
* add function breakpoint tests
* return after sending error response
* revert changes to debugger
* exclude regexp function names
* remove switch statement with one case
* remove ReadFile ambiguous test
* Remove TODO for multiple locs
* remove unnecessary setting of bp.Verified on error
* tighten condition for breakpoint name to match function breakpoint
* add tests for different loc types, add FindLocationSpec
* add test using base name of file
* make functionBreakpoint name a constant
* update stop reason to function breakpoint
* remove comment about optimizing onSetFunctionBreakpoints
* respond to review
* add comments to test
* change functionBpPrefix to const
* handle relative paths
* fix capabilites check
* update function breakpoint tests to check for failure
* use negative line number to determine which are errors
* service/dap: send 'continued' event on next/step/stepout requests
Next/step/stepout requests assume that execution is only resumed
on the thread that was specified. We cannot resume execution on
only a single thread, so we need to send a continued event to let
the client know that all threads have resumed.
* update set breakpoints test with continued event
* service/dap: support pause request
* service/dap: validate the client configurations in initialize request (#2435)
The client can specify certain configurations in the initialize request.
For example, pathFormat determines the pathFormat. We do not currently
support configuring pathFormat, linesStartAt1, or columnsStartAt1, so
we report an error if the client attempts to set these to an
unsupported value.
* TeamCity: fix Windows builds (#2467)
Bintray is shutting down and the URL we used to install mingw is no
longer available. Use chocolatey instead.
* proc/native: low level support for watchpoints in linux/amd64 (#2301)
Adds the low-level support for watchpoints (aka data breakpoints) to
the native linux/amd64 backend.
Does not add user interface or functioning support for watchpoints
on stack variables.
Updates #279
* simplify pause test
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
Co-authored-by: Suzy Mueller <suzmue@golang.org>
Co-authored-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
My local build of delve which apparently pulls in the master branch starting breaking. This is the culprit.
Error:
```
#9 75.27 # github.com/go-delve/delve/service/dap
#9 75.27 /usr/src/github.com/go-delve/delve/service/dap/server.go:107:1: syntax error: unexpected <<, expecting field name or embedded type
#9 75.27 /usr/src/github.com/go-delve/delve/service/dap/server.go:110:1: syntax error: unexpected ==, expecting field name or embedded type
#9 75.27 /usr/src/github.com/go-delve/delve/service/dap/server.go:113:1: syntax error: unexpected >>, expecting field name or embedded type
```
* service/dap: implement exception info
* remove adding additional thread
* Fix tests
* add exceptionInfo tests
* update comments
* map paths to client paths
* remove launch.json
* remove change to ConvertEvalScope
* correct name of supportsExceptionInfoRequest
* Add TODO for deleting output event
* Print Stack header to buffer
* Try to move resolving exception info to onExceptionInfoRequest
* save the error and return if it is the current thread
* rename thread to g
* findgoroutine returns goroutine
* clean up findgoroutine
* log errors
* remove output event
* fix grammar
* service/dap: support setting breakpoints while running
* Review comments, faster test
* Fix comments
* Address review comments
* Do not continue automatically
* Add TODO to resume exeuction
* Handle async test messages in either order
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
* service/dap: add type information to dap variables
* add comment explaining map type choice
* rename to setClientCapabilities
* respond to review
* update TypeString definition
Adds the low-level support for watchpoints (aka data breakpoints) to
the native linux/amd64 backend.
Does not add user interface or functioning support for watchpoints
on stack variables.
Updates #279
The client can specify certain configurations in the initialize request.
For example, pathFormat determines the pathFormat. We do not currently
support configuring pathFormat, linesStartAt1, or columnsStartAt1, so
we report an error if the client attempts to set these to an
unsupported value.
* service/dap: add go build stderr to error response
* service/dap: add go build stderr to error response
* Skip message check for build errors
* test for flag provided message
* service/dap: annotate shadowed variable names in variables request
In order to distinguish variables that are shadowed, this change
updates the names from 'name' to '(name)'. This is the same syntax
used in the terminal package.
* remove unnecessary comment
* Add todo for evaluate name
* Check the evaluateName result is the unshadowed var
* service/dap: add substitutePath configuration
Similar to substitute-path configuration in the dlv cli, substitutePath
in dap allows users to specify path mappings that are applied to the
source files in stacktrace and breakpoint requests.
Updates #2203
* service/dap: refactor the startup of the fixture for attach
Add a helper function for starting up a process to attach to.
* service/dap: update substitute path tests for windows
* service/dap: remove lines that should have been removed in merge
* respond to comments on pr
* move logging to helper functions
* make test comments more clear
* Add comments about absolute paths
* fix log messages
* clarify test comments
* remove comment about absolute paths
We have some places where we use proc.ErrProcessExited and some places
that use &proc.ErrProcessExited, resulting in checks for process exited
errors occasionally failing on some architectures.
Uniform use of ErrProcessExited to the non-pointer version.
Fixes intermittent failure of TestStepOutPreservesGoroutine.
* dap: change how noDebug launch request is served
PR #2400 added support for noDebug launch requests - that was done
by directly starting the target program using os/exec.Cmd and blocking
the launch request indefinitely until the program terminates or
is stopped with a disconnect request (when dlv dap can support it).
Even though the approach seemed to work for user, that adds an extra
control flow and complexity to the codebase.
This change takes a different approach to implement the noDebug
launch feature. Instead of using os/exec.Cmd, this uses the existing
debug launch path, but avoids setting breakpoints. Finally, the program
will start upon receiving onConfigurationDoneRequest and blocks there
until the program terminates. This simplifies the code.
The DAP spec does not explicitly specify what to do about other
requests. It seems like VSCode can issue evaluate requests or other
requests after the configuration done request. Currently they are
blocked because the program is in running state. We can consider checking
s.noDebug and responding with an error in the future if we want/need to.
See the log below for a typical DAP request/response sequence:
2021-03-29T01:42:53-04:00 debug layer=dap building binary at /Users/hakim/projects/s/__debug_bin
2021-03-29T01:42:55-04:00 info layer=debugger launching process with args: [/Users/hakim/projects/s/__debug_bin]
2021-03-29T01:42:55-04:00 debug layer=dap [-> to client]{"seq":0,"type":"event","event":"initialized"}
2021-03-29T01:42:55-04:00 debug layer=dap [-> to client]{"seq":0,"type":"response","request_seq":2,"success":true,"command":"launch"}
2021-03-29T01:42:55-04:00 debug layer=dap [<- from client]{"seq":3,"type":"request","command":"setBreakpoints","arguments":{"source":{"name":"main.go","path":"/Users/hakim/projects/s/main.go"},"breakpoints":[{"line":9}],"lines":[9]}}
2021-03-29T01:42:55-04:00 error layer=dap Unable to set or clear breakpoints: running in noDebug mode
2021-03-29T01:42:55-04:00 debug layer=dap [-> to client]{"seq":0,"type":"response","request_seq":3,"success":false,"command":"setBreakpoints","message":"Unable to set or clear breakpoints","body":{"error":{"id":2002,"format":"Unable to set or clear breakpoints: running in noDebug mode"}}}
2021-03-29T01:42:55-04:00 debug layer=dap [<- from client]{"seq":4,"type":"request","command":"configurationDone","arguments":{}}
2021-03-29T01:42:55-04:00 debug layer=dap [-> to client]{"seq":0,"type":"response","request_seq":4,"success":true,"command":"configurationDone"}
2021-03-29T01:42:55-04:00 debug layer=debugger continuing
Hello
2021-03-29T01:43:00-04:00 debug layer=dap [-> to client]{"seq":0,"type":"event","event":"terminated","body":{}}
2021-03-29T01:43:00-04:00 debug layer=dap [<- from client]{"seq":5,"type":"request","command":"threads"}
2021-03-29T01:43:00-04:00 debug layer=dap [-> to client]{"seq":0,"type":"response","request_seq":5,"success":true,"command":"threads","body":{"threads":null}}
2021-03-29T01:43:00-04:00 debug layer=dap [<- from client]{"seq":6,"type":"request","command":"disconnect","arguments":{}}
2021-03-29T01:43:00-04:00 debug layer=dap [-> to client]{"seq":0,"type":"response","request_seq":6,"success":true,"command":"disconnect"}
2021-03-29T01:43:00-04:00 debug layer=debugger halting
2021-03-29T01:43:00-04:00 error layer=dap Process 27219 has exited with status 0
2021-03-29T01:43:00-04:00 debug layer=debugger detaching
Updates https://github.com/golang/vscode-go/issues/1111
* service/dap: address polina's comments for noDebug logic
Reworked so the noDebug launch request again blocks launch request
handler after responding with the launch response. By skipping
the initialized event, it should prevent well-behaving clients from
sending any further requests. Currently, doesn't matter because
the handler goroutine will be blocked until the program termination
anyway.
Placed mutex back, since I noticed that's the only way to prevent
racing between Stop and the handler goroutine. The synchronization
lotic (in particular, during teardown) should be revisited after
asynchronous request support work is done.
* dap: address more comments
* service/dap: switch goroutine before stepping
The correct goroutine needs to be selected when stepping in order
for the step to execute to the correct location.
* handle next in progress while stepping
* Add tests for steps when switching goroutine
* remove nextInProgress handling
* add new step out test and review debug state check
* update text of stopped event and set goroutine id
Error level logging shows up in the users' consoles/terminals
so be more conservative when logging. Move the followings to
Debug logging.
- DAP error reponses caused by invalid requests.
They are application level errors and DAP clients should handle them.
- Errors reported when debugee already exited.
Fixesgolang/vscode-go#1392
* Truncate long compound map keys and use address suffix only for those
* Remove test typo that causes failures
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
* Add logging and comments to clarify relative output path treatement
* Use absolute output path in one of the unittests
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
If the launch requests has noDebug attribute set, run the built
binary directly. The launch request handler will block until
the binary terminates, so the editor won't send additional requests
like breakpoint setting etc. Still disconnect or restart requests
can flow in though and they should trigger killing of the target
process if it's still running.
In order to run the binary using os/exec on windows, the target
binary has to have .exe as its extension. So, add .exe to the default
output name if it is on windows. I am not sure though yet we want
to modify the user-specified output or not yet. Considering how
go commands behave (not automatically append .exe for 'go build -o')
I think respecting what user specified is right, but the failure
(file not exist) may be mysterious.
* service/dap: use specified working directory for launch requests
If a user specifies a working directory in the launch request,
then the process should use that working directory. This may
affect how the program runs and the user should be able to have
control over this.
* service/dap: add tests for launch with working dir
Added tests to make sure the working directory is set correctly.
* service/dap: fix TestWorkingDir on windows
* service/dap: use %q to print quoted string
* cmd/dlv: update dap warning about working dir
* service/dap: change the launch argument to be wd`
* update warning to specify only launch request
* Use (*api.Variable).SinglelinesString() for dap.Variable values
* Make DeepSource happy
* Adjust unreadable regex in tests
* Use regex for runtime.mutex variable test
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
* service: serialize calls to Command API
Wait until the target process has resumed before accepting new calls to
Command. Before this if a 'continue' was immediately followed by a
'halt' the 'halt' could be processed before the 'continue'.
Fixes#1608Fixes#2216
* service/rpccommon: fix DeepSource issues
* service/dap: support auto-loading of unloaded interfaces
* Make DeepSource happy
* Don't set reference if data failed to auto-load
* Use frame-less expressions
* Refine interface recursion capping test case
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
