Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clean up return->call address translation for backtraces #33380

Merged
merged 4 commits into from
Oct 2, 2019
Merged

Clean up return->call address translation for backtraces #33380

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6bc2065
Clean up return->call address translation for StackTraces.lookup / jl…
c42f Sep 24, 2019
8ba936f
Revert API breaking name change lookupat->lookup from #33190
c42f Sep 25, 2019
141ff51
Clear ARMv7 instruction pointer encoding bit for use in caller lookup
c42f Sep 25, 2019
7528266
Improved jl_unw_stepn: skip internal frames, don't adjust ip for sign…
c42f Sep 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions base/client.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ end

# deprecated function--preserved for DocTests.jl
function ip_matches_func(ip, func::Symbol)
ip isa InterpreterIP || (ip -= 1)
for fr in StackTraces.lookupat(ip)
for fr in StackTraces.lookup(ip)
if fr === StackTraces.UNKNOWN || fr.from_c
return false
end
Expand Down
3 changes: 1 addition & 2 deletions base/deprecated.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ function firstcaller(bt::Vector, funcsyms)
# Identify the calling line
found = false
for ip in bt
ip isa Base.InterpreterIP || (ip -= 1) # convert from return stack to call stack (for inlining info)
lkups = StackTraces.lookupat(ip)
lkups = StackTraces.lookup(ip)
for lkup in lkups
if lkup == StackTraces.UNKNOWN || lkup.from_c
continue
Expand Down
15 changes: 12 additions & 3 deletions base/errorshow.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,10 +611,8 @@ function process_backtrace(t::Vector, limit::Int=typemax(Int); skipC = true)
lkups = t[i]
if lkups isa StackFrame
lkups = [lkups]
elseif lkups isa Base.InterpreterIP
lkups = StackTraces.lookupat(lkups)
else
lkups = StackTraces.lookupat(lkups - 1)
lkups = StackTraces.lookup(lkups)
end
for lkup in lkups
if lkup === StackTraces.UNKNOWN
Expand Down Expand Up @@ -661,3 +659,14 @@ function show_exception_stack(io::IO, stack::Vector)
println(io)
end
end

# Defined here rather than error.jl for bootstrap ordering
function show(io::IO, ip::InterpreterIP)
print(io, typeof(ip))
if ip.code isa Core.CodeInfo
print(io, " in top-level CodeInfo at statement $(Int(ip.stmt))")
else
print(io, " in $(ip.code) at statement $(Int(ip.stmt))")
end
end

9 changes: 4 additions & 5 deletions base/stacktraces.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,13 @@ end


"""
lookupat(pointer::Union{Ptr{Cvoid}, UInt}) -> Vector{StackFrame}
ararslan marked this conversation as resolved.
Show resolved Hide resolved
lookup(pointer::Ptr{Cvoid}) -> Vector{StackFrame}

Given a pointer to an execution context (usually generated by a call to `backtrace`), looks
up stack frame context information. Returns an array of frame information for all functions
inlined at that point, innermost function first.
"""
function lookupat(pointer::Ptr{Cvoid})
function lookup(pointer::Ptr{Cvoid})
infos = ccall(:jl_lookup_code_address, Any, (Ptr{Cvoid}, Cint), pointer, false)
pointer = convert(UInt64, pointer)
isempty(infos) && return [StackFrame(empty_sym, empty_sym, -1, nothing, true, false, pointer)] # this is equal to UNKNOWN
Expand All @@ -117,7 +117,7 @@ end

const top_level_scope_sym = Symbol("top-level scope")

