From 21cb1c9e929e9aaad24a9b64576aceb54b8e4e7a Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Wed, 6 Aug 2025 01:25:27 -0400 Subject: [PATCH 1/7] ZJIT: x86: split: Fix live ranges index-out-of-range panic Previously we crashed panicked due to index bounds check running test_fixnum.rb. On ARM and in other places in the x86 backend, this isn't a problem because they inspect the output of instructions which is never replaced. --- zjit/src/backend/x86_64/mod.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index f7aa1acc4dae63..8027c74b182e66 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -148,6 +148,15 @@ impl Assembler }; } + // When we split an operand, we can create a new VReg not in `live_ranges`. + // So when we see a VReg with out-of-range index, it's created from splitting + // from the loop above and we know it doesn't outlive the current instruction. + let vreg_outlives_insn = |vreg_idx| { + live_ranges + .get(vreg_idx) + .map_or(false, |live_range: &LiveRange| live_range.end() > index) + }; + // We are replacing instructions here so we know they are already // being used. It is okay not to use their output here. #[allow(unused_must_use)] @@ -183,7 +192,7 @@ impl Assembler }, // Instruction output whose live range spans beyond this instruction (Opnd::VReg { idx, .. }, _) => { - if live_ranges[idx].end() > index { + if vreg_outlives_insn(idx) { *left = asm.load(*left); } }, @@ -248,7 +257,7 @@ impl Assembler match opnd { // Instruction output whose live range spans beyond this instruction Opnd::VReg { idx, .. } => { - if live_ranges[*idx].end() > index { + if vreg_outlives_insn(*idx) { *opnd = asm.load(*opnd); } }, @@ -272,7 +281,7 @@ impl Assembler // If we have an instruction output whose live range // spans beyond this instruction, we have to load it. Opnd::VReg { idx, .. } => { - if live_ranges[idx].end() > index { + if vreg_outlives_insn(idx) { *truthy = asm.load(*truthy); } }, @@ -307,7 +316,7 @@ impl Assembler // If we have an instruction output whose live range // spans beyond this instruction, we have to load it. Opnd::VReg { idx, .. } => { - if live_ranges[idx].end() > index { + if vreg_outlives_insn(idx) { *opnd = asm.load(*opnd); } }, From e378a21a32bb0ca26f206f975073760a4e1a8bef Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Wed, 6 Aug 2025 01:27:16 -0400 Subject: [PATCH 2/7] ZJIT: Run TestFixnum --- test/.excludes-zjit/TestFixnum.rb | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 test/.excludes-zjit/TestFixnum.rb diff --git a/test/.excludes-zjit/TestFixnum.rb b/test/.excludes-zjit/TestFixnum.rb deleted file mode 100644 index aaf8760f2f9ded..00000000000000 --- a/test/.excludes-zjit/TestFixnum.rb +++ /dev/null @@ -1,2 +0,0 @@ -# Issue: https://github.com/Shopify/ruby/issues/646 -exclude(/test_/, 'Tests make ZJIT panic on Ubuntu') From 4a70f946a7131da871ca2aef8256a8b94299eba6 Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Wed, 6 Aug 2025 21:51:41 +0100 Subject: [PATCH 3/7] ZJIT: Implement SingleRactorMode invalidation (#14121) * ZJIT: Implement SingleRactorMode invalidation * ZJIT: Add macro for compiling jumps * ZJIT: Fix typo in comment * YJIT: Fix typo in comment * ZJIT: Avoid using unexported types in zjit.h `enum ruby_vminsn_type` is declared in `insns.inc` and is not exported. Using it in `zjit.h` would cause build errors when the file including it doesn't include `insns.inc`. --- depend | 1 + ractor.c | 2 ++ test/ruby/test_zjit.rb | 20 +++++++++++ yjit/src/invariants.rs | 2 +- zjit.h | 8 +++-- zjit/src/codegen.rs | 8 ++--- zjit/src/invariants.rs | 75 ++++++++++++++++++++++++++++-------------- zjit/src/profile.rs | 4 +-- 8 files changed, 85 insertions(+), 35 deletions(-) diff --git a/depend b/depend index ecaf33b1c7710c..ea2486e9e8da1d 100644 --- a/depend +++ b/depend @@ -12702,6 +12702,7 @@ ractor.$(OBJEXT): {$(VPATH)}vm_debug.h ractor.$(OBJEXT): {$(VPATH)}vm_opts.h ractor.$(OBJEXT): {$(VPATH)}vm_sync.h ractor.$(OBJEXT): {$(VPATH)}yjit.h +ractor.$(OBJEXT): {$(VPATH)}zjit.h random.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h random.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h random.$(OBJEXT): $(CCAN_DIR)/list/list.h diff --git a/ractor.c b/ractor.c index dc83ccc1b408c8..096bda5df6dc1a 100644 --- a/ractor.c +++ b/ractor.c @@ -19,6 +19,7 @@ #include "internal/thread.h" #include "variable.h" #include "yjit.h" +#include "zjit.h" VALUE rb_cRactor; static VALUE rb_cRactorSelector; @@ -511,6 +512,7 @@ ractor_create(rb_execution_context_t *ec, VALUE self, VALUE loc, VALUE name, VAL r->debug = cr->debug; rb_yjit_before_ractor_spawn(); + rb_zjit_before_ractor_spawn(); rb_thread_create_ractor(r, args, block); RB_GC_GUARD(rv); diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index e3ef6f65698ad6..ca6293ce040704 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -950,6 +950,26 @@ def test = A::B::C RUBY end + def test_single_ractor_mode_invalidation + # Without invalidating the single-ractor mode, the test would crash + assert_compiles '"errored but not crashed"', <<~RUBY, call_threshold: 2, insns: [:opt_getconstant_path] + C = Object.new + + def test + C + rescue Ractor::IsolationError + "errored but not crashed" + end + + test + test + + Ractor.new { + test + }.value + RUBY + end + def test_dupn assert_compiles '[[1], [1, 1], :rhs, [nil, :rhs]]', <<~RUBY, insns: [:dupn] def test(array) = (array[1, 2] ||= :rhs) diff --git a/yjit/src/invariants.rs b/yjit/src/invariants.rs index a1a7d300aa13b3..6ae1342ce38977 100644 --- a/yjit/src/invariants.rs +++ b/yjit/src/invariants.rs @@ -303,7 +303,7 @@ pub extern "C" fn rb_yjit_cme_invalidate(callee_cme: *const rb_callable_method_e }); } -/// Callback for then Ruby is about to spawn a ractor. In that case we need to +/// Callback for when Ruby is about to spawn a ractor. In that case we need to /// invalidate every block that is assuming single ractor mode. #[no_mangle] pub extern "C" fn rb_yjit_before_ractor_spawn() { diff --git a/zjit.h b/zjit.h index 5ce2826d067a2b..adf47046f83b37 100644 --- a/zjit.h +++ b/zjit.h @@ -14,7 +14,7 @@ extern bool rb_zjit_enabled_p; extern uint64_t rb_zjit_call_threshold; extern uint64_t rb_zjit_profile_threshold; void rb_zjit_compile_iseq(const rb_iseq_t *iseq, rb_execution_context_t *ec, bool jit_exception); -void rb_zjit_profile_insn(enum ruby_vminsn_type insn, rb_execution_context_t *ec); +void rb_zjit_profile_insn(uint32_t insn, rb_execution_context_t *ec); void rb_zjit_profile_enable(const rb_iseq_t *iseq); void rb_zjit_bop_redefined(int redefined_flag, enum ruby_basic_operators bop); void rb_zjit_cme_invalidate(const rb_callable_method_entry_t *cme); @@ -22,15 +22,17 @@ void rb_zjit_invalidate_ep_is_bp(const rb_iseq_t *iseq); void rb_zjit_constant_state_changed(ID id); void rb_zjit_iseq_mark(void *payload); void rb_zjit_iseq_update_references(void *payload); +void rb_zjit_before_ractor_spawn(void); #else #define rb_zjit_enabled_p false static inline void rb_zjit_compile_iseq(const rb_iseq_t *iseq, rb_execution_context_t *ec, bool jit_exception) {} -static inline void rb_zjit_profile_insn(enum ruby_vminsn_type insn, rb_execution_context_t *ec) {} +static inline void rb_zjit_profile_insn(uint32_t insn, rb_execution_context_t *ec) {} static inline void rb_zjit_profile_enable(const rb_iseq_t *iseq) {} static inline void rb_zjit_bop_redefined(int redefined_flag, enum ruby_basic_operators bop) {} static inline void rb_zjit_cme_invalidate(const rb_callable_method_entry_t *cme) {} static inline void rb_zjit_invalidate_ep_is_bp(const rb_iseq_t *iseq) {} static inline void rb_zjit_constant_state_changed(ID id) {} -#endif // #if USE_YJIT +static inline void rb_zjit_before_ractor_spawn(void) {} +#endif // #if USE_ZJIT #endif // #ifndef ZJIT_H diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 0db4d6b781686a..b05aaca68222ae 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -4,7 +4,7 @@ use std::ffi::{c_int, c_void}; use crate::asm::Label; use crate::backend::current::{Reg, ALLOC_REGS}; -use crate::invariants::{track_bop_assumption, track_cme_assumption, track_stable_constant_names_assumption}; +use crate::invariants::{track_bop_assumption, track_cme_assumption, track_single_ractor_assumption, track_stable_constant_names_assumption}; use crate::gc::{get_or_create_iseq_payload, append_gc_offsets}; use crate::state::ZJITState; use crate::stats::{counter_ptr, Counter}; @@ -542,9 +542,9 @@ fn gen_patch_point(jit: &mut JITState, asm: &mut Assembler, invariant: &Invarian let side_exit_ptr = cb.resolve_label(label); track_stable_constant_names_assumption(idlist, code_ptr, side_exit_ptr); } - _ => { - debug!("ZJIT: gen_patch_point: unimplemented invariant {invariant:?}"); - return; + Invariant::SingleRactorMode => { + let side_exit_ptr = cb.resolve_label(label); + track_single_ractor_assumption(code_ptr, side_exit_ptr); } } }); diff --git a/zjit/src/invariants.rs b/zjit/src/invariants.rs index 25cffb970e62dc..c8c91dc45b97a1 100644 --- a/zjit/src/invariants.rs +++ b/zjit/src/invariants.rs @@ -1,7 +1,20 @@ -use std::{collections::{HashMap, HashSet}}; +use std::{collections::{HashMap, HashSet}, mem}; use crate::{backend::lir::{asm_comment, Assembler}, cruby::{rb_callable_method_entry_t, ruby_basic_operators, src_loc, with_vm_lock, IseqPtr, RedefinitionFlag, ID}, hir::Invariant, options::debug, state::{zjit_enabled_p, ZJITState}, virtualmem::CodePtr}; +macro_rules! compile_jumps { + ($cb:expr, $jumps:expr, $($comment_args:tt)*) => { + for jump in $jumps { + $cb.with_write_ptr(jump.from, |cb| { + let mut asm = Assembler::new(); + asm_comment!(asm, $($comment_args)*); + asm.jmp(jump.to.into()); + asm.compile(cb).expect("can write existing code"); + }); + } + }; +} + #[derive(Debug, Eq, Hash, PartialEq)] struct Jump { from: CodePtr, @@ -26,6 +39,9 @@ pub struct Invariants { /// Map from constant ID to patch points that assume the constant hasn't been redefined constant_state_patch_points: HashMap>, + + /// Set of patch points that assume that the interpreter is running with only one ractor + single_ractor_patch_points: HashSet, } /// Called when a basic operator is redefined. Note that all the blocks assuming @@ -46,14 +62,7 @@ pub extern "C" fn rb_zjit_bop_redefined(klass: RedefinitionFlag, bop: ruby_basic debug!("BOP is redefined: {}", bop); // Invalidate all patch points for this BOP - for jump in jumps { - cb.with_write_ptr(jump.from, |cb| { - let mut asm = Assembler::new(); - asm_comment!(asm, "BOP is redefined: {}", bop); - asm.jmp(jump.to.into()); - asm.compile(cb).expect("can write existing code"); - }); - } + compile_jumps!(cb, jumps, "BOP is redefined: {}", bop); cb.mark_all_executable(); } @@ -159,14 +168,8 @@ pub extern "C" fn rb_zjit_cme_invalidate(cme: *const rb_callable_method_entry_t) debug!("CME is invalidated: {:?}", cme); // Invalidate all patch points for this CME - for jump in jumps { - cb.with_write_ptr(jump.from, |cb| { - let mut asm = Assembler::new(); - asm_comment!(asm, "CME is invalidated: {:?}", cme); - asm.jmp(jump.to.into()); - asm.compile(cb).expect("can write existing code"); - }); - } + compile_jumps!(cb, jumps, "CME is invalidated: {:?}", cme); + cb.mark_all_executable(); } }); @@ -187,16 +190,38 @@ pub extern "C" fn rb_zjit_constant_state_changed(id: ID) { debug!("Constant state changed: {:?}", id); // Invalidate all patch points for this constant ID - for jump in jumps { - cb.with_write_ptr(jump.from, |cb| { - let mut asm = Assembler::new(); - asm_comment!(asm, "Constant state changed: {:?}", id); - asm.jmp(jump.to.into()); - asm.compile(cb).expect("can write existing code"); - }); - } + compile_jumps!(cb, jumps, "Constant state changed: {:?}", id); cb.mark_all_executable(); } }); } + +/// Track the JIT code that assumes that the interpreter is running with only one ractor +pub fn track_single_ractor_assumption(patch_point_ptr: CodePtr, side_exit_ptr: CodePtr) { + let invariants = ZJITState::get_invariants(); + invariants.single_ractor_patch_points.insert(Jump { + from: patch_point_ptr, + to: side_exit_ptr, + }); +} + +/// Callback for when Ruby is about to spawn a ractor. In that case we need to +/// invalidate every block that is assuming single ractor mode. +#[unsafe(no_mangle)] +pub extern "C" fn rb_zjit_before_ractor_spawn() { + // If ZJIT isn't enabled, do nothing + if !zjit_enabled_p() { + return; + } + + with_vm_lock(src_loc!(), || { + let cb = ZJITState::get_code_block(); + let jumps = mem::take(&mut ZJITState::get_invariants().single_ractor_patch_points); + + // Invalidate all patch points for single ractor mode + compile_jumps!(cb, jumps, "Another ractor spawned, invalidating single ractor mode assumption"); + + cb.mark_all_executable(); + }); +} diff --git a/zjit/src/profile.rs b/zjit/src/profile.rs index a99229604b0ce2..12b10b98ee57bc 100644 --- a/zjit/src/profile.rs +++ b/zjit/src/profile.rs @@ -39,10 +39,10 @@ impl Profiler { /// API called from zjit_* instruction. opcode is the bare (non-zjit_*) instruction. #[unsafe(no_mangle)] -pub extern "C" fn rb_zjit_profile_insn(bare_opcode: ruby_vminsn_type, ec: EcPtr) { +pub extern "C" fn rb_zjit_profile_insn(bare_opcode: u32, ec: EcPtr) { with_vm_lock(src_loc!(), || { let mut profiler = Profiler::new(ec); - profile_insn(&mut profiler, bare_opcode); + profile_insn(&mut profiler, bare_opcode as ruby_vminsn_type); }); } From ba4a36e226d9a4203b0721f456e894efe4629bd0 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Wed, 6 Aug 2025 13:56:01 -0700 Subject: [PATCH 4/7] ZJIT: Inline attr_reader/attr_accessor (#14126) We can rewrite SendWithoutBlock to GetIvar. --- test/ruby/test_zjit.rb | 32 +++++++++++ zjit/src/hir.rs | 125 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 144 insertions(+), 13 deletions(-) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index ca6293ce040704..8115a601668422 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -887,6 +887,38 @@ def test() = @foo = 1 } end + def test_attr_reader + assert_compiles '[4, 4]', %q{ + class C + attr_reader :foo + + def initialize + @foo = 4 + end + end + + def test(c) = c.foo + c = C.new + [test(c), test(c)] + }, call_threshold: 2, insns: [:opt_send_without_block] + end + + def test_attr_accessor + assert_compiles '[4, 4]', %q{ + class C + attr_accessor :foo + + def initialize + @foo = 4 + end + end + + def test(c) = c.foo + c = C.new + [test(c), test(c)] + }, call_threshold: 2, insns: [:opt_send_without_block] + end + def test_uncached_getconstant_path assert_compiles RUBY_COPYRIGHT.dump, %q{ def test = RUBY_COPYRIGHT diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 1a67037ed35733..7e92f5932922a8 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -1537,22 +1537,31 @@ impl Function { // It allows you to use a faster ISEQ if possible. cme = unsafe { rb_check_overloaded_cme(cme, ci) }; let def_type = unsafe { get_cme_def_type(cme) }; - if def_type != VM_METHOD_TYPE_ISEQ { + if def_type == VM_METHOD_TYPE_ISEQ { // TODO(max): Allow non-iseq; cache cme + // Only specialize positional-positional calls + // TODO(max): Handle other kinds of parameter passing + let iseq = unsafe { get_def_iseq_ptr((*cme).def) }; + if !can_direct_send(iseq) { + self.push_insn_id(block, insn_id); continue; + } + self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state }); + if let Some(profiled_type) = profiled_type { + self_val = self.push_insn(block, Insn::GuardType { val: self_val, guard_type: Type::from_profiled_type(profiled_type), state }); + } + let send_direct = self.push_insn(block, Insn::SendWithoutBlockDirect { self_val, cd, cme, iseq, args, state }); + self.make_equal_to(insn_id, send_direct); + } else if def_type == VM_METHOD_TYPE_IVAR && args.is_empty() { + self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state }); + if let Some(profiled_type) = profiled_type { + self_val = self.push_insn(block, Insn::GuardType { val: self_val, guard_type: Type::from_profiled_type(profiled_type), state }); + } + let id = unsafe { get_cme_def_body_attr_id(cme) }; + let getivar = self.push_insn(block, Insn::GetIvar { self_val, id, state }); + self.make_equal_to(insn_id, getivar); + } else { self.push_insn_id(block, insn_id); continue; } - // Only specialize positional-positional calls - // TODO(max): Handle other kinds of parameter passing - let iseq = unsafe { get_def_iseq_ptr((*cme).def) }; - if !can_direct_send(iseq) { - self.push_insn_id(block, insn_id); continue; - } - self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state }); - if let Some(profiled_type) = profiled_type { - self_val = self.push_insn(block, Insn::GuardType { val: self_val, guard_type: Type::from_profiled_type(profiled_type), state }); - } - let send_direct = self.push_insn(block, Insn::SendWithoutBlockDirect { self_val, cd, cme, iseq, args, state }); - self.make_equal_to(insn_id, send_direct); } Insn::GetConstantPath { ic, state, .. } => { let idlist: *const ID = unsafe { (*ic).segments }; @@ -7422,4 +7431,94 @@ mod opt_tests { Return v7 "#]]); } + + #[test] + fn test_inline_attr_reader_constant() { + eval(" + class C + attr_reader :foo + end + + O = C.new + def test = O.foo + test + test + "); + assert_optimized_method_hir("test", expect![[r#" + fn test@:7: + bb0(v0:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, O) + v9:BasicObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(C@0x1010, foo@0x1018, cme:0x1020) + v11:BasicObject = GetIvar v9, :@foo + Return v11 + "#]]); + } + + #[test] + fn test_inline_attr_accessor_constant() { + eval(" + class C + attr_accessor :foo + end + + O = C.new + def test = O.foo + test + test + "); + assert_optimized_method_hir("test", expect![[r#" + fn test@:7: + bb0(v0:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, O) + v9:BasicObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(C@0x1010, foo@0x1018, cme:0x1020) + v11:BasicObject = GetIvar v9, :@foo + Return v11 + "#]]); + } + + #[test] + fn test_inline_attr_reader() { + eval(" + class C + attr_reader :foo + end + + def test(o) = o.foo + test C.new + test C.new + "); + assert_optimized_method_hir("test", expect![[r#" + fn test@:6: + bb0(v0:BasicObject, v1:BasicObject): + PatchPoint MethodRedefined(C@0x1000, foo@0x1008, cme:0x1010) + v7:BasicObject[class_exact:C] = GuardType v1, BasicObject[class_exact:C] + v8:BasicObject = GetIvar v7, :@foo + Return v8 + "#]]); + } + + #[test] + fn test_inline_attr_accessor() { + eval(" + class C + attr_accessor :foo + end + + def test(o) = o.foo + test C.new + test C.new + "); + assert_optimized_method_hir("test", expect![[r#" + fn test@:6: + bb0(v0:BasicObject, v1:BasicObject): + PatchPoint MethodRedefined(C@0x1000, foo@0x1008, cme:0x1010) + v7:BasicObject[class_exact:C] = GuardType v1, BasicObject[class_exact:C] + v8:BasicObject = GetIvar v7, :@foo + Return v8 + "#]]); + } } From a9f6fe0914cb65f16cdf82390fa070ad76e54a8d Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 6 Aug 2025 14:05:03 -0700 Subject: [PATCH 5/7] Avoid marking CC children after invalidation Once klass becomes Qundef, it's disconnected and won't be invalidated when the CME is. So once that happens we must not mark or attempt to move the cme_ field. --- imemo.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/imemo.c b/imemo.c index 7298d78d65cdb7..2c721ca9112250 100644 --- a/imemo.c +++ b/imemo.c @@ -337,28 +337,37 @@ rb_imemo_mark_and_move(VALUE obj, bool reference_updating) * cc->klass (klass) should not be marked because if the klass is * free'ed, the cc->klass will be cleared by `vm_cc_invalidate()`. * - * cc->cme (cme) should not be marked because if cc is invalidated - * when cme is free'ed. + * For "normal" CCs cc->cme (cme) should not be marked because the cc is + * invalidated through the klass when the cme is free'd. * - klass marks cme if klass uses cme. - * - caller classe's ccs->cme marks cc->cme. - * - if cc is invalidated (klass doesn't refer the cc), - * cc is invalidated by `vm_cc_invalidate()` and cc->cme is - * not be accessed. - * - On the multi-Ractors, cme will be collected with global GC + * - caller class's ccs->cme marks cc->cme. + * - if cc is invalidated (klass doesn't refer the cc), cc is + * invalidated by `vm_cc_invalidate()` after which cc->cme must not + * be accessed. + * - With multi-Ractors, cme will be collected with global GC * so that it is safe if GC is not interleaving while accessing * cc and cme. - * - However, cc_type_super and cc_type_refinement are not chained - * from ccs so cc->cme should be marked; the cme might be - * reachable only through cc in these cases. + * + * However cc_type_super and cc_type_refinement are not chained + * from ccs so cc->cme should be marked as long as the cc is valid; + * the cme might be reachable only through cc in these cases. */ struct rb_callcache *cc = (struct rb_callcache *)obj; - if (reference_updating) { + if (UNDEF_P(cc->klass)) { + /* If it's invalidated, we must not mark anything. + * All fields should are considered invalid + */ + } + else if (reference_updating) { if (moved_or_living_object_strictly_p((VALUE)cc->cme_)) { *((VALUE *)&cc->klass) = rb_gc_location(cc->klass); *((struct rb_callable_method_entry_struct **)&cc->cme_) = (struct rb_callable_method_entry_struct *)rb_gc_location((VALUE)cc->cme_); + + RUBY_ASSERT(RB_TYPE_P(cc->klass, T_CLASS) || RB_TYPE_P(cc->klass, T_ICLASS)); + RUBY_ASSERT(IMEMO_TYPE_P((VALUE)cc->cme_, imemo_ment)); } - else if (vm_cc_valid(cc)) { + else { vm_cc_invalidate(cc); } } From 640a2f1dc77c0ecf226dbd71cf7a1eb876a1f037 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 6 Aug 2025 13:44:46 -0700 Subject: [PATCH 6/7] Ensure ObjectSpace.dump won't call cc_cme on invalidated CC --- ext/objspace/objspace_dump.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/ext/objspace/objspace_dump.c b/ext/objspace/objspace_dump.c index f90ad89b5a6ed7..94a9d43f98243b 100644 --- a/ext/objspace/objspace_dump.c +++ b/ext/objspace/objspace_dump.c @@ -451,13 +451,16 @@ dump_object(VALUE obj, struct dump_config *dc) break; case imemo_callcache: - mid = vm_cc_cme((const struct rb_callcache *)obj)->called_id; - if (mid != 0) { - dump_append(dc, ", \"called_id\":"); - dump_append_id(dc, mid); - + { VALUE klass = ((const struct rb_callcache *)obj)->klass; - if (klass != 0) { + if (klass != Qundef) { + mid = vm_cc_cme((const struct rb_callcache *)obj)->called_id; + if (mid != 0) { + dump_append(dc, ", \"called_id\":"); + dump_append_id(dc, mid); + + } + dump_append(dc, ", \"receiver_class\":"); dump_append_ref(dc, klass); } From fccd96cc6c3cc9f500dc87ae9be65aaa212b02fa Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 6 Aug 2025 13:53:00 -0700 Subject: [PATCH 7/7] Add stricter assertions on CC access --- vm_callinfo.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/vm_callinfo.h b/vm_callinfo.h index 3b6880e3205920..79ccbfa7abb7c3 100644 --- a/vm_callinfo.h +++ b/vm_callinfo.h @@ -418,6 +418,8 @@ static inline const struct rb_callable_method_entry_struct * vm_cc_cme(const struct rb_callcache *cc) { VM_ASSERT(IMEMO_TYPE_P(cc, imemo_callcache)); + VM_ASSERT(cc->klass != Qundef || !vm_cc_markable(cc)); + VM_ASSERT(cc_check_class(cc->klass)); VM_ASSERT(cc->call_ == NULL || // not initialized yet !vm_cc_markable(cc) || cc->cme_ != NULL); @@ -430,6 +432,8 @@ vm_cc_call(const struct rb_callcache *cc) { VM_ASSERT(IMEMO_TYPE_P(cc, imemo_callcache)); VM_ASSERT(cc->call_ != NULL); + VM_ASSERT(cc->klass != Qundef || !vm_cc_markable(cc)); + VM_ASSERT(cc_check_class(cc->klass)); return cc->call_; }