Skip to content

Commit 91a29f4

Browse files
committed
ZJIT: Scan rescue/ensure ISEQs in locals_written_in_block
locals_written_in_block was only scanning blockiseqs reachable through send instructions, missing rescue/ensure handler ISEQs referenced from the catch table. This caused locals written inside rescue blocks to not be reloaded after send-with-block, leading to stale values.
1 parent c153ab2 commit 91a29f4

File tree

5 files changed

+114
-0
lines changed

5 files changed

+114
-0
lines changed

jit.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,19 @@ rb_get_iseq_body_param_opt_table(const rb_iseq_t *iseq)
366366
return iseq->body->param.opt_table;
367367
}
368368

369+
unsigned int
370+
rb_get_iseq_body_catch_table_size(const rb_iseq_t *iseq)
371+
{
372+
const struct iseq_catch_table *ct = iseq->body->catch_table;
373+
return ct ? ct->size : 0;
374+
}
375+
376+
const rb_iseq_t *
377+
rb_get_iseq_catch_table_entry_iseq(const rb_iseq_t *iseq, unsigned int idx)
378+
{
379+
return iseq->body->catch_table->entries[idx].iseq;
380+
}
381+
369382
struct rb_control_frame_struct *
370383
rb_get_ec_cfp(const rb_execution_context_t *ec)
371384
{

zjit/bindgen/src/main.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,8 @@ fn main() {
404404
.allowlist_function("rb_get_iseq_body_param_lead_num")
405405
.allowlist_function("rb_get_iseq_body_param_opt_num")
406406
.allowlist_function("rb_get_iseq_body_param_opt_table")
407+
.allowlist_function("rb_get_iseq_body_catch_table_size")
408+
.allowlist_function("rb_get_iseq_catch_table_entry_iseq")
407409
.allowlist_function("rb_get_cikw_keyword_len")
408410
.allowlist_function("rb_get_cikw_keywords_idx")
409411
.allowlist_function("rb_get_call_data_ci")

zjit/src/cruby_bindings.inc.rs

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

zjit/src/hir.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6280,6 +6280,17 @@ fn locals_written_in_block(parent_iseq: IseqPtr, blockiseq: IseqPtr, target_dept
62806280
}
62816281
insn_idx += insn_len(opcode as usize);
62826282
}
6283+
6284+
// Also scan rescue/ensure handler ISEQs referenced from the catch table.
6285+
// These are child ISEQs (one level deeper than blockiseq) that can write
6286+
// to the parent's locals via setlocal with higher level values.
6287+
let catch_size = unsafe { rb_get_iseq_body_catch_table_size(blockiseq) };
6288+
for i in 0..catch_size {
6289+
let handler_iseq = unsafe { rb_get_iseq_catch_table_entry_iseq(blockiseq, i) };
6290+
if !handler_iseq.is_null() {
6291+
locals_written_in_block(parent_iseq, handler_iseq, target_depth + 1, written);
6292+
}
6293+
}
62836294
}
62846295

62856296
/// Reload locals that may have been modified by the blockiseq.

zjit/src/hir/tests.rs

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4257,6 +4257,89 @@ pub mod hir_build_tests {
42574257
Return v35
42584258
");
42594259
}
4260+
4261+
#[test]
4262+
fn reload_local_written_in_rescue() {
4263+
eval(r#"
4264+
def test
4265+
a = 1
4266+
b = 2
4267+
tap do
4268+
begin
4269+
raise
4270+
rescue
4271+
b = 3
4272+
end
4273+
end
4274+
a + b
4275+
end
4276+
"#);
4277+
assert_contains_opcode("test", YARVINSN_send);
4278+
assert_snapshot!(hir_string("test"), @r"
4279+
fn test@<compiled>:3:
4280+
bb0():
4281+
EntryPoint interpreter
4282+
v1:BasicObject = LoadSelf
4283+
v2:NilClass = Const Value(nil)
4284+
v3:NilClass = Const Value(nil)
4285+
Jump bb2(v1, v2, v3)
4286+
bb1(v6:BasicObject):
4287+
EntryPoint JIT(0)
4288+
v7:NilClass = Const Value(nil)
4289+
v8:NilClass = Const Value(nil)
4290+
Jump bb2(v6, v7, v8)
4291+
bb2(v10:BasicObject, v11:NilClass, v12:NilClass):
4292+
v16:Fixnum[1] = Const Value(1)
4293+
v20:Fixnum[2] = Const Value(2)
4294+
v25:BasicObject = Send v10, 0x1000, :tap # SendFallbackReason: Uncategorized(send)
4295+
v26:BasicObject = GetLocal :b, l0, EP@3
4296+
PatchPoint NoEPEscape(test)
4297+
v35:BasicObject = SendWithoutBlock v16, :+, v26 # SendFallbackReason: Uncategorized(opt_plus)
4298+
CheckInterrupts
4299+
Return v35
4300+
");
4301+
}
4302+
4303+
#[test]
4304+
fn dont_reload_local_not_written_in_rescue() {
4305+
eval(r#"
4306+
def test
4307+
a = 1
4308+
b = 2
4309+
tap do
4310+
begin
4311+
raise
4312+
rescue
4313+
a
4314+
end
4315+
end
4316+
a + b
4317+
end
4318+
"#);
4319+
assert_contains_opcode("test", YARVINSN_send);
4320+
assert_snapshot!(hir_string("test"), @r"
4321+
fn test@<compiled>:3:
4322+
bb0():
4323+
EntryPoint interpreter
4324+
v1:BasicObject = LoadSelf
4325+
v2:NilClass = Const Value(nil)
4326+
v3:NilClass = Const Value(nil)
4327+
Jump bb2(v1, v2, v3)
4328+
bb1(v6:BasicObject):
4329+
EntryPoint JIT(0)
4330+
v7:NilClass = Const Value(nil)
4331+
v8:NilClass = Const Value(nil)
4332+
Jump bb2(v6, v7, v8)
4333+
bb2(v10:BasicObject, v11:NilClass, v12:NilClass):
4334+
v16:Fixnum[1] = Const Value(1)
4335+
v20:Fixnum[2] = Const Value(2)
4336+
v25:BasicObject = Send v10, 0x1000, :tap # SendFallbackReason: Uncategorized(send)
4337+
PatchPoint NoEPEscape(test)
4338+
v34:BasicObject = SendWithoutBlock v16, :+, v20 # SendFallbackReason: Uncategorized(opt_plus)
4339+
CheckInterrupts
4340+
Return v34
4341+
");
4342+
}
42604343
}
42614344

42624345
/// Test successor and predecessor set computations.

0 commit comments

Comments
 (0)