Skip to content

Commit 83ef55d

Browse files
authored
codegen: manually restore ct->scope when no exception handler is emitted (JuliaLang#56612)
This fixes a bug introduced by JuliaLang#55907, which was neglecting that it's possible for `EnterNode` to have no `catch` destination and still have a scope. This can especially happen if the compiler has decided that the body is `nothrow` and chooses to optimize away the `catch` destination, but also JuliaLang#55907 intended to make the scope-only form of `:enter` legal (and not need an exception handler) even if the body is _not_ `nothrow`. This fixes all that up to restore the scope correctly on the happy path. ~~Needs tests - will add those soon~~
1 parent 0bd8292 commit 83ef55d

File tree

2 files changed

+24
-5
lines changed

2 files changed

+24
-5
lines changed

src/codegen.cpp

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1955,7 +1955,7 @@ class jl_codectx_t {
19551955
// local var info. globals are not in here.
19561956
SmallVector<jl_varinfo_t, 0> slots;
19571957
std::map<int, jl_varinfo_t> phic_slots;
1958-
std::map<int, Value*> scope_tokens;
1958+
std::map<int, std::pair<Value*, Value*> > scope_restore;
19591959
SmallVector<jl_cgval_t, 0> SAvalues;
19601960
SmallVector<std::tuple<jl_cgval_t, BasicBlock *, AllocaInst *, PHINode *, SmallVector<PHINode*,0>, jl_value_t *>, 0> PhiNodes;
19611961
SmallVector<bool, 0> ssavalue_assigned;
@@ -6573,6 +6573,7 @@ static void emit_stmtpos(jl_codectx_t &ctx, jl_value_t *expr, int ssaval_result)
65736573
}
65746574
else if (head == jl_leave_sym) {
65756575
int hand_n_leave = 0;
6576+
Value *scope_to_restore = nullptr, *token = nullptr;
65766577
for (size_t i = 0; i < jl_expr_nargs(ex); ++i) {
65776578
jl_value_t *arg = args[i];
65786579
if (arg == jl_nothing)
@@ -6582,7 +6583,7 @@ static void emit_stmtpos(jl_codectx_t &ctx, jl_value_t *expr, int ssaval_result)
65826583
jl_value_t *enter_stmt = jl_array_ptr_ref(ctx.code, enter_idx);
65836584
if (enter_stmt == jl_nothing)
65846585
continue;
6585-
if (ctx.scope_tokens.count(enter_idx)) {
6586+
if (ctx.scope_restore.count(enter_idx)) {
65866587
// TODO: The semantics of `gc_preserve` are not perfect here. An `Expr(:enter, ...)` block may
65876588
// have multiple exits, but effects of `preserve_end` are only extended to the end of the
65886589
// dominance of each `Expr(:leave, ...)`.
@@ -6594,15 +6595,22 @@ static void emit_stmtpos(jl_codectx_t &ctx, jl_value_t *expr, int ssaval_result)
65946595
//
65956596
// This is correct as-is anyway - it just means the scope lives longer than it needs to
65966597
// if the `Expr(:enter, ...)` has multiple exits.
6597-
ctx.builder.CreateCall(prepare_call(gc_preserve_end_func), {ctx.scope_tokens[enter_idx]});
6598+
std::tie(token, scope_to_restore) = ctx.scope_restore[enter_idx];
6599+
ctx.builder.CreateCall(prepare_call(gc_preserve_end_func), {token});
65986600
}
65996601
if (jl_enternode_catch_dest(enter_stmt)) {
66006602
// We're not actually setting up the exception frames for these, so
66016603
// we don't need to exit them.
66026604
hand_n_leave += 1;
6605+
scope_to_restore = nullptr; // restored by exception handler
66036606
}
66046607
}
66056608
ctx.builder.CreateCall(prepare_call(jlleave_noexcept_func), {get_current_task(ctx), ConstantInt::get(getInt32Ty(ctx.builder.getContext()), hand_n_leave)});
6609+
if (scope_to_restore) {
6610+
Value *scope_ptr = get_scope_field(ctx);
6611+
jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe).decorateInst(
6612+
ctx.builder.CreateAlignedStore(scope_to_restore, scope_ptr, ctx.types().alignof_ptr));
6613+
}
66066614
}
66076615
else if (head == jl_pop_exception_sym) {
66086616
jl_cgval_t excstack_state = emit_expr(ctx, jl_exprarg(expr, 0));
@@ -9652,12 +9660,16 @@ static jl_llvm_functions_t
96529660
continue;
96539661
}
96549662
Value *scope_boxed = boxed(ctx, scope);
9655-
StoreInst *scope_store = ctx.builder.CreateAlignedStore(scope_boxed, get_scope_field(ctx), ctx.types().alignof_ptr);
9663+
Value *scope_ptr = get_scope_field(ctx);
9664+
LoadInst *current_scope = ctx.builder.CreateAlignedLoad(ctx.types().T_prjlvalue, scope_ptr, ctx.types().alignof_ptr);
9665+
StoreInst *scope_store = ctx.builder.CreateAlignedStore(scope_boxed, scope_ptr, ctx.types().alignof_ptr);
9666+
jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe).decorateInst(current_scope);
96569667
jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe).decorateInst(scope_store);
96579668
// GC preserve the scope, since it is not rooted in the `jl_handler_t *`
96589669
// and may be removed from jl_current_task by any nested block and then
96599670
// replaced later
9660-
ctx.scope_tokens[cursor] = ctx.builder.CreateCall(prepare_call(gc_preserve_begin_func), {scope_boxed});
9671+
Value *scope_token = ctx.builder.CreateCall(prepare_call(gc_preserve_begin_func), {scope_boxed});
9672+
ctx.scope_restore[cursor] = std::make_pair(scope_token, current_scope);
96619673
}
96629674
}
96639675
else {

test/scopedvalues.jl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,3 +175,10 @@ const inlineable_const_sv = ScopedValue(1)
175175
@test fully_eliminated(; retval=(inlineable_const_sv => 1)) do
176176
inlineable_const_sv => 1
177177
end
178+
179+
# Handle nothrow scope bodies correctly (#56609)
180+
@eval function nothrow_scope()
181+
$(Expr(:tryfinally, :(), nothing, 1))
182+
@test Core.current_scope() === nothing
183+
end
184+
nothrow_scope()

0 commit comments

Comments
 (0)