Skip to content

Commit

Permalink
fix some interpreter issues
Browse files Browse the repository at this point in the history
- `isdefined` on static parameter
- stack overflow when interpreting a block with many exception
  handlers (as in some tests) due to never popping jmp_bufs
- fix backtraces for non-top-level interpreted frames
- remove invalid `pointer_from_objref(immutable)` test, where
  interpreter and compiler had different boxing behavior
  • Loading branch information
JeffBezanson committed Nov 9, 2017
1 parent 5f45c59 commit 4656ccd
Show file tree
Hide file tree
Showing 12 changed files with 163 additions and 81 deletions.
4 changes: 2 additions & 2 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ function _depwarn(msg, opts, bt, caller)
end
end

firstcaller(bt::Array{Ptr{Void},1}, funcsym::Symbol) = firstcaller(bt, (funcsym,))
function firstcaller(bt::Array{Ptr{Void},1}, funcsyms)
firstcaller(bt::Vector, funcsym::Symbol) = firstcaller(bt, (funcsym,))
function firstcaller(bt::Vector, funcsyms)
# Identify the calling line
found = false
lkup = StackTraces.UNKNOWN
Expand Down
42 changes: 21 additions & 21 deletions base/error.jl
Original file line number Diff line number Diff line change
Expand Up @@ -53,36 +53,22 @@ the current exception (if called within a `catch` block).
rethrow() = ccall(:jl_rethrow, Bottom, ())
rethrow(e) = ccall(:jl_rethrow_other, Bottom, (Any,), e)

"""
backtrace()
Get a backtrace object for the current program point.
"""
backtrace() = ccall(:jl_backtrace_from_here, Array{Ptr{Void},1}, (Int32,), false)

struct InterpreterIP
code::CodeInfo
code::Union{CodeInfo,Core.MethodInstance,Void}
stmt::Csize_t
end

"""
catch_backtrace()
Get the backtrace of the current exception, for use within `catch` blocks.
"""
function catch_backtrace()
bt = Ref{Any}(nothing)
bt2 = Ref{Any}(nothing)
ccall(:jl_get_backtrace, Void, (Ref{Any}, Ref{Any}), bt, bt2)
# convert dual arrays (ips, interpreter_frames) to a single array of locations
function _reformat_bt(bt, bt2)
ret = Array{Union{InterpreterIP,Ptr{Void}},1}()
i, j = 1, 1
while i <= length(bt[])
ip = bt[][i]::Ptr{Void}
while i <= length(bt)
ip = bt[i]::Ptr{Void}
if ip == Ptr{Void}(-1%UInt)
# The next one is really a CodeInfo
push!(ret, InterpreterIP(
bt2[][j],
bt[][i+2]))
bt2[j],
bt[i+2]))
j += 1
i += 3
else
Expand All @@ -93,6 +79,20 @@ function catch_backtrace()
ret
end

function backtrace end

"""
catch_backtrace()
Get the backtrace of the current exception, for use within `catch` blocks.
"""
function catch_backtrace()
bt = Ref{Any}(nothing)
bt2 = Ref{Any}(nothing)
ccall(:jl_get_backtrace, Void, (Ref{Any}, Ref{Any}), bt, bt2)
return _reformat_bt(bt[], bt2[])
end

## keyword arg lowering generates calls to this ##
function kwerr(kw, args::Vararg{Any,N}) where {N}
@_noinline_meta
Expand Down
23 changes: 16 additions & 7 deletions base/replutil.jl
Original file line number Diff line number Diff line change
Expand Up @@ -635,17 +635,26 @@ end
global LAST_SHOWN_LINE_INFOS = Tuple{String, Int}[]

function show_backtrace(io::IO, t::Vector)
n_frames = 0
frame_counter = 0
resize!(LAST_SHOWN_LINE_INFOS, 0)
process_backtrace((a,b) -> n_frames += 1, t)
n_frames != 0 && print(io, "\nStacktrace:")
process_entry = (last_frame, n) -> begin
filtered = Any[]
process_backtrace((fr, count) -> push!(filtered, (fr, count)), t)
isempty(filtered) && return

