Move ROBJECT_HEAP into shape

Author: peterzhu2118

debug.c

@@ -52,7 +52,6 @@ const union {
     enum ruby_econv_flag_type   econv_flag_types;
     rb_econv_result_t           econv_result;
     enum ruby_preserved_encindex encoding_index;
-    enum ruby_robject_flags     robject_flags;
     enum ruby_rmodule_flags     rmodule_flags;
     enum ruby_rstring_flags     rstring_flags;
     enum ruby_rarray_flags      rarray_flags;

ext/objspace/objspace_dump.c

@@ -592,7 +592,7 @@ dump_object(VALUE obj, struct dump_config *dc)
         break;
 
       case T_OBJECT:
-        if (!FL_TEST_RAW(obj, ROBJECT_HEAP)) {
+        if (!rb_obj_shape_heap_p(obj)) {
             dump_append(dc, ", \"embedded\":true");
         }
 

gc.c

@@ -1356,7 +1356,7 @@ rb_gc_imemo_needs_cleanup_p(VALUE obj)
         return ((rb_imemo_tmpbuf_t *)obj)->ptr != NULL;
 
       case imemo_fields:
-        return FL_TEST_RAW(obj, OBJ_FIELD_HEAP) || (id2ref_tbl && rb_obj_shape_has_id(obj));
+        return rb_obj_shape_heap_p(obj) || (id2ref_tbl && rb_obj_shape_has_id(obj));
     }
     UNREACHABLE_RETURN(true);
 }
