* 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>
Change debug_info type reader and proc to convert parametric types into
their real types by reading the corresponding dictionary entry and
using the same method used for interfaces to retrieve the DIE from a
runtime._type address.
'2586e9b1'.
With generics a single function can have multiple concrete
instantiations, the old version of FindFileLocation supported at most
one concrete instantiation per function and any number of inlined
calls, this supports any number of inlined calls and concrete
functions.
Internal breakpoints do not need IDs and assigning them from a counter
separate from the user ID counter can be a cause of confusion.
If a user breakpoint is overlayed on top of a pre-existing internal
breakpoint the temporary ID will be surfaced as if it was a user ID,
possibly conflicting with another user ID.
If a temporary breakpoint is overlayed on top of a pre-existing user
breakpoint and the user breakpoint is first deleted and then
re-created, the user ID will be resurrected along with the breakpoint,
instead of allocating a fresh one.
This change removes internal breakpoint IDs entirely, only user
breakpoints receive an ID.
Right now, if (*compositeMemory).WriteMemory needs to write a value to
a register that's smaller than the full size of the register (say, a
uint32 being passed as an argument), then (*AMD64Registers).SetReg can
later fail a sanity check that ensures the passed DwarfRegister is a
full size register.
Fix this by reading the old value of the register and overwriting just
the relevant parts with the new register. For the purposes of an
argument, it would probably be fine to just pad with zeroes, but merging
with the existing value is what gdb does.
Fixes#2698
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.
This patch enables the eBPF tracer backend to parse the ID of the
Goroutine which hit the uprobe. This implementation is specific to AMD64
and will have to be generalized further in order to be used on other
architectures.
* 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
* 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>