function lookupat(ip::Base.InterpreterIP)
function lookup(ip::Base.InterpreterIP)
if ip.code isa Core.MethodInstance && ip.code.def isa Method
codeinfo = ip.code.uninferred
func = ip.code.def.name
Expand Down Expand Up @@ -188,8 +188,7 @@ trace, `stacktrace` first calls `backtrace`.
function stacktrace(trace::Vector{<:Union{Base.InterpreterIP,Ptr{Cvoid}}}, c_funcs::Bool=false)
stack = StackTrace()
for ip in trace
ip isa Base.InterpreterIP || (ip -= 1) # convert from return stack to call stack (for inlining info)
for frame in lookupat(ip)
for frame in lookup(ip)
# Skip frames that come from C calls.
if c_funcs || !frame.from_c
push!(stack, frame)
Expand Down
4 changes: 2 additions & 2 deletions doc/src/base/stacktraces.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ Base.StackTraces.stacktrace
```

The following methods and types in `Base.StackTraces` are not exported and need to be called e.g.
as `StackTraces.lookupat(ptr)`.
as `StackTraces.lookup(ptr)`.

```@docs
Base.StackTraces.lookupat
Base.StackTraces.lookup
Base.StackTraces.remove_frames!
```
4 changes: 2 additions & 2 deletions doc/src/manual/stacktraces.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,12 +300,12 @@ julia> stacktrace(trace, true)
```

Individual pointers returned by [`backtrace`](@ref) can be translated into [`StackTraces.StackFrame`](@ref)
s by passing them into [`StackTraces.lookupat`](@ref):
s by passing them into [`StackTraces.lookup`](@ref):

