Skip to content

Commit c596f0b

Browse files
vchuravyNHDaly
authored andcommitted
Preserve the scope across the exception handler (JuliaLang#60647)
Fixes JuliaLang#60620 (comment) previously we were preserving the new scope across the exception handler, instead of the scope that was actually stored in the eh context. The additionaly write_barriers are probably not necessary (the task should always be young. Alternative to JuliaLang#60620
1 parent 254bc2b commit c596f0b

File tree

3 files changed

+11
-10
lines changed

3 files changed

+11
-10
lines changed

src/codegen.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6159,6 +6159,7 @@ static void emit_stmtpos(jl_codectx_t &ctx, jl_value_t *expr, int ssaval_result)
61596159
Value *scope_ptr = get_scope_field(ctx);
61606160
jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe).decorateInst(
61616161
ctx.builder.CreateAlignedStore(scope_to_restore, scope_ptr, ctx.types().alignof_ptr));
6162+
emit_write_barrier(ctx, get_current_task(ctx), scope_to_restore);
61626163
}
61636164
}
61646165
else if (head == jl_pop_exception_sym) {
@@ -9334,12 +9335,12 @@ static jl_llvm_functions_t
93349335
Value *scope_ptr = get_scope_field(ctx);
93359336
LoadInst *current_scope = ctx.builder.CreateAlignedLoad(ctx.types().T_prjlvalue, scope_ptr, ctx.types().alignof_ptr);
93369337
StoreInst *scope_store = ctx.builder.CreateAlignedStore(scope_boxed, scope_ptr, ctx.types().alignof_ptr);
9338+
// NOTE: wb not needed here, due to store to current_task (always young)
93379339
jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe).decorateInst(current_scope);
93389340
jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe).decorateInst(scope_store);
9339-
// GC preserve the scope, since it is not rooted in the `jl_handler_t *`
9340-
// and may be removed from jl_current_task by any nested block and then
9341-
// replaced later
9342-
Value *scope_token = ctx.builder.CreateCall(prepare_call(gc_preserve_begin_func), {scope_boxed});
9341+
// GC preserve the current_scope, since it is not rooted in the `jl_handler_t *`,
9342+
// the newly entered scope is preserved through the current_task.
9343+
Value *scope_token = ctx.builder.CreateCall(prepare_call(gc_preserve_begin_func), {current_scope});
93439344
ctx.scope_restore[cursor] = std::make_pair(scope_token, current_scope);
93449345
}
93459346
}

src/interpreter.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -539,12 +539,12 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip,
539539
}
540540
s->locals[jl_source_nslots(s->src) + ip] = jl_box_ulong(jl_excstack_state(ct));
541541
if (jl_enternode_scope(stmt)) {
542+
jl_value_t *old_scope = ct->scope; // Identical to __eh.scope
543+
// GC preserve the old_scope, since it is not rooted in the `jl_handler_t *`,
544+
// the newly entered scope is preserved through the current_task.
545+
JL_GC_PUSH1(&old_scope);
542546
jl_value_t *scope = eval_value(jl_enternode_scope(stmt), s);
543-
// GC preserve the scope, since it is not rooted in the `jl_handler_t *`
544-
// and may be removed from jl_current_task by any nested block and then
545-
// replaced later
546-
JL_GC_PUSH1(&scope);
547-
ct->scope = scope;
547+
ct->scope = scope; // NOTE: wb not needed here, due to store to current_task (always young)
548548
if (!jl_setjmp(__eh.eh_ctx, 0)) {
549549
ct->eh = &__eh;
550550
eval_body(stmts, s, next_ip, toplevel);

src/task.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1107,7 +1107,7 @@ JL_DLLEXPORT jl_task_t *jl_new_task(jl_function_t *start, jl_value_t *completion
11071107
t->donenotify = completion_future;
11081108
jl_atomic_store_relaxed(&t->_isexception, 0);
11091109
// Inherit scope from parent task
1110-
t->scope = ct->scope;
1110+
t->scope = ct->scope; // NOTE: wb not needed here
11111111
// Fork task-local random state from parent
11121112
jl_rng_split(t->rngState, ct->rngState);
11131113
// there is no active exception handler available on this stack yet

0 commit comments

Comments
 (0)