From 4c426e98a89015de0ccbd52f3ceb92aa71d31bb4 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Wed, 15 Oct 2025 13:53:57 -0400 Subject: [PATCH 1/4] ZJIT: Use rb_gc_disable() over rb_gc_disable_no_rest() no_rest() trips an assert inside the GC when we allocate with the GC disabled this way: (gc_continue) ../src/gc/default/default.c:2029 (newobj_cache_miss+0x128) [0x105040048] ../src/gc/default/default.c:2370 (rb_gc_impl_new_obj+0x7c) [0x105036374] ../src/gc/default/default.c:2482 (newobj_of) ../src/gc.c:995 (rb_method_entry_alloc+0x40) [0x1051e6c64] ../src/vm_method.c:1102 (rb_method_entry_complement_defined_class) ../src/vm_method.c:1180 (prepare_callable_method_entry+0x14c) [0x1051e87b8] ../src/vm_method.c:1728 (callable_method_entry_or_negative+0x1e8) [0x1051e809c] ../src/vm_method.c:1874 It's tries to continue the GC because it was out of space. Looks like it's not safe to allocate new objects after using rb_gc_disable_no_rest(); existing usages use it for malloc calls. --- zjit/bindgen/src/main.rs | 2 +- zjit/src/cruby.rs | 2 +- zjit/src/cruby_bindings.inc.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index 77d482db4e9d97..43fec090149ef8 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -130,7 +130,7 @@ fn main() { .allowlist_function("rb_singleton_class") .allowlist_function("rb_define_class") .allowlist_function("rb_class_get_superclass") - .allowlist_function("rb_gc_disable_no_rest") + .allowlist_function("rb_gc_disable") .allowlist_function("rb_gc_enable") .allowlist_function("rb_gc_mark") .allowlist_function("rb_gc_mark_movable") diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index 4eb1d3a17c836e..dca4d9180556e8 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -897,7 +897,7 @@ where // 2. If we yield to the GC while compiling, it re-enters our mark and update functions. // This breaks `&mut` exclusivity since mark functions derive fresh `&mut` from statics // while there is a stack frame below it that has an overlapping `&mut`. That's UB. - let gc_disabled_pre_call = unsafe { rb_gc_disable_no_rest() }.test(); + let gc_disabled_pre_call = unsafe { rb_gc_disable() }.test(); let ret = match catch_unwind(func) { Ok(result) => result, diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 6a6263ab15110a..17cda12a0b6204 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -746,6 +746,7 @@ unsafe extern "C" { pub fn rb_gc_mark_movable(obj: VALUE); pub fn rb_gc_location(obj: VALUE) -> VALUE; pub fn rb_gc_enable() -> VALUE; + pub fn rb_gc_disable() -> VALUE; pub fn rb_gc_writebarrier(old: VALUE, young: VALUE); pub fn rb_class_get_superclass(klass: VALUE) -> VALUE; pub static mut rb_cObject: VALUE; @@ -877,7 +878,6 @@ unsafe extern "C" { buff_size: usize, obj: VALUE, ) -> *const ::std::os::raw::c_char; - pub fn rb_gc_disable_no_rest() -> VALUE; pub fn rb_ec_stack_check(ec: *mut rb_execution_context_struct) -> ::std::os::raw::c_int; pub fn rb_gc_writebarrier_remember(obj: VALUE); pub fn rb_shape_id_offset() -> i32; From d272a81f42414d1576dbb44128fcbea74a6cf1d2 Mon Sep 17 00:00:00 2001 From: Aiden Fox Ivey Date: Wed, 15 Oct 2025 14:39:46 -0400 Subject: [PATCH 2/4] ZJIT: Rewrite arm64_split_with_scratch_reg for clarity * The while loop pattern can be rewritten to be more idiomatic, which also allows the iterator to no longer be mutable. --- zjit/src/backend/arm64/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index 6750926b35daa1..74b5210f0fb0a6 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -692,10 +692,10 @@ impl Assembler { /// need to be split with registers after `alloc_regs`, e.g. for `compile_side_exits`, so this /// splits them and uses scratch registers for it. fn arm64_split_with_scratch_reg(mut self) -> Assembler { - let mut iterator = self.insns.into_iter().enumerate().peekable(); + let iterator = self.insns.into_iter().enumerate().peekable(); let mut asm = Assembler::new_with_label_names(take(&mut self.label_names), self.live_ranges.len(), true); - while let Some((_, mut insn)) = iterator.next() { + for (_, mut insn) in iterator { match &mut insn { // For compile_side_exits, support splitting simple C arguments here Insn::CCall { opnds, .. } if !opnds.is_empty() => { From 5a9fac6939af7be2991a5fe16df5fcba8f24eab9 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 14 Oct 2025 20:49:59 -0400 Subject: [PATCH 3/4] Fix assert_equal order in test_namespace.rb The expected value is the first parameter and the actual value is the second in assert_equal. --- test/ruby/test_namespace.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/ruby/test_namespace.rb b/test/ruby/test_namespace.rb index af308ab15c25e3..5661a98ca2b5f1 100644 --- a/test/ruby/test_namespace.rb +++ b/test/ruby/test_namespace.rb @@ -552,7 +552,7 @@ def test_prelude_gems_and_loaded_features # No additional warnings except for experimental warnings assert_includes error.join("\n"), EXPERIMENTAL_WARNINGS - assert_equal error.size, 2 + assert_equal 2, error.size assert_includes output.grep(/^before:/).join("\n"), '/bundled_gems.rb' assert_includes output.grep(/^before:/).join("\n"), '/error_highlight.rb' @@ -574,7 +574,7 @@ def test_prelude_gems_and_loaded_features_with_disable_gems end; assert_includes error.join("\n"), EXPERIMENTAL_WARNINGS - assert_equal error.size, 2 + assert_equal 2, error.size refute_includes output.grep(/^before:/).join("\n"), '/bundled_gems.rb' refute_includes output.grep(/^before:/).join("\n"), '/error_highlight.rb' From 9e4a75696303812d23366d57e4381166b1f88bb1 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 14 Oct 2025 23:59:39 -0700 Subject: [PATCH 4/4] Use BUILTIN_TYPE in gc_mark_check_t_none --- gc/default/default.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gc/default/default.c b/gc/default/default.c index af386a9793ae1d..7c10cc33063b0c 100644 --- a/gc/default/default.c +++ b/gc/default/default.c @@ -4377,7 +4377,7 @@ gc_grey(rb_objspace_t *objspace, VALUE obj) static inline void gc_mark_check_t_none(rb_objspace_t *objspace, VALUE obj) { - if (RB_UNLIKELY(RB_TYPE_P(obj, T_NONE))) { + if (RB_UNLIKELY(BUILTIN_TYPE(obj) == T_NONE)) { enum {info_size = 256}; char obj_info_buf[info_size]; rb_raw_obj_info(obj_info_buf, info_size, obj);