[pull] master from ruby:master

ext/objspace/objspace.c

@@ -30,19 +30,33 @@
 
 /*
  *  call-seq:
- *    ObjectSpace.memsize_of(obj) -> Integer
+ *    ObjectSpace.memsize_of(obj) -> integer
  *
- *  Return consuming memory size of obj in bytes.
+ *  Returns the amount of memory in bytes consumed by +obj+.
  *
- *  Note that the return size is incomplete.  You need to deal with this
- *  information as only a *HINT*. Especially, the size of +T_DATA+ may not be
- *  correct.
+ *  The returned size includes the slot that +obj+ occupies plus any memory
+ *  that +obj+ allocates outside of that slot, such as the storage backing a
+ *  large String, Array, or Hash:
  *
- *  This method is only expected to work with CRuby.
+ *    require 'objspace'
+ *
+ *    ObjectSpace.memsize_of("small")        # => 40
+ *    ObjectSpace.memsize_of("a" * 1000)     # => 1041
+ *    ObjectSpace.memsize_of([1, 2, 3])      # => 40
+ *    ObjectSpace.memsize_of(Array.new(100)) # => 840
+ *
+ *  Special constants such as +true+, +false+, +nil+, small integers, and some
+ *  symbols do not occupy a slot, so their size is reported as +0+:
  *
- *  From Ruby 3.2 with Variable Width Allocation, it returns the actual slot
- *  size used plus any additional memory allocated outside the slot (such
- *  as external strings, arrays, or hash tables).
+ *    ObjectSpace.memsize_of(true)   # => 0
+ *    ObjectSpace.memsize_of(42)     # => 0
+ *
+ *  The returned size is only a hint and may be an underestimate, since it does
+ *  not account for all of the memory that +obj+ references. In particular, the
+ *  size of a +T_DATA+ object (an object implemented in C, such as one defined
+ *  by a C extension) may not be reported correctly.
+ *
+ *  This method is only expected to work with CRuby.
  */
 
 static VALUE

gc/default/default.c