if length(filtered) == 1 && StackTraces.is_top_level_frame(filtered[1][1])
f = filtered[1][1]
if f.line == 0 && f.file == Symbol("")
# don't show a single top-level frame with no location info
return
end
end

print(io, "\nStacktrace:")
frame_counter = 0
for (last_frame, n) in filtered
frame_counter += 1
show_trace_entry(IOContext(io, :backtrace => true), last_frame, n, prefix = string(" [", frame_counter, "] "))
push!(LAST_SHOWN_LINE_INFOS, (string(last_frame.file), last_frame.line))
end
process_backtrace(process_entry, t)
end

function show_backtrace(io::IO, t::Vector{Any})
Expand All @@ -671,7 +680,7 @@ function process_backtrace(process_func::Function, t::Vector, limit::Int=typemax
count += 1
if count > limit; break; end

if lkup.file != last_frame.file || lkup.line != last_frame.line || lkup.func != last_frame.func
if lkup.file != last_frame.file || lkup.line != last_frame.line || lkup.func != last_frame.func || lkup.linfo !== lkup.linfo
if n > 0
process_func(last_frame, n)
end
Expand Down
59 changes: 53 additions & 6 deletions base/stacktraces.jl
Original file line number Diff line number Diff line change
Expand Up @@ -139,16 +139,30 @@ end

lookup(pointer::UInt) = lookup(convert(Ptr{Void}, pointer))

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

using Base.Meta
is_loc_meta(expr, kind) = isexpr(expr, :meta) && length(expr.args) >= 1 && expr.args[1] === kind
function lookup(ip::Base.InterpreterIP)
i = ip.stmt
foundline = false
func = empty_sym
file = empty_sym
line = 0
if ip.code isa Core.MethodInstance
codeinfo = ip.code.inferred
func = ip.code.def.name
file = ip.code.def.file
line = ip.code.def.line
elseif ip.code === nothing
# interpreted top-level expression with no CodeInfo
return [StackFrame(top_level_scope_sym, empty_sym, 0, nothing, false, false, 0)]
else
assert(ip.code isa CodeInfo)
codeinfo = ip.code
func = top_level_scope_sym
file = empty_sym
line = 0
end
while i >= 1
expr = ip.code.code[i]
expr = codeinfo.code[i]
if isa(expr, LineNumberNode)
if line == 0
line = expr.line
Expand All @@ -174,7 +188,7 @@ function lookup(ip::Base.InterpreterIP)
npops = 1
while npops >= 1
i -= 1
expr = ip.code.code[i]
expr = codeinfo.code[i]
is_loc_meta(expr, :pop_loc) && (npops += 1)
is_loc_meta(expr, :push_loc) && (npops -= 1)
end
Expand All @@ -188,6 +202,35 @@ end
lookup(s::StackFrame) = StackFrame[s]
lookup(s::Tuple{StackFrame,Int}) = StackFrame[s[1]]

"""
backtrace()
Get a backtrace object for the current program point.
"""
function Base.backtrace()
bt, bt2 = ccall(:jl_backtrace_from_here, Any, (Int32,), false)
if length(bt) > 2
# remove frames for jl_backtrace_from_here and backtrace()
if bt[2] == Ptr{Void}(-1%UInt)
# backtrace() is interpreted
# Note: win32 is missing the top frame (see https://bugs.chromium.org/p/crashpad/issues/detail?id=53)
@static if Base.Sys.iswindows() && Int === Int32
deleteat!(bt, 1:2)
else
deleteat!(bt, 1:3)
end
unshift!(bt2)
else
@static if Base.Sys.iswindows() && Int === Int32
deleteat!(bt, 1)
else
deleteat!(bt, 1:2)
end
end
end
return Base._reformat_bt(bt, bt2)
end

"""
stacktrace([trace::Vector{Ptr{Void}},] [c_funcs::Bool=false]) -> StackTrace
Expand Down Expand Up @@ -252,10 +295,14 @@ function remove_frames!(stack::StackTrace, m::Module)
return stack
end

is_top_level_frame(f::StackFrame) = f.linfo isa CodeInfo || (f.linfo === nothing && f.func === top_level_scope_sym)

function show_spec_linfo(io::IO, frame::StackFrame)
if frame.linfo == nothing
if frame.func === empty_sym
@printf(io, "ip:%#x", frame.pointer)
elseif frame.func === top_level_scope_sym
print(io, "top-level scope")
else
print_with_color(Base.have_color && get(io, :backtrace, false) ? Base.stackframe_function_color() : :nothing, io, string(frame.func))
end
Expand All @@ -266,7 +313,7 @@ function show_spec_linfo(io::IO, frame::StackFrame)
Base.show(io, frame.linfo)
end
elseif frame.linfo isa CodeInfo
print(io, "In toplevel scope")
print(io, "top-level scope")
end
end

Expand Down
3 changes: 1 addition & 2 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ jl_sym_t *enter_sym; jl_sym_t *leave_sym;
jl_sym_t *exc_sym; jl_sym_t *error_sym;
jl_sym_t *new_sym; jl_sym_t *using_sym;
jl_sym_t *const_sym; jl_sym_t *thunk_sym;
jl_sym_t *anonymous_sym; jl_sym_t *underscore_sym;
jl_sym_t *underscore_sym;
jl_sym_t *abstracttype_sym; jl_sym_t *primtype_sym;
jl_sym_t *structtype_sym; jl_sym_t *foreigncall_sym;
jl_sym_t *global_sym; jl_sym_t *list_sym;
Expand Down Expand Up @@ -312,7 +312,6 @@ void jl_init_frontend(void)
const_sym = jl_symbol("const");
global_sym = jl_symbol("global");
thunk_sym = jl_symbol("thunk");
anonymous_sym = jl_symbol("anonymous");
underscore_sym = jl_symbol("_");
amp_sym = jl_symbol("&");
abstracttype_sym = jl_symbol("abstract_type");
Expand Down
11 changes: 8 additions & 3 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,11 @@ static std::unique_ptr<Module> emit_function(
const jl_cgparams_t *params);
void jl_add_linfo_in_flight(StringRef name, jl_method_instance_t *linfo, const DataLayout &DL);

const char *name_from_method_instance(jl_method_instance_t *li)
{
return jl_is_method(li->def.method) ? jl_symbol_name(li->def.method->name) : "top-level scope";
}

// this generates llvm code for the lambda info
// and adds the result to the jitlayers
// (and the shadow module), but doesn't yet compile
Expand Down Expand Up @@ -1154,7 +1159,7 @@ jl_llvm_functions_t jl_compile_linfo(jl_method_instance_t **pli, jl_code_info_t
li->functionObjectsDecls.specFunctionObject = NULL;
nested_compile = last_n_c;
JL_UNLOCK(&codegen_lock); // Might GC
const char *mname = jl_symbol_name(jl_is_method(li->def.method) ? li->def.method->name : anonymous_sym);
const char *mname = name_from_method_instance(li);
jl_rethrow_with_add("error compiling %s", mname);
}
const char *f = decls.functionObject;
Expand Down Expand Up @@ -1496,7 +1501,7 @@ void *jl_get_llvmf_defn(jl_method_instance_t *linfo, size_t world, bool getwrapp
// something failed!
nested_compile = last_n_c;
JL_UNLOCK(&codegen_lock); // Might GC
const char *mname = jl_symbol_name(jl_is_method(linfo->def.method) ? linfo->def.method->name : anonymous_sym);
const char *mname = name_from_method_instance(linfo);
jl_rethrow_with_add("error compiling %s", mname);
}
// Restore the previous compile context
Expand Down Expand Up @@ -4848,7 +4853,7 @@ static std::unique_ptr<Module> emit_function(
ctx.linfo = lam;
ctx.source = src;
ctx.world = world;
ctx.name = jl_symbol_name(jl_is_method(lam->def.method) ? lam->def.method->name : anonymous_sym);
ctx.name = name_from_method_instance(lam);
ctx.funcName = ctx.name;
ctx.params = params;
ctx.spvals_ptr = NULL;
Expand Down
38 changes: 20 additions & 18 deletions src/interpreter-stacktrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,30 +59,32 @@ uintptr_t __stop_jl_interpreter_frame = (uintptr_t)&__stop_jl_interpreter_frame_
// stack frames.
#ifdef _CPU_X86_64_

#define STR(x) #x
#define XSTR(x) STR(x)

#ifdef _OS_WINDOWS_
size_t STACK_PADDING = 40;
#define STACK_PADDING 32
#else
size_t STACK_PADDING = 8;
#define STACK_PADDING 0
#endif

asm(
ASM_ENTRY
MANGLE("enter_interpreter_frame") ":\n"
".cfi_startproc\n"
// sizeof(struct interpreter_state) is 44, but we need to be 8 byte aligned,
// so subtract 48. For compact unwind info, we need to only have one subq,
// so combine in the stack realignment for a total of 56 bytes.
// Note: make sure stack is 8-byte aligned here even though
// sizeof(struct interpreter_state) might not be.
"\tsubq $56, %rsp\n"
".cfi_def_cfa_offset 64\n"
#ifdef _OS_WINDOWS_
"\tmovq %rcx, %rax\n"
"\tleaq 8(%rsp), %rcx\n"
"\tleaq " XSTR(STACK_PADDING) "(%rsp), %rcx\n"
#else
"\tmovq %rdi, %rax\n"
"\tleaq 8(%rsp), %rdi\n"
"\tleaq " XSTR(STACK_PADDING) "(%rsp), %rdi\n"
#endif
// Zero out the src field
"\tmovq $0, 8(%rsp)\n"
"\tmovq $0, 0(%rdi)\n"
#ifdef _OS_WINDOWS_
// Make space for the register parameter area
"\tsubq $32, %rsp\n"
Expand All @@ -105,11 +107,11 @@ asm(
);

#define CALLBACK_ABI
static_assert(sizeof(interpreter_state) <= 48, "Update assembly code above");
static_assert(sizeof(interpreter_state) <= 56, "Update assembly code above");

#elif defined(_CPU_X86_)

size_t STACK_PADDING = 12;
#define STACK_PADDING 12
asm(
ASM_ENTRY
MANGLE("enter_interpreter_frame") ":\n"
Expand Down Expand Up @@ -148,11 +150,11 @@ asm(
"\tmovl %esp, %ebp\n"
#endif
// sizeof(struct interpreter_state) is 32
"\tsubl $32, %esp\n"
"\tsubl $36, %esp\n"
#ifdef _OS_WINDOWS_
".cfi_def_cfa_offset 40\n"
".cfi_def_cfa_offset 44\n"
#else
".cfi_def_cfa_offset 36\n"
".cfi_def_cfa_offset 40\n"
#endif
"\tmovl %ecx, %eax\n"
"\tmovl %esp, %ecx\n"
Expand All @@ -166,16 +168,16 @@ asm(
#else
"\tsubl $12, %esp\n"
#endif
".cfi_def_cfa_offset 48\n"
".cfi_def_cfa_offset 52\n"
"Lenter_interpreter_frame_start_val:\n"
"\tcalll *%eax\n"
"Lenter_interpreter_frame_end_val:\n"
#ifdef _OS_WINDOWS_
"\taddl $40, %esp\n"
"\taddl $44, %esp\n"
".cfi_def_cfa_offset 8\n"
"\tpopl %ebp\n"
#else
"\taddl $44, %esp\n"
"\taddl $48, %esp\n"
#endif
".cfi_def_cfa_offset 4\n"
"\tret\n"
Expand Down Expand Up @@ -214,11 +216,11 @@ JL_DLLEXPORT size_t jl_capture_interp_frame(uintptr_t *data, uintptr_t sp, uintp
#else
interpreter_state *s = (interpreter_state *)(sp+STACK_PADDING);
#endif
if (space_remaining <= 1 || s->src == 0)
if (space_remaining <= 1)
return 0;
// Sentinel value to indicate an interpreter frame
data[0] = (uintptr_t)-1;
data[1] = (uintptr_t)s->src;
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;
}
Expand Down
Loading

0 comments on commit 4656ccd

Please sign in to comment.