Commit Graph

58 Commits

Author SHA1 Message Date
Alessandro Arzilli
6a85f34966
debugger: report error when switching goroutine is impossible (#2424)
Due to variable shadowing the SwitchGoroutine command never failed.
2021-04-21 13:39:19 -07:00
polinasok
e141c47eb8
service/dap: refine teardown logic (#2414)
* service/dap: refine teardown logic

* Address review comments + add missing lock/unlock

* Narrow lock scope

* Update comments only

* Remove redundan temp var from stopNoDebugProcess

* Clarify comment

* Set debugger to nil after detach to prevent dup teardown in Stop()

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-04-21 13:28:15 -07:00
Suzy Mueller
af1796d171
service/dap: rename launch configuration 'wd' to 'cwd' (#2433)
Rename the launch configuration for specifying the working directory.
2021-04-20 09:40:34 +02:00
Suzy Mueller
2408ed87bf
service/dap: annotate shadowed variable names in variables request (#2428)
* 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
2021-04-19 11:14:50 -07:00
Suzy Mueller
4eb54b01e7
service/dap: add substitutePath configuration (#2379)
* 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
2021-04-15 16:35:37 -07:00
polinasok
747f037883
service/dap: fix temp binary deletion race (#2413)
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-04-14 09:22:40 +02:00
Alessandro Arzilli
3c69f7435e
*: Never use pointer to proc.ErrProcessExited (#2431)
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.
2021-04-13 08:52:29 +02:00
polinasok
fe616c27ff
service/dap: fallback to CurrentThread if SelectedGouroutine is not available (#2422)
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-04-12 15:00:26 -07:00
polinasok
aa426a2e50
service/dap: delay disconnect response and log teardown progress (#2427)
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-04-12 14:50:15 -07:00
Hyang-Ah Hana Kim
8b20609227
dap: suppress error popup for failed evaluation request in repl mode (#2420)
It's expected that users enter invalid expressions through DEBUG
CONSOLE. Pop up for every error is not pleasant.
2021-04-09 10:03:59 +02:00
Hyang-Ah Hana Kim
da27e34217
dap: change how noDebug launch request is served (#2407)
* 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
2021-04-05 11:44:02 -07:00
Suzy Mueller
370ec4e6e4
service/dap: switch goroutines when stepping (#2403)
* 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
2021-04-02 09:19:16 -07:00
Hyang-Ah Hana Kim
743f243841
service/dap: change dap error response logging to debug level (#2412)
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.

Fixes golang/vscode-go#1392
2021-04-02 09:17:43 -07:00
polinasok
2414dcdd30
service/dap: truncate long compound map keys and use unique address suffix for uniqueness (#2399)
* 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>
2021-03-25 09:44:32 -07:00
polinasok
c44252b6fe
service/dap: clarify treatment of relative output path (#2402)
* 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>
2021-03-24 11:02:22 -07:00
Hyang-Ah Hana Kim
3c1b94276a
service/dap: supports noDebug launch requests (#2400)
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.
2021-03-23 12:10:21 -07:00
Suzy Mueller
1c9a10529e
service/dap: use specified working directory for launch requests (#2360)
* 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
2021-03-22 20:06:09 -07:00
polinasok
333e84a0cb
service/dap: use (*api.Variable).SinglelinesString() for dap.Variable values (#2383)
* 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>
2021-03-15 09:36:46 -07:00
Alessandro Arzilli
fe904c14d1
service: serialize calls to Command API (#2370)
* 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 #1608
Fixes #2216

* service/rpccommon: fix DeepSource issues
2021-03-08 10:05:10 -08:00
polinasok
90fb0a535f
service/dap: support auto-loading of unloaded interfaces (#2362)
* 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>
2021-03-08 09:41:47 -08:00
Luis Gabriel Gomez
4364c728f0
dap: reduce branching on onLaunchRequest mode validations (#2364) 2021-03-05 10:07:23 +01:00
polinasok
2e80b32c41
service/dap: avoid double removal of temp built binary (#2335)
* 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>
2021-02-24 08:19:07 -08:00
polinasok
129a9fe46c
service/dap: detect and report unloaded variables (#2353)
* Detect and report unloaded variables

* Make DeepSource happy

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-02-23 08:29:06 -08:00
polinasok
540e63a55e
Include optional breakpoint info in setBreakpoints responses to reliably populate the UI (#2347)
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-02-21 08:02:42 -08:00
Mark Eibes
3269d7a120
service/dap: Update server.go (#2350) 2021-02-21 10:30:04 +01:00
polinasok
82fcd2cb26
service/dap: Add support for evaluateName for variables (#2292)
* 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>
2021-01-14 10:53:12 -08:00
polinasok
a224f17374
service/dap: use address, not index to differentiate compound map keys (#2291)
* Use address, not index to differentiate compound map keys

* Clean up calls to expectVarRegex

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2021-01-08 09:17:54 -08:00
polinasok
796278ecba
service/dap: Support local attach (#2260)
* Support local attach

* Undo loopprog change

* Remove test check for system-specific error message

* Skip attach tests on freebasd

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-12-28 09:14:15 -08:00
Alessandro Arzilli
807664b34b
proc: add flag to distinguish ReturnValues (#2230)
Adds a flag that distinguishes the return values of an injected
function call from the return values of a function call executed by the
target program.
2020-12-10 08:57:50 -08:00
polinasok
98cba03f7d
service/dap: expand goroutine info in threads response (#2233)
* service/dap: expand goroutine info in threads response

* Fix checks

* Make DeepSource happy

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-11-30 09:43:37 -08:00
polinasok
d3d2910de2
Enable supportsDelayedStackTraceLoading capability (#2238)
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-11-30 09:43:09 -08:00
polinasok
c94db60d27
service/dap: support evaluate requests with expressions and calls (#2185)
* Support evaluate request

* Fix failing tests

* Call support

* Remove debugger.CurrentThread() that got accidentally reintroduced during merge

* Address review comments

* Function to stringify stop reason

* Add resetHandlesForStop

* Handle stop inside call

* More tests

* Address review comments

* Check all threads to determine if call completed

* Fix test

* Fix test

* Fix test

* Address review comments

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-11-12 15:24:31 -08:00
Suzy Mueller
5632cf92be
service/dap: add "panic" and "fatal error" as stopped reasons (#2186)
* 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
2020-10-07 08:24:40 -07:00
polinasok
80d0c8e717
service/dap: support clearing breakpoints and setting breakpoint conditions (#2188)
* Support clearing breakpoints and setting conditions

* Return unverified breakpoints with errors

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-10-02 09:18:33 -07:00
polinasok
551c541737
service/dap: make interface variable to dap.Variable conversion more readable (#2184)
* 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>
2020-09-29 07:03:53 -07:00
polinasok
1c4c1fd7fd
Remove irrelevant left-over comment (#2183)
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-09-28 11:05:33 +02:00
Alessandro Arzilli
310a124dba
service/*: remove threadID argument of (*Debugger).PackageVariables (#2182)
* Travis-CI: add ignorechecksum option to chocolatey command

Looks like a configuration problem on chocolatey's end.

* service/*: remove threadID argument of (*Debugger).PackageVariables

Which thread is used doesn't make any difference to the list of package
variables that is returned and this option was only ever used by an old
v1 API call.
2020-09-24 08:02:29 -07:00
polinasok
4980fff8ce
service/dap: Add support for package globals to scopes/variables requests (#2160)
* 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>
2020-09-15 13:14:55 -07:00
aarzilli
6ef7aa8743 service: move some type conversions from service/debugger to rpc pkgs
Move the conversion of some 'proc' types from service/debugger into
service/rpc1 and service/rpc2. The methods of
service/debugger.(*Debugger) are also used by service/dap which
requires these types to be converted differently and converting them
twice is inefficent and doesn't make much sense.

Updates #2161
2020-09-10 16:19:04 +02:00
polinasok
0fd4ff4260
Use 'nil' as string value for interfaces with 0 addr (#2158)
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-09-09 12:47:18 -07:00
polinasok
ec9765359a
service/dap: support next, stepIn and stepOut requests (#2143)
* Issue stopped event on runtime error when continuing

* Support next, stepIn and stepOut

* Refactor stop logic

* Explicitely set AllThreadsContinued

* Make DeepSource happy

* Respond to review comments

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-08-24 10:21:51 -07:00
polinasok
58ea3234ef
service/dap: Initial support for scopes and variables requests (#2111)
* 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>
2020-08-11 08:34:27 -07:00
polinasok
c63ae072bc
service/dap: resolve minor error code TODO (#2103)
Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-07-09 14:39:04 -07:00
polinasok
717e2685db
service/dap: Add launch request support for stackTraceDepth arg (#2099)
* service/dap: Support stackTraceDepth launch argument

* Respond to review comments

* Make DeepSource happy

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-07-08 10:20:05 -07:00
polinasok
7e13791b4a
service/dap: Support stackTrace request (#2093)
* service/dap: Support stackTrace request

* Minor tweaks

* Address review comments

* Fix grammar

* Address review comments

* Address review comments

* Update comments

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-07-01 11:01:17 -07:00
polinasok
4be75bec59
service/dap: Add buildFlags error checking and tests in launch requests (#2056)
* service/dap: Add error checking and tests for buildFlags in launch requests

* Clarify with comments and better naming

* Undo redundant support.go changes

* Undo redundant support.go changes (attempt #2)

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-05-28 14:01:51 -07:00
polinasok
f92afb9c17
service/dap: Adds launch request support for program args (#2040)
* Adds launch request support for program args

* Combine types of function parameters

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-05-11 09:52:26 -07:00
polinasok
2f295f310d
service/dap: Add error handlers for unsupported and not-yet-supported requests (#1918)
* Add no-op handlers for unsupported requests

* Fix whitespace

* Add couple of missing unsupported requests

* More comments

* Separate errors for unsupported and not-yet-implemented requests.

* Fix go.mod/sum dependencies

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
2020-05-01 12:24:44 -07:00
Derek Parker
e5d24a96bf *: Consolidate service/debugger config
Embed the debugger config object in the service config object to avoid needless duplication of fields.
2020-04-14 21:02:38 +02:00
Derek Parker
3e60ae202b *: Add --tty flag for debug / exec
This flag allows users on UNIX systems to set the tty for the program
being debugged by Delve. This is useful for debugging command line
applications which need access to their own TTY, and also for
controlling the output of the debugged programs so that IDEs may open a
dedicated terminal to show the output for the process.
2020-04-10 09:53:13 -07:00