ZJIT: Don't mark control-flow opcodes as invalidating locals (#15694)

jump, branchif, etc don't invalidate locals in the JIT; they might in the interpreter because they can execute arbitrary code, but the JIT side exits before that happens.
This commit is contained in:
Max Bernstein 2025-12-24 17:37:25 -05:00 committed by GitHub
parent 8d097bc472
commit 3e82da7232
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
Notes: git 2025-12-24 22:37:52 +00:00
Merged-By: tekknolagi <donotemailthisaddress@bernsteinbear.com>
3 changed files with 52 additions and 50 deletions

View File

@ -5299,6 +5299,20 @@ impl ProfileOracle {
}
}
fn invalidates_locals(opcode: u32, operands: *const VALUE) -> bool {
match opcode {
// Control-flow is non-leaf in the interpreter because it can execute arbitrary code on
// interrupt. But in the JIT, we side-exit if there is a pending interrupt.
YARVINSN_jump
| YARVINSN_branchunless
| YARVINSN_branchif
| YARVINSN_branchnil
| YARVINSN_leave => false,
// TODO(max): Read the invokebuiltin target from operands and determine if it's leaf
_ => unsafe { !rb_zjit_insn_leaf(opcode as i32, operands) }
}
}
/// The index of the self parameter in the HIR function
pub const SELF_PARAM_IDX: usize = 0;
@ -5434,7 +5448,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
}
// Flag a future getlocal/setlocal to add a patch point if this instruction is not leaf.
if unsafe { !rb_zjit_insn_leaf(opcode as i32, pc.offset(1)) } {
if invalidates_locals(opcode, unsafe { pc.offset(1) }) {
local_inval = true;
}

View File

@ -9638,26 +9638,24 @@ mod hir_opt_tests {
Jump bb2(v8, v9, v10, v11, v12)
bb2(v14:BasicObject, v15:BasicObject, v16:BasicObject, v17:BasicObject, v18:NilClass):
CheckInterrupts
v27:BasicObject = GetLocal :a, l0, EP@6
SetLocal :formatted, l0, EP@3, v27
v39:BasicObject = GetLocal :formatted, l0, EP@3
SetLocal :formatted, l0, EP@3, v15
PatchPoint SingleRactorMode
v56:HeapBasicObject = GuardType v14, HeapBasicObject
v57:HeapBasicObject = GuardShape v56, 0x1000
StoreField v57, :@formatted@0x1001, v39
WriteBarrier v57, v39
v60:CShape[0x1002] = Const CShape(0x1002)
StoreField v57, :_shape_id@0x1003, v60
v45:Class[VMFrozenCore] = Const Value(VALUE(0x1008))
v54:HeapBasicObject = GuardType v14, HeapBasicObject
v55:HeapBasicObject = GuardShape v54, 0x1000
StoreField v55, :@formatted@0x1001, v15
WriteBarrier v55, v15
v58:CShape[0x1002] = Const CShape(0x1002)
StoreField v55, :_shape_id@0x1003, v58
v43:Class[VMFrozenCore] = Const Value(VALUE(0x1008))
PatchPoint MethodRedefined(Class@0x1010, lambda@0x1018, cme:0x1020)
PatchPoint NoSingletonClass(Class@0x1010)
v65:BasicObject = CCallWithFrame v45, :RubyVM::FrozenCore.lambda@0x1048, block=0x1050
v48:BasicObject = GetLocal :a, l0, EP@6
v49:BasicObject = GetLocal :_b, l0, EP@5
v50:BasicObject = GetLocal :_c, l0, EP@4
v51:BasicObject = GetLocal :formatted, l0, EP@3
v63:BasicObject = CCallWithFrame v43, :RubyVM::FrozenCore.lambda@0x1048, block=0x1050
v46:BasicObject = GetLocal :a, l0, EP@6
v47:BasicObject = GetLocal :_b, l0, EP@5
v48:BasicObject = GetLocal :_c, l0, EP@4
v49:BasicObject = GetLocal :formatted, l0, EP@3
CheckInterrupts
Return v65
Return v63
");
}

View File

@ -1069,17 +1069,14 @@ pub mod hir_build_tests {
v18:CBool = Test v11
IfFalse v18, bb3(v10, v11, v12)
v22:Fixnum[3] = Const Value(3)
PatchPoint NoEPEscape(test)
CheckInterrupts
Jump bb4(v10, v11, v22)
bb3(v29:BasicObject, v30:BasicObject, v31:NilClass):
v35:Fixnum[4] = Const Value(4)
PatchPoint NoEPEscape(test)
Jump bb4(v29, v30, v35)
bb4(v40:BasicObject, v41:BasicObject, v42:Fixnum):
PatchPoint NoEPEscape(test)
bb3(v27:BasicObject, v28:BasicObject, v29:NilClass):
v33:Fixnum[4] = Const Value(4)
Jump bb4(v27, v28, v33)
bb4(v36:BasicObject, v37:BasicObject, v38:Fixnum):
CheckInterrupts
Return v42
Return v38
");
}
@ -1366,23 +1363,20 @@ pub mod hir_build_tests {
CheckInterrupts
Jump bb4(v10, v16, v20)
bb4(v26:BasicObject, v27:BasicObject, v28:BasicObject):
PatchPoint NoEPEscape(test)
v34:Fixnum[0] = Const Value(0)
v37:BasicObject = SendWithoutBlock v28, :>, v34 # SendFallbackReason: Uncategorized(opt_gt)
v32:Fixnum[0] = Const Value(0)
v35:BasicObject = SendWithoutBlock v28, :>, v32 # SendFallbackReason: Uncategorized(opt_gt)
CheckInterrupts
v40:CBool = Test v37
IfTrue v40, bb3(v26, v27, v28)
v43:NilClass = Const Value(nil)
PatchPoint NoEPEscape(test)
v38:CBool = Test v35
IfTrue v38, bb3(v26, v27, v28)
v41:NilClass = Const Value(nil)
CheckInterrupts
Return v27
bb3(v53:BasicObject, v54:BasicObject, v55:BasicObject):
PatchPoint NoEPEscape(test)
v62:Fixnum[1] = Const Value(1)
v65:BasicObject = SendWithoutBlock v54, :+, v62 # SendFallbackReason: Uncategorized(opt_plus)
v70:Fixnum[1] = Const Value(1)
v73:BasicObject = SendWithoutBlock v55, :-, v70 # SendFallbackReason: Uncategorized(opt_minus)
Jump bb4(v53, v65, v73)
bb3(v49:BasicObject, v50:BasicObject, v51:BasicObject):
v56:Fixnum[1] = Const Value(1)
v59:BasicObject = SendWithoutBlock v50, :+, v56 # SendFallbackReason: Uncategorized(opt_plus)
v64:Fixnum[1] = Const Value(1)
v67:BasicObject = SendWithoutBlock v51, :-, v64 # SendFallbackReason: Uncategorized(opt_minus)
Jump bb4(v49, v59, v67)
");
}
@ -3064,15 +3058,13 @@ pub mod hir_build_tests {
CheckInterrupts
v35:CBool[true] = Test v32
IfFalse v35, bb3(v16, v17, v18, v19, v20, v25)
PatchPoint NoEPEscape(open)
v42:BasicObject = InvokeBlock, v25 # SendFallbackReason: Uncategorized(invokeblock)
v45:BasicObject = InvokeBuiltin dir_s_close, v16, v25
v40:BasicObject = InvokeBlock, v25 # SendFallbackReason: Uncategorized(invokeblock)
v43:BasicObject = InvokeBuiltin dir_s_close, v16, v25
CheckInterrupts
Return v42
bb3(v51, v52, v53, v54, v55, v56):
PatchPoint NoEPEscape(open)
Return v40
bb3(v49, v50, v51, v52, v53, v54):
CheckInterrupts
Return v56
Return v54
");
}
@ -3548,12 +3540,10 @@ pub mod hir_build_tests {
v22:Fixnum[1] = Const Value(1)
v24:Fixnum[1] = Const Value(1)
v27:BasicObject = SendWithoutBlock v22, :+, v24 # SendFallbackReason: Uncategorized(opt_plus)
PatchPoint NoEPEscape(test)
Jump bb3(v10, v27, v12)
bb3(v32:BasicObject, v33:BasicObject, v34:BasicObject):
PatchPoint NoEPEscape(test)
bb3(v30:BasicObject, v31:BasicObject, v32:BasicObject):
CheckInterrupts
Return v33
Return v31
");
}