Skip to content

Commit 2cc7a56

Browse files
authored
YJIT: Avoid leaks by skipping objects with a singleton class
For receiver with a singleton class, there are multiple vectors YJIT can end up retaining the object. There is a path in jit_guard_known_klass() that bakes the receiver into the code, and the object could also be kept alive indirectly through a path starting at the CME object baked into the code. To avoid these leaks, avoid compiling calls on objects with a singleton class. See: Shopify#552 [Bug #20209]
1 parent f769d68 commit 2cc7a56

File tree

4 files changed

+21
-0
lines changed

4 files changed

+21
-0
lines changed

yjit/bindgen/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,7 @@ fn main() {
447447
.allowlist_function("rb_obj_is_proc")
448448
.allowlist_function("rb_vm_base_ptr")
449449
.allowlist_function("rb_ec_stack_check")
450+
.allowlist_function("rb_vm_top_self")
450451

451452
// We define VALUE manually, don't import it
452453
.blocklist_type("VALUE")

yjit/src/codegen.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7258,6 +7258,17 @@ fn gen_send_general(
72587258
assert_eq!(RUBY_T_CLASS, comptime_recv_klass.builtin_type(),
72597259
"objects visible to ruby code should have a T_CLASS in their klass field");
72607260

7261+
// Don't compile calls through singleton classes to avoid retaining the receiver.
7262+
// Make an exception for class methods since classes tend to be retained anyways.
7263+
// Also compile calls on top_self to help tests.
7264+
if VALUE(0) != unsafe { FL_TEST(comptime_recv_klass, VALUE(RUBY_FL_SINGLETON as usize)) }
7265+
&& comptime_recv != unsafe { rb_vm_top_self() }
7266+
&& !unsafe { RB_TYPE_P(comptime_recv, RUBY_T_CLASS) }
7267+
&& !unsafe { RB_TYPE_P(comptime_recv, RUBY_T_MODULE) } {
7268+
gen_counter_incr(asm, Counter::send_singleton_class);
7269+
return None;
7270+
}
7271+
72617272
// Points to the receiver operand on the stack
72627273
let recv = asm.stack_opnd(recv_idx);
72637274
let recv_opnd: YARVOpnd = recv.into();
@@ -8038,6 +8049,12 @@ fn gen_invokesuper_specialized(
80388049
return None;
80398050
}
80408051

8052+
// Don't compile `super` on objects with singleton class to avoid retaining the receiver.
8053+
if VALUE(0) != unsafe { FL_TEST(comptime_recv.class_of(), VALUE(RUBY_FL_SINGLETON as usize)) } {
8054+
gen_counter_incr(asm, Counter::invokesuper_singleton_class);
8055+
return None;
8056+
}
8057+
80418058
// Do method lookup
80428059
let cme = unsafe { rb_callable_method_entry(comptime_superclass, mid) };
80438060
if cme.is_null() {

yjit/src/cruby_bindings.inc.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -975,6 +975,7 @@ extern "C" {
975975
n: ::std::os::raw::c_long,
976976
elts: *const VALUE,
977977
) -> VALUE;
978+
pub fn rb_vm_top_self() -> VALUE;
978979
pub static mut rb_vm_insns_count: u64;
979980
pub fn rb_method_entry_at(obj: VALUE, id: ID) -> *const rb_method_entry_t;
980981
pub fn rb_callable_method_entry(klass: VALUE, id: ID) -> *const rb_callable_method_entry_t;

yjit/src/stats.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,7 @@ make_counters! {
319319
// Method calls that fallback to dynamic dispatch
320320
send_keywords,
321321
send_kw_splat,
322+
send_singleton_class,
322323
send_args_splat_super,
323324
send_iseq_zsuper,
324325
send_block_arg,
@@ -405,6 +406,7 @@ make_counters! {
405406
invokesuper_no_me,
406407
invokesuper_not_iseq_or_cfunc,
407408
invokesuper_refinement,
409+
invokesuper_singleton_class,
408410

409411
invokeblock_megamorphic,
410412
invokeblock_none,

0 commit comments

Comments
 (0)