Skip to content

Commit ed18a21

Browse files
authored
ZJIT: Check if shape is too complex before reading ivar by index (ruby#15478)
This fixes a crash when the new shape after a transition is too complex; we need to check that it's not complex before trying to read by index.
1 parent 2b66fc7 commit ed18a21

File tree

2 files changed

+34
-2
lines changed

2 files changed

+34
-2
lines changed

zjit/src/hir.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3155,8 +3155,6 @@ impl Function {
31553155
// We're only looking at T_OBJECT so ignore all of the imemo stuff.
31563156
assert!(recv_type.flags().is_t_object());
31573157
next_shape_id = ShapeId(unsafe { rb_shape_transition_add_ivar_no_warnings(class, current_shape_id.0, id) });
3158-
let ivar_result = unsafe { rb_shape_get_iv_index(next_shape_id.0, id, &mut ivar_index) };
3159-
assert!(ivar_result, "New shape must have the ivar index");
31603158
// If the VM ran out of shapes, or this class generated too many leaf,
31613159
// it may be de-optimized into OBJ_TOO_COMPLEX_SHAPE (hash-table).
31623160
let new_shape_too_complex = unsafe { rb_jit_shape_too_complex_p(next_shape_id.0) };
@@ -3165,6 +3163,8 @@ impl Function {
31653163
self.push_insn(block, Insn::IncrCounter(Counter::setivar_fallback_new_shape_too_complex));
31663164
self.push_insn_id(block, insn_id); continue;
31673165
}
3166+
let ivar_result = unsafe { rb_shape_get_iv_index(next_shape_id.0, id, &mut ivar_index) };
3167+
assert!(ivar_result, "New shape must have the ivar index");
31683168
let current_capacity = unsafe { rb_jit_shape_capacity(current_shape_id.0) };
31693169
let next_capacity = unsafe { rb_jit_shape_capacity(next_shape_id.0) };
31703170
// If the new shape has a different capacity, or is TOO_COMPLEX, we'll have to

zjit/src/hir/opt_tests.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3932,6 +3932,38 @@ mod hir_opt_tests {
39323932
");
39333933
}
39343934

3935+
#[test]
3936+
fn test_dont_specialize_setivar_when_next_shape_is_too_complex() {
3937+
eval(r#"
3938+
class AboutToBeTooComplex
3939+
def test = @abc = 5
3940+
end
3941+
SHAPE_MAX_VARIATIONS = 8 # see shape.h
3942+
SHAPE_MAX_VARIATIONS.times do
3943+
AboutToBeTooComplex.new.instance_variable_set(:"@a#{_1}", 1)
3944+
end
3945+
AboutToBeTooComplex.new.test
3946+
TEST = AboutToBeTooComplex.instance_method(:test)
3947+
"#);
3948+
assert_snapshot!(hir_string_proc("TEST"), @r"
3949+
fn test@<compiled>:3:
3950+
bb0():
3951+
EntryPoint interpreter
3952+
v1:BasicObject = LoadSelf
3953+
Jump bb2(v1)
3954+
bb1(v4:BasicObject):
3955+
EntryPoint JIT(0)
3956+
Jump bb2(v4)
3957+
bb2(v6:BasicObject):
3958+
v10:Fixnum[5] = Const Value(5)
3959+
PatchPoint SingleRactorMode
3960+
IncrCounter setivar_fallback_new_shape_too_complex
3961+
SetIvar v6, :@abc, v10
3962+
CheckInterrupts
3963+
Return v10
3964+
");
3965+
}
3966+
39353967
#[test]
39363968
fn test_elide_freeze_with_frozen_hash() {
39373969
eval("

0 commit comments

Comments
 (0)