@@ -3688,10 +3688,6 @@ gc_sweep_plane(rb_objspace_t *objspace, rb_heap_t *heap, uintptr_t p, bits_t bit
 #endif
 
                 if (!rb_gc_obj_needs_cleanup_p(vp)) {
-                    if (RB_UNLIKELY(objspace->hook_events & RUBY_INTERNAL_EVENT_FREEOBJ)) {
-                        rb_gc_event_hook(vp, RUBY_INTERNAL_EVENT_FREEOBJ);
-                    }
-
                     (void)VALGRIND_MAKE_MEM_UNDEFINED((void*)p, slot_size);
                     heap_page_add_freeobj(objspace, sweep_page, vp);
                     gc_report(3, objspace, "page_sweep: %s (fast path) added to freelist\n", rb_obj_info(vp));
@@ -3700,8 +3696,6 @@ gc_sweep_plane(rb_objspace_t *objspace, rb_heap_t *heap, uintptr_t p, bits_t bit
                 else {
                     gc_report(2, objspace, "page_sweep: free %p\n", (void *)p);
 
-                    rb_gc_event_hook(vp, RUBY_INTERNAL_EVENT_FREEOBJ);
-
                     rb_gc_obj_free_vm_weak_references(vp);
                     if (rb_gc_obj_free(objspace, vp)) {
                         (void)VALGRIND_MAKE_MEM_UNDEFINED((void*)p, slot_size);
@@ -3920,12 +3914,72 @@ gc_ractor_newobj_cache_clear(void *c, void *data)
     }
 }
 
+static void
+gc_sweep_freeobj_hooks_page(rb_objspace_t *objspace, struct heap_page *page)
+{
+    bits_t *bits = page->mark_bits;
+    uintptr_t p = (uintptr_t)page->start;
+    short slot_size = page->slot_size;
+    int total_slots = page->total_slots;
+    int bitmap_plane_count = CEILDIV(total_slots, BITS_BITLENGTH);
+
+    int out_of_range_bits = total_slots % BITS_BITLENGTH;
+    bits_t last_plane_mask = (out_of_range_bits != 0)
+        ? ~(((bits_t)1 << out_of_range_bits) - 1)
+        : 0;
+
+    for (int j = 0; j < bitmap_plane_count; j++) {
+        bits_t bitset = ~bits[j];
+        if (j == bitmap_plane_count - 1) {
+            bitset &= ~last_plane_mask;
+        }
+
+        uintptr_t pp = p;
+        while (bitset) {
+            if (bitset & 1) {
+                VALUE vp = (VALUE)pp;
+                asan_unpoisoning_object(vp) {
+                    switch (BUILTIN_TYPE(vp)) {
+                      case T_NONE:
+                      case T_ZOMBIE:
+                      case T_MOVED:
+                        break;
+                      default:
+                        rb_gc_event_hook(vp, RUBY_INTERNAL_EVENT_FREEOBJ);
+                        break;
+                    }
+                }
+            }
+            pp += slot_size;
+            bitset >>= 1;
+        }
+        p += BITS_BITLENGTH * slot_size;
+    }
+}
+
+static void
+gc_sweep_freeobj_hooks(rb_objspace_t *objspace)
+{
+    for (int i = 0; i < HEAP_COUNT; i++) {
+        rb_heap_t *heap = &heaps[i];
+        struct heap_page *page = NULL;
+
+        ccan_list_for_each(&heap->pages, page, page_node) {
+            gc_sweep_freeobj_hooks_page(objspace, page);
+        }
+    }
+}
+
 static void
 gc_sweep_start(rb_objspace_t *objspace)
 {
     gc_mode_transition(objspace, gc_mode_sweeping);
     objspace->rincgc.pooled_slots = 0;
 
+    if (RB_UNLIKELY(objspace->hook_events & RUBY_INTERNAL_EVENT_FREEOBJ)) {
+        gc_sweep_freeobj_hooks(objspace);
+    }
+
 #if GC_CAN_COMPILE_COMPACTION
     if (objspace->flags.during_compacting) {
         gc_sort_heap_by_compare_func(

zjit.h

@@ -57,19 +57,13 @@ void rb_zjit_materialize_frames(const rb_execution_context_t *ec, rb_control_fra
 // instead of a heap-allocated JITFrame pointer.
 #define ZJIT_JIT_RETURN_C_FRAME 0x1
 
-// BADFrame. The high bit is set, so likely SEGV on linux and darwin if dereferenced.
-#define ZJIT_JIT_RETURN_POISON 0xbadfbadfbadfbadfULL
-
 static inline const zjit_jit_frame_t *
 CFP_ZJIT_FRAME(const rb_control_frame_t *cfp)
 {
     if ((VALUE)cfp->jit_return == ZJIT_JIT_RETURN_C_FRAME) {
         return &rb_zjit_c_frame;
     }
     else {
-#if USE_ZJIT
-        RUBY_ASSERT((unsigned long long)((VALUE *)cfp->jit_return)[-1] != ZJIT_JIT_RETURN_POISON);
-#endif
         // Read JITFrame from the stack slot. gen_entry_point() writes an initial
         // frame describing the entry PC + iseq; subsequent gen_save_pc_for_gc()
         // calls update it with a more accurate PC before any non-leaf C call.

zjit/bindgen/src/main.rs

@@ -315,7 +315,6 @@ fn main() {
         .allowlist_function("rb_zjit_insn_leaf")
         .allowlist_type("jit_bindgen_constants")
         .allowlist_type("zjit_struct_offsets")
-        .allowlist_var("ZJIT_JIT_RETURN_POISON")
         .allowlist_var("ZJIT_JIT_RETURN_C_FRAME")
         .allowlist_function("rb_assert_holding_vm_lock")
         .allowlist_function("rb_jit_shape_complex_p")

zjit/src/codegen.rs

@@ -47,13 +47,6 @@ const PC_POISON: Option<*const VALUE> = if cfg!(feature = "runtime_checks") {
     None
 };
 
-/// Sentinel jit_return stored on ISEQ frame push when runtime checks are enabled.
-const JIT_RETURN_POISON: Option<usize> = if cfg!(feature = "runtime_checks") {
-    Some(ZJIT_JIT_RETURN_POISON as usize)
-} else {
-    None
-};
-
 /// Ephemeral code generation state
 struct JITState {
     /// ISEQ version that is being compiled, which will be used by PatchPoint
@@ -94,6 +87,11 @@ impl JITState {
         self.opnds[insn_id.0].unwrap_or_else(|| panic!("Failed to get_opnd({insn_id})"))
     }
 
+    /// Get the ISEQ for the version currently being compiled.
+    fn iseq(&self) -> IseqPtr {
+        unsafe { self.version.as_ref().iseq }
+    }
+
     /// Find or create a label for a given BlockId
     fn get_label(&mut self, asm: &mut Assembler, lir_block_id: lir::BlockId, hir_block_id: BlockId) -> Target {
         // Extend labels vector if the requested index is out of bounds
@@ -698,8 +696,14 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
         &Insn::UnboxFixnum { val } => gen_unbox_fixnum(asm, opnd!(val)),
         Insn::Test { val } => gen_test(asm, opnd!(val)),
         Insn::RefineType { val, .. } => opnd!(val),
-        Insn::HasType { val, expected } => gen_has_type(jit, asm, opnd!(val), *expected),
-        Insn::GuardType { val, guard_type, state } => gen_guard_type(jit, asm, opnd!(val), *guard_type, &function.frame_state(*state)),
+        Insn::HasType { val, expected } => {
+            let val_type = function.type_of(*val);
+            gen_has_type(jit, asm, opnd!(val), val_type, *expected)
+        }
+        Insn::GuardType { val, guard_type, state } => {
+            let val_type = function.type_of(*val);
+            gen_guard_type(jit, asm, opnd!(val), val_type, *guard_type, &function.frame_state(*state))
+        }
         &Insn::GuardBitEquals { val, expected, reason, state, recompile } => gen_guard_bit_equals(jit, asm, opnd!(val), expected, reason, recompile, &function.frame_state(state)),
         &Insn::GuardAnyBitSet { val, mask, reason, state, .. } => gen_guard_any_bit_set(jit, asm, opnd!(val), mask, reason, &function.frame_state(state)),
         &Insn::GuardNoBitsSet { val, mask, reason, state, .. } => gen_guard_no_bits_set(jit, asm, opnd!(val), mask, reason, &function.frame_state(state)),
@@ -2189,6 +2193,19 @@ fn gen_object_alloc_class(asm: &mut Assembler, class: VALUE, state: &FrameState)
     }
 }
 
+/// Map an entry point to the bytecode PC used by its initial JITFrame.
+/// JIT call entries use `opt_table[jit_entry_idx]`; the interpreter entry uses
+/// `opt_table.last()` for the fall-through path where all optionals are filled.
+fn entry_pc(iseq: IseqPtr, jit_entry_idx: Option<usize>) -> *const VALUE {
+    let params = unsafe { iseq.params() };
+    let opt_table = params.opt_table_slice();
+    let entry_idx = jit_entry_idx.unwrap_or_else(|| opt_table.len() - 1);
+    let entry_insn_idx = opt_table.get(entry_idx)
+        .unwrap_or_else(|| panic!("entry_pc: opt_table out of bounds. {params:#?}, entry_idx={entry_idx}"))
+        .as_u32();
+    unsafe { rb_iseq_pc_at_idx(iseq, entry_insn_idx) }
+}
+
 /// Compile a frame setup. If jit_entry_idx is Some, remember the address of it as a JIT entry.
 fn gen_entry_point(jit: &mut JITState, asm: &mut Assembler, jit_entry_idx: Option<usize>) {
     if let Some(jit_entry_idx) = jit_entry_idx {
@@ -2200,16 +2217,10 @@ fn gen_entry_point(jit: &mut JITState, asm: &mut Assembler, jit_entry_idx: Optio
     }
     asm.frame_setup(&[]);
 
-    // Publish the JITFrame slot's location via cfp->jit_return. The slot at
-    // [NATIVE_BASE_PTR - 8] is left uninitialized here; the JIT design relies on
-    // gen_save_pc_for_gc() to populate it before any C call, and on cross-ractor
-    // barriers ensuring that no other ractor scans this CFP before such a call.
+    // Publish a valid entry JITFrame before setting cfp->jit_return.
+    let jit_frame = JITFrame::new_iseq(entry_pc(jit.iseq(), jit_entry_idx), jit.iseq());
+    asm.mov(Opnd::mem(64, NATIVE_BASE_PTR, -SIZEOF_VALUE_I32), Opnd::const_ptr(jit_frame));
     asm.mov(Opnd::mem(64, CFP, RUBY_OFFSET_CFP_JIT_RETURN), NATIVE_BASE_PTR);
-
-    // Poison the JITFrame slot. It should be read only after gen_save_pc_for_gc().
-    if let Some(jit_return_poison) = JIT_RETURN_POISON {
-        asm.mov(Opnd::mem(64, NATIVE_BASE_PTR, -SIZEOF_VALUE_I32), jit_return_poison.into());
-    }
 }
 
 /// Compile code that exits from JIT code with a return value
@@ -2447,7 +2458,7 @@ fn gen_test(asm: &mut Assembler, val: lir::Opnd) -> lir::Opnd {
     asm.csel_e(0.into(), 1.into())
 }
 
-fn gen_has_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, ty: Type) -> lir::Opnd {
+fn gen_has_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, val_type: Type, ty: Type) -> lir::Opnd {
     if ty.is_subtype(types::Fixnum) {
         asm.test(val, Opnd::UImm(RUBY_FIXNUM_FLAG as u64));
         asm.csel_nz(Opnd::Imm(1), Opnd::Imm(0))
@@ -2490,13 +2501,16 @@ fn gen_has_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, ty: Typ
         // TODO: Max thinks codegen should not care about the shapes of the operands except to create them. (Shopify/ruby#685)
         let val = asm.load_mem(val);
 
-        // Immediate -> definitely not the class
-        asm.test(val, (RUBY_IMMEDIATE_MASK as u64).into());
-        asm.jnz(jit, result_edge(Opnd::Imm(0)));
+        let is_known_heap_basic_object = val_type.is_subtype(types::HeapBasicObject);
+        if !is_known_heap_basic_object {
+            // Immediate -> definitely not the class
+            asm.test(val, (RUBY_IMMEDIATE_MASK as u64).into());
+            asm.jnz(jit, result_edge(Opnd::Imm(0)));
 
-        // Qfalse -> definitely not the class
-        asm.cmp(val, Qfalse.into());
-        asm.je(jit, result_edge(Opnd::Imm(0)));
+            // Qfalse -> definitely not the class
+            asm.cmp(val, Qfalse.into());
+            asm.je(jit, result_edge(Opnd::Imm(0)));
+        }
 
         // Heap object -> check klass field
         let klass = asm.load(Opnd::mem(64, val, RUBY_OFFSET_RBASIC_KLASS));
@@ -2517,7 +2531,8 @@ fn gen_has_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, ty: Typ
 }
 
 /// Compile a type check with a side exit
-fn gen_guard_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, guard_type: Type, state: &FrameState) -> lir::Opnd {
+fn gen_guard_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, val_type: Type, guard_type: Type, state: &FrameState) -> lir::Opnd {
+    let is_known_heap_basic_object = val_type.is_subtype(types::HeapBasicObject);
     gen_incr_counter(asm, Counter::guard_type_count);
     if guard_type.is_subtype(types::Fixnum) {
         asm.test(val, Opnd::UImm(RUBY_FIXNUM_FLAG as u64));
@@ -2553,14 +2568,16 @@ fn gen_guard_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, guard
         // TODO: Max thinks codegen should not care about the shapes of the operands except to create them. (Shopify/ruby#685)
         let val = asm.load_mem(val);
 
-        // Check if it's a special constant
         let side_exit = side_exit(jit, state, GuardType(guard_type));
-        asm.test(val, (RUBY_IMMEDIATE_MASK as u64).into());
-        asm.jnz(jit, side_exit.clone());
-
-        // Check if it's false
-        asm.cmp(val, Qfalse.into());
-        asm.je(jit, side_exit.clone());
+        if !is_known_heap_basic_object {
+            // Check if it's a special constant
+            asm.test(val, (RUBY_IMMEDIATE_MASK as u64).into());
+            asm.jnz(jit, side_exit.clone());
+
+            // Check if it's false
+            asm.cmp(val, Qfalse.into());
+            asm.je(jit, side_exit.clone());
+        }
 
         // Load the class from the object's klass field
         let klass = asm.load(Opnd::mem(64, val, RUBY_OFFSET_RBASIC_KLASS));
@@ -2570,13 +2587,15 @@ fn gen_guard_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, guard
     } else if let Some(builtin_type) = guard_type.builtin_type_equivalent() {
         let side = side_exit(jit, state, GuardType(guard_type));
 
-        // Check special constant
-        asm.test(val, Opnd::UImm(RUBY_IMMEDIATE_MASK as u64));
-        asm.jnz(jit, side.clone());
+        if !is_known_heap_basic_object {
+            // Check special constant
+            asm.test(val, Opnd::UImm(RUBY_IMMEDIATE_MASK as u64));
+            asm.jnz(jit, side.clone());
 
-        // Check false
-        asm.cmp(val, Qfalse.into());
-        asm.je(jit, side.clone());
+            // Check false
+            asm.cmp(val, Qfalse.into());
+            asm.je(jit, side.clone());
+        }
 
         // Mask and check the builtin type
         let val = asm.load_mem(val);
@@ -2683,7 +2702,7 @@ fn gen_incr_send_fallback_counter(asm: &mut Assembler, reason: SendFallbackReaso
 /// (send, sendforward, invokesuper, invokesuperforward, invokeblock).
 /// These instructions call vm_caller_setup_arg_block which writes to cfp->block_code.
 #[allow(non_upper_case_globals)]
-fn iseq_may_write_block_code(iseq: IseqPtr) -> bool {
+pub(crate) fn iseq_may_write_block_code(iseq: IseqPtr) -> bool {
     let encoded_size = unsafe { rb_iseq_encoded_size(iseq) };
     let mut insn_idx: u32 = 0;
 
@@ -2715,7 +2734,7 @@ fn gen_save_pc_for_gc(asm: &mut Assembler, state: &FrameState) {
 
     gen_incr_counter(asm, Counter::vm_write_jit_frame_count);
     asm_comment!(asm, "save JITFrame to CFP");
-    let jit_frame = JITFrame::new_iseq(next_pc, state.iseq, !iseq_may_write_block_code(state.iseq));
+    let jit_frame = JITFrame::new_iseq(next_pc, state.iseq);
     asm.mov(Opnd::mem(64, NATIVE_BASE_PTR, -SIZEOF_VALUE_I32), Opnd::const_ptr(jit_frame));
 
     // CFP_PC for a live JIT frame routes through the JITFrame on the native

zjit/src/codegen_tests.rs

@@ -4269,14 +4269,9 @@ fn test_getspecial_multiple_groups() {
     assert_snapshot!(assert_compiles(r#"test("123-456")"#), @r#""456""#);
 }
 
-// In a JIT-to-JIT call, gen_push_frame writes JIT_RETURN_POISON to the
-// callee's cfp->jit_return (runtime_checks builds). On the *first* such
-// call the function stub trampoline clears jit_return to NULL, so the
-// crash only manifests on the second JIT-to-JIT hit when the stub has
-// been patched to jump directly to the callee's JIT entry. Putting $& as
-// the first C call in the callee keeps the poison live until
-// gen_getspecial_symbol calls rb_backref_get → rb_vm_svar_lep → CFP_PC →
-// CFP_ZJIT_FRAME, which dereferences the poison without the prep fix.
+// In a JIT-to-JIT call, the callee's cfp->jit_return is published at entry.
+// Putting $& as the first C call in the callee exercises CFP_ZJIT_FRAME before
+// gen_save_pc_for_gc has a chance to update the entry JITFrame.
 #[test]
 fn test_getspecial_symbol_in_jit_to_jit_callee() {
     eval(r#"
@@ -4287,10 +4282,6 @@ fn test_getspecial_symbol_in_jit_to_jit_callee() {
         callee
         callee
 
-        # First call to caller_method profiles; second JITs caller_method
-        # and runs through the function-stub-hit path which clears
-        # jit_return. The third call goes through the patched stub with
-        # POISON intact, hitting the bug.
         caller_method
         caller_method
     "#);