Refactor obj_ivar_set and vm_setivar

obj_ivar_set and vm_setivar_slowpath is essentially doing the same thing,
but the code is duplicated and not quite implemented in the same way,
which could cause bugs. This commit refactors vm_setivar_slowpath to use
obj_ivar_set.

internal/variable.h

@@ -57,6 +57,7 @@ void rb_const_warn_if_deprecated(const rb_const_entry_t *, VALUE, ID);
 rb_shape_t * rb_grow_iv_list(VALUE obj);
 void rb_ensure_iv_list_size(VALUE obj, uint32_t len, uint32_t newsize);
 struct gen_ivtbl * rb_ensure_generic_iv_list_size(VALUE obj, uint32_t newsize);
+attr_index_t rb_obj_ivar_set(VALUE obj, ID id, VALUE val);
 MJIT_SYMBOL_EXPORT_END
 
 static inline bool

object.c

@@ -2790,7 +2790,7 @@ rb_obj_ivar_get(VALUE obj, VALUE iv)
  */
 
 static VALUE
-rb_obj_ivar_set(VALUE obj, VALUE iv, VALUE val)
+rb_obj_ivar_set_m(VALUE obj, VALUE iv, VALUE val)
 {
     ID id = id_for_var(obj, iv, instance);
     if (!id) id = rb_intern_str(iv);
@@ -4399,7 +4399,7 @@ InitVM_Object(void)
     rb_define_method(rb_mKernel, "public_methods", rb_obj_public_methods, -1); /* in class.c */
     rb_define_method(rb_mKernel, "instance_variables", rb_obj_instance_variables, 0); /* in variable.c */
     rb_define_method(rb_mKernel, "instance_variable_get", rb_obj_ivar_get, 1);
-    rb_define_method(rb_mKernel, "instance_variable_set", rb_obj_ivar_set, 2);
+    rb_define_method(rb_mKernel, "instance_variable_set", rb_obj_ivar_set_m, 2);
     rb_define_method(rb_mKernel, "instance_variable_defined?", rb_obj_ivar_defined, 1);
     rb_define_method(rb_mKernel, "remove_instance_variable",
                      rb_obj_remove_instance_variable, 1); /* in variable.c */

variable.c

@@ -1415,36 +1415,39 @@ rb_grow_iv_list(VALUE obj)
     return res;
 }
 
-static VALUE
+attr_index_t
-obj_ivar_set(VALUE obj, ID id, VALUE val)
+rb_obj_ivar_set(VALUE obj, ID id, VALUE val)
 {
     attr_index_t index;
 
-    // Get the current shape
+    shape_id_t next_shape_id = ROBJECT_SHAPE_ID(obj);
-    rb_shape_t * shape = rb_shape_get_shape_by_id(ROBJECT_SHAPE_ID(obj));
+    rb_shape_t *shape = rb_shape_get_shape_by_id(next_shape_id);
+    uint32_t num_iv = shape->capacity;
 
-    bool found = true;
     if (!rb_shape_get_iv_index(shape, id, &index)) {
         index = shape->next_iv_index;
-        found = false;
+        if (index >= MAX_IVARS) {
+            rb_raise(rb_eArgError, "too many instance variables");
         }
 
-    // Reallocating can kick off GC.  We can't set the new shape
+        if (UNLIKELY(shape->next_iv_index >= num_iv)) {
-    // on this object until the buffer has been allocated, otherwise
+            RUBY_ASSERT(shape->next_iv_index == num_iv);
-    // GC could read off the end of the buffer.
+
-    if (shape->capacity <= index) {
             shape = rb_grow_iv_list(obj);
+            RUBY_ASSERT(shape->type == SHAPE_CAPACITY_CHANGE);
         }
 
-    if (!found) {
+        rb_shape_t *next_shape = rb_shape_get_next(shape, obj, id);
-        shape = rb_shape_get_next(shape, obj, id);
+        RUBY_ASSERT(next_shape->type == SHAPE_IVAR);
-        RUBY_ASSERT(index == (shape->next_iv_index - 1));
+        RUBY_ASSERT(index == (next_shape->next_iv_index - 1));
-        rb_shape_set_shape(obj, shape);
+        next_shape_id = rb_shape_id(next_shape);
+
+        rb_shape_set_shape(obj, next_shape);
     }
 
     RB_OBJ_WRITE(obj, &ROBJECT_IVPTR(obj)[index], val);
 
-    return val;
+    return index;
 }
 
 /* Set the instance variable +val+ on object +obj+ at ivar name +id+.
@@ -1455,7 +1458,7 @@ VALUE
 rb_vm_set_ivar_id(VALUE obj, ID id, VALUE val)
 {
     rb_check_frozen_internal(obj);
-    obj_ivar_set(obj, id, val);
+    rb_obj_ivar_set(obj, id, val);
     return val;
 }
 
@@ -1526,7 +1529,7 @@ ivar_set(VALUE obj, ID id, VALUE val)
     switch (BUILTIN_TYPE(obj)) {
       case T_OBJECT:
       {
-          obj_ivar_set(obj, id, val);
+          rb_obj_ivar_set(obj, id, val);
           break;
       }
       case T_CLASS:

vm_insnhelper.c

@@ -1299,38 +1299,12 @@ vm_setivar_slowpath(VALUE obj, ID id, VALUE val, const rb_iseq_t *iseq, IVC ic,
         {
             rb_check_frozen_internal(obj);
 
-            attr_index_t index;
+            attr_index_t index = rb_obj_ivar_set(obj, id, val);
 
-            rb_shape_t* shape = rb_shape_get_shape(obj);
-            uint32_t num_iv = shape->capacity;
             shape_id_t next_shape_id = ROBJECT_SHAPE_ID(obj);
 
-            if (!rb_shape_get_iv_index(shape, id, &index)) {
-                if (UNLIKELY(shape->next_iv_index >= num_iv)) {
-                    RUBY_ASSERT(shape->next_iv_index == num_iv);
-
-                    shape = rb_grow_iv_list(obj);
-                    RUBY_ASSERT(shape->type == SHAPE_CAPACITY_CHANGE);
-                }
-
-                index = shape->next_iv_index;
-
-                if (index >= MAX_IVARS) {
-                    rb_raise(rb_eArgError, "too many instance variables");
-                }
-
-                rb_shape_t * next_shape = rb_shape_get_next(shape, obj, id);
-                RUBY_ASSERT(next_shape->type == SHAPE_IVAR);
-                RUBY_ASSERT(index == (next_shape->next_iv_index - 1));
-                next_shape_id = rb_shape_id(next_shape);
-
-                rb_shape_set_shape(obj, next_shape);
-            }
-
             populate_cache(index, next_shape_id, id, iseq, ic, cc, is_attr);
 
-            VALUE *ptr = ROBJECT_IVPTR(obj);
-            RB_OBJ_WRITE(obj, &ptr[index], val);
             RB_DEBUG_COUNTER_INC(ivar_set_ic_miss_iv_hit);
             return val;
         }