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
* 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
* 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>
Handle the signaled status for the thread leader like we handle the
exited status, by returning ErrProcessExited and recording the killer
signal in it.
Prior to this commit we would find out about the death of the thread
later in the loop, the condition would still be reported as
ErrProcessExited, but without recording the signal number anywhere.
Also fixes a bug in TestAttachStopOnEntry where the test would
inadvertently cause a SIGPIPE to be sent to the target process, making
it terminate early.
* 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
daptest has type assertion functions that panic
if the read response/event message is not
the expected type. This is not only against the
recommended style guideline (Don't Panic, Useful
Test Failures, ...), but also it prevents from
quickly diagnosing test failures occurred in remote
CIs.
This PR changes the type assertion to the two
return value type assertion, and t.Fatal with details
if the type is not expected.
service/dap/daptest/main.go is a program that auto
generates those assertion functions in resp.go.
Run `go generate` from the service/dap directory
to update resp.go.
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
* 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
* 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/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>
* Avoid double removal of temp binary
* Add back accidentally removed empty line
* Simplify regex
* Use unique build output directories in test cases
* Recover TestLaunchDebugRequest hidden logging and refine error check
* Special case access-denied error on Windows
* Remove special case for access denied on Windows
* Increase remove delay on Win
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
* DAP test for github.com/golang/vscode-go/issues/1056
* DAP test for github.com/golang/vscode-go/issues/884
* DAP test for github.com/golang/vscode-go/issues/851
* DAP test for github.com/golang/vscode-go/issues/1053
* DAP test for github.com/golang/vscode-go/issues/1054
* Make DeepSource happy
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
Both structMember and findMethod implemented a depth-first search in
embedded fields but the Go specification requires a breadth-first
search. They also allowed promotion of fields in the concrete type of
embedded interfaces even though this is not allowed by Go.
Furthermore they both lacked protection from infinite recursion
when a type embeds itself and the user requests a non-existent field.
Fixes#2316
* add -json flag when running tests on TeamCity
* introduce TeamCity builds
* restore gdbserial constants for 386
Otherwise compilation fails.
* skip TestAttachRequest on Windows as it never finishes
* run tests on 1.16beta1
* Add support for evaluateName for variables
* More evaluateName logic tweaks and tests
* Make DeepSource happy
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
* Use address, not index to differentiate compound map keys
* Clean up calls to expectVarRegex
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
Fix bug in DAP test: TestEvaluateCallRequest.
In Go 1.15 the call injection will be executed on a different goroutine
from the goroutine where it was started on to avoid confusing the
garbage collector, the test must be aware of this fact and use the
goroutine ID from the stopped response instead of assuming 1 is the
currently selected goroutine.
Disables TestAttachDetach when running in Github Actions.
Disable some coredump tests when running in Github Actions (core size
limits?).
* service/dap: add "panic" and "fatal error" as stopped reasons
The unrecovered panic and fatal throw breakpoints are not set by the
user. We now check for these special breakpoints and send appropriate
stopped reasons to the client.
* Add getter for StopReason
* Set threadID and stop reason correctly
If there is no selected goroutine, no goroutine ID should be set in
the stopped event.
The stopped reason can be better determined using the process
StopReason.
* Update panic breakpoint on next test to work with Go 1.13 runtime
When running panic.go with Go1.13, the next line that is stepped to
after panic('boom') is the defer function in the runtime package. The
unrecovered panic breakpoint is not hit until after several steps.
The test now steps until the breakpoint is hit, or the program terminates
without hitting the unrecovered panic breakpoint, in which case it fails.
* Skip breakpoint on next test in < Go 1.14
* Add underlying type when printing interface type
* Add todo to remove one level from interface printing
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
* Support global variables
* Respond to review comments
* Clarify comment
* Add more details to test error messages
* Remove flaky main..inittask checks
* Rename globals flag to match vscode-go
* Normalize filepath with slash separator
* Improve handling for unknown package
* Tweak error message
* More refactoring, normalization and error details to deal with Win test failures
* Clean up optional launch args processing
* Add CurrentPackage to debugger and use instead of ListPackagesBuildInfo
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
* Initial support for scopes and variables requests
* Add detailed variables test
* Address review comments
* Fix typo and redudant escaped characters
* Bug fix for uninitialized interfaces; no refs needed for 0-size vars
* Minor cosmetic tweaks
* Add incomplete loading test
* Make DeepSource happy
* Remove unnecessary t.Helper() calls
* Update broken test after merge
* Add missing return
* Rework test harness to abort testvariables2 before stack overflow
* Remove accidentally duplicated disconnet
* Test for invalid interface type with regex
* Drop testvariables3, clean up and test unreadable case
* Respond to review comments
* Make expectVar test helper less fragile
* Make DeepSource happy
* Use proc.LoadConfig directly
* Adjust test to avoid var count discrepency between Go 1.15 and earlier
* Make compound keys in a map unique for correct display
* Remove locals num check that will break if more vars are added
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
* Adds launch request support for program args
* Combine types of function parameters
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
Using a stop channel to signal clean shutdown to the server's
goroutines. Also updates the tests to trigger disconnectChan.
This helps avoid spurious error logs when the client disconnects, when
Ctrl+C is pressed, etc.
