From 197180d8589ad14fc4bc4c23782b76739c4ec5a4 Mon Sep 17 00:00:00 2001 From: Yi Lin Date: Thu, 31 Aug 2023 02:03:28 +1200 Subject: [PATCH] Add GC write barrier in jl_reserve_excstack (#51096) `jl_push_excstack` and `jl_reserve_excstack` take an address to the field `excstack` in `jl_task_t`, and may update the field. In such a case, a GC write barrier is needed. This PR adds the missing write barrier. Note: I am not sure if this affects the correctness of Julia GC or not. This could be special cased in the GC code, as the task is always rooted. However, generally if the task is in the old generation, it does not have to be scanned even if it is a root. A write barrier is needed to make sure the GC is aware of the update. --- src/julia_internal.h | 2 +- src/rtutils.c | 7 ++++--- src/task.c | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/julia_internal.h b/src/julia_internal.h index 1a0373f0f9131..721271480fbb1 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -1219,7 +1219,7 @@ STATIC_INLINE size_t jl_excstack_next(jl_excstack_t *stack, size_t itr) JL_NOTSA return itr-2 - jl_excstack_bt_size(stack, itr); } // Exception stack manipulation -void jl_push_excstack(jl_excstack_t **stack JL_REQUIRE_ROOTED_SLOT JL_ROOTING_ARGUMENT, +void jl_push_excstack(jl_task_t* task, jl_excstack_t **stack JL_REQUIRE_ROOTED_SLOT JL_ROOTING_ARGUMENT, jl_value_t *exception JL_ROOTED_ARGUMENT, jl_bt_element_t *bt_data, size_t bt_size); diff --git a/src/rtutils.c b/src/rtutils.c index eefd1b25f9bc4..35ab89d856783 100644 --- a/src/rtutils.c +++ b/src/rtutils.c @@ -325,7 +325,7 @@ static void jl_copy_excstack(jl_excstack_t *dest, jl_excstack_t *src) JL_NOTSAFE dest->top = src->top; } -static void jl_reserve_excstack(jl_excstack_t **stack JL_REQUIRE_ROOTED_SLOT, +static void jl_reserve_excstack(jl_task_t* task, jl_excstack_t **stack JL_REQUIRE_ROOTED_SLOT, size_t reserved_size) { jl_excstack_t *s = *stack; @@ -339,13 +339,14 @@ static void jl_reserve_excstack(jl_excstack_t **stack JL_REQUIRE_ROOTED_SLOT, if (s) jl_copy_excstack(new_s, s); *stack = new_s; + jl_gc_wb(task, new_s); } -void jl_push_excstack(jl_excstack_t **stack JL_REQUIRE_ROOTED_SLOT JL_ROOTING_ARGUMENT, +void jl_push_excstack(jl_task_t* task, jl_excstack_t **stack JL_REQUIRE_ROOTED_SLOT JL_ROOTING_ARGUMENT, jl_value_t *exception JL_ROOTED_ARGUMENT, jl_bt_element_t *bt_data, size_t bt_size) { - jl_reserve_excstack(stack, (*stack ? (*stack)->top : 0) + bt_size + 2); + jl_reserve_excstack(task, stack, (*stack ? (*stack)->top : 0) + bt_size + 2); jl_excstack_t *s = *stack; jl_bt_element_t *rawstack = jl_excstack_raw(s); memcpy(rawstack + s->top, bt_data, sizeof(jl_bt_element_t)*bt_size); diff --git a/src/task.c b/src/task.c index 1dab8688cb079..73d9033f0cb50 100644 --- a/src/task.c +++ b/src/task.c @@ -721,7 +721,7 @@ JL_DLLEXPORT JL_NORETURN void jl_no_exc_handler(jl_value_t *e, jl_task_t *ct) /* 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_push_excstack(&ct->excstack, exception, \ + jl_push_excstack(ct, &ct->excstack, exception, \ ptls->bt_data, ptls->bt_size); \ ptls->bt_size = 0; \ } \ @@ -1224,7 +1224,7 @@ CFI_NORETURN jl_timing_block_task_enter(ct, ptls, NULL); if (jl_atomic_load_relaxed(&ct->_isexception)) { record_backtrace(ptls, 0); - jl_push_excstack(&ct->excstack, ct->result, + jl_push_excstack(ct, &ct->excstack, ct->result, ptls->bt_data, ptls->bt_size); res = ct->result; }