Commit Graph

75 Commits

Author SHA1 Message Date
aarzilli
60c58acb8e proc,service: display return values when stepping out of a function
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.
2018-06-12 11:35:56 +02:00
aarzilli
f38a2816d1 proc: move AllGCache to a common struct
Add a new method "Common" to proc.Process that returns a pointer to a
struct that pkg/proc can use to store its things, independently of the
backend.

This is used here to replace the AllGCache typecasts, it will also be
used to store the return values of the stepout breakpoint and the state
for injected function calls.
2018-06-12 11:35:56 +02:00
aarzilli
5155ef047f proc,dwarf/line: support is_stmt and prologue_end flags
Go1.11 uses the is_stmt flag of .debug_line to communicate which
assembly instructions are good places for breakpoints, we should
respect this flag.

These changes were introduced by:
* https://go-review.googlesource.com/c/go/+/102435/

Additionally when setting next breakpoints ignore all PC addresses that
belong to the same line as the one currently under at the cursor. This
matches the behavior of gdb and avoids stopping multiple times at the
heading line of a for statement with go1.11.

Change: https://go-review.googlesource.com/c/go/+/110416 adds the
prologue_end flag to the .debug_line section to communicate the end of
the stack-split prologue. We should use it instead of pattern matching
the disassembly when available.

Fixes #550

type of interfaces
'c7cde8b'.
2018-06-11 11:09:02 -07:00
aarzilli
77056de757 proc_test: miscellaneous line number changes in go1.11 2018-06-11 11:09:02 -07:00
aarzilli
5d26d333bf proc: handle new way of panic'ing in 1.11 2018-06-11 11:09:02 -07:00
aarzilli
3381404604 pkg/proc: respect specified load configuration after reslicing a map
Maps were always loaded with using the default configuration during a
reslice. This is probably a remnant from when we didn't let clients
configure the load parameters.
2018-06-08 13:20:54 -07:00
Sergio Lopez
bafa512067 proc_test: implement test for DWZ compressed DWARF
If dwz binary is available in the system, test delve's ability to find
deduplicated symbols in the DWARF information.

dwzcompression.go contains a small C function (void fortytwo()) which
calls glibc's fprintf with stdin as first argument. Normally, stdin
will be present as a DW_TAG_variable as part of a DW_TAG_compile_unit
named dwzcompression.cgo2.c.

After running dwz on the binary, stdin is moved to a
DW_TAG_partial_unit, which is imported from dwzcompression.cgo2.c with
a DW_TAG_imported_unit.

This test verifies that delve is able to find stdin symbol's type, as a
way to confirm it understands dwz's compressed/deduplicated DWARF
information.
2018-05-23 13:59:45 -07:00
aarzilli
cc86bde549 proc/native,proc/gdbserial: let target access terminal
Change the linux verison of proc/native and proc/gdbserial (with
debugserver) so that they let the target process use the terminal when
delve is launched in headless mode.

Windows already worked, proc/gdbserial (with rr) already worked.
I couldn't find a way to make proc/gdbserial (with lldb-server) work.

No tests are added because I can't think of a way to test for
foregroundness of a process.

Fixes #65
2018-05-18 09:53:29 -07:00
aarzilli
7fd47749ef proc: Flag shadowed arguments as shadowed
Fixes #951
2018-04-23 10:13:21 -07:00
aarzilli
21be59469a proc: cache entire frame in FrameToScope instead of variablesByTag
Caching the frame in variablesByTag is problematic:

