proc.Next and proc.Step will call, after setting their temp
breakpoints, curthread.SetCurrentBreakpoint. This is intended to find
if one of the newly created breakpoints happens to be at the same
instruction that curthread is stopped at.
However SetCurrentBreakpoint is intended to be called after a Continue
and StepInstruction operation so it will also detect if curthread is
stopped one byte after a breakpoint.
If the instruction immediately preceeding the current instruction of
curthread happens to:
1. have one of the newly created temp breakpoints
2. be one byte long
SetCurrentBreakpoint will believe that we just hit that breakpoint and
therefore the instruction should be repeated, and thus rewind the PC of
curthread by 1.
We should distinguish between the two uses of SetCurrentBreakpoint and
disable the check for "just hit" breakpoints when inappropriate.
Fixes#1656
The repository is being switched from the personal account
github.com/derekparker/delve to the organization account
github.com/go-delve/delve. This patch updates imports and docs, while
preserving things which should not be changed such as my name in the
CHANGELOG and in TODO comments.
Fncall.go was written with the assumption that the object returned by
proc.Thread.Registers does not change after we call
proc.Thread.SetPC/etc.
This is true for the native backend but not for gdbserial. I had
anticipated this problem and introduced the Save/SavedRegisters
mechanism during the first implementation of fncall.go but that's
insufficient.
Instead:
1. clarify that the object returned by proc.Thread.Registers could
change when the CPU registers are modified.
2. add a Copy method to Registers that returns a copy of the registers
that are guaranteed not to change when the CPU registers change.
3. remove the Save/SavedRegisters mechanism.
This solution leaves us the option, in the future, to cache the output
of proc.(Thread).Registers, avoiding a system call every time it's
called.
Implements the function call injection protocol introduced in go 1.11
by https://go-review.googlesource.com/c/go/+/109699.
This is only the basic support, see TODO comments in pkg/proc/fncall.go
for a list of missing features.
Updates #119
Displays the return values of the current function when we step out of
it after executing a step, next or stepout command.
Implementation of this feature is tricky: when the function has
returned the return variables are not in scope anymore. Implementing
this feature requires evaluating variables that are out of scope, using
a stack frame that doesn't exist anymore.
We can't calculate the address of these variables when the
next/step/stepout command is initiated either, because between that
point and the time where the stepout breakpoint is actually hit the
goroutine stack could grow and be moved to a different memory address.
If a breakpoint is hit close to process death on a thread that isn't
the group leader the process could die while we are trying to stop it.
This can be easily reproduced by having the goroutine that's executing
main.main (which will almost always run on the thread group leader)
wait for a second goroutine before exiting, then setting a breakpoint
on the second goroutine and stepping through it (see TestIssue1101 in
proc_test.go).
When stepping over the return instruction of main.f the deferred
wg.Done() call will be executed which will cause the main goroutine to
resume and proceed to exit. Both the temporary breakpoint on wg.Done
and the temporary breakpoint on the return address of main.f will be in
close proximity to main.main calling os.Exit() and causing the death of
the thread group leader.
Under these circumstances the call to native.(*Thread).waitFast in
native.(*Thread).halt can hang forever due to a bug similar to
https://sourceware.org/bugzilla/show_bug.cgi?id=12702 (see comment in
native.(*Thread).wait for an explanation).
Replacing waitFast with a normal wait work in most circumstances,
however, besides the performance hit, it looks like in this
circumstances trapWait sometimes receives a spurious SIGTRAP on the
dying group leader which would cause the subsequent call to wait in
halt to accidentally reap the process without noting that it did exit.
Instead this patch removes the call to wait from halt and instead calls
trapWait in a loop in setCurrentBreakpoints until all threads are set
to running=false. This is also a better fix than the workaround to
ESRCH error while setting current breakpoints implemented in 94b50d.
Fixes#1101
Conditional breakpoints with unmet conditions would cause next and step
to skip the line.
This breakpoint changes the Kind field of proc.Breakpoint from a single
value to a bit field, each breakpoint object can represent
simultaneously a user breakpoint and one internal breakpoint (of which
we have several different kinds).
The breakpoint condition for internal breakpoints is stored in the new
internalCond field of proc.Breakpoint so that it will not conflict with
user specified conditions.
The breakpoint setting code is changed to allow overlapping one
internal breakpoint on a user breakpoint, or a user breakpoint on an
existing internal breakpoint. All other combinations are rejected. The
breakpoint clearing code is changed to clear the UserBreakpoint bit and
only remove the phisical breakpoint if no other bits are set in the
Kind field. ClearInternalBreakpoints does the same thing but clearing
all bits that aren't the UserBreakpoint bit.
Fixes#844
Move some duplicate code, related to breakpoints, that was in both
backends into a single place.
This is in preparation to solve issue #844 (conditional breakpoints
make step and next fail) which will make this common breakpoint code
more complicated.
* Fix various issues detected by megacheck
I've ran honnef.co/go/tools/cmd/megacheck and fixed a few of the
things that came up there.
* Cleanup using Gogland