```julia-repl
julia> pointer = backtrace()[1];

julia> frame = StackTraces.lookupat(pointer - 1)
julia> frame = StackTraces.lookup(pointer)
1-element Array{Base.StackTraces.StackFrame,1}:
jl_apply_generic at gf.c:2167

Expand Down
6 changes: 4 additions & 2 deletions src/interpreter-stacktrace.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,13 +397,14 @@ JL_DLLEXPORT size_t jl_capture_interp_frame(uintptr_t *data, uintptr_t sp, uintp
#else
interpreter_state *s = (interpreter_state *)(sp+TOTAL_STACK_PADDING);
#endif
if (space_remaining <= 1)
int required_space = 3;
if (space_remaining < required_space)
return 0;
// Sentinel value to indicate an interpreter frame
data[0] = JL_BT_INTERP_FRAME;
data[1] = s->mi ? (uintptr_t)s->mi : s->src ? (uintptr_t)s->src : (uintptr_t)jl_nothing;
data[2] = (uintptr_t)s->ip;
return 2;
return required_space;
}

extern void * CALLBACK_ABI enter_interpreter_frame(void * CALLBACK_ABI (*callback)(interpreter_state *, void *), void *arg);
Expand All @@ -420,6 +421,7 @@ JL_DLLEXPORT int jl_is_enter_interpreter_frame(uintptr_t ip)

JL_DLLEXPORT size_t jl_capture_interp_frame(uintptr_t *data, uintptr_t sp, uintptr_t fp, size_t space_remaining)
{
// Leave bt_entry[0] as the native instruction ptr
return 0;
}
#define CALLBACK_ABI
Expand Down
8 changes: 5 additions & 3 deletions src/julia_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,10 +636,12 @@ typedef int bt_cursor_t;
#endif
// Special marker in backtrace data for encoding interpreter frames
#define JL_BT_INTERP_FRAME (((uintptr_t)0)-1)
size_t rec_backtrace(uintptr_t *data, size_t maxsize) JL_NOTSAFEPOINT;
size_t rec_backtrace_ctx(uintptr_t *data, size_t maxsize, bt_context_t *ctx, int add_interp_frames) JL_NOTSAFEPOINT;
// Maximum number of elements of bt_data taken up by interpreter frame
#define JL_BT_MAX_ENTRY_SIZE 3
size_t rec_backtrace(uintptr_t *bt_data, size_t maxsize) JL_NOTSAFEPOINT;
size_t rec_backtrace_ctx(uintptr_t *bt_data, size_t maxsize, bt_context_t *ctx, int add_interp_frames) JL_NOTSAFEPOINT;
#ifdef LIBOSXUNWIND
size_t rec_backtrace_ctx_dwarf(uintptr_t *data, size_t maxsize, bt_context_t *ctx, int add_interp_frames) JL_NOTSAFEPOINT;
size_t rec_backtrace_ctx_dwarf(uintptr_t *bt_data, size_t maxsize, bt_context_t *ctx, int add_interp_frames) JL_NOTSAFEPOINT;
#endif
JL_DLLEXPORT void jl_get_backtrace(jl_array_t **bt, jl_array_t **bt2);
void jl_critical_error(int sig, bt_context_t *context, uintptr_t *bt_data, size_t *bt_size);
Expand Down
150 changes: 104 additions & 46 deletions src/stackwalk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,28 @@ extern "C" {
static int jl_unw_init(bt_cursor_t *cursor, bt_context_t *context) JL_NOTSAFEPOINT;
static int jl_unw_step(bt_cursor_t *cursor, uintptr_t *ip, uintptr_t *sp, uintptr_t *fp) JL_NOTSAFEPOINT;

size_t jl_unw_stepn(bt_cursor_t *cursor, uintptr_t *ip, uintptr_t *sp, size_t maxsize, int add_interp_frames) JL_NOTSAFEPOINT
// Record backtrace entries into bt_data by stepping cursor with jl_unw_step
// until the outermost frame is encountered or the buffer bt_data is (close to)
// full. Native instruction pointers are adjusted to point to the address of
// the call instruction.
//
// `maxsize` is the size of the buffer `bt_data` (and `sp` if non-NULL). It
// must be at least JL_BT_MAX_ENTRY_SIZE to accommodate extended backtrace
// entries. If `sp != NULL`, the stack pointer corresponding `bt_data[i]` is
// stored in `sp[i]`.
//
// jl_unw_stepn will return 1 if there are more frames to come. The number of
// elements of bt_data (and sp if non-NULL) which were used are returned in
// bt_size.
int jl_unw_stepn(bt_cursor_t *cursor, uintptr_t *bt_data, size_t *bt_size,
uintptr_t *sp, size_t maxsize, int add_interp_frames) JL_NOTSAFEPOINT
{
jl_ptls_t ptls = jl_get_ptls_states();
volatile size_t n = 0;
uintptr_t thesp;
uintptr_t thefp;
volatile int need_more_space = 0;
uintptr_t return_ip = 0;
uintptr_t thesp = 0;
uintptr_t thefp = 0;
#if defined(_OS_WINDOWS_) && !defined(_CPU_X86_64_)
assert(!jl_in_stackwalk);
jl_in_stackwalk = 1;
Expand All @@ -44,21 +60,58 @@ size_t jl_unw_stepn(bt_cursor_t *cursor, uintptr_t *ip, uintptr_t *sp, size_t ma
ptls->safe_restore = &buf;
#endif
while (1) {
if (n >= maxsize) {
n = maxsize; // return maxsize + 1 if ran out of space
break;
}
if (!jl_unw_step(cursor, &ip[n], &thesp, &thefp))
break;
if (sp)
sp[n] = thesp;
if (add_interp_frames && jl_is_enter_interpreter_frame(ip[n])) {
n += jl_capture_interp_frame(&ip[n], thesp, thefp, maxsize-n-1) + 1;
} else {
n++;
}
if (n + JL_BT_MAX_ENTRY_SIZE > maxsize) {
// Postpone advancing the cursor: may need more space
need_more_space = 1;
break;
}
int have_more_frames = jl_unw_step(cursor, &return_ip, &thesp, &thefp);
if (sp)
sp[n] = thesp;
// ARM instruction pointer encoding uses the low bit as a flag for
// thumb mode, which must be cleared before further use. (Note not
// needed for ARM AArch64.) See
// https://github.com/libunwind/libunwind/pull/131
#ifdef _CPU_ARM_
return_ip &= ~(uintptr_t)0x1;
#endif
// For the purposes of looking up debug info for functions, we want
// to harvest addresses for the *call* instruction `call_ip` during
// stack walking. However, this information isn't directly
// available. Instead, the stack walk discovers the address
// `return_ip` which would be *returned to* as the stack is
// unwound.
//
// To infer `call_ip` in full generality we would need to
// understand each platform ABI instruction pointer encoding and
// calling conventions, noting that these may vary per stack frame.
// (For example signal frames on linux x86_64 have `call_ip ==
// return_ip`.)
//
// However for our current purposes it seems sufficient to assume
// that `call_ip = return_ip-1`. See also:
//
// * The LLVM unwinder functions step() and setInfoBasedOnIPRegister()
// https://github.com/llvm/llvm-project/blob/master/libunwind/src/UnwindCursor.hpp
// * The way that libunwind handles it in `unw_get_proc_name`:
// https://lists.nongnu.org/archive/html/libunwind-devel/2014-06/msg00025.html
Copy link
Member Author

