From 7d2b724e08a06146887cb7912064b597ae8f69b6 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 6 Aug 2025 18:18:21 -0700 Subject: [PATCH 1/4] Add missing writebarrier on complex obj dup When we dup a complex object we need to issue a writebarrier_remember on the new object. This was caught by wbcheck inside the rubygems test suite. Co-authored-by: Aaron Patterson --- shape.c | 1 + 1 file changed, 1 insertion(+) diff --git a/shape.c b/shape.c index df4faf960c0726..b10b52d76f9458 100644 --- a/shape.c +++ b/shape.c @@ -1180,6 +1180,7 @@ rb_shape_copy_complex_ivars(VALUE dest, VALUE obj, shape_id_t src_shape_id, st_t st_delete(table, &id, NULL); } rb_obj_init_too_complex(dest, table); + rb_gc_writebarrier_remember(dest); } size_t From 0b098a93dbb9cb2112e7c0c8519e09cca4478fd0 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 12 Aug 2025 10:41:49 -0700 Subject: [PATCH 2/4] Add missing write barriers to Random We recently converted this to be WB_PROTECTED, but missed some locations the seed was set. This is a little bit confusing because as far as I can tell some of the seeds are set in Ractor local storage, and others on actual objects. --- random.c | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/random.c b/random.c index 85d72057cd6997..9b8cec40b4abb7 100644 --- a/random.c +++ b/random.c @@ -142,18 +142,21 @@ static const rb_random_interface_t random_mt_if = { }; static rb_random_mt_t * -rand_mt_start(rb_random_mt_t *r) +rand_mt_start(rb_random_mt_t *r, VALUE obj) { if (!genrand_initialized(&r->mt)) { r->base.seed = rand_init(&random_mt_if, &r->base, random_seed(Qundef)); + if (obj) { + RB_OBJ_WRITTEN(obj, Qundef, r->base.seed); + } } return r; } static rb_random_t * -rand_start(rb_random_mt_t *r) +rand_start(rb_random_mt_t *r, VALUE obj) { - return &rand_mt_start(r)->base; + return &rand_mt_start(r, obj)->base; } static rb_ractor_local_key_t default_rand_key; @@ -192,7 +195,13 @@ default_rand(void) static rb_random_mt_t * default_mt(void) { - return rand_mt_start(default_rand()); + return rand_mt_start(default_rand(), Qfalse); +} + +static rb_random_t * +default_rand_start(void) +{ + return &default_mt()->base; } unsigned int @@ -293,7 +302,7 @@ get_rnd(VALUE obj) rb_random_t *ptr; TypedData_Get_Struct(obj, rb_random_t, &rb_random_data_type, ptr); if (RTYPEDDATA_TYPE(obj) == &random_mt_type) - return rand_start((rb_random_mt_t *)ptr); + return rand_start((rb_random_mt_t *)ptr, obj); return ptr; } @@ -309,11 +318,11 @@ static rb_random_t * try_get_rnd(VALUE obj) { if (obj == rb_cRandom) { - return rand_start(default_rand()); + return default_rand_start(); } if (!rb_typeddata_is_kind_of(obj, &rb_random_data_type)) return NULL; if (RTYPEDDATA_TYPE(obj) == &random_mt_type) - return rand_start(DATA_PTR(obj)); + return rand_start(DATA_PTR(obj), obj); rb_random_t *rnd = DATA_PTR(obj); if (!rnd) { rb_raise(rb_eArgError, "uninitialized random: %s", @@ -829,6 +838,7 @@ rand_mt_copy(VALUE obj, VALUE orig) mt = &rnd1->mt; *rnd1 = *rnd2; + RB_OBJ_WRITTEN(obj, Qundef, rnd1->base.seed); mt->next = mt->state + numberof(mt->state) - mt->left + 1; return obj; } @@ -916,7 +926,7 @@ rand_mt_load(VALUE obj, VALUE dump) } mt->left = (unsigned int)x; mt->next = mt->state + numberof(mt->state) - x + 1; - rnd->base.seed = rb_to_int(seed); + RB_OBJ_WRITE(obj, &rnd->base.seed, rb_to_int(seed)); return obj; } @@ -975,7 +985,7 @@ static VALUE rb_f_srand(int argc, VALUE *argv, VALUE obj) { VALUE seed, old; - rb_random_mt_t *r = rand_mt_start(default_rand()); + rb_random_mt_t *r = default_mt(); if (rb_check_arity(argc, 0, 1) == 0) { seed = random_seed(obj); @@ -1337,7 +1347,7 @@ rb_random_bytes(VALUE obj, long n) static VALUE random_s_bytes(VALUE obj, VALUE len) { - rb_random_t *rnd = rand_start(default_rand()); + rb_random_t *rnd = default_rand_start(); return rand_bytes(&random_mt_if, rnd, NUM2LONG(rb_to_int(len))); } @@ -1359,7 +1369,7 @@ random_s_bytes(VALUE obj, VALUE len) static VALUE random_s_seed(VALUE obj) { - rb_random_mt_t *rnd = rand_mt_start(default_rand()); + rb_random_mt_t *rnd = default_mt(); return rnd->base.seed; } @@ -1689,7 +1699,7 @@ static VALUE rb_f_rand(int argc, VALUE *argv, VALUE obj) { VALUE vmax; - rb_random_t *rnd = rand_start(default_rand()); + rb_random_t *rnd = default_rand_start(); if (rb_check_arity(argc, 0, 1) && !NIL_P(vmax = argv[0])) { VALUE v = rand_range(obj, rnd, vmax); @@ -1716,7 +1726,7 @@ rb_f_rand(int argc, VALUE *argv, VALUE obj) static VALUE random_s_rand(int argc, VALUE *argv, VALUE obj) { - VALUE v = rand_random(argc, argv, Qnil, rand_start(default_rand())); + VALUE v = rand_random(argc, argv, Qnil, default_rand_start()); check_random_number(v, argv); return v; } From 6f472c8b61bf8bbf4c2fd3c0af9ddaf92538c94e Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Mon, 18 Aug 2025 12:59:06 -0700 Subject: [PATCH 3/4] ZJIT: Fix BorrowError on --zjit-dump-disasm (#14267) --- zjit/src/codegen.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index d2810cddb7bb72..5780a2635778f9 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -152,8 +152,9 @@ fn gen_iseq_call(cb: &mut CodeBlock, caller_iseq: IseqPtr, iseq_call: &Rc>) // Update the stub to call the code pointer let code_addr = code_ptr.raw_ptr(cb); + let iseq = iseq_call.borrow().iseq; iseq_call.borrow_mut().regenerate(cb, |asm| { - asm_comment!(asm, "call compiled function: {}", iseq_get_location(iseq_call.borrow().iseq, 0)); + asm_comment!(asm, "call compiled function: {}", iseq_get_location(iseq, 0)); asm.ccall(code_addr, vec![]); }); From 306d50811dd060d876d1eb364a0d5e6106f5e4f1 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Mon, 18 Aug 2025 15:58:38 -0700 Subject: [PATCH 4/4] Don't allow looking at the shape ID of immediates (#14266) It only makes sense for heap objects. --- shape.c | 5 ++- test/ruby/test_shapes.rb | 22 +++++++++---- yjit/src/codegen.rs | 4 +-- zjit/src/profile.rs | 70 ++++++++++++++++++++++++++++++++-------- 4 files changed, 79 insertions(+), 22 deletions(-) diff --git a/shape.c b/shape.c index b10b52d76f9458..d58f9f1d0ca568 100644 --- a/shape.c +++ b/shape.c @@ -369,7 +369,7 @@ RUBY_FUNC_EXPORTED shape_id_t rb_obj_shape_id(VALUE obj) { if (RB_SPECIAL_CONST_P(obj)) { - return SPECIAL_CONST_SHAPE_ID; + rb_bug("rb_obj_shape_id: called on a special constant"); } if (BUILTIN_TYPE(obj) == T_CLASS || BUILTIN_TYPE(obj) == T_MODULE) { @@ -1425,6 +1425,9 @@ rb_shape_parent(VALUE self) static VALUE rb_shape_debug_shape(VALUE self, VALUE obj) { + if (RB_SPECIAL_CONST_P(obj)) { + rb_raise(rb_eArgError, "Can't get shape of special constant"); + } return shape_id_t_to_rb_cShape(rb_obj_shape_id(obj)); } diff --git a/test/ruby/test_shapes.rb b/test/ruby/test_shapes.rb index 77bba6421baee0..50b1679c187e69 100644 --- a/test/ruby/test_shapes.rb +++ b/test/ruby/test_shapes.rb @@ -1032,12 +1032,22 @@ def test_array_has_root_shape assert_shape_equal(RubyVM::Shape.root_shape, RubyVM::Shape.of([])) end - def test_true_has_special_const_shape_id - assert_equal(RubyVM::Shape::SPECIAL_CONST_SHAPE_ID, RubyVM::Shape.of(true).id) - end - - def test_nil_has_special_const_shape_id - assert_equal(RubyVM::Shape::SPECIAL_CONST_SHAPE_ID, RubyVM::Shape.of(nil).id) + def test_raise_on_special_consts + assert_raise ArgumentError do + RubyVM::Shape.of(true) + end + assert_raise ArgumentError do + RubyVM::Shape.of(false) + end + assert_raise ArgumentError do + RubyVM::Shape.of(nil) + end + assert_raise ArgumentError do + RubyVM::Shape.of(0) + end + assert_raise ArgumentError do + RubyVM::Shape.of(:foo) + end end def test_root_shape_transition_to_special_const_on_frozen diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 9644b948d7d515..44d458020db307 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -3104,7 +3104,7 @@ fn gen_set_ivar( // Get the iv index let shape_too_complex = comptime_receiver.shape_too_complex(); - let ivar_index = if !shape_too_complex { + let ivar_index = if !comptime_receiver.special_const_p() && !shape_too_complex { let shape_id = comptime_receiver.shape_id_of(); let mut ivar_index: u16 = 0; if unsafe { rb_shape_get_iv_index(shape_id, ivar_name, &mut ivar_index) } { @@ -3369,7 +3369,7 @@ fn gen_definedivar( // Specialize base on compile time values let comptime_receiver = jit.peek_at_self(); - if comptime_receiver.shape_too_complex() || asm.ctx.get_chain_depth() >= GET_IVAR_MAX_DEPTH { + if comptime_receiver.special_const_p() || comptime_receiver.shape_too_complex() || asm.ctx.get_chain_depth() >= GET_IVAR_MAX_DEPTH { // Fall back to calling rb_ivar_defined // Save the PC and SP because the callee may allocate diff --git a/zjit/src/profile.rs b/zjit/src/profile.rs index 7ffaea29dcdc81..771d90cb0ec426 100644 --- a/zjit/src/profile.rs +++ b/zjit/src/profile.rs @@ -98,19 +98,32 @@ fn profile_operands(profiler: &mut Profiler, profile: &mut IseqProfile, n: usize let obj = profiler.peek_at_stack((n - i - 1) as isize); // TODO(max): Handle GC-hidden classes like Array, Hash, etc and make them look normal or // drop them or something - let ty = ProfiledType::new(obj.class_of(), obj.shape_id_of()); + let ty = ProfiledType::new(obj); unsafe { rb_gc_writebarrier(profiler.iseq.into(), ty.class()) }; types[i].observe(ty); } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct Flags(u32); + +impl Flags { + const NONE: u32 = 0; + const IS_IMMEDIATE: u32 = 1 << 0; + + pub fn none() -> Self { Self(Self::NONE) } + + pub fn immediate() -> Self { Self(Self::IS_IMMEDIATE) } + pub fn is_immediate(self) -> bool { (self.0 & Self::IS_IMMEDIATE) != 0 } +} + /// opt_send_without_block/opt_plus/... should store: /// * the class of the receiver, so we can do method lookup /// * the shape of the receiver, so we can optimize ivar lookup /// with those two, pieces of information, we can also determine when an object is an immediate: -/// * Integer + SPECIAL_CONST_SHAPE_ID == Fixnum -/// * Float + SPECIAL_CONST_SHAPE_ID == Flonum -/// * Symbol + SPECIAL_CONST_SHAPE_ID == StaticSymbol +/// * Integer + IS_IMMEDIATE == Fixnum +/// * Float + IS_IMMEDIATE == Flonum +/// * Symbol + IS_IMMEDIATE == StaticSymbol /// * NilClass == Nil /// * TrueClass == True /// * FalseClass == False @@ -118,6 +131,7 @@ fn profile_operands(profiler: &mut Profiler, profile: &mut IseqProfile, n: usize pub struct ProfiledType { class: VALUE, shape: ShapeId, + flags: Flags, } impl Default for ProfiledType { @@ -127,12 +141,42 @@ impl Default for ProfiledType { } impl ProfiledType { - fn new(class: VALUE, shape: ShapeId) -> Self { - Self { class, shape } + fn new(obj: VALUE) -> Self { + if obj == Qfalse { + return Self { class: unsafe { rb_cFalseClass }, + shape: INVALID_SHAPE_ID, + flags: Flags::immediate() }; + } + if obj == Qtrue { + return Self { class: unsafe { rb_cTrueClass }, + shape: INVALID_SHAPE_ID, + flags: Flags::immediate() }; + } + if obj == Qnil { + return Self { class: unsafe { rb_cNilClass }, + shape: INVALID_SHAPE_ID, + flags: Flags::immediate() }; + } + if obj.fixnum_p() { + return Self { class: unsafe { rb_cInteger }, + shape: INVALID_SHAPE_ID, + flags: Flags::immediate() }; + } + if obj.flonum_p() { + return Self { class: unsafe { rb_cFloat }, + shape: INVALID_SHAPE_ID, + flags: Flags::immediate() }; + } + if obj.static_sym_p() { + return Self { class: unsafe { rb_cSymbol }, + shape: INVALID_SHAPE_ID, + flags: Flags::immediate() }; + } + Self { class: obj.class_of(), shape: obj.shape_id_of(), flags: Flags::none() } } pub fn empty() -> Self { - Self { class: VALUE(0), shape: INVALID_SHAPE_ID } + Self { class: VALUE(0), shape: INVALID_SHAPE_ID, flags: Flags::none() } } pub fn is_empty(&self) -> bool { @@ -148,27 +192,27 @@ impl ProfiledType { } pub fn is_fixnum(&self) -> bool { - self.class == unsafe { rb_cInteger } && self.shape == SPECIAL_CONST_SHAPE_ID + self.class == unsafe { rb_cInteger } && self.flags.is_immediate() } pub fn is_flonum(&self) -> bool { - self.class == unsafe { rb_cFloat } && self.shape == SPECIAL_CONST_SHAPE_ID + self.class == unsafe { rb_cFloat } && self.flags.is_immediate() } pub fn is_static_symbol(&self) -> bool { - self.class == unsafe { rb_cSymbol } && self.shape == SPECIAL_CONST_SHAPE_ID + self.class == unsafe { rb_cSymbol } && self.flags.is_immediate() } pub fn is_nil(&self) -> bool { - self.class == unsafe { rb_cNilClass } && self.shape == SPECIAL_CONST_SHAPE_ID + self.class == unsafe { rb_cNilClass } && self.flags.is_immediate() } pub fn is_true(&self) -> bool { - self.class == unsafe { rb_cTrueClass } && self.shape == SPECIAL_CONST_SHAPE_ID + self.class == unsafe { rb_cTrueClass } && self.flags.is_immediate() } pub fn is_false(&self) -> bool { - self.class == unsafe { rb_cFalseClass } && self.shape == SPECIAL_CONST_SHAPE_ID + self.class == unsafe { rb_cFalseClass } && self.flags.is_immediate() } }