1. accounting for variables that are (partially) stored in registers is
complicated (see issue #1106)
2. for some types (strings, interfaces...) simply creating the Variable
object reads memory, which therefore happens before we can do any
caching.

Instead cache the entire frame when the EvalScope object is created.
The cached range is between the SP value of the current frame and the
CFA of the preceeding frame, if available, or the CFA of the current
frame otherwise.

Fixes #1106
2018-04-23 10:13:21 -07:00
aarzilli
e1f2626b56 proc_test: instrument TestFrameEvaluation
I've seen TestFrameEvaluation fail in CI in the past. It's been a while
since the last time and I couldn't reproduce it locally at all. I'd
like to have some instrumentation in case it happens again.
2018-04-13 15:32:33 -07:00
aarzilli
290e8e7528 proc: support inlining
Go 1.10 added inlined calls to debug_info, this commit adds support
for DW_TAG_inlined_call to delve, both for stack traces (where
inlined calls will appear as normal stack frames) and to correct
the behavior of next, step and stepout.

The calls to Next and Frame of stackIterator continue to work
unchanged and only return real stack frames, after reading each line
appendInlinedCalls is called to unpacked all the inlined calls that
involve the current PC.

The fake stack frames produced by appendInlinedCalls are
distinguished from real stack frames by having the Inlined attribute
set to true. Also their Current and Call locations are treated
differently. The Call location will be changed to represent the
position inside the inlined call, while the Current location will
always reference the real stack frame. This is done because:

* next, step and stepout need to access the debug_info entry of
the real function they are stepping through
* we are already manipulating Call in different ways while Current
is just what we read from the call stack

The strategy remains mostly the same, we disassemble the function
and we set a breakpoint on each instruction corresponding to a
different file:line. The function in question will be the one
corresponding to the first real (i.e. non-inlined) stack frame.

* If the current function contains inlined calls, 'next' will not
set any breakpoints on instructions that belong to inlined calls. We
do not do this for 'step'.

* If we are inside an inlined call that makes other inlined
functions, 'next' will not set any breakpoints that belong to
inlined calls that are children of the current inlined call.

* If the current function is inlined the breakpoint on the return
address won't be set, because inlined frames don't have a return
address.

* The code we use for stepout doesn't work at all if we are inside
an inlined call, instead we call 'next' but instruct it to remove
all PCs belonging to the current inlined call.
2018-03-26 14:30:38 -04:00
Yasushi Saito
82aff3f18a Extend the "frame" command to set the current frame. (#1110)
* Extend the "frame" command to set the current frame.

Command

  frame 3

sets up so that subsequent "print", "set", "whatis" command
will operate on frame 3.

  frame 3 print foo

continues to work.

Added "up", "down". They move the current frame up or down.

Implementation note:

This changes removes "scopePrefix" mode from the terminal/command.go and instead
have the command examine the goroutine/frame value to see if it is invoked in a
scoped context.

* Rename Command.Frame -> Command.frame.
2018-03-22 10:02:15 -07:00
aarzilli
ec8dc3a10d proc,vendor: show global variables in disassembly
updates vendored version of x86asm, adds a symbol lookup function to
pass to the disassembler.

This will show global symbol names in the disassembly like go tool
objdump does.
2018-03-22 10:01:00 -07:00
Josh Soref
1d3b41f64e all: Spelling 2018-03-20 11:05:35 +01:00
aarzilli
449b3cedef proc: manual stop requests should clear internal breakpoints
Fixes #1145
2018-03-08 12:02:29 -08:00
aarzilli
e47599d09a proc: remove proc.Process.Halt
The proper way to stop a running process is to call RequestManualStop,
which most code already did with the exception of some old test code.
2018-03-06 09:06:19 -08:00
aarzilli
ac1aa98378 proc: remove proc.Process.Kill
the proper way to kill the target process is to pass true to Detach.
Everything except old test code did that already.
2018-03-06 09:06:19 -08:00
aarzilli
f32ce1b21d proc/native: fix race condition between Halt and process death (linux)
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
2018-03-06 09:06:19 -08:00
aarzilli
dc7a4ccb16 proc: support interface type resolution for packages containing a dot
If the last entry of the package path contains a '.' the corresponding
DIEs for its types will replace the '.' character with '%2e'. We must
do the same when resolving the package path of the concrete type of an
interface variable.

Fixes #1137
2018-03-05 10:07:11 -08:00
Alessandro Arzilli
0c40a8f52a dwarf/reader,proc: support DW_AT_abstract_origin (#1111)
debug_info entries can use DW_AT_abstract_origin to inherit the
attributes of another entry, supporting this attribute is necessary to
support DW_TAG_inlined_subroutine.

Go, starting with 1.10, emits DW_TAG_inlined_subroutine entries when
inlining is enabled.
2018-02-13 09:20:45 -08:00
aarzilli
74d330a013 proc: Remove unused code 2018-01-26 12:58:21 -08:00
aarzilli
91fdc5202d proc_test: clean some repeated patterns in proc_test 2018-01-26 12:58:21 -08:00
Alessandro Arzilli
bc77ff4534
proc_test: deflake TestSystemstackOnRuntimeNewstack (#1078)
Depending on how the runtime schedules our goroutines we can get
unlucky and have the first call to runtime.newstack we intercept be for
a different goroutine (usually the garbage collector).
Only check stacktraces that happen on the same goroutine that executed
main.main.
2018-01-19 15:42:23 +01:00
aarzilli
7bec20e5fc proc: avoid scanning system stack if it's not executing cgo
The runtime calls into g0 in many places, not necessarily using
runtime.systemstack or runtime.asmcgocall.
One example of this is the call to runtime.newstack inside
runtime.morestack.

If we stop the process while one goroutine is executing
runtime.newstack we would be unable to fully scan its stack because we
don't know that we have to switch back to the goroutine stack after
runtime.newstack.

Instead of tracking down every possible way that the runtime switches
to g0 we switch to the goroutine stack immediately after the top of the
stack, unless cgo is being executed on the systemstack.

Fixes #1066
2018-01-05 10:29:31 -08:00
aarzilli
07c716818e proc/test: miscellaneous test changes for go1.10 2017-12-13 12:18:18 -08:00
aarzilli
85669434f6 pkg/proc: use DW_AT_decl_line to determine var visibility
Fixes #186, #83
2017-12-13 12:18:18 -08:00
aarzilli
b3246296d7 dwarf/line: handle DW_LNE_end_of_sequence correctly
We need to reset the current file and line number.

Fixes #1008
2017-12-08 02:16:26 -08:00
aarzilli
77c955365f proc: handle DW_TAG_subprogram with a nochildren abbrev
On macOS, externally linked programs will have an abbrev for
DW_TAG_subprogram without the haschildren flag set. We should handle
this case instead of expecting all DW_TAG_subprogram entries to have
list of children.

Fixes #1034
2017-12-07 15:00:18 -08:00
aarzilli
5372588c61 proc: support cgo stacktraces
When creating a stack trace we should switch between the goroutine
stack and the system stack (where cgo code is executed) as appropriate
to reconstruct the logical stacktrace.

Goroutines that are currently executing on the system stack will have
the SystemStack flag set, frames of the goroutine stack will have a
negative FrameOffset (like always) and frames of the system stack will
have a positive FrameOffset (which is actually just the CFA value for
the frame).

Updates #935
2017-11-28 11:00:53 -08:00
aarzilli
5fdcd2c91a cmd, proc/test: disable optimizations on the C compiler
Pass CGO_FLAGS='-O0 -g' to go build to disable optimizations when
calling the C compiler.
2017-11-28 11:00:53 -08:00
aarzilli
1ced7c3a60 proc: next should not skip lines with conditional bps
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
2017-11-20 11:25:35 -08:00
aarzilli
178589a4e7 proc: breakpoints refactoring
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.
2017-11-20 11:25:35 -08:00
aarzilli
f4e2000fc8 proc: refactor stack.go to use DWARF registers
Instead of only tracking a few cherrypicked registers in stack.go track
all DWARF registers.

This is needed for cgo code and for the locationlists emitted by go in
1.10:
* The debug_frame sections emitted by C compilers can not be used
  without tracking all registers
* the loclists emitted by go1.10 need all registers of a frame to be
  interpreted.
2017-11-17 10:17:24 -08:00
aarzilli
5c9b2009ca proc: change next to skip deferred functions
Make 'next' skip deferred functions unless they are called via a panic.
Call to a deferred function through 'return' are predictable, if the
user wants to step into them 'step' can be used but without this change
there is no way to avoid stepping into them.

Implements #956
2017-09-25 12:46:25 -07:00
aarzilli
2c0e31160d proc/gdbserial: return error if stopped with a sginal
On macOS we can also stop when we receive a signal,
propagate this reason upwards to the client.

Also clear internal breakpoints after an unrecovered-panic since they
can not be reached anymore.

Fixes #872
2017-09-20 12:39:29 -07:00
aarzilli
9ee21686e6 proc: report errors when loading executable on attach
Fixes #940
2017-08-30 11:20:20 -07:00
aarzilli
f553c95eeb proc/tests: fix intermittent failure of TestNextParked
Always pick a goroutine that we know will have to be resumed before the
program terminates instead of relying on luck.

Fixes #803
2017-08-14 13:24:30 -06:00
aarzilli
2ad9ce6fe3 proc: lexical block support
Fixes #106
2017-08-01 11:20:25 -06:00
aarzilli
1128c26b87 cmd/dlv: do not pass "linkmode internal" for windows on go1.9 and later
go1.9 no longer needs "linkmode internal" on windows.

Fixes #755
Fixes #477
Fixes #631
2017-08-01 11:20:25 -06:00
aarzilli
a17de32c23 proc/variables: support embedded struct fields on go1.9
Before go1.9 embedded struct fields had name == "" in runtime and ==
type name in DWARF. After go1.9 both runtime and DWARF use a simplified
version of the type as name.
Embedded structs are distinguished from normal fields by setting a flag
in the runtime.structfield, for runtime, and by adding a custom
attribute in DWARF.
2017-08-01 11:20:25 -06:00
aarzilli
038fd33999 proc_test: line numbering changed 2017-08-01 11:20:25 -06:00
aarzilli
731829c349 proc: auto-dereference local variables that escape to the heap
The compiler a variable 'v' that escapes to the heap with a '&v' entry.
Auto dereference those local variables.

Fixe #871
2017-08-01 11:20:25 -06:00
aarzilli
2d9a9a76eb proc: fix next when current function is unknown on macOS
Updates #893
2017-07-26 12:50:09 -06:00
aarzilli
5c2673a632 proc/native, proc/gdbserial: StepInstruction without goroutine
proc.Process.StepInstruction should work even if there is no goroutine
selected.
2017-07-26 12:50:09 -06:00
Florin Pățan
135330cbb2 Pass LD_/DYLD_ env vars to debugserver (fixes #877) (#910) 2017-07-18 12:57:41 -06:00
Florin Pățan
32a005de2b Fix various issues detected by megacheck (#880)
* 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
2017-06-29 11:15:59 -07:00
aarzilli
16d8bd647f proc/*: remove Process.Running
Implementing proc.Process.Running in a thread safe way is complicated
and nothing actually uses it besides tests, so we are better off
rewriting the tests without Running and removing it.

In particular:

* The call to d.target.Running() in service/debugger/debugger.go
  (Restart) can never return true because that line executes while
  holding processMutex and all continue operations are also executed
  while holding processMutex.
* The call to dbp.Running() pkg/proc/native/proc.go (Detach) can never
  return true, because it's only called from
  debugger.(*Debugger).detach() which is also always called while
  holding processMutex.

Since some tests are hard to write correctly without Process.Running a
simpler interface, Process.NotifyResumed, is introduced.

Fixes #830
2017-06-13 08:53:54 +02:00
aarzilli
40b482130a proc/variables: protect form invalid G struct in parseG
A waitreason string that has invalid length (because the G struct is
corrupted or being modified) could cause a crash.
2017-05-25 21:04:09 +02:00
Alessandro Arzilli
354055836a proc: next, stepout should work on recursive goroutines (#831)
Before this commit our temp breakpoints only checked that we would stay
on the same goroutine.
However this isn't enough for recursive functions we must check that we
stay on the same goroutine AND on the same stack frame (or, in the case
of the StepOut breakpoint, the previous stack frame).

This commit:
1. adds a new synthetic variable runtime.frameoff that returns the
   offset of the current frame from the base of the call stack.
   This is similar to runtime.curg
2. Changes the condition used for breakpoints on the lines of the
   current function to check that runtime.frameoff hasn't changed.
3. Changes the condition used for breakpoints on the return address to
   check that runtime.frameoff corresponds to the previous frame in the
   stack.
4. All other temporary breakpoints (the step-into breakpoints and defer
   breakpoints) remain unchanged.

Fixes #828
2017-05-16 11:23:33 -07:00