Skip to content

Commit 5fbb151

Browse files
committed
ZJIT: Clear cfp->block_code before exposing a stub frame to the GC
1 parent dc6b98d commit 5fbb151

2 files changed

Lines changed: 75 additions & 1 deletion

File tree

‎zjit/src/codegen.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3832,14 +3832,15 @@ c_callable! {
38323832
let entry_insn_idx = params.opt_table_slice().get(entry_idx)
38333833
.unwrap_or_else(|| panic!("function_stub: opt_table out of bounds. {params:#?}, entry_idx={entry_idx}"))
38343834
.as_u32();
3835-
// gen_push_frame() doesn't set PC or ISEQ, so we need to set them before exit.
3835+
// gen_push_frame() doesn't set PC, ISEQ, or block_code, so we need to set them before exit.
38363836
// function_stub_hit_body() may allocate and call gc_validate_pc(), so we always set PC and ISEQ.
38373837
// Clear jit_return so the interpreter reads cfp->pc and cfp->iseq directly.
38383838
// cfp->sp is set to the base pointer, so it needs to be fixed before exit.
38393839
let pc = unsafe { rb_iseq_pc_at_idx(iseq, entry_insn_idx) };
38403840
unsafe { rb_set_cfp_pc(cfp, pc) };
38413841
unsafe { (*cfp)._iseq = iseq };
38423842
unsafe { (*cfp).jit_return = std::ptr::null_mut() };
3843+
unsafe { (*cfp).block_code = std::ptr::null() };
38433844
let ec_cfp = unsafe { ec.byte_add(RUBY_OFFSET_EC_CFP as usize) as *mut CfpPtr };
38443845
unsafe { *ec_cfp = cfp };
38453846
unsafe { rb_set_cfp_sp(cfp, sp) };

‎zjit/src/codegen_tests.rs‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9057,3 +9057,76 @@ fn test_regression_stub_frame_sp_published_for_gc() {
90579057
let caller_version = unsafe { caller_payload.versions.last().unwrap().as_ref() };
90589058
assert_eq!(1, caller_version.outgoing.len(), "expected a JIT-to-JIT function stub");
90599059
}
9060+
9061+
#[test]
9062+
fn test_regression_stub_frame_block_code_cleared_for_gc() {
9063+
rb_zjit_prepare_options();
9064+
set_inline_threshold(0); // don't inline the callee; we need a function stub
9065+
set_call_threshold(2000);
9066+
eval("nil"); // boot the VM before touching ZJITState
9067+
9068+
assert_snapshot!(inspect(r#"
9069+
class Integer
9070+
# No send/invokesuper/invokeblock, so iseq_may_write_block_code() is false
9071+
# and gen_push_frame() leaves this frame's cfp->block_code alone.
9072+
def zjit_bc_callee(x) = x + 1
9073+
end
9074+
9075+
def zjit_bc_caller(run, x)
9076+
1.zjit_bc_callee(x) if run
9077+
end
9078+
9079+
def zjit_bc_deep(n)
9080+
if n > 0
9081+
zjit_bc_deep(n - 1)
9082+
else
9083+
# Arm an incremental mark that can't finish in one step. Nothing
9084+
# allocates between here and the function stub hit below, so the
9085+
# pending mark is still in progress when the stub takes the VM lock.
9086+
GC.start(full_mark: true, immediate_mark: false, immediate_sweep: false)
9087+
$zjit_bc_armed += 1 if GC.latest_gc_info(:state) == :marking
9088+
zjit_bc_caller(true, 0) # JIT-to-JIT call through the function stub
9089+
end
9090+
end
9091+
9092+
# Warm the caller past the call threshold so it compiles with a function
9093+
# stub for the still-uncompiled callee.
9094+
i = 0
9095+
while i < 2100
9096+
zjit_bc_caller(false, nil)
9097+
i += 1
9098+
end
9099+
9100+
# Give the incremental mark enough work that it can't finish in one step.
9101+
$zjit_bc_bloat = Array.new(300_000) { Object.new }
9102+
$zjit_bc_armed = 0
9103+
9104+
# Leave a pointer to this block ISEQ in 60 consecutive CFP slots.
9105+
# The module is anonymous and the entry call lives inside the eval'd code,
9106+
# so nothing outside keeps the module, the method or the block ISEQ alive.
9107+
Module.new.module_eval(<<~PLANT)
9108+
def self.plant(n)
9109+
plant(n - 1) { } if n > 0
9110+
end
9111+
plant(60)
9112+
PLANT
9113+
9114+
# FREE: the block ISEQ is garbage now, so those slots dangle at a T_NONE slot.
9115+
GC.start
9116+
9117+
results = []
9118+
depth = 20
9119+
6.times do
9120+
results << zjit_bc_deep(depth)
9121+
depth += 1 # land on a planted slot no frame has pushed over since
9122+
end
9123+
[results.uniq, $zjit_bc_armed > 0]
9124+
"#), @"[[1], true]");
9125+
9126+
// Guard the shape of the repro: the caller must really call the callee
9127+
// through a JIT-to-JIT function stub.
9128+
let caller_iseq = get_method_iseq("self", "zjit_bc_caller");
9129+
let caller_payload = get_or_create_iseq_payload(caller_iseq);
9130+
let caller_version = unsafe { caller_payload.versions.last().unwrap().as_ref() };
9131+
assert_eq!(1, caller_version.outgoing.len(), "expected a JIT-to-JIT function stub");
9132+
}

0 commit comments

Comments
 (0)