Skip to content

Commit a68e42c

Browse files
committed
ZJIT: Track block ISEQ pointers baked into yield guards for compaction
The `yield` dispatch guards `captured->code.iseq` against the profiled block ISEQ, but it baked the ISEQ pointer into JIT code as a raw `Const::CPtr`. The GC neither marked it nor updated it, so once compaction moved the block ISEQ, the guard kept comparing against the old address. Any block ISEQ allocated at that address later passed the guard, and the JIT-to-JIT call that had already been patched for the original block ran the original block's code with the new block's `self` and EP. This showed up as `Array#each` running an unrelated Bundler block in place of a test's block. Bake the ISEQ in as a `Const::Value` so it is recorded as a GC offset, marked, and updated on compaction. Add an `Iseq` HIR type so that `Type::from_value` can type an `imemo_iseq`, like `CallableMethodEntry`.
1 parent e5a2031 commit a68e42c

7 files changed

Lines changed: 97 additions & 50 deletions

File tree

‎zjit/src/codegen_tests.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,6 +1127,25 @@ fn test_yield_inlined_caller_block_dispatches_without_guards() {
11271127
});
11281128
}
11291129

1130+
#[test]
1131+
fn test_yield_block_iseq_guard_survives_compaction() {
1132+
// The yield dispatch guards captured->code.iseq against the profiled block ISEQ baked into
1133+
// JIT code. ISEQs are movable GC objects, so compaction must update the baked pointer.
1134+
// Otherwise the guard would side-exit after compaction, or worse, match an unrelated ISEQ
1135+
// allocated at the old address later and dispatch to the wrong block.
1136+
// Disable inlining: when foo is inlined into test, yield takes the guard-free path instead.
1137+
with_inlining_threshold(0, || {
1138+
eval("
1139+
def foo = yield
1140+
def test = foo { 42 }
1141+
# Call it enough times to compile both test and foo (through test's JIT-to-JIT stub)
1142+
4.times { test }
1143+
GC.verify_compaction_references(expand_heap: true, toward: :empty) if GC.respond_to?(:compact)
1144+
");
1145+
assert_snapshot!(assert_compiles("test"), @"42");
1146+
});
1147+
}
1148+
11301149
#[test]
11311150
fn test_yield_with_lambda_arg() {
11321151
// A lambda passed via &l is a proc handler (not imemo_iseq): yield falls back but runs.

‎zjit/src/cruby.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,12 @@ impl VALUE {
757757
unsafe { rb_IMEMO_TYPE_P(self, imemo_ment) == 1 }
758758
}
759759

760+
/// Return true if `self` is an ISEQ (`imemo_iseq`)
761+
pub fn iseq_p(self) -> bool {
762+
if self == VALUE(0) { return false; }
763+
unsafe { rb_IMEMO_TYPE_P(self, imemo_iseq) == 1 }
764+
}
765+
760766
/// Assert that `self` is a method entry in debug builds
761767
pub fn as_cme(self) -> *const rb_callable_method_entry_t {
762768
let ptr: *const rb_callable_method_entry_t = self.as_ptr();

‎zjit/src/hir.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3226,7 +3226,7 @@ impl Function {
32263226
/// Load `captured->code.iseq` from a `struct rb_captured_block *`.
32273227
fn load_captured_code_iseq(&mut self, block: BlockId, captured: InsnId) -> InsnId {
32283228
let offset: i32 = std::mem::offset_of!(rb_captured_block, code).try_into().unwrap();
3229-
self.load_field(block, captured, FieldName::code_iseq, offset, types::CPtr)
3229+
self.load_field(block, captured, FieldName::code_iseq, offset, types::Iseq)
32303230
}
32313231

32323232
/// Untag an ISEQ block handler into its `struct rb_captured_block *`:
@@ -3272,10 +3272,11 @@ impl Function {
32723272
self.push_insn(block, Insn::GuardBitEquals { val: tag, expected: Const::CInt64(0x1), reason: Box::new(SideExitReason::InvokeBlockHandlerNotIseq), state, recompile: Some(Recompile) });
32733273
let captured = self.untag_block_handler(block, block_handler);
32743274

3275-
// Guard captured->code.iseq is the profiled block iseq. Compare the raw imemo pointer:
3276-
// type inference (from_value) can't type an iseq imemo, so guard it as a CPtr identity.
3275+
// Guard captured->code.iseq is the profiled block iseq. The ISEQ is a GC object that
3276+
// can be moved by compaction, so bake it in as a `Const::Value` rather than a raw
3277+
// `Const::CPtr` to let the GC mark it and update the pointer embedded in JIT code.
32773278
let captured_iseq = self.load_captured_code_iseq(block, captured);
3278-
self.push_insn(block, Insn::GuardBitEquals { val: captured_iseq, expected: Const::CPtr(block_iseq as *const u8), reason: Box::new(SideExitReason::InvokeBlockIseqChanged), state, recompile: Some(Recompile) });
3279+
self.push_insn(block, Insn::GuardBitEquals { val: captured_iseq, expected: Const::Value(VALUE::from(block_iseq)), reason: Box::new(SideExitReason::InvokeBlockIseqChanged), state, recompile: Some(Recompile) });
32793280

32803281
let result = self.push_insn(block, Insn::InvokeBlockIseqDirect { iseq: block_iseq, captured, args, state });
32813282
return (block, result);
@@ -3300,7 +3301,8 @@ impl Function {
33003301

33013302
let mut compare_block = dispatch_block;
33023303
for &block_iseq in iseqs {
3303-
let expected = self.push_insn(compare_block, Insn::Const { val: Const::CPtr(block_iseq as *const u8) });
3304+
// Use `Const::Value` so that the GC updates the ISEQ pointer on compaction. See above.
3305+
let expected = self.push_insn(compare_block, Insn::Const { val: Const::Value(VALUE::from(block_iseq)) });
33043306
let iseq_matches = self.push_insn(compare_block, Insn::IsBitEqual { left: captured_iseq, right: expected });
33053307
let direct_block = self.new_block(insn_idx);
33063308
let miss_block = self.new_block(insn_idx);

‎zjit/src/hir/opt_tests.rs‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4436,9 +4436,9 @@ mod hir_opt_tests {
44364436
v30:CInt64[1] = GuardBitEquals v29, CInt64(1) recompile
44374437
v31:CInt64[-4] = Const CInt64(-4)
44384438
v32:CInt64 = IntAnd v27, v31
4439-
v33:CPtr = LoadField v32, :code_iseq@0x1001
4440-
v34:CPtr[CPtr(0x1002)] = GuardBitEquals v33, CPtr(0x1002) recompile
4441-
v35:BasicObject = InvokeBlockIseqDirect (0x1002), v32, v10, v12, v14, v16, v18, v20, v22, v24
4439+
v33:Iseq = LoadField v32, :code_iseq@0x1001
4440+
v34:Iseq[VALUE(0x1008)] = GuardBitEquals v33, Value(VALUE(0x1008)) recompile
4441+
v35:BasicObject = InvokeBlockIseqDirect (0x1008), v32, v10, v12, v14, v16, v18, v20, v22, v24
44424442
CheckInterrupts
44434443
Return v35
44444444
");
@@ -4558,19 +4558,19 @@ mod hir_opt_tests {
45584558
bb5():
45594559
v20:CInt64[-4] = Const CInt64(-4)
45604560
v21:CInt64 = IntAnd v13, v20
4561-
v22:CPtr = LoadField v21, :code_iseq@0x1001
4562-
v23:CPtr[CPtr(0x1002)] = Const CPtr(0x1002)
4561+
v22:Iseq = LoadField v21, :code_iseq@0x1001
4562+
v23:Iseq[VALUE(0x1008)] = Const Value(VALUE(0x1008))
45634563
v24:CBool = IsBitEqual v22, v23
45644564
CondBranch v24, bb7(), bb8()
45654565
bb7():
4566-
v26:BasicObject = InvokeBlockIseqDirect (0x1002), v21, v10
4566+
v26:BasicObject = InvokeBlockIseqDirect (0x1008), v21, v10
45674567
Jump bb4(v26)
45684568
bb8():
4569-
v28:CPtr[CPtr(0x1003)] = Const CPtr(0x1003)
4569+
v28:Iseq[VALUE(0x1010)] = Const Value(VALUE(0x1010))
45704570
v29:CBool = IsBitEqual v22, v28
45714571
CondBranch v29, bb9(), bb10()
45724572
bb9():
4573-
v31:BasicObject = InvokeBlockIseqDirect (0x1003), v21, v10
4573+
v31:BasicObject = InvokeBlockIseqDirect (0x1010), v21, v10
45744574
Jump bb4(v31)
45754575
bb10():
45764576
Jump bb6()
@@ -4623,19 +4623,19 @@ mod hir_opt_tests {
46234623
bb5():
46244624
v20:CInt64[-4] = Const CInt64(-4)
46254625
v21:CInt64 = IntAnd v13, v20
4626-
v22:CPtr = LoadField v21, :code_iseq@0x1001
4627-
v23:CPtr[CPtr(0x1002)] = Const CPtr(0x1002)
4626+
v22:Iseq = LoadField v21, :code_iseq@0x1001
4627+
v23:Iseq[VALUE(0x1008)] = Const Value(VALUE(0x1008))
46284628
v24:CBool = IsBitEqual v22, v23
46294629
CondBranch v24, bb7(), bb8()
46304630
bb7():
4631-
v26:BasicObject = InvokeBlockIseqDirect (0x1002), v21, v10
4631+
v26:BasicObject = InvokeBlockIseqDirect (0x1008), v21, v10
46324632
Jump bb4(v26)
46334633
bb8():
4634-
v28:CPtr[CPtr(0x1003)] = Const CPtr(0x1003)
4634+
v28:Iseq[VALUE(0x1010)] = Const Value(VALUE(0x1010))
46354635
v29:CBool = IsBitEqual v22, v28
46364636
CondBranch v29, bb9(), bb10()
46374637
bb9():
4638-
v31:BasicObject = InvokeBlockIseqDirect (0x1003), v21, v10
4638+
v31:BasicObject = InvokeBlockIseqDirect (0x1010), v21, v10
46394639
Jump bb4(v31)
46404640
bb10():
46414641
Jump bb6()
@@ -4687,19 +4687,19 @@ mod hir_opt_tests {
46874687
bb5():
46884688
v20:CInt64[-4] = Const CInt64(-4)
46894689
v21:CInt64 = IntAnd v13, v20
4690-
v22:CPtr = LoadField v21, :code_iseq@0x1001
4691-
v23:CPtr[CPtr(0x1002)] = Const CPtr(0x1002)
4690+
v22:Iseq = LoadField v21, :code_iseq@0x1001
4691+
v23:Iseq[VALUE(0x1008)] = Const Value(VALUE(0x1008))
46924692
v24:CBool = IsBitEqual v22, v23
46934693
CondBranch v24, bb7(), bb8()
46944694
bb7():
4695-
v26:BasicObject = InvokeBlockIseqDirect (0x1002), v21, v10
4695+
v26:BasicObject = InvokeBlockIseqDirect (0x1008), v21, v10
46964696
Jump bb4(v26)
46974697
bb8():
4698-
v28:CPtr[CPtr(0x1003)] = Const CPtr(0x1003)
4698+
v28:Iseq[VALUE(0x1010)] = Const Value(VALUE(0x1010))
46994699
v29:CBool = IsBitEqual v22, v28
47004700
CondBranch v29, bb9(), bb10()
47014701
bb9():
4702-
v31:BasicObject = InvokeBlockIseqDirect (0x1003), v21, v10
4702+
v31:BasicObject = InvokeBlockIseqDirect (0x1010), v21, v10
47034703
Jump bb4(v31)
47044704
bb10():
47054705
Jump bb6()
@@ -4752,12 +4752,12 @@ mod hir_opt_tests {
47524752
bb5():
47534753
v20:CInt64[-4] = Const CInt64(-4)
47544754
v21:CInt64 = IntAnd v13, v20
4755-
v22:CPtr = LoadField v21, :code_iseq@0x1011
4756-
v23:CPtr[CPtr(0x1012)] = Const CPtr(0x1012)
4755+
v22:Iseq = LoadField v21, :code_iseq@0x1011
4756+
v23:Iseq[VALUE(0x1018)] = Const Value(VALUE(0x1018))
47574757
v24:CBool = IsBitEqual v22, v23
47584758
CondBranch v24, bb7(), bb8()
47594759
bb7():
4760-
v26:BasicObject = InvokeBlockIseqDirect (0x1012), v21, v10
4760+
v26:BasicObject = InvokeBlockIseqDirect (0x1018), v21, v10
47614761
Jump bb4(v26)
47624762
bb8():
47634763
Jump bb6()
@@ -19914,9 +19914,9 @@ mod hir_opt_tests {
1991419914
v83:CInt64[1] = GuardBitEquals v82, CInt64(1) recompile
1991519915
v84:CInt64[-4] = Const CInt64(-4)
1991619916
v85:CInt64 = IntAnd v80, v84
19917-
v86:CPtr = LoadField v85, :code_iseq@0x1001
19918-
v87:CPtr[CPtr(0x1002)] = GuardBitEquals v86, CPtr(0x1002) recompile
19919-
v88:BasicObject = InvokeBlockIseqDirect (0x1002), v85, v77
19917+
v86:Iseq = LoadField v85, :code_iseq@0x1001
19918+
v87:Iseq[VALUE(0x1008)] = GuardBitEquals v86, Value(VALUE(0x1008)) recompile
19919+
v88:BasicObject = InvokeBlockIseqDirect (0x1008), v85, v77
1992019920
v92:Fixnum[1] = Const Value(1)
1992119921
v93:Fixnum = FixnumAdd v49, v92
1992219922
PatchPoint NoEPEscape(each)

‎zjit/src/hir_type/gen_hir_type.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ def to_graphviz type, f
5151
undef_ = value.subtype "Undef"
5252
# TODO(max): Figure out if CME should be a subtype of IMemo
5353
value.subtype "CallableMethodEntry" # rb_callable_method_entry_t*
54+
value.subtype "Iseq" # rb_iseq_t*
5455
basic_object = value.subtype "BasicObject"
5556
basic_object_exact = basic_object.subtype "BasicObjectExact"
5657
basic_object_subclass = basic_object.subtype "BasicObjectSubclass"

‎zjit/src/hir_type/hir_type.inc.rs‎

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

‎zjit/src/hir_type/mod.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,10 @@ impl Type {
333333
// valid on imemo.
334334
Type::new(bits::CallableMethodEntry, Specialization::Object(val))
335335
}
336+
else if val.iseq_p() {
337+
// Same as CME: an ISEQ is an imemo, so class_of is not valid on it.
338+
Type::new(bits::Iseq, Specialization::Object(val))
339+
}
336340
else {
337341
Self::from_heap_object(val)
338342
}
@@ -1119,6 +1123,18 @@ mod tests {
11191123
});
11201124
}
11211125

1126+
#[test]
1127+
fn iseq() {
1128+
crate::cruby::with_rubyvm(|| {
1129+
let iseq = crate::cruby::test_utils::get_proc_iseq("proc { 1 }");
1130+
assert!(!iseq.is_null());
1131+
let iseq_value: VALUE = iseq.into();
1132+
let ty = Type::from_value(iseq_value);
1133+
assert_subtype(ty, types::Iseq);
1134+
assert!(ty.ruby_object_known());
1135+
});
1136+
}
1137+
11221138
#[test]
11231139
fn string_subclass_is_string_subtype() {
11241140
crate::cruby::with_rubyvm(|| {

0 commit comments

Comments
 (0)