https://git.ruby-lang.org/ruby.git/commit/?id=d741c77b5f

From d741c77b5fd976300815c1ea987e76e92b71122f Mon Sep 17 00:00:00 2001
From: Koichi Sasada <ko1@a...>
Date: Fri, 11 Dec 2020 16:37:20 +0900
Subject: fix ivar with shareable objects issue

Instance variables of sharable objects are accessible only from
main ractor, so we need to check it correctly.

diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb
index 2bbf823..81bea95 100644
--- a/bootstraptest/test_ractor.rb
+++ b/bootstraptest/test_ractor.rb
@@ -809,6 +809,53 @@ assert_equal 'can not access instance variables of shareable objects from non-ma https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_ractor.rb#L809
   end
 }
 
+# ivar in shareable-objects are not allowed to access from non-main Ractor, by @iv (get)
+assert_equal 'can not access instance variables of shareable objects from non-main Ractors', %q{
+  class Ractor
+    def setup
+      @foo = ''
+    end
+
+    def foo
+      @foo
+    end
+  end
+
+  shared = Ractor.new{}
+  shared.setup
+
+  r = Ractor.new shared do |shared|
+    p shared.foo
+  end
+
+  begin
+    r.take
+  rescue Ractor::RemoteError => e
+    e.cause.message
+  end
+}
+
+# ivar in shareable-objects are not allowed to access from non-main Ractor, by @iv (set)
+assert_equal 'can not access instance variables of shareable objects from non-main Ractors', %q{
+  class Ractor
+    def setup
+      @foo = ''
+    end
+  end
+
+  shared = Ractor.new{}
+
+  r = Ractor.new shared do |shared|
+    p shared.setup
+  end
+
+  begin
+    r.take
+  rescue Ractor::RemoteError => e
+    e.cause.message
+  end
+}
+
 # But a shareable object is frozen, it is allowed to access ivars from non-main Ractor
 assert_equal '11', %q{
   [Object.new, [], ].map{|obj|
diff --git a/variable.c b/variable.c
index 81c0a7d..45385fb 100644
--- a/variable.c
+++ b/variable.c
@@ -926,6 +926,8 @@ generic_ivtbl(VALUE obj, ID id, bool force_check_ractor) https://github.com/ruby/ruby/blob/trunk/variable.c#L926
         !RB_OBJ_FROZEN_RAW(obj) &&
         UNLIKELY(!rb_ractor_main_p()) &&
         UNLIKELY(rb_ractor_shareable_p(obj))) {
+
+        // TODO: RuntimeError?
         rb_raise(rb_eRuntimeError, "can not access instance variables of shareable objects from non-main Ractors");
     }
     return generic_iv_tbl_;
@@ -961,6 +963,21 @@ rb_ivar_generic_ivtbl_lookup(VALUE obj, struct gen_ivtbl **ivtbl) https://github.com/ruby/ruby/blob/trunk/variable.c#L963
     return gen_ivtbl_get(obj, 0, ivtbl);
 }
 
+MJIT_FUNC_EXPORTED VALUE
+rb_ivar_generic_lookup_with_index(VALUE obj, ID id, uint32_t index)
+{
+    struct gen_ivtbl *ivtbl;
+
+    if (gen_ivtbl_get(obj, id, &ivtbl)) {
+        if (LIKELY(index < ivtbl->numiv)) {
+            VALUE val = ivtbl->ivptr[index];
+            return val;
+        }
+    }
+
+    return Qundef;
+}
+
 static VALUE
 generic_ivar_delete(VALUE obj, ID id, VALUE undef)
 {
diff --git a/variable.h b/variable.h
index 4d71f87..bfe1be2 100644
--- a/variable.h
+++ b/variable.h
@@ -17,5 +17,6 @@ struct gen_ivtbl { https://github.com/ruby/ruby/blob/trunk/variable.h#L17
 };
 
 int rb_ivar_generic_ivtbl_lookup(VALUE obj, struct gen_ivtbl **);
+VALUE rb_ivar_generic_lookup_with_index(VALUE obj, ID id, uint32_t index);
 
 #endif /* RUBY_TOPLEVEL_VARIABLE_H */
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 9205273..9a94b10 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -1095,6 +1095,21 @@ iv_index_tbl_lookup(struct st_table *iv_index_tbl, ID id, struct rb_iv_index_tbl https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1095
     return found ? true : false;
 }
 
+ALWAYS_INLINE(static void fill_ivar_cache(const rb_iseq_t *iseq, IVC ic, const struct rb_callcache *cc, int is_attr, struct rb_iv_index_tbl_entry *ent));
+
+static inline void
+fill_ivar_cache(const rb_iseq_t *iseq, IVC ic, const struct rb_callcache *cc, int is_attr, struct rb_iv_index_tbl_entry *ent)
+{
+    // fill cache
+    if (!is_attr) {
+        ic->entry = ent;
+        RB_OBJ_WRITTEN(iseq, Qundef, ent->class_value);
+    }
+    else {
+        vm_cc_attr_index_set(cc, (int)ent->index + 1);
+    }
+}
+
 ALWAYS_INLINE(static VALUE vm_getivar(VALUE, ID, const rb_iseq_t *, IVC, const struct rb_callcache *, int));
 static inline VALUE
 vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_callcache *cc, int is_attr)
