[前][次][番号順一覧][スレッド一覧]

ruby-changes:64174

From: Koichi <ko1@a...>
Date: Tue, 15 Dec 2020 13:29:51 +0900 (JST)
Subject: [ruby-changes:64174] aa6287cd26 (master): fix inline method cache sync bug

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

From aa6287cd26582e64c19e37dea3fd90b380b85d5b Mon Sep 17 00:00:00 2001
From: Koichi Sasada <ko1@a...>
Date: Tue, 15 Dec 2020 05:40:38 +0900
Subject: fix inline method cache sync bug

`cd` is passed to method call functions to method invocation
functions, but `cd` can be manipulated by other ractors simultaneously
so it contains thread-safety issue.

To solve this issue, this patch stores `ci` and found `cc` to `calling`
and stops to pass `cd`.

diff --git a/insns.def b/insns.def
index f912b51..09bb9e1 100644
--- a/insns.def
+++ b/insns.def
@@ -891,11 +891,6 @@ invokeblock https://github.com/ruby/ruby/blob/trunk/insns.def#L891
 // attr rb_snum_t sp_inc = sp_inc_of_invokeblock(cd->ci);
 // attr rb_snum_t comptime_sp_inc = sp_inc_of_invokeblock(ci);
 {
-    if (UNLIKELY(vm_cc_call(cd->cc) != vm_invokeblock_i)) {
-        const struct rb_callcache *cc = vm_cc_new(0, NULL, vm_invokeblock_i);
-        RB_OBJ_WRITE(GET_ISEQ(), &cd->cc, cc);
-    }
-
     VALUE bh = VM_BLOCK_HANDLER_NONE;
     val = vm_sendish(ec, GET_CFP(), cd, bh, vm_search_invokeblock);
 
diff --git a/internal/vm.h b/internal/vm.h
index d146bc6..6a525b8 100644
--- a/internal/vm.h
+++ b/internal/vm.h
@@ -27,8 +27,7 @@ struct rb_callable_method_entry_struct; /* in method.h */ https://github.com/ruby/ruby/blob/trunk/internal/vm.h#L27
 struct rb_method_definition_struct;     /* in method.h */
 struct rb_execution_context_struct;     /* in vm_core.h */
 struct rb_control_frame_struct;         /* in vm_core.h */
-struct rb_calling_info;                 /* in vm_core.h */
-struct rb_call_data;
+struct rb_callinfo;                     /* in vm_core.h */
 
 enum method_missing_reason {
     MISSING_NOENTRY   = 0x00,
@@ -93,7 +92,7 @@ VALUE rb_eql_opt(VALUE obj1, VALUE obj2); https://github.com/ruby/ruby/blob/trunk/internal/vm.h#L92
 
 struct rb_iseq_struct;
 MJIT_SYMBOL_EXPORT_BEGIN
-void rb_vm_search_method_slowpath(VALUE cd_owner, struct rb_call_data *cd, VALUE klass);
+const struct rb_callcache *rb_vm_search_method_slowpath(const struct rb_callinfo *ci, VALUE klass);
 MJIT_SYMBOL_EXPORT_END
 
 /* vm_method.c */
diff --git a/template/call_iseq_optimized.inc.tmpl b/template/call_iseq_optimized.inc.tmpl
index f8883a1..2d3ad40 100644
--- a/template/call_iseq_optimized.inc.tmpl
+++ b/template/call_iseq_optimized.inc.tmpl
@@ -16,10 +16,10 @@ https://github.com/ruby/ruby/blob/trunk/template/call_iseq_optimized.inc.tmpl#L16
 % P.each{|param|
 %   L.each{|local|
 static VALUE
-<%= fname(param, local) %>(rb_execution_context_t *ec, rb_control_frame_t *cfp, struct rb_calling_info *calling, struct rb_call_data *cd)
+<%= fname(param, local) %>(rb_execution_context_t *ec, rb_control_frame_t *cfp, struct rb_calling_info *calling)
 {
     RB_DEBUG_COUNTER_INC(ccf_iseq_fix);
-    return vm_call_iseq_setup_normal(ec, cfp, calling, vm_cc_cme(cd->cc), 0, <%= param %>, <%= local %>);
+    return vm_call_iseq_setup_normal(ec, cfp, calling, vm_cc_cme(calling->cc), 0, <%= param %>, <%= local %>);
 }
 
 %   }
diff --git a/tool/ruby_vm/views/_mjit_compile_send.erb b/tool/ruby_vm/views/_mjit_compile_send.erb
index 01ae593..382e8a3 100644
--- a/tool/ruby_vm/views/_mjit_compile_send.erb
+++ b/tool/ruby_vm/views/_mjit_compile_send.erb
@@ -76,8 +76,9 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_send.erb#L76
 
             if (vm_cc_cme(captured_cc)->def->type == VM_METHOD_TYPE_CFUNC) {
 %               # TODO: optimize this more
-                fprintf(f, "        struct rb_call_data cc_cd = { .ci = (CALL_INFO)0x%"PRIxVALUE", .cc = cc };\n", (VALUE)ci); // creating local cd here because operand's cd->cc may not be the same as inlined cc.
-                fprintf(f, "        val = vm_call_cfunc_with_frame(ec, reg_cfp, &calling, &cc_cd);\n");
+                fprintf(f, "        calling.ci = (CALL_INFO)0x%"PRIxVALUE";\n", (VALUE)ci); // creating local cd here because operand's cd->cc may not be the same as inlined cc.
+                fprintf(f, "        calling.cc = cc;");
+                fprintf(f, "        val = vm_call_cfunc_with_frame(ec, reg_cfp, &calling);\n");
             }
             else { // VM_METHOD_TYPE_ISEQ
 %               # fastpath_applied_iseq_p checks rb_simple_iseq_p, which ensures has_opt == FALSE
diff --git a/vm_callinfo.h b/vm_callinfo.h
index b268783..4ee6fa7 100644
--- a/vm_callinfo.h
+++ b/vm_callinfo.h
@@ -267,8 +267,7 @@ vm_ci_markable(const struct rb_callinfo *ci) https://github.com/ruby/ruby/blob/trunk/vm_callinfo.h#L267
 typedef VALUE (*vm_call_handler)(
     struct rb_execution_context_struct *ec,
     struct rb_control_frame_struct *cfp,
-    struct rb_calling_info *calling,
-    struct rb_call_data *cd);
+    struct rb_calling_info *calling);
 
 // imemo_callcache
 
diff --git a/vm_core.h b/vm_core.h
index 117671b..8540f8d 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -238,6 +238,8 @@ union iseq_inline_storage_entry { https://github.com/ruby/ruby/blob/trunk/vm_core.h#L238
 };
 
 struct rb_calling_info {
+    const struct rb_callinfo *ci;
+    const struct rb_callcache *cc;
     VALUE block_handler;
     VALUE recv;
     int argc;
diff --git a/vm_eval.c b/vm_eval.c
index 9cf1bfc..d98bd07 100644
--- a/vm_eval.c
+++ b/vm_eval.c
@@ -38,32 +38,30 @@ typedef enum call_type { https://github.com/ruby/ruby/blob/trunk/vm_eval.c#L38
 } call_type;
 
 static VALUE send_internal(int argc, const VALUE *argv, VALUE recv, call_type scope);
-static VALUE vm_call0_body(rb_execution_context_t* ec, struct rb_calling_info *calling, struct rb_call_data *cd, const VALUE *argv);
+static VALUE vm_call0_body(rb_execution_context_t* ec, struct rb_calling_info *calling, const VALUE *argv);
 
 #ifndef MJIT_HEADER
 
 MJIT_FUNC_EXPORTED VALUE
-rb_vm_call0(rb_execution_context_t *ec, VALUE recv, ID id, int argc, const VALUE *argv, const rb_callable_method_entry_t *me, int kw_splat)
+rb_vm_call0(rb_execution_context_t *ec, VALUE recv, ID id, int argc, const VALUE *argv, const rb_callable_method_entry_t *cme, int kw_splat)
 {
     struct rb_calling_info calling = {
+        .ci = &VM_CI_ON_STACK(id, kw_splat ? VM_CALL_KW_SPLAT : 0, argc, NULL),
+        .cc = &VM_CC_ON_STACK(Qfalse, vm_call_general, { 0 }, cme),
         .block_handler = VM_BLOCK_HANDLER_NONE,
         .recv = recv,
         .argc = argc,
         .kw_splat = kw_splat,
     };
-    struct rb_call_data cd = {
-        .ci = &VM_CI_ON_STACK(id, kw_splat ? VM_CALL_KW_SPLAT : 0, argc, NULL),
-        .cc = &VM_CC_ON_STACK(Qfalse, vm_call_general, { 0 }, me),
-    };
 
-    return vm_call0_body(ec, &calling, &cd, argv);
+    return vm_call0_body(ec, &calling, argv);
 }
 
 static VALUE
-vm_call0_cfunc_with_frame(rb_execution_context_t* ec, struct rb_calling_info *calling, struct rb_call_data *cd, const VALUE *argv)
+vm_call0_cfunc_with_frame(rb_execution_context_t* ec, struct rb_calling_info *calling, const VALUE *argv)
 {
-    const struct rb_callinfo *ci = cd->ci;
-    const struct rb_callcache *cc = cd->cc;
+    const struct rb_callinfo *ci = calling->ci;
+    const struct rb_callcache *cc = calling->cc;
     VALUE val;
     const rb_callable_method_entry_t *me = vm_cc_cme(cc);
     const rb_method_cfunc_t *cfunc = UNALIGNED_MEMBER_PTR(me->def, body.cfunc);
@@ -106,17 +104,17 @@ vm_call0_cfunc_with_frame(rb_execution_context_t* ec, struct rb_calling_info *ca https://github.com/ruby/ruby/blob/trunk/vm_eval.c#L104
 }
 
 static VALUE
-vm_call0_cfunc(rb_execution_context_t *ec, struct rb_calling_info *calling, struct rb_call_data *cd, const VALUE *argv)
+vm_call0_cfunc(rb_execution_context_t *ec, struct rb_calling_info *calling, const VALUE *argv)
 {
-    return vm_call0_cfunc_with_frame(ec, calling, cd, argv);
+    return vm_call0_cfunc_with_frame(ec, calling, argv);
 }
 
 /* `ci' should point temporal value (on stack value) */
 static VALUE
-vm_call0_body(rb_execution_context_t *ec, struct rb_calling_info *calling, struct rb_call_data *cd, const VALUE *argv)
+vm_call0_body(rb_execution_context_t *ec, struct rb_calling_info *calling, const VALUE *argv)
 {
-    const struct rb_callinfo *ci = cd->ci;
-    const struct rb_callcache *cc = cd->cc;
+    const struct rb_callinfo *ci = calling->ci;
+    const struct rb_callcache *cc = calling->cc;
 
     VALUE ret;
 
@@ -137,13 +135,13 @@ vm_call0_body(rb_execution_context_t *ec, struct rb_calling_info *calling, struc https://github.com/ruby/ruby/blob/trunk/vm_eval.c#L135
 		*reg_cfp->sp++ = argv[i];
 	    }
 
-            vm_call_iseq_setup(ec, reg_cfp, calling, cd);
+            vm_call_iseq_setup(ec, reg_cfp, calling);
 	    VM_ENV_FLAGS_SET(ec->cfp->ep, VM_FRAME_FLAG_FINISH);
 	    return vm_exec(ec, TRUE); /* CHECK_INTS in this function */
 	}
       case VM_METHOD_TYPE_NOTIMPLEMENTED:
       case VM_METHOD_TYPE_CFUNC:
-        ret = vm_call0_cfunc(ec, calling, cd, argv);
+        ret = vm_call0_cfunc(ec, calling, argv);
 	goto success;
       case VM_METHOD_TYPE_ATTRSET:
         if (calling->kw_splat &&
@@ -168,7 +166,7 @@ vm_call0_body(rb_execution_context_t *ec, struct rb_calling_info *calling, struc https://github.com/ruby/ruby/blob/trunk/vm_eval.c#L166
 	ret = rb_attr_get(calling->recv, vm_cc_cme(cc)->def->body.attr.id);
 	goto success;
       case VM_METHOD_TYPE_BMETHOD:
-        ret = vm_call_bmethod_body(ec, calling, cd, argv);
+        ret = vm_call_bmethod_body(ec, calling, argv);
 	goto success;
       case VM_METHOD_TYPE_ZSUPER:
       case VM_METHOD_TYPE_REFINED:
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index ac0f999..039c376 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -1523,7 +1523,7 @@ vm_expandarray(VALUE *sp, VALUE ary, rb_num_t num, int flag) https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1523
     RB_GC_GUARD(ary);
 }
 
-static VALUE vm_call_general(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, struct rb_calling_info *calling, struct rb_call_data *cd);
+static VALUE vm_call_general(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, struct rb_calling_info *calling);
 
 static VALU (... truncated)

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

[前][次][番号順一覧][スレッド一覧]