Skip to content

Commit f51d741

Browse files
committed
ZJIT: Guard receiver type in inline Array method handlers
inline_array_push, inline_array_pop, and inline_array_aref did not guard/coerce the receiver to Array type before emitting Array-specific HIR instructions (ArrayPush, ArrayPop, ArrayLength, ArrayAref). This caused a MismatchedOperandType validation failure when these handlers were called from the InvokeSuper CFUNC path, where the receiver (self) has type BasicObject rather than Array. The crash was triggered by representable gem's Binding::Map#<< calling super(binding) to invoke Array#<< on an Array subclass. Add likely_a + coerce_to checks matching the pattern used by inline_array_aset. This is a no-op in the normal SendWithoutBlock path where the caller already adds a GuardType, but correctly inserts the guard when called from InvokeSuper.
1 parent c272297 commit f51d741

File tree

2 files changed

+161
-1
lines changed

2 files changed

+161
-1
lines changed

zjit/src/cruby_methods.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,10 @@ fn inline_kernel_block_given_p(fun: &mut hir::Function, block: hir::BlockId, _re
342342

343343
fn inline_array_aref(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option<hir::InsnId> {
344344
if let &[index] = args {
345-
if fun.likely_a(index, types::Fixnum, state) {
345+
if fun.likely_a(recv, types::Array, state)
346+
&& fun.likely_a(index, types::Fixnum, state)
347+
{
348+
let recv = fun.coerce_to(block, recv, types::Array, state);
346349
let index = fun.coerce_to(block, index, types::Fixnum, state);
347350
let index = fun.push_insn(block, hir::Insn::UnboxFixnum { val: index });
348351
let length = fun.push_insn(block, hir::Insn::ArrayLength { array: recv });
@@ -386,6 +389,8 @@ fn inline_array_aset(fun: &mut hir::Function, block: hir::BlockId, recv: hir::In
386389
fn inline_array_push(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option<hir::InsnId> {
387390
// Inline only the case of `<<` or `push` when called with a single argument.
388391
if let &[val] = args {
392+
if !fun.likely_a(recv, types::Array, state) { return None; }
393+
let recv = fun.coerce_to(block, recv, types::Array, state);
389394
let _ = fun.push_insn(block, hir::Insn::ArrayPush { array: recv, val, state });
390395
return Some(recv);
391396
}
@@ -395,6 +400,8 @@ fn inline_array_push(fun: &mut hir::Function, block: hir::BlockId, recv: hir::In
395400
fn inline_array_pop(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option<hir::InsnId> {
396401
// Only inline the case of no arguments.
397402
let &[] = args else { return None; };
403+
if !fun.likely_a(recv, types::Array, state) { return None; }
404+
let recv = fun.coerce_to(block, recv, types::Array, state);
398405
fun.guard_not_shared(block, recv, state);
399406
Some(fun.push_insn(block, hir::Insn::ArrayPop { array: recv, state }))
400407
}

zjit/src/hir/opt_tests.rs

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7846,6 +7846,159 @@ mod hir_opt_tests {
78467846
");
78477847
}
78487848

7849+
#[test]
7850+
fn test_optimize_array_push_with_array_subclass() {
7851+
eval("
7852+
class PushSubArray < Array
7853+
def <<(val) = super
7854+
end
7855+
test = PushSubArray.new
7856+
test << 1
7857+
");
7858+
assert_snapshot!(hir_string_proc("PushSubArray.new.method(:<<)"), @r"
7859+
fn <<@<compiled>:3:
7860+
bb0():
7861+
EntryPoint interpreter
7862+
v1:BasicObject = LoadSelf
7863+
v2:BasicObject = GetLocal :val, l0, SP@4
7864+
Jump bb2(v1, v2)
7865+
bb1(v5:BasicObject, v6:BasicObject):
7866+
EntryPoint JIT(0)
7867+
Jump bb2(v5, v6)
7868+
bb2(v8:BasicObject, v9:BasicObject):
7869+
PatchPoint MethodRedefined(Array@0x1000, <<@0x1008, cme:0x1010)
7870+
v21:CPtr = GetLEP
7871+
v22:RubyValue = LoadField v21, :_ep_method_entry@0x1038
7872+
v23:CallableMethodEntry[VALUE(0x1040)] = GuardBitEquals v22, Value(VALUE(0x1040))
7873+
v24:RubyValue = LoadField v21, :_ep_specval@0x1048
7874+
v25:FalseClass = GuardBitEquals v24, Value(false)
7875+
v26:Array = GuardType v8, Array
7876+
ArrayPush v26, v9
7877+
IncrCounter inline_cfunc_optimized_send_count
7878+
CheckInterrupts
7879+
Return v26
7880+
");
7881+
}
7882+
7883+
#[test]
7884+
fn test_optimize_array_pop_with_array_subclass() {
7885+
eval("
7886+
class PopSubArray < Array
7887+
def pop = super
7888+
end
7889+
test = PopSubArray.new([1])
7890+
test.pop
7891+
");
7892+
assert_snapshot!(hir_string_proc("PopSubArray.new.method(:pop)"), @r"
7893+
fn pop@<compiled>:3:
7894+
bb0():
7895+
EntryPoint interpreter
7896+
v1:BasicObject = LoadSelf
7897+
Jump bb2(v1)
7898+
bb1(v4:BasicObject):
7899+
EntryPoint JIT(0)
7900+
Jump bb2(v4)
7901+
bb2(v6:BasicObject):
7902+
PatchPoint MethodRedefined(Array@0x1000, pop@0x1008, cme:0x1010)
7903+
v17:CPtr = GetLEP
7904+
v18:RubyValue = LoadField v17, :_ep_method_entry@0x1038
7905+
v19:CallableMethodEntry[VALUE(0x1040)] = GuardBitEquals v18, Value(VALUE(0x1040))
7906+
v20:RubyValue = LoadField v17, :_ep_specval@0x1048
7907+
v21:FalseClass = GuardBitEquals v20, Value(false)
7908+
PatchPoint MethodRedefined(Array@0x1000, pop@0x1008, cme:0x1010)
7909+
v27:CPtr = GetLEP
7910+
v28:RubyValue = LoadField v27, :_ep_method_entry@0x1038
7911+
v29:CallableMethodEntry[VALUE(0x1040)] = GuardBitEquals v28, Value(VALUE(0x1040))
7912+
v30:RubyValue = LoadField v27, :_ep_specval@0x1048
7913+
v31:FalseClass = GuardBitEquals v30, Value(false)
7914+
v22:Array = GuardType v6, Array
7915+
v23:CUInt64 = LoadField v22, :_rbasic_flags@0x1049
7916+
v24:CUInt64 = GuardNoBitsSet v23, RUBY_ELTS_SHARED=CUInt64(4096)
7917+
v25:BasicObject = ArrayPop v22
7918+
IncrCounter inline_cfunc_optimized_send_count
7919+
CheckInterrupts
7920+
Return v25
7921+
");
7922+
}
7923+
7924+
#[test]
7925+
fn test_optimize_array_aref_with_array_subclass_and_fixnum() {
7926+
eval("
7927+
class ArefSubArray < Array
7928+
def [](idx) = super
7929+
end
7930+
test = ArefSubArray.new([1])
7931+
test[0]
7932+
");
7933+
assert_snapshot!(hir_string_proc("ArefSubArray.new.method(:[])"), @r"
7934+
fn []@<compiled>:3:
7935+
bb0():
7936+
EntryPoint interpreter
7937+
v1:BasicObject = LoadSelf
7938+
v2:BasicObject = GetLocal :idx, l0, SP@4
7939+
Jump bb2(v1, v2)
7940+
bb1(v5:BasicObject, v6:BasicObject):
7941+
EntryPoint JIT(0)
7942+
Jump bb2(v5, v6)
7943+
bb2(v8:BasicObject, v9:BasicObject):
7944+
PatchPoint MethodRedefined(Array@0x1000, []@0x1008, cme:0x1010)
7945+
v21:CPtr = GetLEP
7946+
v22:RubyValue = LoadField v21, :_ep_method_entry@0x1038
7947+
v23:CallableMethodEntry[VALUE(0x1040)] = GuardBitEquals v22, Value(VALUE(0x1040))
7948+
v24:RubyValue = LoadField v21, :_ep_specval@0x1048
7949+
v25:FalseClass = GuardBitEquals v24, Value(false)
7950+
PatchPoint MethodRedefined(Array@0x1000, []@0x1008, cme:0x1010)
7951+
v35:CPtr = GetLEP
7952+
v36:RubyValue = LoadField v35, :_ep_method_entry@0x1038
7953+
v37:CallableMethodEntry[VALUE(0x1040)] = GuardBitEquals v36, Value(VALUE(0x1040))
7954+
v38:RubyValue = LoadField v35, :_ep_specval@0x1048
7955+
v39:FalseClass = GuardBitEquals v38, Value(false)
7956+
v26:Array = GuardType v8, Array
7957+
v27:Fixnum = GuardType v9, Fixnum
7958+
v28:CInt64 = UnboxFixnum v27
7959+
v29:CInt64 = ArrayLength v26
7960+
v30:CInt64 = GuardLess v28, v29
7961+
v31:CInt64[0] = Const CInt64(0)
7962+
v32:CInt64 = GuardGreaterEq v30, v31
7963+
v33:BasicObject = ArrayAref v26, v32
7964+
IncrCounter inline_cfunc_optimized_send_count
7965+
CheckInterrupts
7966+
Return v33
7967+
");
7968+
}
7969+
7970+
#[test]
7971+
fn test_dont_optimize_array_aref_with_array_subclass_and_non_fixnum() {
7972+
eval("
7973+
class ArefSubArrayRange < Array
7974+
def [](idx) = super
7975+
end
7976+
test = ArefSubArrayRange.new([1, 2, 3])
7977+
test[0..1]
7978+
");
7979+
assert_snapshot!(hir_string_proc("ArefSubArrayRange.new.method(:[])"), @r"
7980+
fn []@<compiled>:3:
7981+
bb0():
7982+
EntryPoint interpreter
7983+
v1:BasicObject = LoadSelf
7984+
v2:BasicObject = GetLocal :idx, l0, SP@4
7985+
Jump bb2(v1, v2)
7986+
bb1(v5:BasicObject, v6:BasicObject):
7987+
EntryPoint JIT(0)
7988+
Jump bb2(v5, v6)
7989+
bb2(v8:BasicObject, v9:BasicObject):
7990+
PatchPoint MethodRedefined(Array@0x1000, []@0x1008, cme:0x1010)
7991+
v21:CPtr = GetLEP
7992+
v22:RubyValue = LoadField v21, :_ep_method_entry@0x1038
7993+
v23:CallableMethodEntry[VALUE(0x1040)] = GuardBitEquals v22, Value(VALUE(0x1040))
7994+
v24:RubyValue = LoadField v21, :_ep_specval@0x1048
7995+
v25:FalseClass = GuardBitEquals v24, Value(false)
7996+
v26:BasicObject = CCallVariadic v8, :Array#[]@0x1050, v9
7997+
CheckInterrupts
7998+
Return v26
7999+
");
8000+
}
8001+
78498002
#[test]
78508003
fn test_optimize_array_length() {
78518004
eval("

0 commit comments

Comments
 (0)