From 228d13f6ed914d1e7f6bd2416e3f5be8283be865 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 3 Dec 2025 18:08:14 +0100 Subject: [PATCH 01/13] gc.c: Pass shape_id to `newobj_init` Attempt to fix the following SEGV: ``` ruby(gc_mark) ../src/gc/default/default.c:4429 ruby(gc_mark_children+0x45) [0x560b380bf8b5] ../src/gc/default/default.c:4625 ruby(gc_mark_stacked_objects) ../src/gc/default/default.c:4647 ruby(gc_mark_stacked_objects_all) ../src/gc/default/default.c:4685 ruby(gc_marks_rest) ../src/gc/default/default.c:5707 ruby(gc_marks+0x4e7) [0x560b380c41c1] ../src/gc/default/default.c:5821 ruby(gc_start) ../src/gc/default/default.c:6502 ruby(heap_prepare+0xa4) [0x560b380c4efc] ../src/gc/default/default.c:2074 ruby(heap_next_free_page) ../src/gc/default/default.c:2289 ruby(newobj_cache_miss) ../src/gc/default/default.c:2396 ruby(RB_SPECIAL_CONST_P+0x0) [0x560b380c5df4] ../src/gc/default/default.c:2420 ruby(RB_BUILTIN_TYPE) ../src/include/ruby/internal/value_type.h:184 ruby(newobj_init) ../src/gc/default/default.c:2136 ruby(rb_gc_impl_new_obj) ../src/gc/default/default.c:2500 ruby(newobj_of) ../src/gc.c:996 ruby(rb_imemo_new+0x37) [0x560b380d8bed] ../src/imemo.c:46 ruby(imemo_fields_new) ../src/imemo.c:105 ruby(rb_imemo_fields_new) ../src/imemo.c:120 ``` I have no reproduction, but my understanding based on the backtrace and error is that GC is triggered inside `newobj_init` causing the new object to be marked while in a incomplete state. I believe the fix is to pass the `shape_id` down to `newobj_init` so it can be set before the GC has a chance to trigger. --- gc.c | 5 ++--- gc/default/default.c | 33 ++++++++++++++++----------------- gc/gc_impl.h | 2 +- gc/mmtk/mmtk.c | 5 +++-- shape.h | 9 +++++++-- 5 files changed, 29 insertions(+), 25 deletions(-) diff --git a/gc.c b/gc.c index 9ccaffdc0b371c..1020a461dce11f 100644 --- a/gc.c +++ b/gc.c @@ -622,7 +622,7 @@ typedef struct gc_function_map { void (*stress_set)(void *objspace_ptr, VALUE flag); VALUE (*stress_get)(void *objspace_ptr); // Object allocation - VALUE (*new_obj)(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags, bool wb_protected, size_t alloc_size); + VALUE (*new_obj)(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags, shape_id_t shape_id, bool wb_protected, size_t alloc_size); size_t (*obj_slot_size)(VALUE obj); size_t (*heap_id_for_size)(void *objspace_ptr, size_t size); bool (*size_allocatable_p)(size_t size); @@ -993,8 +993,7 @@ gc_validate_pc(VALUE obj) static inline VALUE newobj_of(rb_ractor_t *cr, VALUE klass, VALUE flags, shape_id_t shape_id, bool wb_protected, size_t size) { - VALUE obj = rb_gc_impl_new_obj(rb_gc_get_objspace(), cr->newobj_cache, klass, flags, wb_protected, size); - RBASIC_SET_SHAPE_ID_NO_CHECKS(obj, shape_id); + VALUE obj = rb_gc_impl_new_obj(rb_gc_get_objspace(), cr->newobj_cache, klass, flags, shape_id, wb_protected, size); gc_validate_pc(obj); diff --git a/gc/default/default.c b/gc/default/default.c index 54b93dc8d07cf6..2cacb2cfd9e26b 100644 --- a/gc/default/default.c +++ b/gc/default/default.c @@ -32,6 +32,7 @@ #include "darray.h" #include "gc/gc.h" #include "gc/gc_impl.h" +#include "shape.h" #ifndef BUILDING_MODULAR_GC # include "probes.h" @@ -2131,15 +2132,13 @@ rb_gc_impl_source_location_cstr(int *ptr) #endif static inline VALUE -newobj_init(VALUE klass, VALUE flags, int wb_protected, rb_objspace_t *objspace, VALUE obj) +newobj_init(VALUE klass, VALUE flags, shape_id_t shape_id, int wb_protected, rb_objspace_t *objspace, VALUE obj) { GC_ASSERT(BUILTIN_TYPE(obj) == T_NONE); GC_ASSERT((flags & FL_WB_PROTECTED) == 0); RBASIC(obj)->flags = flags; *((VALUE *)&RBASIC(obj)->klass) = klass; -#if RBASIC_SHAPE_ID_FIELD - RBASIC(obj)->shape_id = 0; -#endif + RBASIC_SET_SHAPE_ID_NO_CHECKS(obj, shape_id); int t = flags & RUBY_T_MASK; if (t == T_CLASS || t == T_MODULE || t == T_ICLASS) { @@ -2423,10 +2422,10 @@ newobj_alloc(rb_objspace_t *objspace, rb_ractor_newobj_cache_t *cache, size_t he return obj; } -ALWAYS_INLINE(static VALUE newobj_slowpath(VALUE klass, VALUE flags, rb_objspace_t *objspace, rb_ractor_newobj_cache_t *cache, int wb_protected, size_t heap_idx)); +ALWAYS_INLINE(static VALUE newobj_slowpath(VALUE klass, VALUE flags, shape_id_t shape_id, rb_objspace_t *objspace, rb_ractor_newobj_cache_t *cache, int wb_protected, size_t heap_idx)); static inline VALUE -newobj_slowpath(VALUE klass, VALUE flags, rb_objspace_t *objspace, rb_ractor_newobj_cache_t *cache, int wb_protected, size_t heap_idx) +newobj_slowpath(VALUE klass, VALUE flags, shape_id_t shape_id, rb_objspace_t *objspace, rb_ractor_newobj_cache_t *cache, int wb_protected, size_t heap_idx) { VALUE obj; unsigned int lev; @@ -2451,32 +2450,32 @@ newobj_slowpath(VALUE klass, VALUE flags, rb_objspace_t *objspace, rb_ractor_new } obj = newobj_alloc(objspace, cache, heap_idx, true); - newobj_init(klass, flags, wb_protected, objspace, obj); + newobj_init(klass, flags, shape_id, wb_protected, objspace, obj); } RB_GC_CR_UNLOCK(lev); return obj; } -NOINLINE(static VALUE newobj_slowpath_wb_protected(VALUE klass, VALUE flags, +NOINLINE(static VALUE newobj_slowpath_wb_protected(VALUE klass, VALUE flags, shape_id_t shape_id, rb_objspace_t *objspace, rb_ractor_newobj_cache_t *cache, size_t heap_idx)); -NOINLINE(static VALUE newobj_slowpath_wb_unprotected(VALUE klass, VALUE flags, +NOINLINE(static VALUE newobj_slowpath_wb_unprotected(VALUE klass, VALUE flags, shape_id_t shape_id, rb_objspace_t *objspace, rb_ractor_newobj_cache_t *cache, size_t heap_idx)); static VALUE -newobj_slowpath_wb_protected(VALUE klass, VALUE flags, rb_objspace_t *objspace, rb_ractor_newobj_cache_t *cache, size_t heap_idx) +newobj_slowpath_wb_protected(VALUE klass, VALUE flags, shape_id_t shape_id, rb_objspace_t *objspace, rb_ractor_newobj_cache_t *cache, size_t heap_idx) { - return newobj_slowpath(klass, flags, objspace, cache, TRUE, heap_idx); + return newobj_slowpath(klass, flags, shape_id, objspace, cache, TRUE, heap_idx); } static VALUE -newobj_slowpath_wb_unprotected(VALUE klass, VALUE flags, rb_objspace_t *objspace, rb_ractor_newobj_cache_t *cache, size_t heap_idx) +newobj_slowpath_wb_unprotected(VALUE klass, VALUE flags, shape_id_t shape_id, rb_objspace_t *objspace, rb_ractor_newobj_cache_t *cache, size_t heap_idx) { - return newobj_slowpath(klass, flags, objspace, cache, FALSE, heap_idx); + return newobj_slowpath(klass, flags, shape_id, objspace, cache, FALSE, heap_idx); } VALUE -rb_gc_impl_new_obj(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags, bool wb_protected, size_t alloc_size) +rb_gc_impl_new_obj(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags, shape_id_t shape_id, bool wb_protected, size_t alloc_size) { VALUE obj; rb_objspace_t *objspace = objspace_ptr; @@ -2497,14 +2496,14 @@ rb_gc_impl_new_obj(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags if (!RB_UNLIKELY(during_gc || ruby_gc_stressful) && wb_protected) { obj = newobj_alloc(objspace, cache, heap_idx, false); - newobj_init(klass, flags, wb_protected, objspace, obj); + newobj_init(klass, flags, shape_id, wb_protected, objspace, obj); } else { RB_DEBUG_COUNTER_INC(obj_newobj_slowpath); obj = wb_protected ? - newobj_slowpath_wb_protected(klass, flags, objspace, cache, heap_idx) : - newobj_slowpath_wb_unprotected(klass, flags, objspace, cache, heap_idx); + newobj_slowpath_wb_protected(klass, flags, shape_id, objspace, cache, heap_idx) : + newobj_slowpath_wb_unprotected(klass, flags, shape_id, objspace, cache, heap_idx); } return obj; diff --git a/gc/gc_impl.h b/gc/gc_impl.h index 3250483639e775..65c0586d46bfad 100644 --- a/gc/gc_impl.h +++ b/gc/gc_impl.h @@ -55,7 +55,7 @@ GC_IMPL_FN VALUE rb_gc_impl_stress_get(void *objspace_ptr); GC_IMPL_FN VALUE rb_gc_impl_config_get(void *objspace_ptr); GC_IMPL_FN void rb_gc_impl_config_set(void *objspace_ptr, VALUE hash); // Object allocation -GC_IMPL_FN VALUE rb_gc_impl_new_obj(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags, bool wb_protected, size_t alloc_size); +GC_IMPL_FN VALUE rb_gc_impl_new_obj(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags, uint32_t /* shape_id_t */ shape_id, bool wb_protected, size_t alloc_size); GC_IMPL_FN size_t rb_gc_impl_obj_slot_size(VALUE obj); GC_IMPL_FN size_t rb_gc_impl_heap_id_for_size(void *objspace_ptr, size_t size); GC_IMPL_FN bool rb_gc_impl_size_allocatable_p(size_t size); diff --git a/gc/mmtk/mmtk.c b/gc/mmtk/mmtk.c index e1678dcf6ab0b4..a3bdf1cd4a9a42 100644 --- a/gc/mmtk/mmtk.c +++ b/gc/mmtk/mmtk.c @@ -7,6 +7,7 @@ #include "gc/gc.h" #include "gc/gc_impl.h" +#include "shape.h" #include "gc/mmtk/mmtk.h" #include "ccan/list/list.h" @@ -603,7 +604,7 @@ rb_gc_impl_config_set(void *objspace_ptr, VALUE hash) // Object allocation VALUE -rb_gc_impl_new_obj(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags, bool wb_protected, size_t alloc_size) +rb_gc_impl_new_obj(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags, shape_id_t shape_id, bool wb_protected, size_t alloc_size) { #define MMTK_ALLOCATION_SEMANTICS_DEFAULT 0 struct objspace *objspace = objspace_ptr; @@ -625,7 +626,7 @@ rb_gc_impl_new_obj(void *objspace_ptr, void *cache_ptr, VALUE klass, VALUE flags VALUE *alloc_obj = mmtk_alloc(ractor_cache->mutator, alloc_size + 8, MMTk_MIN_OBJ_ALIGN, 0, MMTK_ALLOCATION_SEMANTICS_DEFAULT); alloc_obj++; alloc_obj[-1] = alloc_size; - alloc_obj[0] = flags; + alloc_obj[0] = RSHAPE_COMBINE_IN_FLAGS(flags, shape_id);; alloc_obj[1] = klass; mmtk_post_alloc(ractor_cache->mutator, (void*)alloc_obj, alloc_size + 8, MMTK_ALLOCATION_SEMANTICS_DEFAULT); diff --git a/shape.h b/shape.h index 9478d4b3a95dc6..28c2a7eef9421a 100644 --- a/shape.h +++ b/shape.h @@ -162,6 +162,12 @@ RBASIC_SHAPE_ID_FOR_READ(VALUE obj) bool rb_shape_verify_consistency(VALUE obj, shape_id_t shape_id); #endif +static inline VALUE +RSHAPE_COMBINE_IN_FLAGS(VALUE flags, shape_id_t shape_id) +{ + return (flags &SHAPE_FLAG_MASK) | (((VALUE)shape_id) << SHAPE_FLAG_SHIFT); +} + static inline void RBASIC_SET_SHAPE_ID_NO_CHECKS(VALUE obj, shape_id_t shape_id) { @@ -169,8 +175,7 @@ RBASIC_SET_SHAPE_ID_NO_CHECKS(VALUE obj, shape_id_t shape_id) RBASIC(obj)->shape_id = (VALUE)shape_id; #else // Object shapes are occupying top bits - RBASIC(obj)->flags &= SHAPE_FLAG_MASK; - RBASIC(obj)->flags |= ((VALUE)(shape_id) << SHAPE_FLAG_SHIFT); + RBASIC(obj)->flags = RSHAPE_COMBINE_IN_FLAGS(RBASIC(obj)->flags, shape_id); #endif } From 8d1a6bc48b1a45cacd8b1f1c42f71d5967a27bba Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 3 Dec 2025 20:40:22 +0100 Subject: [PATCH 02/13] gc.c: check if the struct has fields before marking the fields_obj If GC trigger in the middle of `struct_alloc`, and the struct has more than 3 elements, then `fields_obj` reference is garbage. We must first check the shape to know if it was actually initialized. --- gc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gc.c b/gc.c index 1020a461dce11f..7b4547e2ea0bca 100644 --- a/gc.c +++ b/gc.c @@ -3303,7 +3303,7 @@ rb_gc_mark_children(void *objspace, VALUE obj) gc_mark_internal(ptr[i]); } - if (!FL_TEST_RAW(obj, RSTRUCT_GEN_FIELDS)) { + if (rb_shape_obj_has_fields(obj) && !FL_TEST_RAW(obj, RSTRUCT_GEN_FIELDS)) { gc_mark_internal(RSTRUCT_FIELDS_OBJ(obj)); } From 9913d8da1ffcd00e8c648ac52abefdc086662797 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Mon, 1 Dec 2025 23:19:14 -0800 Subject: [PATCH 03/13] Group malloc counters together --- gc/default/default.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/gc/default/default.c b/gc/default/default.c index 2cacb2cfd9e26b..1d50172739fdfd 100644 --- a/gc/default/default.c +++ b/gc/default/default.c @@ -468,8 +468,14 @@ enum gc_mode { typedef struct rb_objspace { struct { - size_t limit; size_t increase; +#if RGENGC_ESTIMATE_OLDMALLOC + size_t oldmalloc_increase; +#endif + } malloc_counters; + + struct { + size_t limit; #if MALLOC_ALLOCATED_SIZE size_t allocated_size; size_t allocations; @@ -590,7 +596,6 @@ typedef struct rb_objspace { size_t old_objects_limit; #if RGENGC_ESTIMATE_OLDMALLOC - size_t oldmalloc_increase; size_t oldmalloc_increase_limit; #endif @@ -864,7 +869,7 @@ RVALUE_AGE_SET(VALUE obj, int age) } #define malloc_limit objspace->malloc_params.limit -#define malloc_increase objspace->malloc_params.increase +#define malloc_increase objspace->malloc_counters.increase #define malloc_allocated_size objspace->malloc_params.allocated_size #define heap_pages_lomem objspace->heap_pages.range[0] #define heap_pages_himem objspace->heap_pages.range[1] @@ -4910,7 +4915,7 @@ gc_marks_check(rb_objspace_t *objspace, st_foreach_callback_func *checker_func, { size_t saved_malloc_increase = objspace->malloc_params.increase; #if RGENGC_ESTIMATE_OLDMALLOC - size_t saved_oldmalloc_increase = objspace->rgengc.oldmalloc_increase; + size_t saved_oldmalloc_increase = objspace->malloc_counters.oldmalloc_increase; #endif VALUE already_disabled = rb_objspace_gc_disable(objspace); @@ -4933,7 +4938,7 @@ gc_marks_check(rb_objspace_t *objspace, st_foreach_callback_func *checker_func, if (already_disabled == Qfalse) rb_objspace_gc_enable(objspace); objspace->malloc_params.increase = saved_malloc_increase; #if RGENGC_ESTIMATE_OLDMALLOC - objspace->rgengc.oldmalloc_increase = saved_oldmalloc_increase; + objspace->malloc_counters.oldmalloc_increase = saved_oldmalloc_increase; #endif } #endif /* RGENGC_CHECK_MODE >= 4 */ @@ -6331,7 +6336,7 @@ gc_reset_malloc_info(rb_objspace_t *objspace, bool full_mark) /* reset oldmalloc info */ #if RGENGC_ESTIMATE_OLDMALLOC if (!full_mark) { - if (objspace->rgengc.oldmalloc_increase > objspace->rgengc.oldmalloc_increase_limit) { + if (objspace->malloc_counters.oldmalloc_increase > objspace->rgengc.oldmalloc_increase_limit) { gc_needs_major_flags |= GPR_FLAG_MAJOR_BY_OLDMALLOC; objspace->rgengc.oldmalloc_increase_limit = (size_t)(objspace->rgengc.oldmalloc_increase_limit * gc_params.oldmalloc_limit_growth_factor); @@ -6344,13 +6349,13 @@ gc_reset_malloc_info(rb_objspace_t *objspace, bool full_mark) if (0) fprintf(stderr, "%"PRIdSIZE"\t%d\t%"PRIuSIZE"\t%"PRIuSIZE"\t%"PRIdSIZE"\n", rb_gc_count(), gc_needs_major_flags, - objspace->rgengc.oldmalloc_increase, + objspace->malloc_counters.oldmalloc_increase, objspace->rgengc.oldmalloc_increase_limit, gc_params.oldmalloc_limit_max); } else { /* major GC */ - objspace->rgengc.oldmalloc_increase = 0; + objspace->malloc_counters.oldmalloc_increase = 0; if ((objspace->profile.latest_gc_info & GPR_FLAG_MAJOR_BY_OLDMALLOC) == 0) { objspace->rgengc.oldmalloc_increase_limit = @@ -7583,7 +7588,7 @@ rb_gc_impl_stat(void *objspace_ptr, VALUE hash_or_sym) SET(old_objects, objspace->rgengc.old_objects); SET(old_objects_limit, objspace->rgengc.old_objects_limit); #if RGENGC_ESTIMATE_OLDMALLOC - SET(oldmalloc_increase_bytes, objspace->rgengc.oldmalloc_increase); + SET(oldmalloc_increase_bytes, objspace->malloc_counters.oldmalloc_increase); SET(oldmalloc_increase_bytes_limit, objspace->rgengc.oldmalloc_increase_limit); #endif @@ -8040,13 +8045,13 @@ objspace_malloc_increase_body(rb_objspace_t *objspace, void *mem, size_t new_siz if (new_size > old_size) { RUBY_ATOMIC_SIZE_ADD(malloc_increase, new_size - old_size); #if RGENGC_ESTIMATE_OLDMALLOC - RUBY_ATOMIC_SIZE_ADD(objspace->rgengc.oldmalloc_increase, new_size - old_size); + RUBY_ATOMIC_SIZE_ADD(objspace->malloc_counters.oldmalloc_increase, new_size - old_size); #endif } else { atomic_sub_nounderflow(&malloc_increase, old_size - new_size); #if RGENGC_ESTIMATE_OLDMALLOC - atomic_sub_nounderflow(&objspace->rgengc.oldmalloc_increase, old_size - new_size); + atomic_sub_nounderflow(&objspace->malloc_counters.oldmalloc_increase, old_size - new_size); #endif } From a773bbf0cc35cd4b73509edd58a0757d06abaca6 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 2 Dec 2025 09:46:10 -0800 Subject: [PATCH 04/13] Track small malloc/free changes in thread local --- gc/default/default.c | 76 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 67 insertions(+), 9 deletions(-) diff --git a/gc/default/default.c b/gc/default/default.c index 1d50172739fdfd..f8fd40b44fd8b4 100644 --- a/gc/default/default.c +++ b/gc/default/default.c @@ -159,6 +159,17 @@ #define GC_OLDMALLOC_LIMIT_MAX (128 * 1024 * 1024 /* 128MB */) #endif +#ifndef GC_MALLOC_INCREASE_LOCAL_THRESHOLD +#define GC_MALLOC_INCREASE_LOCAL_THRESHOLD (8 * 1024 /* 8KB */) +#endif + +#ifdef RB_THREAD_LOCAL_SPECIFIER +#define USE_MALLOC_INCREASE_LOCAL 1 +static RB_THREAD_LOCAL_SPECIFIER int malloc_increase_local; +#else +#define USE_MALLOC_INCREASE_LOCAL 0 +#endif + #ifndef GC_CAN_COMPILE_COMPACTION #if defined(__wasi__) /* WebAssembly doesn't support signals */ # define GC_CAN_COMPILE_COMPACTION 0 @@ -7531,6 +7542,8 @@ ns_to_ms(uint64_t ns) return ns / (1000 * 1000); } +static void malloc_increase_local_flush(rb_objspace_t *objspace); + VALUE rb_gc_impl_stat(void *objspace_ptr, VALUE hash_or_sym) { @@ -7540,6 +7553,7 @@ rb_gc_impl_stat(void *objspace_ptr, VALUE hash_or_sym) setup_gc_stat_symbols(); ractor_cache_flush_count(objspace, rb_gc_get_ractor_newobj_cache()); + malloc_increase_local_flush(objspace); if (RB_TYPE_P(hash_or_sym, T_HASH)) { hash = hash_or_sym; @@ -8027,6 +8041,45 @@ objspace_malloc_gc_stress(rb_objspace_t *objspace) } } +static void +malloc_increase_commit(rb_objspace_t *objspace, size_t new_size, size_t old_size) +{ + if (new_size > old_size) { + RUBY_ATOMIC_SIZE_ADD(malloc_increase, new_size - old_size); +#if RGENGC_ESTIMATE_OLDMALLOC + RUBY_ATOMIC_SIZE_ADD(objspace->malloc_counters.oldmalloc_increase, new_size - old_size); +#endif + } + else { + atomic_sub_nounderflow(&malloc_increase, old_size - new_size); +#if RGENGC_ESTIMATE_OLDMALLOC + atomic_sub_nounderflow(&objspace->malloc_counters.oldmalloc_increase, old_size - new_size); +#endif + } +} + +#if USE_MALLOC_INCREASE_LOCAL +static void +malloc_increase_local_flush(rb_objspace_t *objspace) +{ + int delta = malloc_increase_local; + if (delta == 0) return; + + malloc_increase_local = 0; + if (delta > 0) { + malloc_increase_commit(objspace, (size_t)delta, 0); + } + else { + malloc_increase_commit(objspace, 0, (size_t)(-delta)); + } +} +#else +static void +malloc_increase_local_flush(rb_objspace_t *objspace) +{ +} +#endif + static inline bool objspace_malloc_increase_report(rb_objspace_t *objspace, void *mem, size_t new_size, size_t old_size, enum memop_type type, bool gc_allowed) { @@ -8042,18 +8095,23 @@ objspace_malloc_increase_report(rb_objspace_t *objspace, void *mem, size_t new_s static bool objspace_malloc_increase_body(rb_objspace_t *objspace, void *mem, size_t new_size, size_t old_size, enum memop_type type, bool gc_allowed) { - if (new_size > old_size) { - RUBY_ATOMIC_SIZE_ADD(malloc_increase, new_size - old_size); -#if RGENGC_ESTIMATE_OLDMALLOC - RUBY_ATOMIC_SIZE_ADD(objspace->malloc_counters.oldmalloc_increase, new_size - old_size); -#endif +#if USE_MALLOC_INCREASE_LOCAL + if (new_size < GC_MALLOC_INCREASE_LOCAL_THRESHOLD && + old_size < GC_MALLOC_INCREASE_LOCAL_THRESHOLD) { + malloc_increase_local += (int)new_size - (int)old_size; + + if (malloc_increase_local >= GC_MALLOC_INCREASE_LOCAL_THRESHOLD || + malloc_increase_local <= -GC_MALLOC_INCREASE_LOCAL_THRESHOLD) { + malloc_increase_local_flush(objspace); + } } else { - atomic_sub_nounderflow(&malloc_increase, old_size - new_size); -#if RGENGC_ESTIMATE_OLDMALLOC - atomic_sub_nounderflow(&objspace->malloc_counters.oldmalloc_increase, old_size - new_size); -#endif + malloc_increase_local_flush(objspace); + malloc_increase_commit(objspace, new_size, old_size); } +#else + malloc_increase_commit(objspace, new_size, old_size); +#endif if (type == MEMOP_TYPE_MALLOC && gc_allowed) { retry: From 2b23b05bf2c0f30f2c4ee9bb3030fa58f2cba3a6 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 24 Nov 2025 16:16:08 -0800 Subject: [PATCH 05/13] ZJIT: Add a specialized instruction iterator to the assembler This commit adds a specialized instruction iterator to the assembler with a custom "peek" method. The reason is that we want to add basic blocks to LIR. When we split instructions, we need to add any new instructions to the correct basic block. The custom iterator will maintain the correct basic block inside the assembler, that way when we push any new instructions they will be appended to the correct place. --- zjit/src/backend/lir.rs | 47 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index 3c9bf72023d90b..d7c6740d13a1e4 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -1399,6 +1399,15 @@ impl Assembler } } + pub fn instruction_iterator(&mut self) -> InsnIter { + let insns = take(&mut self.insns); + InsnIter { + old_insns_iter: insns.into_iter(), + peeked: None, + index: 0, + } + } + pub fn expect_leaf_ccall(&mut self, stack_size: usize) { self.leaf_ccall_stack_size = Some(stack_size); } @@ -1998,6 +2007,44 @@ impl fmt::Debug for Assembler { } } +pub struct InsnIter { + old_insns_iter: std::vec::IntoIter, + peeked: Option<(usize, Insn)>, + index: usize, +} + +impl InsnIter { + // We're implementing our own peek() because we don't want peek to + // cross basic blocks as we're iterating. + pub fn peek(&mut self) -> Option<&(usize, Insn)> { + // If we don't have a peeked value, get one + if self.peeked.is_none() { + let insn = self.old_insns_iter.next()?; + let idx = self.index; + self.index += 1; + self.peeked = Some((idx, insn)); + } + // Return a reference to the peeked value + self.peeked.as_ref() + } + + // Get the next instruction. Right now we're passing the "new" assembler + // (the assembler we're copying in to) as a parameter. Once we've + // introduced basic blocks to LIR, we'll use the to set the correct BB + // on the new assembler, but for now it is unused. + pub fn next(&mut self, _new_asm: &mut Assembler) -> Option<(usize, Insn)> { + // If we have a peeked value, return it + if let Some(item) = self.peeked.take() { + return Some(item); + } + // Otherwise get the next from underlying iterator + let insn = self.old_insns_iter.next()?; + let idx = self.index; + self.index += 1; + Some((idx, insn)) + } +} + impl Assembler { #[must_use] pub fn add(&mut self, left: Opnd, right: Opnd) -> Opnd { From d7e55f84f2bd62d302b29513d4c4dc8ae9aef96f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 25 Nov 2025 16:51:54 -0800 Subject: [PATCH 06/13] ZJIT: Use the custom iterator This commit uses the custom instruction iterator in arm64 / x86_64 instruction splitting. Once we introduce basic blocks to LIR, the custom iterator will ensure that instructions are added to the correct place. --- zjit/src/backend/arm64/mod.rs | 25 +++++++++++++------------ zjit/src/backend/x86_64/mod.rs | 21 +++++++++++---------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index 013fd315833409..78fb69b0b04d5b 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -391,10 +391,10 @@ impl Assembler { let mut asm_local = Assembler::new_with_asm(&self); let live_ranges: Vec = take(&mut self.live_ranges); - let mut iterator = self.insns.into_iter().enumerate().peekable(); + let mut iterator = self.instruction_iterator(); let asm = &mut asm_local; - while let Some((index, mut insn)) = iterator.next() { + while let Some((index, mut insn)) = iterator.next(asm) { // Here we're going to map the operands of the instruction to load // any Opnd::Value operands into registers if they are heap objects // such that only the Op::Load instruction needs to handle that @@ -428,13 +428,13 @@ impl Assembler { *right = split_shifted_immediate(asm, other_opnd); // Now `right` is either a register or an immediate, both can try to // merge with a subsequent mov. - merge_three_reg_mov(&live_ranges, &mut iterator, left, left, out); + merge_three_reg_mov(&live_ranges, &mut iterator, asm, left, left, out); asm.push_insn(insn); } _ => { *left = split_load_operand(asm, *left); *right = split_shifted_immediate(asm, *right); - merge_three_reg_mov(&live_ranges, &mut iterator, left, right, out); + merge_three_reg_mov(&live_ranges, &mut iterator, asm, left, right, out); asm.push_insn(insn); } } @@ -444,7 +444,7 @@ impl Assembler { *right = split_shifted_immediate(asm, *right); // Now `right` is either a register or an immediate, // both can try to merge with a subsequent mov. - merge_three_reg_mov(&live_ranges, &mut iterator, left, left, out); + merge_three_reg_mov(&live_ranges, &mut iterator, asm, left, left, out); asm.push_insn(insn); } Insn::And { left, right, out } | @@ -454,7 +454,7 @@ impl Assembler { *left = opnd0; *right = opnd1; - merge_three_reg_mov(&live_ranges, &mut iterator, left, right, out); + merge_three_reg_mov(&live_ranges, &mut iterator, asm, left, right, out); asm.push_insn(insn); } @@ -567,7 +567,7 @@ impl Assembler { if matches!(out, Opnd::VReg { .. }) && *out == *src && live_ranges[out.vreg_idx()].end() == index + 1 => { *out = Opnd::Reg(*reg); asm.push_insn(insn); - iterator.next(); // Pop merged Insn::Mov + iterator.next(asm); // Pop merged Insn::Mov } _ => { asm.push_insn(insn); @@ -694,7 +694,7 @@ impl Assembler { /// VRegs, most splits should happen in [`Self::arm64_split`]. However, some instructions /// need to be split with registers after `alloc_regs`, e.g. for `compile_exits`, so this /// splits them and uses scratch registers for it. - fn arm64_scratch_split(self) -> Assembler { + fn arm64_scratch_split(mut self) -> Assembler { /// If opnd is Opnd::Mem with a too large disp, make the disp smaller using lea. fn split_large_disp(asm: &mut Assembler, opnd: Opnd, scratch_opnd: Opnd) -> Opnd { match opnd { @@ -753,9 +753,9 @@ impl Assembler { let mut asm_local = Assembler::new_with_asm(&self); let asm = &mut asm_local; asm.accept_scratch_reg = true; - let mut iterator = self.insns.into_iter().enumerate().peekable(); + let iterator = &mut self.instruction_iterator(); - while let Some((_, mut insn)) = iterator.next() { + while let Some((_, mut insn)) = iterator.next(asm) { match &mut insn { Insn::Add { left, right, out } | Insn::Sub { left, right, out } | @@ -1660,7 +1660,8 @@ impl Assembler { /// If a, b, and c are all registers. fn merge_three_reg_mov( live_ranges: &[LiveRange], - iterator: &mut std::iter::Peekable>, + iterator: &mut InsnIter, + asm: &mut Assembler, left: &Opnd, right: &Opnd, out: &mut Opnd, @@ -1671,7 +1672,7 @@ fn merge_three_reg_mov( = (left, right, iterator.peek()) { if out == src && live_ranges[out.vreg_idx()].end() == *mov_idx && matches!(*dest, Opnd::Reg(_) | Opnd::VReg{..}) { *out = *dest; - iterator.next(); // Pop merged Insn::Mov + iterator.next(asm); // Pop merged Insn::Mov } } } diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index a9bd57f368c59b..5e975e1bd03ccf 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -138,11 +138,12 @@ impl Assembler { /// Split IR instructions for the x86 platform fn x86_split(mut self) -> Assembler { - let mut asm = Assembler::new_with_asm(&self); + let mut asm_local = Assembler::new_with_asm(&self); + let asm = &mut asm_local; let live_ranges: Vec = take(&mut self.live_ranges); - let mut iterator = self.insns.into_iter().enumerate().peekable(); + let mut iterator = self.instruction_iterator(); - while let Some((index, mut insn)) = iterator.next() { + while let Some((index, mut insn)) = iterator.next(asm) { let is_load = matches!(insn, Insn::Load { .. } | Insn::LoadInto { .. }); let mut opnd_iter = insn.opnd_iter_mut(); @@ -187,13 +188,13 @@ impl Assembler { if out == src && left == dest && live_ranges[out.vreg_idx()].end() == index + 1 && uimm_num_bits(*value) <= 32 => { *out = *dest; asm.push_insn(insn); - iterator.next(); // Pop merged Insn::Mov + iterator.next(asm); // Pop merged Insn::Mov } (Opnd::Reg(_), Opnd::Reg(_), Some(Insn::Mov { dest, src })) if out == src && live_ranges[out.vreg_idx()].end() == index + 1 && *dest == *left => { *out = *dest; asm.push_insn(insn); - iterator.next(); // Pop merged Insn::Mov + iterator.next(asm); // Pop merged Insn::Mov } _ => { match (*left, *right) { @@ -373,7 +374,7 @@ impl Assembler { (Insn::Lea { opnd, out }, Some(Insn::Mov { dest: Opnd::Reg(reg), src })) if matches!(out, Opnd::VReg { .. }) && out == src && live_ranges[out.vreg_idx()].end() == index + 1 => { asm.push_insn(Insn::Lea { opnd: *opnd, out: Opnd::Reg(*reg) }); - iterator.next(); // Pop merged Insn::Mov + iterator.next(asm); // Pop merged Insn::Mov } _ => asm.push_insn(insn), } @@ -384,14 +385,14 @@ impl Assembler { } } - asm + asm_local } /// Split instructions using scratch registers. To maximize the use of the register pool /// for VRegs, most splits should happen in [`Self::x86_split`]. However, some instructions /// need to be split with registers after `alloc_regs`, e.g. for `compile_exits`, so /// this splits them and uses scratch registers for it. - pub fn x86_scratch_split(self) -> Assembler { + pub fn x86_scratch_split(mut self) -> Assembler { /// For some instructions, we want to be able to lower a 64-bit operand /// without requiring more registers to be available in the register /// allocator. So we just use the SCRATCH0_OPND register temporarily to hold @@ -470,9 +471,9 @@ impl Assembler { let mut asm_local = Assembler::new_with_asm(&self); let asm = &mut asm_local; asm.accept_scratch_reg = true; - let mut iterator = self.insns.into_iter().enumerate().peekable(); + let mut iterator = self.instruction_iterator(); - while let Some((_, mut insn)) = iterator.next() { + while let Some((_, mut insn)) = iterator.next(asm) { match &mut insn { Insn::Add { left, right, out } | Insn::Sub { left, right, out } | From 612a66805f35b2c732ec843d78f1dcad5c6456cd Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Wed, 3 Dec 2025 16:02:31 -0500 Subject: [PATCH 07/13] Remove spurious obj != klass check in shape_get_next This should never be true. I added an `rb_bug` in case it was and it wasn't true in any of btest or test-all. Co-authored-by: Aaron Patterson --- shape.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shape.c b/shape.c index 754be1cfd64e65..c6f5af2519323e 100644 --- a/shape.c +++ b/shape.c @@ -839,7 +839,7 @@ shape_get_next(rb_shape_t *shape, enum shape_type shape_type, VALUE obj, ID id, } // Check if we should update max_iv_count on the object's class - if (obj != klass && new_shape->next_field_index > RCLASS_MAX_IV_COUNT(klass)) { + if (new_shape->next_field_index > RCLASS_MAX_IV_COUNT(klass)) { RCLASS_SET_MAX_IV_COUNT(klass, new_shape->next_field_index); } From f1670733249fb30d755bad1f88c0e54b26bdf49e Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Wed, 3 Dec 2025 16:06:41 -0500 Subject: [PATCH 08/13] Move imemo fields check out of shape_get_next Not every caller (for example, YJIT) actually needs to pass the object. YJIT (and, in the future, ZJIT) only need to pass the class. --- shape.c | 50 +++++++++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/shape.c b/shape.c index c6f5af2519323e..13d1edf36c928a 100644 --- a/shape.c +++ b/shape.c @@ -801,7 +801,7 @@ shape_get_iv_index(rb_shape_t *shape, ID id, attr_index_t *value) } static inline rb_shape_t * -shape_get_next(rb_shape_t *shape, enum shape_type shape_type, VALUE obj, ID id, bool emit_warnings) +shape_get_next(rb_shape_t *shape, enum shape_type shape_type, VALUE klass, ID id, bool emit_warnings) { RUBY_ASSERT(!is_instance_id(id) || RTEST(rb_sym2str(ID2SYM(id)))); @@ -812,23 +812,6 @@ shape_get_next(rb_shape_t *shape, enum shape_type shape_type, VALUE obj, ID id, } #endif - VALUE klass; - if (IMEMO_TYPE_P(obj, imemo_fields)) { - VALUE owner = rb_imemo_fields_owner(obj); - switch (BUILTIN_TYPE(owner)) { - case T_CLASS: - case T_MODULE: - klass = rb_singleton_class(owner); - break; - default: - klass = rb_obj_class(owner); - break; - } - } - else { - klass = rb_obj_class(obj); - } - bool allow_new_shape = RCLASS_VARIATION_COUNT(klass) < SHAPE_MAX_VARIATIONS; bool variation_created = false; rb_shape_t *new_shape = get_next_shape_internal(shape, id, shape_type, &variation_created, allow_new_shape); @@ -862,6 +845,28 @@ shape_get_next(rb_shape_t *shape, enum shape_type shape_type, VALUE obj, ID id, return new_shape; } +static VALUE +obj_get_owner_class(VALUE obj) +{ + VALUE klass; + if (IMEMO_TYPE_P(obj, imemo_fields)) { + VALUE owner = rb_imemo_fields_owner(obj); + switch (BUILTIN_TYPE(owner)) { + case T_CLASS: + case T_MODULE: + klass = rb_singleton_class(owner); + break; + default: + klass = rb_obj_class(owner); + break; + } + } + else { + klass = rb_obj_class(obj); + } + return klass; +} + static rb_shape_t * remove_shape_recursive(VALUE obj, rb_shape_t *shape, ID id, rb_shape_t **removed_shape) { @@ -884,7 +889,8 @@ remove_shape_recursive(VALUE obj, rb_shape_t *shape, ID id, rb_shape_t **removed // We found a new parent. Create a child of the new parent that // has the same attributes as this shape. if (new_parent) { - rb_shape_t *new_child = shape_get_next(new_parent, shape->type, obj, shape->edge_name, true); + VALUE klass = obj_get_owner_class(obj); + rb_shape_t *new_child = shape_get_next(new_parent, shape->type, klass, shape->edge_name, true); RUBY_ASSERT(!new_child || new_child->capacity <= shape->capacity); return new_child; } @@ -933,7 +939,8 @@ rb_shape_transition_add_ivar(VALUE obj, ID id) shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj); RUBY_ASSERT(!shape_frozen_p(original_shape_id)); - rb_shape_t *next_shape = shape_get_next(RSHAPE(original_shape_id), SHAPE_IVAR, obj, id, true); + VALUE klass = obj_get_owner_class(obj); + rb_shape_t *next_shape = shape_get_next(RSHAPE(original_shape_id), SHAPE_IVAR, klass, id, true); if (next_shape) { return shape_id(next_shape, original_shape_id); } @@ -948,7 +955,8 @@ rb_shape_transition_add_ivar_no_warnings(VALUE obj, ID id) shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj); RUBY_ASSERT(!shape_frozen_p(original_shape_id)); - rb_shape_t *next_shape = shape_get_next(RSHAPE(original_shape_id), SHAPE_IVAR, obj, id, false); + VALUE klass = obj_get_owner_class(obj); + rb_shape_t *next_shape = shape_get_next(RSHAPE(original_shape_id), SHAPE_IVAR, klass, id, false); if (next_shape) { return shape_id(next_shape, original_shape_id); } From b43e66d3b37d4bd029a90dbee376e475aed79d2a Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Wed, 3 Dec 2025 16:11:30 -0500 Subject: [PATCH 09/13] YJIT: Pass class and shape ID directly instead of object --- shape.c | 4 +--- shape.h | 2 +- yjit/src/codegen.rs | 4 +++- yjit/src/cruby_bindings.inc.rs | 6 +++++- zjit/src/cruby_bindings.inc.rs | 6 +++++- 5 files changed, 15 insertions(+), 7 deletions(-) diff --git a/shape.c b/shape.c index 13d1edf36c928a..90036722f10026 100644 --- a/shape.c +++ b/shape.c @@ -950,12 +950,10 @@ rb_shape_transition_add_ivar(VALUE obj, ID id) } shape_id_t -rb_shape_transition_add_ivar_no_warnings(VALUE obj, ID id) +rb_shape_transition_add_ivar_no_warnings(VALUE klass, shape_id_t original_shape_id, ID id) { - shape_id_t original_shape_id = RBASIC_SHAPE_ID(obj); RUBY_ASSERT(!shape_frozen_p(original_shape_id)); - VALUE klass = obj_get_owner_class(obj); rb_shape_t *next_shape = shape_get_next(RSHAPE(original_shape_id), SHAPE_IVAR, klass, id, false); if (next_shape) { return shape_id(next_shape, original_shape_id); diff --git a/shape.h b/shape.h index 28c2a7eef9421a..bebfaba608c7fa 100644 --- a/shape.h +++ b/shape.h @@ -230,7 +230,7 @@ shape_id_t rb_shape_transition_frozen(VALUE obj); shape_id_t rb_shape_transition_complex(VALUE obj); shape_id_t rb_shape_transition_remove_ivar(VALUE obj, ID id, shape_id_t *removed_shape_id); shape_id_t rb_shape_transition_add_ivar(VALUE obj, ID id); -shape_id_t rb_shape_transition_add_ivar_no_warnings(VALUE obj, ID id); +shape_id_t rb_shape_transition_add_ivar_no_warnings(VALUE klass, shape_id_t original_shape_id, ID id); shape_id_t rb_shape_transition_object_id(VALUE obj); shape_id_t rb_shape_transition_heap(VALUE obj, size_t heap_index); shape_id_t rb_shape_object_id(shape_id_t original_shape_id); diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 50762c64d3eceb..865f80ca4f0cab 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -3101,7 +3101,9 @@ fn gen_set_ivar( let mut new_shape_too_complex = false; let new_shape = if !shape_too_complex && receiver_t_object && ivar_index.is_none() { let current_shape_id = comptime_receiver.shape_id_of(); - let next_shape_id = unsafe { rb_shape_transition_add_ivar_no_warnings(comptime_receiver, ivar_name) }; + // We don't need to check about imemo_fields here because we're definitely looking at a T_OBJECT. + let klass = unsafe { rb_obj_class(comptime_receiver) }; + let next_shape_id = unsafe { rb_shape_transition_add_ivar_no_warnings(klass, current_shape_id, ivar_name) }; // If the VM ran out of shapes, or this class generated too many leaf, // it may be de-optimized into OBJ_TOO_COMPLEX_SHAPE (hash-table). diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index 6efef9c55c39c4..b9a8197184a21a 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -1064,7 +1064,11 @@ extern "C" { pub fn rb_shape_id_offset() -> i32; pub fn rb_obj_shape_id(obj: VALUE) -> shape_id_t; pub fn rb_shape_get_iv_index(shape_id: shape_id_t, id: ID, value: *mut attr_index_t) -> bool; - pub fn rb_shape_transition_add_ivar_no_warnings(obj: VALUE, id: ID) -> shape_id_t; + pub fn rb_shape_transition_add_ivar_no_warnings( + klass: VALUE, + original_shape_id: shape_id_t, + id: ID, + ) -> shape_id_t; pub fn rb_ivar_get_at(obj: VALUE, index: attr_index_t, id: ID) -> VALUE; pub fn rb_ivar_get_at_no_ractor_check(obj: VALUE, index: attr_index_t) -> VALUE; pub fn rb_gvar_get(arg1: ID) -> VALUE; diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 7bd392563987a3..0048fd6daece27 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -2033,7 +2033,11 @@ unsafe extern "C" { pub fn rb_shape_id_offset() -> i32; pub fn rb_obj_shape_id(obj: VALUE) -> shape_id_t; pub fn rb_shape_get_iv_index(shape_id: shape_id_t, id: ID, value: *mut attr_index_t) -> bool; - pub fn rb_shape_transition_add_ivar_no_warnings(obj: VALUE, id: ID) -> shape_id_t; + pub fn rb_shape_transition_add_ivar_no_warnings( + klass: VALUE, + original_shape_id: shape_id_t, + id: ID, + ) -> shape_id_t; pub fn rb_ivar_get_at_no_ractor_check(obj: VALUE, index: attr_index_t) -> VALUE; pub fn rb_gvar_get(arg1: ID) -> VALUE; pub fn rb_gvar_set(arg1: ID, arg2: VALUE) -> VALUE; From b79ef73a3bb911c8d5ac9016092eab08b146fb3a Mon Sep 17 00:00:00 2001 From: yui-knk Date: Wed, 3 Dec 2025 11:27:06 +0900 Subject: [PATCH 10/13] Remove needless parse.y `value_expr` macro In the past parse.y and ripper had different `value_expr` definition so that `value_expr` does nothing for ripper. ```c // parse.y #define value_expr(node) value_expr_gen(p, (node)) // ripper #define value_expr(node) ((void)(node)) ``` However Rearchitect Ripper (89cfc1520717257073012ec07105c551e4b8af7c) removed `value_expr` definition for ripper then this commit removes needless parse.y macro and uses `value_expr_gen` directly. --- parse.y | 47 +++++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/parse.y b/parse.y index a9fccdd5f721e0..e65159257632c0 100644 --- a/parse.y +++ b/parse.y @@ -1402,10 +1402,9 @@ static NODE *logop(struct parser_params*,ID,NODE*,NODE*,const YYLTYPE*,const YYL static NODE *newline_node(NODE*); static void fixpos(NODE*,NODE*); -static int value_expr_gen(struct parser_params*,NODE*); +static int value_expr(struct parser_params*,NODE*); static void void_expr(struct parser_params*,NODE*); static NODE *remove_begin(NODE*); -#define value_expr(node) value_expr_gen(p, (node)) static NODE *void_stmts(struct parser_params*,NODE*); static void reduce_nodes(struct parser_params*,NODE**); static void block_dup_check(struct parser_params*,NODE*,NODE*); @@ -3103,39 +3102,39 @@ rb_parser_ary_free(rb_parser_t *p, rb_parser_ary_t *ary) %rule range_expr(range) : range tDOT2 range { - value_expr($1); - value_expr($3); + value_expr(p, $1); + value_expr(p, $3); $$ = NEW_DOT2($1, $3, &@$, &@2); /*% ripper: dot2!($:1, $:3) %*/ } | range tDOT3 range { - value_expr($1); - value_expr($3); + value_expr(p, $1); + value_expr(p, $3); $$ = NEW_DOT3($1, $3, &@$, &@2); /*% ripper: dot3!($:1, $:3) %*/ } | range tDOT2 { - value_expr($1); + value_expr(p, $1); $$ = NEW_DOT2($1, new_nil_at(p, &@2.end_pos), &@$, &@2); /*% ripper: dot2!($:1, Qnil) %*/ } | range tDOT3 { - value_expr($1); + value_expr(p, $1); $$ = NEW_DOT3($1, new_nil_at(p, &@2.end_pos), &@$, &@2); /*% ripper: dot3!($:1, Qnil) %*/ } | tBDOT2 range { - value_expr($2); + value_expr(p, $2); $$ = NEW_DOT2(new_nil_at(p, &@1.beg_pos), $2, &@$, &@1); /*% ripper: dot2!(Qnil, $:2) %*/ } | tBDOT3 range { - value_expr($2); + value_expr(p, $2); $$ = NEW_DOT3(new_nil_at(p, &@1.beg_pos), $2, &@$, &@1); /*% ripper: dot3!(Qnil, $:2) %*/ } @@ -3144,7 +3143,7 @@ rb_parser_ary_free(rb_parser_t *p, rb_parser_ary_t *ary) %rule value_expr(value) : value { - value_expr($1); + value_expr(p, $1); $$ = $1; } ; @@ -3467,7 +3466,7 @@ expr : command_call } | arg tASSOC { - value_expr($arg); + value_expr(p, $arg); } p_in_kwarg[ctxt] p_pvtbl p_pktbl p_top_expr_body[body] @@ -3482,7 +3481,7 @@ expr : command_call } | arg keyword_in { - value_expr($arg); + value_expr(p, $arg); } p_in_kwarg[ctxt] p_pvtbl p_pktbl p_top_expr_body[body] @@ -4059,7 +4058,7 @@ arg : asgn(arg_rhs) ternary : arg '?' arg '\n'? ':' arg { - value_expr($1); + value_expr(p, $1); $$ = new_if(p, $1, $3, $6, &@$, &NULL_LOC, &@5, &NULL_LOC); fixpos($$, $1); /*% ripper: ifop!($:1, $:3, $:6) %*/ @@ -4138,13 +4137,13 @@ aref_args : none arg_rhs : arg %prec tOP_ASGN { - value_expr($1); + value_expr(p, $1); $$ = $1; } | arg modifier_rescue after_rescue arg { p->ctxt.in_rescue = $3.in_rescue; - value_expr($1); + value_expr(p, $1); $$ = rescued_expr(p, $1, $4, &@1, &@2, &@4); /*% ripper: rescue_mod!($:1, $:4) %*/ } @@ -12820,8 +12819,8 @@ call_bin_op(struct parser_params *p, NODE *recv, ID id, NODE *arg1, const YYLTYPE *op_loc, const YYLTYPE *loc) { NODE *expr; - value_expr(recv); - value_expr(arg1); + value_expr(p, recv); + value_expr(p, arg1); expr = NEW_OPCALL(recv, id, NEW_LIST(arg1, &arg1->nd_loc), loc); nd_set_line(expr, op_loc->beg_pos.lineno); return expr; @@ -12831,7 +12830,7 @@ static NODE * call_uni_op(struct parser_params *p, NODE *recv, ID id, const YYLTYPE *op_loc, const YYLTYPE *loc) { NODE *opcall; - value_expr(recv); + value_expr(p, recv); opcall = NEW_OPCALL(recv, id, 0, loc); nd_set_line(opcall, op_loc->beg_pos.lineno); return opcall; @@ -12881,8 +12880,8 @@ match_op(struct parser_params *p, NODE *node1, NODE *node2, const YYLTYPE *op_lo NODE *n; int line = op_loc->beg_pos.lineno; - value_expr(node1); - value_expr(node2); + value_expr(p, node1); + value_expr(p, node2); if ((n = last_expr_once_body(node1)) != 0) { switch (nd_type(n)) { @@ -13922,7 +13921,7 @@ value_expr_check(struct parser_params *p, NODE *node) } static int -value_expr_gen(struct parser_params *p, NODE *node) +value_expr(struct parser_params *p, NODE *node) { NODE *void_node = value_expr_check(p, node); if (void_node) { @@ -14191,7 +14190,7 @@ range_op(struct parser_params *p, NODE *node, const YYLTYPE *loc) if (node == 0) return 0; type = nd_type(node); - value_expr(node); + value_expr(p, node); if (type == NODE_INTEGER) { if (!e_option_supplied(p)) rb_warn0L(nd_line(node), "integer literal in flip-flop"); ID lineno = rb_intern("$."); @@ -14328,7 +14327,7 @@ logop(struct parser_params *p, ID id, NODE *left, NODE *right, { enum node_type type = id == idAND || id == idANDOP ? NODE_AND : NODE_OR; NODE *op; - value_expr(left); + value_expr(p, left); if (left && nd_type_p(left, type)) { NODE *node = left, *second; while ((second = RNODE_AND(node)->nd_2nd) != 0 && nd_type_p(second, type)) { From e96bbd718e4aef81c749080016a44364a2b8e8c9 Mon Sep 17 00:00:00 2001 From: yui-knk Date: Wed, 3 Dec 2025 15:52:30 +0900 Subject: [PATCH 11/13] Remove needless parse.y `new_nil` macro In the past parse.y and ripper had different `new_nil` definition so that `new_nil` returns `nil` for ripper. ```c // parse.y #define new_nil(loc) NEW_NIL(loc) // ripper #define new_nil(loc) Qnil ``` However Rearchitect Ripper (89cfc1520717257073012ec07105c551e4b8af7c) removed `new_nil` definition for ripper then this commit removes needless parse.y macro and uses `NEW_NIL` directly. --- parse.y | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/parse.y b/parse.y index e65159257632c0..32431434de9734 100644 --- a/parse.y +++ b/parse.y @@ -1393,7 +1393,6 @@ last_expr_node(NODE *expr) static NODE* cond(struct parser_params *p, NODE *node, const YYLTYPE *loc); static NODE* method_cond(struct parser_params *p, NODE *node, const YYLTYPE *loc); -#define new_nil(loc) NEW_NIL(loc) static NODE *new_nil_at(struct parser_params *p, const rb_code_position_t *pos); static NODE *new_if(struct parser_params*,NODE*,NODE*,NODE*,const YYLTYPE*,const YYLTYPE*,const YYLTYPE*,const YYLTYPE*); static NODE *new_unless(struct parser_params*,NODE*,NODE*,NODE*,const YYLTYPE*,const YYLTYPE*,const YYLTYPE*,const YYLTYPE*); @@ -4451,7 +4450,7 @@ primary : inline_primary } | keyword_not '(' rparen { - $$ = call_uni_op(p, method_cond(p, new_nil(&@2), &@2), METHOD_NOT, &@1, &@$); + $$ = call_uni_op(p, method_cond(p, NEW_NIL(&@2), &@2), METHOD_NOT, &@1, &@$); /*% ripper: unary!(ID2VAL(idNOT), Qnil) %*/ } | fcall brace_block From 8f7d821dbae8bba00e1e905bbda1b639998288aa Mon Sep 17 00:00:00 2001 From: Rune Philosof Date: Tue, 28 Oct 2025 13:55:59 +0100 Subject: [PATCH 12/13] [ruby/psych] Remove y Object extension in IRB Fixes: ruby#685 This feature can easily break how you use other gems like factory_bot or prawn. https://github.com/ruby/psych/pull/747#issuecomment-3413139525 > But I kind of think we should leave `psych/y` around. If people really want to use it they could require the file. If you miss the function in Kernel, you can require it interactively or add it to `.irbrc`: ```ruby require 'psych/y' ``` https://github.com/ruby/psych/commit/f1610b3f05 --- ext/psych/lib/psych/core_ext.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ext/psych/lib/psych/core_ext.rb b/ext/psych/lib/psych/core_ext.rb index 78fe262239a391..6dfd0f16962896 100644 --- a/ext/psych/lib/psych/core_ext.rb +++ b/ext/psych/lib/psych/core_ext.rb @@ -14,10 +14,6 @@ def to_yaml options = {} end end -if defined?(::IRB) - require_relative 'y' -end - # Up to Ruby 3.4, Set was a regular object and was dumped as such # by Pysch. # Starting from Ruby 4.0 it's a core class written in C, so we have to implement From 0e7e6858e3a3689708371028cb1553317b905bb0 Mon Sep 17 00:00:00 2001 From: Caleb Stewart Date: Mon, 4 Aug 2025 20:52:52 -0500 Subject: [PATCH 13/13] [ruby/psych] Add option to disable symbol parsing https://github.com/ruby/psych/commit/4e9d08c285 --- ext/psych/lib/psych.rb | 13 +++++++------ ext/psych/lib/psych/nodes/node.rb | 4 ++-- ext/psych/lib/psych/scalar_scanner.rb | 5 +++-- ext/psych/lib/psych/visitors/to_ruby.rb | 4 ++-- test/psych/test_scalar_scanner.rb | 5 +++++ 5 files changed, 19 insertions(+), 12 deletions(-) diff --git a/ext/psych/lib/psych.rb b/ext/psych/lib/psych.rb index 0c158c9ff329b1..850a6d19374d87 100644 --- a/ext/psych/lib/psych.rb +++ b/ext/psych/lib/psych.rb @@ -269,10 +269,10 @@ module Psych # YAML documents that are supplied via user input. Instead, please use the # load method or the safe_load method. # - def self.unsafe_load yaml, filename: nil, fallback: false, symbolize_names: false, freeze: false, strict_integer: false + def self.unsafe_load yaml, filename: nil, fallback: false, symbolize_names: false, freeze: false, strict_integer: false, parse_symbols: true result = parse(yaml, filename: filename) return fallback unless result - result.to_ruby(symbolize_names: symbolize_names, freeze: freeze, strict_integer: strict_integer) + result.to_ruby(symbolize_names: symbolize_names, freeze: freeze, strict_integer: strict_integer, parse_symbols: parse_symbols) end ### @@ -320,13 +320,13 @@ def self.unsafe_load yaml, filename: nil, fallback: false, symbolize_names: fals # Psych.safe_load("---\n foo: bar") # => {"foo"=>"bar"} # Psych.safe_load("---\n foo: bar", symbolize_names: true) # => {:foo=>"bar"} # - def self.safe_load yaml, permitted_classes: [], permitted_symbols: [], aliases: false, filename: nil, fallback: nil, symbolize_names: false, freeze: false, strict_integer: false + def self.safe_load yaml, permitted_classes: [], permitted_symbols: [], aliases: false, filename: nil, fallback: nil, symbolize_names: false, freeze: false, strict_integer: false, parse_symbols: true result = parse(yaml, filename: filename) return fallback unless result class_loader = ClassLoader::Restricted.new(permitted_classes.map(&:to_s), permitted_symbols.map(&:to_s)) - scanner = ScalarScanner.new class_loader, strict_integer: strict_integer + scanner = ScalarScanner.new class_loader, strict_integer: strict_integer, parse_symbols: parse_symbols visitor = if aliases Visitors::ToRuby.new scanner, class_loader, symbolize_names: symbolize_names, freeze: freeze else @@ -366,7 +366,7 @@ def self.safe_load yaml, permitted_classes: [], permitted_symbols: [], aliases: # Raises a TypeError when `yaml` parameter is NilClass. This method is # similar to `safe_load` except that `Symbol` objects are allowed by default. # - def self.load yaml, permitted_classes: [Symbol], permitted_symbols: [], aliases: false, filename: nil, fallback: nil, symbolize_names: false, freeze: false, strict_integer: false + def self.load yaml, permitted_classes: [Symbol], permitted_symbols: [], aliases: false, filename: nil, fallback: nil, symbolize_names: false, freeze: false, strict_integer: false, parse_symbols: true safe_load yaml, permitted_classes: permitted_classes, permitted_symbols: permitted_symbols, aliases: aliases, @@ -374,7 +374,8 @@ def self.load yaml, permitted_classes: [Symbol], permitted_symbols: [], aliases: fallback: fallback, symbolize_names: symbolize_names, freeze: freeze, - strict_integer: strict_integer + strict_integer: strict_integer, + parse_symbols: parse_symbols end ### diff --git a/ext/psych/lib/psych/nodes/node.rb b/ext/psych/lib/psych/nodes/node.rb index 6ae5c591484539..f89c82b7f76df8 100644 --- a/ext/psych/lib/psych/nodes/node.rb +++ b/ext/psych/lib/psych/nodes/node.rb @@ -45,8 +45,8 @@ def each &block # Convert this node to Ruby. # # See also Psych::Visitors::ToRuby - def to_ruby(symbolize_names: false, freeze: false, strict_integer: false) - Visitors::ToRuby.create(symbolize_names: symbolize_names, freeze: freeze, strict_integer: strict_integer).accept(self) + def to_ruby(symbolize_names: false, freeze: false, strict_integer: false, parse_symbols: true) + Visitors::ToRuby.create(symbolize_names: symbolize_names, freeze: freeze, strict_integer: strict_integer, parse_symbols: true).accept(self) end alias :transform :to_ruby diff --git a/ext/psych/lib/psych/scalar_scanner.rb b/ext/psych/lib/psych/scalar_scanner.rb index 8cf868f8638e9e..6a556fb3b89bc2 100644 --- a/ext/psych/lib/psych/scalar_scanner.rb +++ b/ext/psych/lib/psych/scalar_scanner.rb @@ -27,10 +27,11 @@ class ScalarScanner attr_reader :class_loader # Create a new scanner - def initialize class_loader, strict_integer: false + def initialize class_loader, strict_integer: false, parse_symbols: true @symbol_cache = {} @class_loader = class_loader @strict_integer = strict_integer + @parse_symbols = parse_symbols end # Tokenize +string+ returning the Ruby object @@ -72,7 +73,7 @@ def tokenize string -Float::INFINITY elsif string.match?(/^\.nan$/i) Float::NAN - elsif string.match?(/^:./) + elsif @parse_symbols && string.match?(/^:./) if string =~ /^:(["'])(.*)\1/ @symbol_cache[string] = class_loader.symbolize($2.sub(/^:/, '')) else diff --git a/ext/psych/lib/psych/visitors/to_ruby.rb b/ext/psych/lib/psych/visitors/to_ruby.rb index 2814ce1a695df0..e62311ae12d177 100644 --- a/ext/psych/lib/psych/visitors/to_ruby.rb +++ b/ext/psych/lib/psych/visitors/to_ruby.rb @@ -12,9 +12,9 @@ module Visitors ### # This class walks a YAML AST, converting each node to Ruby class ToRuby < Psych::Visitors::Visitor - def self.create(symbolize_names: false, freeze: false, strict_integer: false) + def self.create(symbolize_names: false, freeze: false, strict_integer: false, parse_symbols: true) class_loader = ClassLoader.new - scanner = ScalarScanner.new class_loader, strict_integer: strict_integer + scanner = ScalarScanner.new class_loader, strict_integer: strict_integer, parse_symbols: parse_symbols new(scanner, class_loader, symbolize_names: symbolize_names, freeze: freeze) end diff --git a/test/psych/test_scalar_scanner.rb b/test/psych/test_scalar_scanner.rb index 2637a74df82b31..bc6a74ad8b2a98 100644 --- a/test/psych/test_scalar_scanner.rb +++ b/test/psych/test_scalar_scanner.rb @@ -138,6 +138,11 @@ def test_scan_strings_with_strict_int_delimiters assert_equal '-0b___', scanner.tokenize('-0b___') end + def test_scan_without_parse_symbols + scanner = Psych::ScalarScanner.new ClassLoader.new, parse_symbols: false + assert_equal ':foo', scanner.tokenize(':foo') + end + def test_scan_int_commas_and_underscores # NB: This test is to ensure backward compatibility with prior Psych versions, # not to test against any actual YAML specification.