fix some interpreter issues

- `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
JuliaLang · Nov 7, 2017 · 50e8e83 · 50e8e83
1 parent 0ff420c
commit 50e8e83
Show file tree

Hide file tree

Showing 8 changed files with 114 additions and 55 deletions.
diff --git a/base/deprecated.jl b/base/deprecated.jl
@@ -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

diff --git a/base/error.jl b/base/error.jl
@@ -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
@@ -93,6 +79,28 @@ function catch_backtrace()
     ret
 end
 
+"""
+    backtrace()
+
+Get a backtrace object for the current program point.
+"""
+function backtrace()
+    bt, bt2 = ccall(:jl_backtrace_from_here, Any, (Int32,), false)
+    return _reformat_bt(bt, bt2)
+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

diff --git a/base/replutil.jl b/base/replutil.jl
@@ -671,7 +671,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

diff --git a/base/stacktraces.jl b/base/stacktraces.jl
@@ -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 !foundline && isa(expr, LineNumberNode)
             line = expr.line
             file = expr.file
@@ -170,7 +184,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
@@ -252,6 +266,8 @@ 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
@@ -262,7 +278,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
 

diff --git a/src/interpreter-stacktrace.c b/src/interpreter-stacktrace.c
@@ -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"
@@ -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"
@@ -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"
@@ -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"
@@ -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;
 }

diff --git a/src/interpreter.c b/src/interpreter.c
@@ -16,11 +16,13 @@ extern "C" {
 
 typedef struct {
     jl_code_info_t *src; // contains the names and number of slots
+    jl_method_instance_t *mi; // MethodInstance we're executing, or NULL if toplevel
     jl_module_t *module; // context for globals
     jl_value_t **locals; // slots for holding local slots and ssavalues
     jl_svec_t *sparam_vals; // method static parameters, if eval-ing a method body
     size_t ip; // Points to the currently-evaluating statement
     int preevaluation; // use special rules for pre-evaluating expressions
+    int continue_at; // statement index to jump to after leaving exception handler (0 if none)
 } interpreter_state;
 
 #include "interpreter-stacktrace.c"
@@ -53,6 +55,8 @@ SECT_INTERP CALLBACK_ABI void *jl_interpret_toplevel_expr_in_callback(interprete
     s->module = args->m;
     s->sparam_vals = args->sparam_vals;
     s->preevaluation = (s->sparam_vals != NULL);
+    s->continue_at = 0;
+    s->mi = NULL;
 
     JL_TRY {
         ptls->current_task->current_module = ptls->current_module = args->m;
@@ -259,7 +263,7 @@ SECT_INTERP static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
             defined = jl_boundp(modu, (jl_sym_t*)sym);
         }
         else if (jl_is_expr(sym) && ((jl_expr_t*)sym)->head == static_parameter_sym) {
-            ssize_t n = jl_unbox_long(args[0]);
+            ssize_t n = jl_unbox_long(jl_exprarg(sym, 0));
             assert(n > 0);
             if (s->sparam_vals && n <= jl_svec_len(s->sparam_vals)) {
                 jl_value_t *sp = jl_svecref(s->sparam_vals, n - 1);
@@ -544,6 +548,8 @@ SECT_INTERP CALLBACK_ABI void *jl_toplevel_eval_body_callback(interpreter_state
     struct jl_toplevel_eval_body_args *args =
         (struct jl_toplevel_eval_body_args*)vargs;
     s->module = args->m;
+    s->continue_at = 0;
+    s->mi = NULL;
     jl_value_t *ret = eval_body(args->stmts, s, 0, 1);
     jl_get_ptls_states()->world_age = last_age;
     return ret;
@@ -627,6 +633,11 @@ SECT_INTERP static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s
                 if (!jl_setjmp(__eh.eh_ctx,1)) {
                     return eval_body(stmts, s, s->ip + 1, toplevel);
                 }
+                else if (s->continue_at) {
+                    s->ip = s->continue_at;
+                    s->continue_at = 0;
+                    continue;
+                }
                 else {
 #ifdef _OS_WINDOWS_
                     if (jl_get_ptls_states()->exception_in_transit == jl_stackovf_exception)
@@ -638,7 +649,16 @@ SECT_INTERP static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s
             }
             else if (head == leave_sym) {
                 int hand_n_leave = jl_unbox_long(jl_exprarg(stmt,0));
-                jl_pop_handler(hand_n_leave);
+                assert(hand_n_leave > 0);
+                // equivalent to jl_pop_handler(hand_n_leave) :
+                jl_ptls_t ptls = jl_get_ptls_states();
+                jl_handler_t *eh = ptls->current_task->eh;
+                while (--hand_n_leave > 0)
+                    eh = eh->prev;
+                jl_eh_restore_state(eh);
+                // pop jmp_bufs from stack
+                s->continue_at = s->ip + 1;
+                jl_longjmp(eh->eh_ctx, 1);
             }
             else if (toplevel && jl_is_toplevel_only_expr(stmt)) {
                 jl_toplevel_eval(s->module, stmt);
@@ -715,6 +735,8 @@ SECT_INTERP CALLBACK_ABI void *jl_interpret_call_callback(interpreter_state *s,
     s->module = args->lam->def.method->module;
     s->locals = locals + 2;
     s->sparam_vals = args->lam->sparam_vals;
+    s->continue_at = 0;
+    s->mi = args->lam;
     size_t i;
     for (i = 0; i < args->lam->def.method->nargs; i++) {
         if (args->lam->def.method->isva && i == args->lam->def.method->nargs - 1)
@@ -750,6 +772,8 @@ SECT_INTERP CALLBACK_ABI void *jl_interpret_toplevel_thunk_callback(interpreter_
     s->locals = locals;
     s->module = args->m;
     s->sparam_vals = jl_emptysvec;
+    s->continue_at = 0;
+    s->mi = NULL;
     size_t last_age = jl_get_ptls_states()->world_age;
     jl_value_t *r = eval_body(stmts, s, 0, 1);
     jl_get_ptls_states()->world_age = last_age;

diff --git a/src/stackwalk.c b/src/stackwalk.c
@@ -100,12 +100,14 @@ JL_DLLEXPORT jl_value_t *jl_backtrace_from_here(int returnsp)
 {
     jl_array_t *ip = NULL;
     jl_array_t *sp = NULL;
-    JL_GC_PUSH2(&ip, &sp);
+    jl_array_t *bt2 = NULL;
+    JL_GC_PUSH3(&ip, &sp, &bt2);
     if (array_ptr_void_type == NULL) {
         array_ptr_void_type = jl_apply_type2((jl_value_t*)jl_array_type, (jl_value_t*)jl_voidpointer_type, jl_box_long(1));
     }
     ip = jl_alloc_array_1d(array_ptr_void_type, 0);
     sp = returnsp ? jl_alloc_array_1d(array_ptr_void_type, 0) : NULL;
+    bt2 = jl_alloc_array_1d(jl_array_any_type, 0);
     const size_t maxincr = 1000;
     bt_context_t context;
     bt_cursor_t cursor;
@@ -117,13 +119,22 @@ JL_DLLEXPORT jl_value_t *jl_backtrace_from_here(int returnsp)
             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, 0);
+                    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);
+
+        n = 0;
+        while (n < jl_array_len(ip)) {
+            if ((uintptr_t)jl_array_ptr_ref(ip, n) == (uintptr_t)-1) {
+                jl_array_ptr_1d_push(bt2, jl_array_ptr_ref(ip, n+1));
+                n += 2;
+            }
+            n++;
+        }
     }
-    jl_value_t *bt = returnsp ? (jl_value_t*)jl_svec2(ip, sp) : (jl_value_t*)ip;
+    jl_value_t *bt = returnsp ? (jl_value_t*)jl_svec(3, ip, bt2, sp) : (jl_value_t*)jl_svec(2, ip, bt2);
     JL_GC_POP();
     return bt;
 }

diff --git a/test/core.jl b/test/core.jl
@@ -5580,8 +5580,6 @@ let x5 = UnionField5(nothing, Int8(3))
     @test x5 == x5copy
     @test object_id(x5) === object_id(x5copy)
     @test hash(x5) === hash(x5copy)
-    @test pointer_from_objref(x5) === pointer_from_objref(x5)
-    @test pointer_from_objref(x5) !== pointer_from_objref(x5copy)
 end