@c42f c42f Sep 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you can see, I spent a while looking up what people do for this stuff. AFAICT it seems that return_ip - 1 should be sufficient for our use cases. libunwind does something slightly more fancy, but I guess we don't want to trace through signal handler frames. Also libunwind's dwarf.use_prev_instr field is hidden from us behind an opaque pointer in any case (see unw_get_proc_name).

The LLVM unwinder seems to have a special case for ARM in thumb mode and I tried to do the same for our ARMv7 support but I don't know how to test that yet. @yuyichao does the above use of _CPU_ARM_ look right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you can see, I spent a while looking up what people do for this stuff. AFAICT it seems that return_ip - 1 should be sufficient for our use cases. libunwind does something slightly more fancy, but I guess we don't want to trace through signal handler frames. Also libunwind's dwarf.use_prev_instr field is hidden from us behind an opaque pointer in any case (see unw_get_proc_name).

As long as we don't give that correct pointer back to libunwind and don't add the offset when we get the backtrace from the signal frame it should be fine.

does the above use of CPU_ARM look right?

LGTM.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't add the offset when we get the backtrace from the signal frame

Oops, looks like I was doing that part wrong. This should now be cleaned up in the latest commit by avoiding doing the offset for contexts we know have come from a signal handler. I checked that this matches the behavior of libunwind's unw_backtrace() for those contexts on my machine.

