Commit Graph

171 Commits

Author SHA1 Message Date
aarzilli
e19cbcefa9 proc,command: fix optimized function warning in 1.9
A user complained on the mailing list about having continuous
"optimized function warnings" on non-optimized functions when using 1.9.
This commit fixes the problem by disabling optimized function detection
on 1.9 and earlier (where it's impossible) and adds a test so we don't
break it again in the future.
2018-06-20 07:56:32 -07:00
aarzilli
2a2d1040e9 proc: allow "package/path".varname syntax
If the application being debugged imports two packages with the same
name (but different paths) there was no way to disambiguate the two,
since the character '/' can not appear inside a go identifier.

By allowing users to use a string literal as the package name a package
path can be specified.
2018-06-14 09:29:23 -07:00
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
d04c60e0b9 proc/gdbserial: add syscall.Syscall* to list of blocking functions
Fixes #1227
2018-06-11 14:25:05 -07:00
aarzilli
9464c23740 proc: in go1.11 runtime.g.waitreason is not a string 2018-06-11 11:09:02 -07:00
aarzilli
8bbccb3e66 proc: use new extended attribute to resolve concrete type of interfaces
go1.11 adds a new extended attribute to all type DIEs containing the
address of the corresponding runtime._type struct, use this attribute
to find the DIE of the concrete type of interface variables when
available.
2018-06-11 11:09:02 -07:00
aarzilli
42747a0951 proc: correctly set G struct offset for 1.11
The offset of G changed in go1.11 from 0x8a0 to 0x30. See:
https://github.com/golang/go/issues/23617
2018-06-11 11:09:02 -07: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
aarzilli
d85cb61cad tests: call RunTestsWithFixtures everywhere we use BuildFixture
If we don't build artifacts aren't removed after the tests run. Also
add a check to prevent this mistake from reoccuring.
2018-06-08 11:39:47 -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
Sergio Lopez
ed71248f9b proc/test/support: implement EnableDWZCompression
The EnableDWZCompression flag allows tests to request BuildFixture to
run "dwz" on the Fixture's resulting binary to compress/deduplicate its
DWARF sections.
2018-05-23 13:59:45 -07:00
Sergio Lopez
38307f92d4 proc: add support for dwz compressed DWARF
'dwz' is a tool that reduces the size of DWARF sections by
deduplicating symbols. The deduplicated symbols are moved from their
original 'compile unit' to a 'partial unit', which is then referenced
from its original location with an 'imported unit' tag.

In the case of Go binaries, all symbols are located in a single
'compile unit', and the name of each symbol contains a reference to its
package, so 'dwz' is not able to deduplicate them. But still, some C
symbols included in the binary are deduplicated, which also alters the
structure of the DWARF sections, making delve unable to parse them
(crashing in the attempt).

While it would've been possible to simply ignore the C symbols, or
blindly loading all then into BinaryInfo members (packageVars,
Functions...), for correctness sake this change tries to do the right
thing, staging symbols into temporary partialUnit objects, moving them
to BinaryInfo when they are actually requested  by a 'imported unit'
tag.
2018-05-23 13:59:45 -07:00
aarzilli
d2c48bac35 proc/gdbserial: disable foreground if /dev/tty can't be opened
Passing --stdin-path /dev/tty will crash debugserver if /dev/tty can't
be open.

Fixes #1215
2018-05-21 14:32:20 -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
Jay Mundrawala
c7cde8b151 Keep searching for file:line until found
Go seems to be generating multiple compilation units that have
the same file. I think this happens for functions that get inlined.
Without this patch, those inlined functions break the ability to set
a breakpoint at other lines in the file. I was able to load the same
binary in gdb and set a breakpoints throughout the file without issue.

```
➜ objdump --dwarf=decodedline automate-gateway | grep handler/users.go
.../handler/users.go:[++]
s/.../handler/users.go           20            0xb6dd88
.../handler/users.go:[++]
s/.../handler/users.go           20            0xb6e50f
.../handler/users.go:[++]
s/automate-gateway/handler/users.go           32            0xb66640
```

Inlined functions are still a little weird. setting a breakpoint on
a function that gets inlined picks the first occurence. That being
said, I think delve should still do something reasonable for the rest
of the lines in the file.
2018-05-15 15:41:23 -07:00
aarzilli
49d1206b94 proc: do not assume g.gopc is valid
Fixes #1203
2018-05-09 12:33:41 -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
a5574bcdc7 proc: change memCache to delay reading
Change memCache so that the preloaded memory is not read immediately
but only after the actual read to the preloaded range.

This allows us to request caching the entire stack frame every time we
create an eval scope and no unnecessary reads will be made even if the
user is just trying to evaluate a global variable.
2018-04-23 10:13:21 -07:00
Functionary Robot
35622fd667 pkg/proc: fix errors found by vet
Vet found the following errors:
	pkg/proc/moduledata.go:152: namedata[1] (8 bits) too small for shift of 8
	pkg/proc/moduledata.go:170: taglendata[0] (8 bits) too small for shift of 8

The fix is to convert before shifting.
2018-04-19 13:29:13 -07:00
aarzilli
119cb93460 tests: cache and delete fixtures built with non-zero flags 2018-04-19 13:28:11 -07:00
aarzilli
b9c4a1d68c proc: Short circuit evaluation of && and || like go does
Change evaluation of binary operators so that both && and || only
evaluate their second argument conditionally, like go does.
2018-04-13 15:34:03 -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
4f70ff0a77 terminal: make printcontext use SelectedGoroutine
printcontext should use SelectedGoroutine instead of trusting that the
goroutine running on current thread matches the SelectedGoroutine.

When the user switches to a parked goroutine CurrentThread and
SelectedGoroutine will diverge.

Almost all calls to printcontext are safe, they happen after a continue
command returns when SelectedGoroutine and CurrentThread always agree,
but the calls in frameCommand and listCommand are wrong.

Additionally we should stop reporting an error when the debugger is
stopped on an unknown PC address.
2018-04-13 15:31:50 -07:00
aarzilli
4e177bb99a proc/gdbserial,dlv/cmds: make logGdbWire command line configurable 2018-04-13 15:31:02 -07:00
aarzilli
a708d00e78 proc/eval: strings of different length are never equal 2018-04-10 14:46:31 -07:00
aarzilli
2497caea4a proc/native/windows: when there is an error during Attach dbp might be nil 2018-04-10 14:45:46 -07:00
aarzilli
edf4cebe25 proc/gdbserial: disable g command on version 902 of debugserver
The g command is bugged and causes that version of debugserver to
crash. See: https://bugs.llvm.org/show_bug.cgi?id=36968

Fixes #1165
2018-04-10 14:44:58 -07:00
aarzilli
166121d18e gdbserial,debugger: better error message when debugserver not installed
When gdbserial can not find debugserver or lldb-server the error
message is always the same and it complains about lldb-server not being
found.

This is fine on linux (where the backend is unnecessary) but incomplete
on macOS (where the backend is actually used).

Make the error message clearer so that users who do not bother reading
install instructions are not confused.
2018-04-10 13:49:57 -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
aarzilli
84ce278352 proc: allow evaluating constants specified with a partial package path
Fixes #1151
2018-03-20 09:46:35 -07:00
aarzilli
918ab760a4 proc/core: Make TestCoreFpTest less flaky
Registers XMM1 and XMM2 get sometimes clobbered between the time we set
them and the panic. There is no guarantee that they won't in the go
spec so we shouldn't expect any register to keep its value. However
since this seems to only affect 1 and 2 let's try to use 9 and 10
instead.
2018-03-20 09:34:05 -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
cd5203e305 proc: fix reading of empty strings in core files
Every time we read an empty string we accidentally issue a read for 0
bytes at address 0, this is fine for real memory but the core file
reader doesn't like it.

Fixes an issue reported on the mailing list.
2018-03-08 11:58:03 -08:00
aarzilli
6c973bf230 proc/native: move halt to os specific struct
The windows backend isn't using the halt field so it can be removed
there.
On linux it can be replaced with a parameter passed to trapWait.
2018-03-06 09:06:19 -08:00
aarzilli
8561db8c2c proc/native: move Thread.running to os struct
Windows and macOS aren't using this field so move it to the os-specific
thread struct and remove it from everything except linux.
2018-03-06 09:06:19 -08:00
aarzilli
f26bb0b875 proc/native: refactor Halt/setCurrentBreakpoint call pair 2018-03-06 09:06:19 -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
Derek Parker
4da05d62cd pkg/proc: Fix reporting of 'go' statement (#1136)
Fixes a bug where the 'go' statement that created a goroutine was
incorrectly reported.

Fixes #1135
2018-03-05 10:40:42 +01: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