Fix exc_stack GC by making exc stacks always belong to tasks

Make exc_stack always belong to a task and never to the TLS, to
correctly handle exception stacks as GC'd buffers.  A downside is that
we now allocate on the first throw within any given task. This should
generally be ok, but could loose the root cause of an error if we
coincidentally exhaust memory at the same time.

Also simplify save and restore of exception stack state in codegen and
interpreter by removing mention of this from jl_handler_t, and providing
a runtime function jl_exc_stack_state().

src/cgutils.cpp

@@ -2568,17 +2568,6 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
     }
 }
 
-static Value *emit_pexc_stack_top(jl_codectx_t &ctx)
-{
-    // Load the pointer ptls->exc_stack
-    // Types here are a bit of a lie, but we didn't declare jl_tls_states_t to LLVM.
-    Value *ptls_val = emit_bitcast(ctx, ctx.ptlsStates, PointerType::get(T_psize,0));
-    Constant *offset = ConstantInt::getSigned(T_int32,
-        offsetof(jl_tls_states_t, exc_stack) / sizeof(size_t**));
-    Value *ppexc_stack_top = ctx.builder.CreateInBoundsGEP(ptls_val, offset);
-    return ctx.builder.CreateLoad(ppexc_stack_top, true/*isvolatile*/);
-}
-
 static void emit_signal_fence(jl_codectx_t &ctx)
 {
 #if defined(_CPU_ARM_) || defined(_CPU_AARCH64_)

src/codegen.cpp

@@ -290,6 +290,7 @@ static Function *jlenter_func;
 static Function *jlcurrent_exception_func;
 static Function *jlleave_func;
 static Function *jl_restore_exc_stack_func;
+static Function *jl_exc_stack_state_func;
 static Function *jlegal_func;
 static Function *jl_alloc_obj_func;
 static Function *jl_newbits_func;
@@ -3762,9 +3763,9 @@ static void emit_stmtpos(jl_codectx_t &ctx, jl_value_t *expr, int ssaval_result)
                            ConstantInt::get(T_int32, jl_unbox_long(args[0])));
     }
     else if (head == pop_exc_sym) {
-        jl_cgval_t exc_stack_top = emit_expr(ctx, jl_exprarg(expr, 0));
-        assert(exc_stack_top.V && exc_stack_top.V->getType() == T_size);
-        ctx.builder.CreateCall(prepare_call(jl_restore_exc_stack_func), exc_stack_top.V);
+        jl_cgval_t exc_stack_state = emit_expr(ctx, jl_exprarg(expr, 0));
+        assert(exc_stack_state.V && exc_stack_state.V->getType() == T_size);
+        ctx.builder.CreateCall(prepare_call(jl_restore_exc_stack_func), exc_stack_state.V);
         return;
     }
     else {
@@ -6182,10 +6183,11 @@ static std::unique_ptr<Module> emit_function(
             assert(jl_is_long(args[0]));
             int lname = jl_unbox_long(args[0]);
             // Save exception stack depth at enter for use in pop_exc
-            Value *exc_stack_top = ctx.builder.CreateLoad(emit_pexc_stack_top(ctx),
-                                                          "exc_stack_top");
+
+            Value *exc_stack_state =
+                ctx.builder.CreateCall(prepare_call(jl_exc_stack_state_func));
             assert(!ctx.ssavalue_assigned.at(cursor));
-            ctx.SAvalues.at(cursor) = jl_cgval_t(exc_stack_top, NULL, false,
+            ctx.SAvalues.at(cursor) = jl_cgval_t(exc_stack_state, NULL, false,
                                                  (jl_value_t*)jl_ulong_type, NULL);
             ctx.ssavalue_assigned.at(cursor) = true;
             CallInst *sj = ctx.builder.CreateCall(prepare_call(except_enter_func));
@@ -7139,6 +7141,12 @@ static void init_julia_llvm_env(Module *m)
                          "jl_restore_exc_stack", m);
     add_named_global(jl_restore_exc_stack_func, &jl_restore_exc_stack);
 
+    jl_exc_stack_state_func =
+        Function::Create(FunctionType::get(T_size, false),
+                         Function::ExternalLinkage,
+                         "jl_exc_stack_state", m);
+    add_named_global(jl_exc_stack_state_func, &jl_exc_stack_state);
+
     std::vector<Type *> args_2vals_callee_rooted(0);
     args_2vals_callee_rooted.push_back(PointerType::get(T_jlvalue, AddressSpace::CalleeRooted));
     args_2vals_callee_rooted.push_back(PointerType::get(T_jlvalue, AddressSpace::CalleeRooted));

src/gc.c

@@ -1472,26 +1472,6 @@ STATIC_INLINE int gc_mark_queue_obj(jl_gc_mark_cache_t *gc_cache, gc_mark_sp_t *
     return (int)nptr;
 }
 
-// Mark and queue an exception stack to be scanned.
-void gc_mark_queue_scan_exc_stack(jl_gc_mark_cache_t *gc_cache, gc_mark_sp_t *sp,
-                                  jl_exc_stack_t* exc_stack)
-{
-    if (gc_marked(jl_astaggedvalue(exc_stack)->header)) {
-        return;
-    }
-    // TODO: Should we be testing `if (gc_marked(jl_astaggedvalue(exc_stack)->header))`?
-    // FIXME: How does gc_try_setmark differ from gc_setmark_buf_ ?
-    // What's the difference between a buffer and object?
-    // if (!gc_try_setmark((jl_value_t*)exc_stack, &nptr, &tag, &bits)) {
-    gc_setmark_buf_(jl_get_ptls_states(), exc_stack, 0,
-                    sizeof(exc_stack) + sizeof(uintptr_t)*exc_stack->reserved_size);
-    if (exc_stack->top == 0)
-        return;
-    gc_mark_exc_stack_t data = {exc_stack, exc_stack->top, 0};
-    gc_mark_stack_push(gc_cache, sp, gc_mark_label_addrs[GC_MARK_L_excstack],
-                       &data, sizeof(data), 1);
-}
-
 // Check if `nptr` is tagged for `old + refyoung`,
 // Push the object to the remset and update the `nptr` counter if necessary.
 STATIC_INLINE void gc_mark_push_remset(jl_ptls_t ptls, jl_value_t *obj, uintptr_t nptr) JL_NOTSAFEPOINT
@@ -1720,10 +1700,9 @@ STATIC_INLINE int gc_mark_scan_obj32(jl_ptls_t ptls, gc_mark_sp_t *sp, gc_mark_o
 // (OTOH, the spill will likely make use of the stack engine which is otherwise idle so
 //  the performance impact is minimum as long as it's not in the hottest path)
 
-// There are four external entry points to the loop, corresponding to labels `marked_obj`,
+// There are three external entry points to the loop, corresponding to labels `marked_obj`,
 // `scan_only`, `finlist` and `excstack` (see the corresponding functions
-// `gc_mark_queue_obj`, `gc_mark_queue_scan_obj`, `gc_mark_queue_finlist` and
-// `gc_mark_queue_scan_exc_stack` above).
+// `gc_mark_queue_obj`, `gc_mark_queue_scan_obj` and `gc_mark_queue_finlist` above).
 //
 // The scanning of the object starts with `goto mark`, which updates the metadata and scans
 // the object whose information is stored in `new_obj`, `tag` and `bits`.
@@ -2210,7 +2189,6 @@ mark: {
                                    &stackdata, sizeof(stackdata), 1);
             }
             if (ta->exc_stack) {
-                assert(ta->exc_stack->top > 0); // FIXME?
                 gc_setmark_buf_(ptls, ta->exc_stack, bits, sizeof(jl_exc_stack_t) +
                                 sizeof(uintptr_t)*ta->exc_stack->reserved_size);
                 gc_mark_exc_stack_t stackdata = {ta->exc_stack, ta->exc_stack->top, 0};
@@ -2315,7 +2293,6 @@ static void jl_gc_queue_thread_local(jl_gc_mark_cache_t *gc_cache, gc_mark_sp_t
     gc_mark_queue_obj(gc_cache, sp, ptls2->root_task);
     if (ptls2->previous_exception)
         gc_mark_queue_obj(gc_cache, sp, ptls2->previous_exception);
-    gc_mark_queue_scan_exc_stack(gc_cache, sp, ptls2->exc_stack);
 }
 
 // mark the initial root set

src/interpreter.c

@@ -615,8 +615,8 @@ SECT_INTERP static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s
                     }
                     catch_ip += 1;
                 }
-                // store previous top of exception stack for use by pop_exc.
-                s->locals[jl_source_nslots(s->src) + s->ip] = jl_box_ulong(__eh.exc_stack_top);
+                // store current top of exception stack for restore in pop_exc.
+                s->locals[jl_source_nslots(s->src) + s->ip] = jl_box_ulong(jl_exc_stack_state());
                 if (!jl_setjmp(__eh.eh_ctx,1)) {
                     eval_body(stmts, s, s->ip + 1, toplevel); // returns via continue_at
                 }
@@ -648,10 +648,8 @@ SECT_INTERP static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s
                 jl_longjmp(eh->eh_ctx, 1);
             }
             else if (head == pop_exc_sym) {
-                // note that `__eh->exc_stack_top` may be already overwritten
-                // at this point.
-                size_t prev_stack_top = jl_unbox_ulong(eval_value(jl_exprarg(stmt, 0), s));
-                jl_restore_exc_stack(prev_stack_top);
+                size_t prev_state = jl_unbox_ulong(eval_value(jl_exprarg(stmt, 0), s));
+                jl_restore_exc_stack(prev_state);
             }
             else if (head == const_sym) {
                 jl_sym_t *sym = (jl_sym_t*)jl_exprarg(stmt, 0);

src/jlapi.c

@@ -106,8 +106,8 @@ JL_DLLEXPORT jl_value_t *jl_eval_string(const char *str)
 
 JL_DLLEXPORT jl_value_t *jl_current_exception(void)
 {
-    jl_exc_stack_t *s = jl_get_ptls_states()->exc_stack;
-    return s->top != 0 ? jl_exc_stack_exception(s, s->top) : jl_nothing;
+    jl_exc_stack_t *s = jl_get_ptls_states()->current_task->exc_stack;
+    return s && s->top != 0 ? jl_exc_stack_exception(s, s->top) : jl_nothing;
 }
 
 JL_DLLEXPORT jl_value_t *jl_exception_occurred(void)

src/julia.h

@@ -1589,7 +1589,6 @@ typedef struct _jl_handler_t {
     jl_jmp_buf eh_ctx;
     jl_gcframe_t *gcstack;
     struct _jl_handler_t *prev;
-    size_t exc_stack_top;
     int8_t gc_state;
 #ifdef JULIA_ENABLE_THREADING
     size_t locks_len;
@@ -1624,7 +1623,7 @@ typedef struct _jl_task_t {
     jl_handler_t *eh;
     // saved gc stack top for context switches
     jl_gcframe_t *gcstack;
-    // saved exception stack (NULL if empty)
+    // saved exception stack
     jl_exc_stack_t *exc_stack;
     // current module, or NULL if this task has not set one
     jl_module_t *current_module;
@@ -1654,7 +1653,8 @@ JL_DLLEXPORT void JL_NORETURN jl_no_exc_handler(jl_value_t *e);
 JL_DLLEXPORT void jl_enter_handler(jl_handler_t *eh);
 JL_DLLEXPORT void jl_eh_restore_state(jl_handler_t *eh);
 JL_DLLEXPORT void jl_pop_handler(int n);
-JL_DLLEXPORT void jl_restore_exc_stack(size_t prev_top);
+JL_DLLEXPORT size_t jl_exc_stack_state(void);
+JL_DLLEXPORT void jl_restore_exc_stack(size_t state);
 
 #if defined(_OS_WINDOWS_)
 #if defined(_COMPILER_MINGW_)
@@ -1694,13 +1694,14 @@ extern int had_exception;
 
 #define JL_TRY                                                    \
     int i__tr, i__ca; jl_handler_t __eh;                          \
+    size_t __exc_stack_state = jl_exc_stack_state();              \
     jl_enter_handler(&__eh);                                      \
     if (!jl_setjmp(__eh.eh_ctx,0))                                \
         for (i__tr=1; i__tr; i__tr=0, jl_eh_restore_state(&__eh))
 
 #define JL_CATCH                                                \
     else                                                        \
-        for (i__ca=1, jl_eh_restore_state(&__eh); i__ca; i__ca=0, jl_restore_exc_stack(__eh.exc_stack_top))
+        for (i__ca=1, jl_eh_restore_state(&__eh); i__ca; i__ca=0, jl_restore_exc_stack(__exc_stack_state))
 
 #endif
 

src/julia_threads.h

@@ -110,11 +110,9 @@ struct _jl_tls_states_t {
     int16_t tid;
     // Temp storage for exception thrown in signal handler. Not rooted.
     struct _jl_value_t *sig_exception;
-    // Temporary backtrace buffer.
+    // Temporary backtrace buffer. Scanned for gc roots when bt_size > 0.
     uintptr_t *bt_data; // JL_MAX_BT_SIZE + 1 elements long
     size_t bt_size;    // Size for backtrace in transit in bt_data
-    // Stack of exceptions being handled by task.
-    jl_exc_stack_t *exc_stack;
     // Atomically set by the sender, reset by the handler.
     volatile sig_atomic_t signal_request;
     // Allow the sigint to be raised asynchronously

src/rtutils.c

@@ -219,7 +219,6 @@ JL_DLLEXPORT void jl_enter_handler(jl_handler_t *eh)
     // Must have no safepoint
     eh->prev = current_task->eh;
     eh->gcstack = ptls->pgcstack;
-    eh->exc_stack_top = ptls->exc_stack->top;
 #ifdef JULIA_ENABLE_THREADING
     eh->gc_state = ptls->gc_state;
     eh->locks_len = current_task->locks.len;
@@ -286,11 +285,21 @@ JL_DLLEXPORT void jl_pop_handler(int n)
     jl_eh_restore_state(eh);
 }
 
-JL_DLLEXPORT void jl_restore_exc_stack(size_t prev_top)
+JL_DLLEXPORT size_t jl_exc_stack_state(void)
 {
     jl_ptls_t ptls = jl_get_ptls_states();
-    assert(ptls->exc_stack->top >= prev_top);
-    ptls->exc_stack->top = prev_top;
+    jl_exc_stack_t *s = ptls->current_task->exc_stack;
+    return s ? s->top : 0;
+}
+
+JL_DLLEXPORT void jl_restore_exc_stack(size_t state)
+{
+    jl_ptls_t ptls = jl_get_ptls_states();
+    jl_exc_stack_t *s = ptls->current_task->exc_stack;
+    if (s) {
+        assert(s->top >= state);
+        s->top = state;
+    }
 }
 
 JL_DLLEXPORT jl_value_t *jl_apply_with_saved_exception_state(jl_value_t **args, uint32_t nargs, int drop_exceptions)
@@ -326,17 +335,17 @@ void jl_reserve_exc_stack(jl_exc_stack_t **stack, size_t reserved_size)
     size_t bufsz = sizeof(jl_exc_stack_t) + sizeof(uintptr_t)*reserved_size;
     jl_exc_stack_t *new_s = (jl_exc_stack_t*)jl_gc_alloc_buf(jl_get_ptls_states(), bufsz);
     new_s->top = 0;
+    new_s->reserved_size = reserved_size;
     if (s)
         jl_copy_exc_stack(new_s, s);
-    new_s->reserved_size = reserved_size;
     *stack = new_s;
 }
 
 void jl_push_exc_stack(jl_exc_stack_t **stack, jl_value_t *exception,
                        uintptr_t *bt_data, size_t bt_size)
 {
+    jl_reserve_exc_stack(stack, (*stack ? (*stack)->top : 0) + bt_size + 2);
     jl_exc_stack_t *s = *stack;
-    jl_reserve_exc_stack(stack, (s ? s->top : 0) + bt_size + 2);
     memcpy(jl_excstk_raw(s) + s->top, bt_data, sizeof(uintptr_t)*bt_size);
     s->top += bt_size + 2;
     jl_excstk_raw(s)[s->top-2] = bt_size;

src/stackwalk.c

@@ -167,10 +167,10 @@ void decode_backtrace(uintptr_t *bt_data, size_t bt_size,
 
 JL_DLLEXPORT void jl_get_backtrace(jl_array_t **btout, jl_array_t **bt2out)
 {
-    jl_exc_stack_t *s = jl_get_ptls_states()->exc_stack;
+    jl_exc_stack_t *s = jl_get_ptls_states()->current_task->exc_stack;
     uintptr_t *bt_data = NULL;
     size_t bt_size = 0;
-    if (s->top) {
+    if (s && s->top) {
         bt_data = jl_exc_stack_bt_data(s, s->top);
         bt_size = jl_exc_stack_bt_size(s, s->top);
     }
@@ -187,7 +187,9 @@ JL_DLLEXPORT jl_value_t *jl_get_exc_stack(int include_bt, int max_entries)
     jl_array_t *bt2 = NULL;
     JL_GC_PUSH3(&stack, &bt, &bt2);
     stack = jl_alloc_array_1d(jl_array_any_type, 0);
-    jl_exc_stack_t *s = jl_get_ptls_states()->exc_stack;
+    jl_exc_stack_t *s = jl_get_ptls_states()->current_task->exc_stack;
+    if (!s)
+        return (jl_value_t*)stack;
     size_t itr = s->top;
     int i = 0;
     while (itr > 0 && i < max_entries) {
@@ -514,7 +516,9 @@ JL_DLLEXPORT void jl_gdblookup(uintptr_t ip)
 
 JL_DLLEXPORT void jlbacktrace(void)
 {
-    jl_exc_stack_t *s = jl_get_ptls_states()->exc_stack;
+    jl_exc_stack_t *s = jl_get_ptls_states()->current_task->exc_stack;
+    if (!s)
+        return;
     size_t bt_size = jl_exc_stack_bt_size(s, s->top);
     uintptr_t *bt_data = jl_exc_stack_bt_data(s, s->top);
     for (size_t i = 0; i < bt_size; i++)

src/task.c

@@ -252,7 +252,6 @@ static void NOINLINE JL_NORETURN JL_USED_FUNC start_task(void)
     t->started = 1;
     if (t->exception != jl_nothing) {
         size_t bt_size = rec_backtrace(ptls->bt_data, JL_MAX_BT_SIZE);
-        jl_reserve_exc_stack(&t->exc_stack, bt_size+2);
         jl_push_exc_stack(&t->exc_stack, t->exception, ptls->bt_data, bt_size);
         res = t->exception;
     }
@@ -267,11 +266,12 @@ static void NOINLINE JL_NORETURN JL_USED_FUNC start_task(void)
             res = jl_apply(&t->start, 1);
         }
         JL_CATCH {
-            // TODO: Persist exception stack as above?
             res = jl_current_exception();
             t->exception = res;
             jl_gc_wb(t, res);
+            goto skip_pop_exc;
         }
+skip_pop_exc:;
     }
     finish_task(t, res);
     gc_debug_critical_error();
@@ -314,30 +314,11 @@ static void ctx_switch(jl_ptls_t ptls, jl_task_t **pt)
     if (!jl_setjmp(ptls->current_task->ctx, 0)) {
         jl_task_t *lastt = ptls->current_task;
 
-        // Swap exception stacks. Note that the state must be left consistent
-        // if allocations throw, so we reserve space before doing anything else.
-        if (ptls->exc_stack->top) {
-            // May throw
-            jl_reserve_exc_stack(&lastt->exc_stack, ptls->exc_stack->top);
-        }
-        if (t->exc_stack && t->exc_stack->top) {
-            // Only necessary if t->exc_stack was run on a different thread.
-            jl_reserve_exc_stack(&ptls->exc_stack, t->exc_stack->top);
-        }
-
 #ifdef COPY_STACKS
         save_stack(ptls, lastt, pt); // allocates (gc-safepoint, and can also fail)
 #else
         *pt = lastt; // can't fail after here: clear the gc-root for the target task now
 #endif
-        // Cannot throw.
-        if (ptls->exc_stack->top) {
-            jl_copy_exc_stack(lastt->exc_stack, ptls->exc_stack);
-        }
-        if (t->exc_stack && t->exc_stack->top) {
-            jl_copy_exc_stack(ptls->exc_stack, t->exc_stack);
-            t->exc_stack->top = 0;
-        }
 
         // set up global state for new task
         lastt->gcstack = ptls->pgcstack;
@@ -567,12 +548,16 @@ void JL_NORETURN throw_internal(jl_value_t *exception JL_MAYBE_UNROOTED,
     ptls->io_wait = 0;
     if (ptls->safe_restore)
         jl_longjmp(*ptls->safe_restore, 1);
+    jl_gc_unsafe_enter(ptls);
     if (exception) {
         // Persist exception in transit before a new one can be generated.
-        jl_push_exc_stack(&ptls->exc_stack, exception, bt_data, bt_size);
+        JL_GC_PUSH1(&exception);
+        // May allocate. FIXME: Ensure bt_data is rooted.
+        assert(ptls->current_task);
+        jl_push_exc_stack(&ptls->current_task->exc_stack, exception, bt_data, bt_size);
+        JL_GC_POP();
     }
-    assert(ptls->exc_stack->top);
-    jl_gc_unsafe_enter(ptls);
+    assert(ptls->current_task->exc_stack && ptls->current_task->exc_stack->top);
     jl_handler_t *eh = ptls->current_task->eh;
     if (eh != NULL) {
 #ifdef ENABLE_TIMINGS
@@ -612,17 +597,18 @@ JL_DLLEXPORT void jl_sig_throw(void)
     jl_ptls_t ptls = jl_get_ptls_states();
     jl_value_t *e = ptls->sig_exception;
     assert(e);
+    ptls->sig_exception = NULL;
     throw_internal(e, ptls->bt_data, ptls->bt_size);
 }
 
 JL_DLLEXPORT void jl_rethrow_other(jl_value_t *e)
 {
     // TODO: Uses of this function could be replaced with jl_throw
     // if we rely on exc_stack for root cause analysis.
-    jl_ptls_t ptls = jl_get_ptls_states();
-    assert(ptls->exc_stack->top != 0);
+    jl_exc_stack_t *exc_stack = jl_get_ptls_states()->current_task->exc_stack;
+    assert(exc_stack && exc_stack->top != 0);
     // overwrite exception on top of stack. see jl_exc_stack_exception
-    jl_excstk_raw(ptls->exc_stack)[ptls->exc_stack->top-1] = (uintptr_t)e;
+    jl_excstk_raw(exc_stack)[exc_stack->top-1] = (uintptr_t)e;
     jl_rethrow();
 }