The comment on the advanceRegs() method was referencing several
nonexistent fields. This patch fixes that, and improves the comment to
reference the peculiar interaction between the method and it.regs.
evalFunctionCall needs to remove the breakpoint from the current thread
after starting the function call injection, otherwise Continue will
think that the thread is stopped at a breakpoint and return to the user
instead of continuing the call injection.
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.
* Use the active xcode-select path instead of a hardcoded Xcode path
* Refactored exec.Command to invoke Output instead of running with a custom buffer for stdout
Addresses review comment by @derekparker
1. Forward stdin/stdout/stderr to the target process when in foreground
mode instead of always forwarding the current tty (issue #1964)
2. When redirecting a file descriptor make sure to also specify
something for all three otherwise debugserver will misbehave (either
exit on launch or run but giving the target process a closed file
descriptor).
Fixes#1964
On linux we can not read memory if the thread we use to do it is
occupied doing certain system calls. The exact conditions when this
happens have never been clear.
This problem was worked around by using the Blocked method which
recognized the most common circumstances where this would happen.
However this is a hack: Blocked returning true doesn't mean that the
problem will manifest and Blocked returning false doesn't necessarily
mean the problem will not manifest. A side effect of this is issue
#2151 where sometimes we can't read the memory of a thread and find its
associated goroutine.
This commit fixes this problem by always reading memory using a thread
we know to be good for this, specifically the one returned by
ContinueOnce. In particular the changes are as follows:
1. Remove (ProcessInternal).CurrentThread and
(ProcessInternal).SetCurrentThread, the "current thread" becomes a
field of Target, CurrentThread becomes a (*Target) method and
(*Target).SwitchThread basically just sets a field Target.
2. The backends keep track of their own internal idea of what the
current thread is, to use it to read memory, this is the thread they
return from ContinueOnce as trapthread
3. The current thread in the backend and the current thread in Target
only ever get synchronized in two places: when the backend creates a
Target object the currentThread field of Target is initialized with the
backend's current thread and when (*Target).Restart gets called (when a
recording is rewound the currentThread used by Target might not exist
anymore).
4. We remove the MemoryReadWriter interface embedded in Thread and
instead add a Memory method to Process that returns a MemoryReadWriter.
The backends will return something here that will read memory using
the current thread saved by the backend.
5. The Thread.Blocked method is removed
One possible problem with this change is processes that have threads
with different memory maps. As far as I can determine this could happen
on old versions of linux but this option was removed in linux 2.5.
Fixes#2151
TestStepConcurrentDirect will occasionally fail (7% of the time on my
setup) by either causing the target processs to execute an invalid
instruction or (more infrequently) by switching to the wrong thread.
Both of those are caused by receiving SIGTRAPs for threads hitting a
breakpoint after it has been removed (the thread hits the breakpoint,
we stop everything and remove the breakpoint and only after we receive
the signal).
Change native.(*nativeProcess).stop to handle SIGTRAPs that can't be
attributed to a breakpoint, a hardcoded breakpoint in the program's
text, or manual stops (and therefore are likely caused by phantom
breakpoint hits).
Co-authored-by: a <a@kra>
Due to a missing check TestCgoStacktrace2 didn't actually check
anything. Enable it and then skip it on linux/386 and linux/arm64 where
it's broken.
Co-authored-by: a <a@kra>
If the process receives a signal (or sends a singal to itself) and then
dies before we can route the signal back to it we still need to
retrieve its exit status.
Fixes a rare failure of TestIssue1101 in proc_test.go
Co-authored-by: a <a@kra>
* proc/tests: keep track of tests skipped due to backend problems
Mark tests skipped due to backend problems and add a script to keep
track of them.
* Travis-CI: add ignorechecksum option to chocolatey command
Looks like a configuration problem on chocolatey's end.
* proc: use argument position for addr only when injecting function calls
We can not, in general, use the argument position to determine the
address of a formal parameter, it will not work in presence of
optimizations or inlining. In those cases formal arguments could be
stored in registers.
Fixes#2176
* Travis-CI: add ignorechecksum option to chocolatey command
Looks like a configuration problem on chocolatey's end.
Co-authored-by: a <a@kra>
* Revert "proc: Find executable should follow symbol links."
This reverts commit 3e04ad0fada0c3ab57caf58bc024e4c0f9a3e01a.
* proc: resolve symlinks when searching for split debug_info if path is /proc/pid/exe
Fixes#2168
During the testing of the core dump generation feature two bugs were
discovered in gdbserial:
1. we don't check that both bytes of the checksum are read, if the
buffer only has one byte we can end up reading only one byte instead
of two and the second byte will mess up the parsing of the next
packet
2. binary encoded packets can start with an 'E' and not be errors, when
using binary responses add an extra check for the lenght of the
response before deciding that the response is an error.
Unfortunately this encoding is inherently ambiguous (we can't
distinguish a 3 byte response starting with 'E' from an error) so
binary requests that lead to short responses should be avoided.
Testing this is complicated, they will be tested implicitly by the
upcoming core dump test.
Co-authored-by: a <a@kra>
Since proc is supposed to work independently from the target
architecture it shouldn't use architecture-dependent types, like
uintptr. For example when reading a 64bit core file on a 32bit
architecture, uintptr will be 32bit but the addresses proc needs to
represent will be 64bit.
Adds features to support default file descriptor redirects for the
target process:
1. A new command line flag '--redirect' and '-r' are added to specify
file redirects for the target process
2. New syntax is added to the 'restart' command to specify file
redirects.
3. Interactive instances will check if stdin/stdout and stderr are
terminals and print a helpful error message if they aren't.
An internal breakpoint condition shouldn't ever error:
* use a ThreadContext to evaluate conditions if a goroutine isn't
available
* evaluate runtime.curg to a fake g variable containing only
`goid == 0` when there is no current goroutine
Fixes#2113
Recent changes to the way registers are handled broke reporting of AVX
registers (i.e. YMMx). This change restores the functionality by:
- concatenating the higher half of the YMMx registers to their
corresponding XMMx lower half (YMMx registers do not have an
independent DWARF register number)
- modifying the formatSSEReg function to handle them when they are
present.
Fixes#2033
Commit 1ee8d5c reviewed in Pull Request #1960 relaxed some tests using
goroutinestackprog but missed others.
Fixes some test flakiness that isn't relevant.
* proc: start variable visibility one line after their decl line
In most cases variables shouldn't be visible on their declaration line
because they won't be initialized there.
Function arguments are treated as an exception.
This fix is only applied to programs compiled with Go 1.15 or later as
previous versions of Go did not report the correct declaration line for
variables captured by closures.
Fixes#1134
* proc: silence go vet error
* Makefile: enable PIE tests on windows/Go 1.15
* core: support core files for PIEs on windows
* goversion: add Go 1.15 to supported versions
* proc: fix function call injection for Go 1.15
Go 1.15 changed the call injection protocol so that the runtime will
execute the injected call on a different (new) goroutine.
This commit changes the function call support in delve to:
1. correctly track down the call injection state after the runtime
switches to a different goroutine.
2. correctly perform the escapeCheck when stack values can come from
multiple goroutine stacks.
* proc: miscellaneous fixed for call injection under macOS with go 1.15
- create copy of SP in debugCallAXCompleteCall case because the code
used to assume that regs doesn't change
- fix automatic address calculation for function arguments when an
argument has a spurious DW_OP_piece at entry
The file:line information for the entrypoint is more acccurate than the
file:line information at a return point, which could be affected by a
compiler bug.
Fixes#2086
Recent change #2061:
292f5c69f0c769fd32c2e8b1e7153b56e908efd7
proc: step into unexported runtime funcs when already inside runtime
means that TestIssue414 (which tries to step repeatedly until the
program exits) can now steps through way more runtime code than it ever
did before. This causes this test to occasionally fail. Stepping
blindly through runtime code has never been particularly safe as the
runtime can switch to a different goroutine causing Delve to misbehave.
This change restores the previous behavior of TestIssue414 where Step
behaved like Next inside runtime code.
These methods only work if registers have been loaded once after the
last resume, there's probably no code path that calls SetXX before
Thread.Registers but lets make sure it can't happen anyway.
On platforms other than macOS this doesn't matter but on macOS a
segmentation fault will cause ContinueOnce to return an error, before
returning it we should still fix the current thread and selected
goroutine values.
Fixes#2078
Normally we don't step into unexported runtime functions because the
compiler is free to insert them into the code and they are not relevant
to the user, however if we are already stepping through a runtime
function we should let step into work normally and step into other
runtime functions.
Changes implementations of proc.Registers interface and the
op.DwarfRegisters struct so that floating point registers can be loaded
only when they are needed.
Removes the floatingPoint parameter from proc.Thread.Registers.
This accomplishes three things:
1. it simplifies the proc.Thread.Registers interface
2. it makes it impossible to accidentally create a broken set of saved
registers or of op.DwarfRegisters by accidentally calling
Registers(false)
3. it improves general performance of Delve by avoiding to load
floating point registers as much as possible
Floating point registers are loaded under two circumstances:
1. When the Slice method is called with floatingPoint == true
2. When the Copy method is called
Benchmark before:
BenchmarkConditionalBreakpoints-4 1 4327350142 ns/op
Benchmark after:
BenchmarkConditionalBreakpoints-4 1 3852642917 ns/op
Updates #1549
Unexport `GetDebugServerAbsolutePath` and avoid unnecessary repeated calls.
Remove `os.Stat` because `Exec.LookPath` has already used `os.Stat`.And Fix
some comments.
On linux platform, we simply treated `/proc/$pid/exe` as the
executable of targeting process when doing `dlv attach`. The
`/proc/$pid/exe` is a symbol link of the real executable file.
Delve couldn't find the corrsponding external debug file based on the
symbol link:
```
could not attach to pid $pid: could not open debug info
```
The fix is to evaluate the symbol links to the actual executable path.
The process could quit while we are inside stop, we should report the
error otherwise the following code will try to send on the closed
ptrace channel.
Fixes a sporadic error in TestIssue1101.
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.
* gdbserial/gdbserver: Dynamically resolve debugserver binary
Instead of hardcoding the absolute path to the Command Line
Tools (CLT) binary, will attempt to resolve the path at the
$PATH, or at the Xcode bundle. If none are available, will
fallback to the default CLT location.
Fixes#986
* gdbserial/gdbserver: Log outgoing executed commands
Add logging to capture the executable and associated arguments used
in LLDBLaunch and LLDBAttach
Related to #986
* gdbserial/gdbserver: Add unit tests for helper function
Define unit tests for helper function. Setup each test to temporarily make
PATH variable, and file system changes, and subsequently revert them.
Related to #986
* gdbserial/gdbserver: Lazily load function
Lazily obtain absolute path to avoid increasing load times.
Remove flaky tests.
Related to #986
The test was always flaky because we can't fully control the state of
all goroutines in the target program, Go 1.14's asynchronous preemption
exacerbates the problem.
See for example:
https://travis-ci.com/github/go-delve/delve/jobs/302407282
This commit relaxes the checks made by the test to avoid irrelevante
flakiness.
Under some circumstances (methods with non-pointer receivers or from
embedded fields called through an interface) the compiler will
autogenerate wrapper functions.
This commit changes next, step and stepout to skip all autogenerated
wrappers.
Fixes#1908
Allows Delve clients to stop a recording midway by sending a
Command('halt')
request.
This is implemented by changing debugger.New to start recording the
process on a separate goroutine while holding the processMutex locked.
By locking the processMutex we ensure that almost all RPC requests will
block until the recording is done, since we can not respond correctly
to any of them.
API calls that do not require manipulating or examining the target
process, such as "IsMulticlient", "SetApiVersion" and
"GetState(nowait=true)" will work while we are recording the process.
Two other internal changes are made to the API: both GetState and
Restart become asynchronous requests, like Command. Restart because
this way it can be interrupted by a StopRecording request if the
rerecord option is passed.
GetState because clients need a call that will block until the
recording is compelted and can also be interrupted with a
StopRecording.
Clients that are uninterested in allowing the user to stop a recording
can ignore this change, since eventually they will make a request to
Delve that will block until the recording is completed.
Clients that wish to support this feature must:
1. call GetState(nowait=false) after connecting to Delve, before any
call that would need to manipulate the target process
2. allow the user to send a StopRecording request during the initial
GetState call
3. allow the user to send a StopRecording request during any subsequent
Restart(rerecord=true) request (if supported).
Implements #1747
Reset the timer once we start executing the code paths we actually care
to measure, since the setup and continue steps can be relatively
expensive.
Also, disable recording on benchmarks. This seems less useful, and also
cuts out a lot of noise when benchmarks are run.
When switching to the goroutine stack the stack iterator registers
might not have any entry for the BP register, make sure to add it
instead of just trying to change it.
Instead of rescanning debug_info every time we want to read a function
(either to find inlined calls or its variables) cache the tree of
dwarf.Entry that we would generate and use that.
Benchmark before:
BenchmarkConditionalBreakpoints-4 1 5164689165 ns/op
Benchmark after:
BenchmarkConditionalBreakpoints-4 1 4817425836 ns/op
Updates #1549
Add logging for Issue #1927. The bug happens during the call to
api.ConvertThread, returning an error will not suffice since
ConvertThread will not surface it.
Updates #1927
Inserts a call to ClearAllGCache into stepInstructionOut so that cached
goroutine state is not inconsistent after an injected function call.\
Fixes#1925
If we call one expression which is the fake method of meanless
string, `delve` will panic. Strengthen the inspection of boundary
conditions when supporting function calls on non-struct types.
Update: #1871
* *: Fix go vet struct complaints
* *: Fix struct vet issue on linux
* *: Ignore proc/native in go vet check
We have to do some unsafe pointer manipulation that will never make go
vet happy within the proc/native package. Ignore it for runs of go vet.
* proc: move defer breakpoint code into a function
Moves the code that sets a breakpoint on the first deferred function,
used by both next and StepOut, to its function.
* proc: implement reverse step/next/stepout
When the direction of execution is reversed (on a recording) Step, Next and
StepOut will behave similarly to their forward version. However there are
some subtle interactions between their behavior, prologue skipping, deferred
calls and normal calls. Specifically:
- when stepping backwards we need to set a breakpoint on the first
instruction after each CALL instruction, once this breakpoint is reached we
need to execute a single StepInstruction operation to reverse step into the
CALL.
- to insure that the prologue is skipped reverse next needs to check if it
is on the first instruction after the prologue, and if it is behave like
reverse stepout.
- there is no reason to set breakpoints on deferred calls when reverse
nexting or reverse stepping out, they will never be hit.
- reverse step out should generally place its breakpoint on the CALL
instruction that created the current stack frame (which will be the CALL
instruction immediately preceding the instruction at the return address).
- reverse step out needs to treat panic calls and deferreturn calls
specially.
* service,terminal: implement reverse step, next, stepout
* proc,proc/*: move SelectedGoroutine to proc.Target, remove PostInitializationSetup
moves SelectedGoroutine, SwitchThread and SwitchGoroutine to
proc.Target, merges PostInitializationSetup with NewTarget.
* proc,proc/*: add StopReason field to Target
Adds a StopReason field to the Target object describing why the target
process is currently stopped. This will be useful for the DAP server
(which needs to report this reason in one of its requests) as well as
making pull request #1785 (reverse step) conformant to the new
architecture.
* proc: collect NewTarget arguments into a struct
Implement debugging function for 386 on linux with reference to AMD64.
There are a few remaining problems that need to be solved in another time.
1. The stacktrace of cgo are not exactly as expected.
2. Not implement `core` for now.
3. Not implement `call` for now. Can't not find `runtime·debugCallV1` or
similar function in $GOROOT/src/runtime/asm_386.s.
Update #20
* proc/native/linux: only set breakpoints on threads that receive SIGTRAP
* proc/native/linux: do not call (*Thread).Stopped inside (*Process).stop
(*Thread).Stopped is slow because it needs to open, read and parse a
file in /proc, we don't actually need to do that, we can just rely on
the value of Thread.os.running.
Benchmark before:
BenchmarkConditionalBreakpoints-4 1 12476166303 ns/op
Benchmark after:
BenchmarkConditionalBreakpoints-4 1 10403533675 ns/op
Conditional breakpoint evaluation: 1.24ms -> 1ms
Updates #1549
This change adds `ProcessVmRead` and `ProcessVmWrite` wrappers around
the syscalls `process_vm_readv` and `process_vm_writev`, available since
Linux 3.2. These follow the same permission model as `ptrace`, but they
don't actually require being attached, which means they can be called
directly from any thread in the debugger. They also use `iovec` to write
entire blocks at once, rather than having to peek/poke each `uintptr`.
These wrappers are used in `Thread.ReadMemory` and `WriteMemory`, still
falling back to `ptrace` if that fails for any reason. Notably,
`process_vm_writev` respects memory protection, so it can't modify
read-only memory like `ptrace`. This frequently occurs when writing
breakpoints in read-only `.text`, so to avoid a lot of wasted `EFAULT`
calls, we only try `process_vm_writev` for larger writes.
Adds an optional scope prefix to the `regs` command which allows
printing registers for any stack frame (as long as they were somehow
saved). Issue #1838 is not yet to be closed since we are still not
recovering the registers of a segfaulting frame.
Updates #1838
As we rearrange the code and the Go compiler changes the error message
returned by the compiler on unsupported architectures will change too,
making it un-googlable. Since the error message tends to be rather
obscure too this regularly confuses newbies.
This is an effort to make the error message for unsupported GOOS/GOARCH
combinations the same across all unsupported combinations and to make
it more user friendly.
Directories containing Go source code are supposed to contain a single
package. This property happens to be checked by cmd/go itself so it
will happen even before the syntax is fully checked and therefore has a
high probability of being the first (and only) error message being
print.
Here we take advantage of this by adding to the pkg/proc/native
directory a file with a bad package line that only gets compiled in on
unsupported GOOS/GOARCH combinations.
At present the error message for compiling Delve on unsupported systems
will be:
service/debugger/debugger.go:21:2: found packages native (proc.go) and your_operating_system_and_architecture_combination_is_not_supported_by_delve (support_sentinel.go) in $PATH_TO_DELVE/pkg/proc/native
Go 1.8 has been out of date for a while and the code to support it is
potentially dangerous if a future version of Go removes the runtime
types from debug_info and also changes their representation.
