Skip to content

Commit

Permalink
YJIT: Relax --yjit-verify-ctx after singleton class creation
Browse files Browse the repository at this point in the history
Types like `Type::CString` really only assert that at one point the object had
its class field equal to `String`. Once a singleton class is created for any
strings, the type makes no assertion about any class field anymore, and becomes
the same as `Type::TString`.

Previously, the `--yjit-verify-ctx` option wasn't allowing objects of these
kind that have have singleton classes to pass verification even though the code
generators handle it just fine.

Found through `ruby/spec`.
  • Loading branch information
XrXr committed Apr 25, 2024
1 parent 49753cd commit 9b5bc8e
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 4 deletions.
14 changes: 14 additions & 0 deletions bootstraptest/test_yjit.rb
Expand Up @@ -4830,6 +4830,20 @@ def to_int_99.to_int = 99
str
}

# test --yjit-verify-ctx for arrays with a singleton class
assert_equal "ok", %q{
class Array
def foo
self.singleton_class.define_method(:first) { :ok }
first
end
end
def test = [].foo
test
}

assert_equal '["raised", "Module", "Object"]', %q{
def foo(obj)
obj.superclass.name
Expand Down
26 changes: 25 additions & 1 deletion yjit/src/codegen.rs
Expand Up @@ -567,14 +567,36 @@ fn verify_ctx(jit: &JITState, ctx: &Context) {
unsafe { CStr::from_ptr(rb_obj_info(val)).to_str().unwrap() }
}

// Some types such as CString only assert the class field of the object
// when there has never been a singleton class created for objects of that class.
// Once there is a singleton class created they become their weaker
// `T*` variant, and we more objects should pass the verification.
fn relax_type_with_singleton_class_assumption(ty: Type) -> Type {
if let Type::CString | Type::CArray | Type::CHash = ty {
if has_singleton_class_of(ty.known_class().unwrap()) {
match ty {
Type::CString => return Type::TString,
Type::CArray => return Type::TArray,
Type::CHash => return Type::THash,
_ => (),
}
}
}

ty
}

// Only able to check types when at current insn
assert!(jit.at_current_insn());

let self_val = jit.peek_at_self();
let self_val_type = Type::from(self_val);
let learned_self_type = ctx.get_opnd_type(SelfOpnd);
let learned_self_type = relax_type_with_singleton_class_assumption(learned_self_type);


// Verify self operand type
if self_val_type.diff(ctx.get_opnd_type(SelfOpnd)) == TypeDiff::Incompatible {
if self_val_type.diff(learned_self_type) == TypeDiff::Incompatible {
panic!(
"verify_ctx: ctx self type ({:?}) incompatible with actual value of self {}",
ctx.get_opnd_type(SelfOpnd),
Expand All @@ -587,6 +609,7 @@ fn verify_ctx(jit: &JITState, ctx: &Context) {
for i in 0..top_idx {
let learned_mapping = ctx.get_opnd_mapping(StackOpnd(i));
let learned_type = ctx.get_opnd_type(StackOpnd(i));
let learned_type = relax_type_with_singleton_class_assumption(learned_type);

let stack_val = jit.peek_at_stack(ctx, i as isize);
let val_type = Type::from(stack_val);
Expand Down Expand Up @@ -632,6 +655,7 @@ fn verify_ctx(jit: &JITState, ctx: &Context) {
let top_idx: usize = cmp::min(local_table_size as usize, MAX_TEMP_TYPES);
for i in 0..top_idx {
let learned_type = ctx.get_local_type(i);
let learned_type = relax_type_with_singleton_class_assumption(learned_type);
let local_val = jit.peek_at_local(i as i32);
let local_type = Type::from(local_val);

Expand Down
6 changes: 3 additions & 3 deletions yjit/src/core.rs
Expand Up @@ -53,11 +53,11 @@ pub enum Type {
ImmSymbol,

TString, // An object with the T_STRING flag set, possibly an rb_cString
CString, // An un-subclassed string of type rb_cString (can have instance vars in some cases)
CString, // An object that at one point had its class field equal rb_cString (creating a singleton class changes it)
TArray, // An object with the T_ARRAY flag set, possibly an rb_cArray
CArray, // An un-subclassed array of type rb_cArray (can have instance vars in some cases)
CArray, // An object that at one point had its class field equal rb_cArray (creating a singleton class changes it)
THash, // An object with the T_HASH flag set, possibly an rb_cHash
CHash, // An un-subclassed hash of type rb_cHash (can have instance vars in some cases)
CHash, // An object that at one point had its class field equal rb_cHash (creating a singleton class changes it)

BlockParamProxy, // A special sentinel value indicating the block parameter should be read from
// the current surrounding cfp
Expand Down

0 comments on commit 9b5bc8e

Please sign in to comment.