@@ -1412,7 +1412,7 @@ rb_gc_obj_needs_cleanup_p(VALUE obj)
 
     switch (flags & RUBY_T_MASK) {
       case T_OBJECT:
-        if (flags & ROBJECT_HEAP) return true;
+        if (rb_obj_shape_heap_p(obj)) return true;
         return false;
 
       case T_DATA:
@@ -1557,7 +1557,7 @@ rb_gc_obj_free(void *objspace, VALUE obj)
 
     switch (BUILTIN_TYPE(obj)) {
       case T_OBJECT:
-        if (FL_TEST_RAW(obj, ROBJECT_HEAP)) {
+        if (rb_obj_shape_heap_p(obj)) {
             if (rb_obj_shape_complex_p(obj)) {
                 RB_DEBUG_COUNTER_INC(obj_obj_complex);
                 st_free_table(ROBJECT_FIELDS_HASH(obj));
@@ -2198,7 +2198,7 @@ object_id0(VALUE obj)
     id = generate_next_object_id();
     rb_obj_field_set(obj, object_id_shape_id, 0, id);
 
-    RUBY_ASSERT(RBASIC_SHAPE_ID(obj) == object_id_shape_id);
+    RUBY_ASSERT(RBASIC_SHAPE_ID(obj) == object_id_shape_id || RBASIC_SHAPE_ID(obj) == rb_shape_transition_heap(object_id_shape_id));
     RUBY_ASSERT(rb_obj_shape_has_id(obj));
 
     if (RB_UNLIKELY(id2ref_tbl)) {
@@ -2604,7 +2604,7 @@ rb_obj_memsize_of(VALUE obj)
 
     switch (BUILTIN_TYPE(obj)) {
       case T_OBJECT:
-        if (FL_TEST_RAW(obj, ROBJECT_HEAP)) {
+        if (rb_obj_shape_heap_p(obj)) {
             if (rb_obj_shape_complex_p(obj)) {
                 size += rb_st_memsize(ROBJECT_FIELDS_HASH(obj));
             }
@@ -3882,7 +3882,7 @@ gc_ref_update_object(void *objspace, VALUE v)
 {
     VALUE *ptr = ROBJECT_FIELDS(v);
 
-    if (FL_TEST_RAW(v, ROBJECT_HEAP)) {
+    if (rb_obj_shape_heap_p(v)) {
         if (rb_obj_shape_complex_p(v)) {
             gc_ref_update_table_values_only(ROBJECT_FIELDS_HASH(v));
             return;
@@ -3894,7 +3894,7 @@ gc_ref_update_object(void *objspace, VALUE v)
             // Object can be re-embedded
             memcpy(ROBJECT(v)->as.ary, ptr, sizeof(VALUE) * ROBJECT_FIELDS_COUNT(v));
             SIZED_FREE_N(ptr, ROBJECT_FIELDS_CAPACITY(v));
-            FL_UNSET_RAW(v, ROBJECT_HEAP);
+            RBASIC_SET_SHAPE_ID(v, rb_obj_shape_transition_embedded(v));
             ptr = ROBJECT(v)->as.ary;
         }
     }
@@ -5087,7 +5087,7 @@ rb_raw_obj_info_buitin_type(char *const buff, const size_t buff_size, const VALU
             }
           case T_OBJECT:
             {
-                if (FL_TEST_RAW(obj, ROBJECT_HEAP)) {
+                if (rb_obj_shape_heap_p(obj)) {
                     if (rb_obj_shape_complex_p(obj)) {
                         size_t hash_len = rb_st_table_size(ROBJECT_FIELDS_HASH(obj));
                         APPEND_F("(complex) len:%zu", hash_len);

imemo.c

@@ -141,7 +141,7 @@ rb_imemo_fields_new(VALUE owner, shape_id_t shape_id, bool shareable)
     // layout in the shape describes the layout of the thing on which it is set.
     // Imemo fields have the same layout as robject, therefore the layout
     // should reflect that fact.
-    RBASIC_SET_SHAPE_ID(fields, rb_shape_id_with_robject_layout(shape_id));
+    RBASIC_SET_SHAPE_ID(fields, rb_shape_id_with_robject_layout(rb_shape_transition_embedded(shape_id)));
     RUBY_ASSERT(IMEMO_TYPE_P(fields, imemo_fields));
     return fields;
 }
@@ -151,12 +151,11 @@ rb_imemo_fields_new_complex(VALUE owner, shape_id_t shape_id, size_t capa, bool
 {
     VALUE fields = rb_imemo_new(imemo_fields, owner, sizeof(struct rb_fields), shareable);
     IMEMO_OBJ_FIELDS(fields)->as.complex.table = st_init_numtable_with_size(capa);
-    FL_SET_RAW(fields, OBJ_FIELD_HEAP);
     // imemo fields objects should always have "RObject" layout.  The
     // layout in the shape describes the layout of the thing on which it is set.
     // Imemo fields have the same layout as robject, therefore the layout
     // should reflect that fact.
-    RBASIC_SET_SHAPE_ID(fields, rb_shape_id_with_robject_layout(shape_id));
+    RBASIC_SET_SHAPE_ID(fields, rb_shape_id_with_robject_layout(rb_shape_transition_heap(shape_id)));
     return fields;
 }
 
@@ -180,12 +179,11 @@ rb_imemo_fields_new_complex_tbl(VALUE owner, shape_id_t shape_id, st_table *tbl,
 {
     VALUE fields = rb_imemo_new(imemo_fields, owner, sizeof(struct rb_fields), shareable);
     IMEMO_OBJ_FIELDS(fields)->as.complex.table = tbl;
-    FL_SET_RAW(fields, OBJ_FIELD_HEAP);
     // imemo fields objects should always have "RObject" layout.  The
     // layout in the shape describes the layout of the thing on which it is set.
     // Imemo fields have the same layout as robject, therefore the layout
     // should reflect that fact.
-    RBASIC_SET_SHAPE_ID(fields, rb_shape_id_with_robject_layout(shape_id));
+    RBASIC_SET_SHAPE_ID(fields, rb_shape_id_with_robject_layout(rb_shape_transition_heap(shape_id)));
     st_foreach(tbl, imemo_fields_trigger_wb_i, (st_data_t)fields);
     return fields;
 }
@@ -628,7 +626,7 @@ rb_free_const_table(struct rb_id_table *tbl)
 static inline void
 imemo_fields_free(struct rb_fields *fields)
 {
-    if (FL_TEST_RAW((VALUE)fields, OBJ_FIELD_HEAP)) {
+    if (rb_obj_shape_heap_p((VALUE)fields)) {
         RUBY_ASSERT(rb_shape_complex_p(RBASIC_SHAPE_ID((VALUE)fields)));
         st_free_table(fields->as.complex.table);
     }

include/ruby/internal/core/robject.h

@@ -48,34 +48,6 @@
 #define ROBJECT_FIELDS              ROBJECT_FIELDS
 /** @endcond */
 
-/**
- * @private
- *
- * Bits that you can set to ::RBasic::flags.
- */
-enum ruby_robject_flags {
-    /**
-     * This flag marks that the object's instance variables are stored in an
-     * external heap buffer.
-     * Normally, instance variable references are stored inside the object slot,
-     * but if it overflow, Ruby may have to allocate a separate buffer and spills
-     * the instance variables there.
-     * This flag denotes that situation.
-     *
-     * @warning  This  bit has  to be  considered read-only.   Setting/clearing
-     *           this  bit without  corresponding fix  up must  cause immediate
-     *           SEGV.   Also,   internal  structures   of  an   object  change
-     *           dynamically  and  transparently  throughout of  its  lifetime.
-     *           Don't assume it being persistent.
-     *
-     * @internal
-     *
-     * 3rd parties must  not be aware that  there even is more than  one way to
-     * store instance variables.  Might better be hidden.
-     */
-    ROBJECT_HEAP = RUBY_FL_USER4
-};
-
 struct st_table;
 
 /**

internal/imemo.h

@@ -10,6 +10,7 @@
  */
 #include "ruby/internal/config.h"
 #include <stddef.h>             /* for size_t */
+#include "shape.h"
 #include "id_table.h"
 #include "internal/array.h"     /* for rb_ary_hidden_new_fill */
 #include "ruby/internal/stdbool.h"     /* for bool */
@@ -171,7 +172,7 @@ imemo_type(VALUE imemo)
 static inline int
 imemo_type_p(VALUE imemo, enum imemo_type imemo_type)
 {
-    if (LIKELY(!RB_SPECIAL_CONST_P(imemo))) {
+    if (RB_LIKELY(!RB_SPECIAL_CONST_P(imemo))) {
         /* fixed at compile time if imemo_type is given. */
         const VALUE mask = (IMEMO_MASK << FL_USHIFT) | RUBY_T_MASK;
         const VALUE expected_type = (imemo_type << FL_USHIFT) | T_IMEMO;
@@ -258,7 +259,6 @@ struct rb_fields {
 // IMEMO/fields and T_OBJECT have exactly the same layout.
 // This is useful for JIT and common codepaths.
 #define OBJ_FIELD_HEAP ROBJECT_HEAP
-STATIC_ASSERT(imemo_fields_flags, OBJ_FIELD_HEAP == IMEMO_FL_USER0);
 STATIC_ASSERT(imemo_fields_embed_offset, offsetof(struct RObject, as.ary) == offsetof(struct rb_fields, as.embed.fields));
 STATIC_ASSERT(imemo_fields_external_offset, offsetof(struct RObject, as.heap.fields) == offsetof(struct rb_fields, as.external.ptr));
 STATIC_ASSERT(imemo_fields_complex_offset, offsetof(struct RObject, as.heap.fields) == offsetof(struct rb_fields, as.complex.table));
@@ -308,7 +308,7 @@ rb_imemo_fields_ptr(VALUE fields_obj)
 
     RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields) || RB_TYPE_P(fields_obj, T_OBJECT));
 
-    if (UNLIKELY(FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP))) {
+    if (RB_UNLIKELY(rb_obj_shape_heap_p(fields_obj))) {
         return IMEMO_OBJ_FIELDS(fields_obj)->as.external.ptr;
     }
     else {
@@ -324,7 +324,7 @@ rb_imemo_fields_complex_tbl(VALUE fields_obj)
     }
 
     RUBY_ASSERT(IMEMO_TYPE_P(fields_obj, imemo_fields) || RB_TYPE_P(fields_obj, T_OBJECT));
-    RUBY_ASSERT(FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP));
+    RUBY_ASSERT(rb_obj_shape_heap_p(fields_obj));
 
     // Some codepaths unconditionally access the fields_ptr, and assume it can be used as st_table if the
     // shape is complex.

internal/object.h

@@ -9,6 +9,7 @@
  * @brief      Internal header for Object.
  */
 #include "ruby/ruby.h"          /* for VALUE */
+#include "shape.h"
 
 /* object.c */
 
@@ -68,7 +69,7 @@ ROBJECT_FIELDS(VALUE obj)
 
     struct RObject *const ptr = ROBJECT(obj);
 
-    if (RB_UNLIKELY(RB_FL_ANY_RAW(obj, ROBJECT_HEAP))) {
+    if (RB_UNLIKELY(rb_obj_shape_heap_p(obj))) {
         return ptr->as.heap.fields;
     }
     else {

internal/variable.h

@@ -14,6 +14,8 @@
 #include "ruby/internal/stdbool.h"     /* for bool */
 #include "ruby/ruby.h"          /* for VALUE */
 #include "shape.h"              /* for shape_id_t */
+#include "box.h"
+#include "internal/gc.h"
 
 /* variable.c */
 void rb_gc_mark_global_tbl(void);

object.c

@@ -364,6 +364,7 @@ rb_obj_copy_ivar(VALUE dest, VALUE obj)
     RUBY_ASSERT(src_num_ivs <= dest_capa);
     if (initial_capa < dest_capa) {
         rb_ensure_iv_list_size(dest, 0, dest_capa);
+        dest_shape_id = rb_shape_transition_heap(dest_shape_id);
         dest_buf = ROBJECT_FIELDS(dest);
     }
 

repro_yjit_shape_capacity.rb

@@ -0,0 +1,96 @@
+# frozen_string_literal: true
+
+# Reproducer/probe for dynamic T_OBJECT slot capacity being narrowed before
+# shape code clamps it. Run from build/ with:
+#
+#   ./ruby --yjit --yjit-call-threshold=1 ../repro_yjit_shape_capacity.rb
+#
+# A crashing YJIT run depends on the allocator returning a slot whose capacity
+# gets truncated to a smaller attr_index_t value. This script first checks the
+# underlying invariant directly so it is useful even when it does not crash.
+
+require "objspace"
+
+POINTER_SIZE = [0].pack("J").bytesize
+RBASIC_SIZE = GC::INTERNAL_CONSTANTS.fetch(:RBASIC_SIZE)
+
+def dump_hash(obj)
+  ObjectSpace.dump(obj).then do |dump|
+    require "json"
+    JSON.parse(dump)
+  end
+end
+
+def object_slot_capacity(obj)
+  dump = dump_hash(obj)
+  (dump.fetch("slot_size") - RBASIC_SIZE) / POINTER_SIZE
+end
+
+def shape_capacity(obj)
+  return RubyVM::Shape.of(obj).capacity if defined?(RubyVM::Shape)
+
+  # Without SHAPE_DEBUG, infer only from behavior below.
+  nil
+end
+
+def embedded?(obj)
+  dump_hash(obj).key?("embedded")
+end
+
+shape_max_fields = if defined?(RubyVM::Shape::SHAPE_MAX_FIELDS)
+  RubyVM::Shape::SHAPE_MAX_FIELDS
+else
+  126
+end
+
+class ShapeCapacityProbe
+  def read_last
+    @v_last
+  end
+end
+
+# Force future ShapeCapacityProbe instances to request the largest normal
+# T_OBJECT slot that shapes are supposed to support.
+seed = ShapeCapacityProbe.new
+shape_max_fields.times do |i|
+  seed.instance_variable_set(:"@seed_#{i}", i)
+end
+
+probe = ShapeCapacityProbe.new
+slot_capacity = object_slot_capacity(probe)
+root_capacity = shape_capacity(probe)
+expected_capacity = [slot_capacity, shape_max_fields].min
+
+puts "slot_capacity=#{slot_capacity} expected_shape_capacity=#{expected_capacity} shape_capacity=#{root_capacity.inspect}"
+puts "initial=#{ObjectSpace.dump(probe).strip}"
+
+if root_capacity && root_capacity < expected_capacity
+  abort "BUG: root shape capacity #{root_capacity} is smaller than actual supported capacity #{expected_capacity}"
+end
+
+# Behavior-only check for non-SHAPE_DEBUG builds: fill up to the capacity that
+# should be embeddable in this slot. A bad truncated root capacity externalizes
+# this object before the slot is full.
+expected_capacity.times do |i|
+  name = i == expected_capacity - 1 ? :@v_last : :"@v_#{i}"
+  probe.instance_variable_set(name, i)
+end
+
+puts "filled=#{ObjectSpace.dump(probe).strip}"
+
+unless embedded?(probe)
+  abort "BUG: object externalized after #{expected_capacity} ivars even though its slot can embed them"
+end
+
+if RubyVM.const_defined?(:YJIT) && RubyVM::YJIT.enabled?
+  expected = expected_capacity - 1
+  20_000.times do
+    value = probe.read_last
+    abort "BUG: YJIT read wrong value #{value.inspect}, expected #{expected.inspect}" unless value == expected
+  end
+  puts "YJIT read stress passed"
+else
+  warn "YJIT is not enabled; rerun with --yjit --yjit-call-threshold=1 for the JIT stress path."
+end
+
+puts "No capacity mismatch reproduced on this build."