@@ -1116,38 +1131,38 @@ vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_call https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1131
         if (LIKELY(BUILTIN_TYPE(obj) == T_OBJECT) &&
             LIKELY(index < ROBJECT_NUMIV(obj))) {
             val = ROBJECT_IVPTR(obj)[index];
+
+            VM_ASSERT(rb_ractor_shareable_p(obj) ? rb_ractor_shareable_p(val) : true);
         }
         else if (FL_TEST_RAW(obj, FL_EXIVAR)) {
-            struct gen_ivtbl *ivtbl;
-
-            if (LIKELY(rb_ivar_generic_ivtbl_lookup(obj, &ivtbl)) &&
-                LIKELY(index < ivtbl->numiv)) {
-                val = ivtbl->ivptr[index];
-            }
+            val = rb_ivar_generic_lookup_with_index(obj, id, index);
         }
+
         goto ret;
     }
     else {
-        struct st_table *iv_index_tbl;
-        st_index_t numiv;
-        VALUE *ivptr;
+        struct rb_iv_index_tbl_entry *ent;
 
         if (BUILTIN_TYPE(obj) == T_OBJECT) {
-            iv_index_tbl = ROBJECT_IV_INDEX_TBL(obj);
-            numiv = ROBJECT_NUMIV(obj);
-            ivptr = ROBJECT_IVPTR(obj);
-            goto fill;
-	}
-        else if (FL_TEST_RAW(obj, FL_EXIVAR)) {
-            struct gen_ivtbl *ivtbl;
-            if (LIKELY(rb_ivar_generic_ivtbl_lookup(obj, &ivtbl))) {
-                numiv = ivtbl->numiv;
-                ivptr = ivtbl->ivptr;
-                iv_index_tbl = RCLASS_IV_INDEX_TBL(rb_obj_class(obj));
-                goto fill;
+            struct st_table *iv_index_tbl = ROBJECT_IV_INDEX_TBL(obj);
+
+            if (iv_index_tbl && iv_index_tbl_lookup(iv_index_tbl, id, &ent)) {
+                fill_ivar_cache(iseq, ic, cc, is_attr, ent);
+
+                // get value
+                if (ent->index < ROBJECT_NUMIV(obj)) {
+                    val = ROBJECT_IVPTR(obj)[ent->index];
+
+                    VM_ASSERT(rb_ractor_shareable_p(obj) ? rb_ractor_shareable_p(val) : true);
+                }
             }
-            else {
-                goto ret;
+        }
+        else if (FL_TEST_RAW(obj, FL_EXIVAR)) {
+            struct st_table *iv_index_tbl = RCLASS_IV_INDEX_TBL(rb_obj_class(obj));
+
+            if (iv_index_tbl && iv_index_tbl_lookup(iv_index_tbl, id, &ent)) {
+                fill_ivar_cache(iseq, ic, cc, is_attr, ent);
+                val = rb_ivar_generic_lookup_with_index(obj, id, ent->index);
             }
         }
         else {
@@ -1155,25 +1170,6 @@ vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_call https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1170
             goto general_path;
         }
 
-      fill:
-        if (iv_index_tbl) {
-            struct rb_iv_index_tbl_entry *ent;
-
-            if (iv_index_tbl_lookup(iv_index_tbl, id, &ent)) {
-                if (!is_attr) {
-                    ic->entry = ent;
-                    RB_OBJ_WRITTEN(iseq, Qundef, ent->class_value);
-                }
-                else { /* call_info */
-                    vm_cc_attr_index_set(cc, (int)ent->index + 1);
-                }
-
-                if (ent->index < numiv) {
-                    val = ivptr[ent->index];
-                }
-            }
-        }
-
       ret:
         if (LIKELY(val != Qundef)) {
             return val;
@@ -1204,6 +1200,8 @@ vm_setivar(VALUE obj, ID id, VALUE val, const rb_iseq_t *iseq, IVC ic, const str https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1200
 	VALUE klass = RBASIC(obj)->klass;
 	uint32_t index;
 
+        VM_ASSERT(!rb_ractor_shareable_p(obj));
+
 	if (LIKELY(
 	    (!is_attr && RB_DEBUG_COUNTER_INC_UNLESS(ivar_set_ic_miss_serial, ic->entry && ic->entry->class_serial  == RCLASS_SERIAL(klass))) ||
             ( is_attr && RB_DEBUG_COUNTER_INC_UNLESS(ivar_set_ic_miss_unset, vm_cc_attr_index(cc) > 0)))) {
-- 
cgit v0.10.2


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/