Two fixes for GC rooting / scanning of backtrace data

* Exception stack scanning was busted on on 32 bit systems due to an
  errant uint64_t. Duh!

* Ensure that ptls->bt_data is always scanned for roots when bt_size is
  nonzero, so we can't loose references while allocating size for the
  exception stack.

Also a cleanup of an unnecessary backtrace when passing exceptions
between tasks.

src/gc.c

@@ -1901,7 +1901,7 @@ excstack: {
         size_t i = stackitr->i;
         while (itr > 0) {
             size_t bt_size = jl_exc_stack_bt_size(exc_stack, itr);
-            uint64_t *bt_data = jl_exc_stack_bt_data(exc_stack, itr);
+            uintptr_t *bt_data = jl_exc_stack_bt_data(exc_stack, itr);
             while (i+2 < bt_size) {
                 if (bt_data[i] != (uintptr_t)-1) {
                     i++;
@@ -2472,6 +2472,18 @@ static void jl_gc_queue_remset(jl_gc_mark_cache_t *gc_cache, gc_mark_sp_t *sp, j
     ptls2->heap.rem_bindings.len = n_bnd_refyoung;
 }
 
+static void jl_gc_queue_bt_buf(jl_gc_mark_cache_t *gc_cache, gc_mark_sp_t *sp, jl_ptls_t ptls2)
+{
+    size_t n = 0;
+    while (n+2 < ptls2->bt_size) {
+        if (ptls2->bt_data[n] == (uintptr_t)-1) {
+            gc_mark_queue_obj(gc_cache, sp, (jl_value_t*)ptls2->bt_data[n+1]);
+            n += 2;
+        }
+        n++;
+    }
+}
+
 // Only one thread should be running in this function
 static int _jl_gc_collect(jl_ptls_t ptls, int full)
 {
@@ -2492,6 +2504,8 @@ static int _jl_gc_collect(jl_ptls_t ptls, int full)
         jl_gc_queue_remset(gc_cache, &sp, ptls2);
         // 2.2. mark every thread local root
         jl_gc_queue_thread_local(gc_cache, &sp, ptls2);
+        // 2.3. mark any managed objects in the backtrace buffer
+        jl_gc_queue_bt_buf(gc_cache, &sp, ptls2);
     }
 
     // 3. walk roots

src/julia_internal.h

@@ -668,8 +668,8 @@ JL_DLLEXPORT size_t jl_capture_interp_frame(uintptr_t *data, uintptr_t sp, uintp
 typedef struct _jl_exc_stack_t {
     size_t top;
     size_t reserved_size;
-    // Pack all stack entries into a buffer to eliminate allocation during
-    // exception handling, at least in the common case.
+    // Pack all stack entries into a growable buffer to amortize allocation
+    // across repeated exception handling.
     // uintptr_t data[]; // Access with jl_excstk_raw
 #define jl_excstk_raw(stack) ((uintptr_t*)((char*)(stack) + sizeof(jl_exc_stack_t)))
 } jl_exc_stack_t;

src/task.c

@@ -251,8 +251,6 @@ static void NOINLINE JL_NORETURN JL_USED_FUNC start_task(void)
     jl_value_t *res;
     t->started = 1;
     if (t->exception != jl_nothing) {
-        size_t bt_size = rec_backtrace(ptls->bt_data, JL_MAX_BT_SIZE);
-        jl_push_exc_stack(&t->exc_stack, t->exception, ptls->bt_data, bt_size);
         res = t->exception;
     }
     else {
@@ -541,20 +539,22 @@ JL_DLLEXPORT JL_NORETURN void jl_no_exc_handler(jl_value_t *e) JL_NOTSAFEPOINT
 }
 
 // yield to exception handler
-void JL_NORETURN throw_internal(jl_value_t *exception JL_MAYBE_UNROOTED,
-                                uintptr_t* bt_data, size_t bt_size)
+void JL_NORETURN throw_internal(jl_value_t *exception JL_MAYBE_UNROOTED)
 {
     jl_ptls_t ptls = jl_get_ptls_states();
     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.
+        // The temporary ptls->bt_data is rooted by special purpose code in the
+        // GC. This exists only for the purpose of preserving bt_data until we
+        // set ptls->bt_size=0 below.
         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_push_exc_stack(&ptls->current_task->exc_stack, exception,
+                          ptls->bt_data, ptls->bt_size);
+        ptls->bt_size = 0;
         JL_GC_POP();
     }
     assert(ptls->current_task->exc_stack && ptls->current_task->exc_stack->top);
@@ -581,9 +581,9 @@ JL_DLLEXPORT void jl_throw(jl_value_t *e)
     jl_ptls_t ptls = jl_get_ptls_states();
     assert(e != NULL);
     if (ptls->safe_restore)
-        throw_internal(NULL, NULL, 0);
-    size_t bt_size = rec_backtrace(ptls->bt_data, JL_MAX_BT_SIZE);
-    throw_internal(e, ptls->bt_data, bt_size);
+        throw_internal(NULL);
+    ptls->bt_size = rec_backtrace(ptls->bt_data, JL_MAX_BT_SIZE);
+    throw_internal(e);
 }
 
 // rethrow with current exc_stack state
@@ -592,28 +592,31 @@ JL_DLLEXPORT void jl_rethrow(void)
     jl_exc_stack_t *exc_stack = jl_get_ptls_states()->current_task->exc_stack;
     if (!exc_stack || exc_stack->top == 0)
         jl_error("rethrow() not allowed outside a catch block");
-    throw_internal(NULL, NULL, 0);
+    throw_internal(NULL);
 }
 
+// Special case throw for errors detected inside signal handlers.  This is not
+// (cannot be) called directly in the signal handler itself, but is returned to
+// after the signal handler exits.
 JL_DLLEXPORT void jl_sig_throw(void)
 {
     jl_ptls_t ptls = jl_get_ptls_states();
     jl_value_t *e = ptls->sig_exception;
-    assert(e);
+    assert(e && ptls->bt_size != 0);
     ptls->sig_exception = NULL;
-    throw_internal(e, ptls->bt_data, ptls->bt_size);
+    throw_internal(e);
 }
 
 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.
+    // TODO: Should uses of `rethrow(exc)` be replaced with a normal throw, now
+    // that exception stacks allow root cause analysis?
     jl_exc_stack_t *exc_stack = jl_get_ptls_states()->current_task->exc_stack;
     if (!exc_stack || exc_stack->top == 0)
         jl_error("rethrow(exc) not allowed outside a catch block");
     // overwrite exception on top of stack. see jl_exc_stack_exception
     jl_excstk_raw(exc_stack)[exc_stack->top-1] = (uintptr_t)e;
-    jl_rethrow();
+    throw_internal(NULL);
 }
 
 JL_DLLEXPORT jl_task_t *jl_new_task(jl_function_t *start, size_t ssize)

test/exceptions.jl

@@ -116,8 +116,6 @@ end
     @test length(Base.catch_stack()) == 0
     test_exc_stack_catch_break()
     try
-        # FIXME!! Running this with n=1000 aborts with a memory error.
-        # Likely due to the GC changes.
         test_exc_stack_deep(100)
     catch
         @test length(Base.catch_stack()) == 100