uintptr_t call_ip = return_ip - 1;
if (call_ip == JL_BT_INTERP_FRAME) {
// Never leave special marker in the bt data as it can corrupt the GC.
call_ip = 0;
}
uintptr_t *bt_entry = bt_data + n;
size_t entry_sz = 0;
if (add_interp_frames && jl_is_enter_interpreter_frame(call_ip) &&
(entry_sz = jl_capture_interp_frame(bt_entry, thesp, thefp, maxsize-n)) != 0) {
n += entry_sz;
} else {
*bt_entry = call_ip;
n++;
}
if (!have_more_frames)
break;
}
n++;
#if !defined(_OS_WINDOWS_)
}
else {
Expand All @@ -73,26 +126,27 @@ size_t jl_unw_stepn(bt_cursor_t *cursor, uintptr_t *ip, uintptr_t *sp, size_t ma
#if defined(_OS_WINDOWS_) && !defined(_CPU_X86_64_)
jl_in_stackwalk = 0;
#endif
return n;
*bt_size = n;
return need_more_space;
}

size_t rec_backtrace_ctx(uintptr_t *data, size_t maxsize,
size_t rec_backtrace_ctx(uintptr_t *bt_data, size_t maxsize,
bt_context_t *context, int add_interp_frames)
{
size_t n = 0;
size_t bt_size = 0;
bt_cursor_t cursor;
if (!jl_unw_init(&cursor, context))
return 0;
n = jl_unw_stepn(&cursor, data, NULL, maxsize, add_interp_frames);
return n > maxsize ? maxsize : n;
jl_unw_stepn(&cursor, bt_data, &bt_size, NULL, maxsize, add_interp_frames);
return bt_size;
}

size_t rec_backtrace(uintptr_t *data, size_t maxsize)
size_t rec_backtrace(uintptr_t *bt_data, size_t maxsize)
{
bt_context_t context;
memset(&context, 0, sizeof(context));
jl_unw_get(&context);
return rec_backtrace_ctx(data, maxsize, &context, 1);
return rec_backtrace_ctx(bt_data, maxsize, &context, 1);
}

static jl_value_t *array_ptr_void_type JL_ALWAYS_LEAFTYPE = NULL;
Expand All @@ -114,18 +168,27 @@ JL_DLLEXPORT jl_value_t *jl_backtrace_from_here(int returnsp)
memset(&context, 0, sizeof(context));
jl_unw_get(&context);
if (jl_unw_init(&cursor, &context)) {
size_t n = 0, offset = 0;
do {
size_t offset = 0;
while (1) {
jl_array_grow_end(ip, maxincr);
if (returnsp) jl_array_grow_end(sp, maxincr);
n = jl_unw_stepn(&cursor, (uintptr_t*)jl_array_data(ip) + offset,
returnsp ? (uintptr_t*)jl_array_data(sp) + offset : NULL, maxincr, 1);
offset += maxincr;
} while (n > maxincr);
jl_array_del_end(ip, maxincr - n);
if (returnsp) jl_array_del_end(sp, maxincr - n);
uintptr_t *sp_ptr = NULL;
if (returnsp) {
sp_ptr = (uintptr_t*)jl_array_data(sp) + offset;
jl_array_grow_end(sp, maxincr);
}
size_t size_incr = 0;
int need_more_space = jl_unw_stepn(&cursor, (uintptr_t*)jl_array_data(ip) + offset,
&size_incr, sp_ptr, maxincr, 1);
offset += size_incr;
if (!need_more_space) {
jl_array_del_end(ip, jl_array_len(ip) - offset);
if (returnsp)
jl_array_del_end(sp, jl_array_len(sp) - offset);
break;
}
}

n = 0;
size_t n = 0;
while (n < jl_array_len(ip)) {
if ((uintptr_t)jl_array_ptr_ref(ip, n) == JL_BT_INTERP_FRAME) {
jl_array_ptr_1d_push(bt2, jl_array_ptr_ref(ip, n+1));
Expand Down Expand Up @@ -331,9 +394,7 @@ static int jl_unw_step(bt_cursor_t *cursor, uintptr_t *ip, uintptr_t *sp, uintpt
*sp = (uintptr_t)cursor->stackframe.AddrStack.Offset;
if (fp)
*fp = (uintptr_t)cursor->stackframe.AddrFrame.Offset;
if (*ip == 0 || *ip == JL_BT_INTERP_FRAME) {
// don't leave special marker in the bt data as it can corrupt the GC.
*ip = 0;
if (*ip == 0) {
if (!readable_pointer((LPCVOID)*sp))
return 0;
cursor->stackframe.AddrPC.Offset = *(DWORD32*)*sp; // POP EIP (aka RET)
Expand All @@ -349,9 +410,7 @@ static int jl_unw_step(bt_cursor_t *cursor, uintptr_t *ip, uintptr_t *sp, uintpt
*sp = (uintptr_t)cursor->Rsp;
if (fp)
*fp = (uintptr_t)cursor->Rbp;
if (*ip == 0 || *ip == JL_BT_INTERP_FRAME) {
// don't leave special marker in the bt data as it can corrupt the GC.
*ip = 0;
if (*ip == 0) {
if (!readable_pointer((LPCVOID)*sp))
return 0;
cursor->Rip = *(DWORD64*)*sp; // POP RIP (aka RET)
Expand Down Expand Up @@ -404,8 +463,7 @@ static int jl_unw_step(bt_cursor_t *cursor, uintptr_t *ip, uintptr_t *sp, uintpt
unw_word_t reg;
if (unw_get_reg(cursor, UNW_REG_IP, &reg) < 0)
return 0;
// don't leave special marker in the bt data as it can corrupt the GC.
*ip = reg == JL_BT_INTERP_FRAME ? 0 : reg;
*ip = reg;
if (unw_get_reg(cursor, UNW_REG_SP, &reg) < 0)
return 0;
*sp = reg;
Expand All @@ -426,15 +484,15 @@ int jl_unw_init_dwarf(bt_cursor_t *cursor, bt_context_t *uc)
{
return unw_init_local_dwarf(cursor, uc) != 0;
}
size_t rec_backtrace_ctx_dwarf(uintptr_t *data, size_t maxsize,
size_t rec_backtrace_ctx_dwarf(uintptr_t *bt_data, size_t maxsize,
bt_context_t *context, int add_interp_frames)
{
size_t n;
size_t bt_size = 0;
bt_cursor_t cursor;
if (!jl_unw_init_dwarf(&cursor, context))
return 0;
n = jl_unw_stepn(&cursor, data, NULL, maxsize, add_interp_frames);
return n > maxsize ? maxsize : n;
jl_unw_stepn(&cursor, bt_data, &bt_size, NULL, maxsize, add_interp_frames);
return bt_size;
}
#endif

Expand Down
Loading