ruby-changes:53420
From: ko1 <ko1@a...>
Date: Fri, 9 Nov 2018 10:02:31 +0900 (JST)
Subject: [ruby-changes:53420] ko1:r65636 (trunk): fix passing wrong `passed_bmethod_me`.
ko1 2018-11-09 10:02:13 +0900 (Fri, 09 Nov 2018) New Revision: 65636 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=65636 Log: fix passing wrong `passed_bmethod_me`. * vm_core.h: remove `rb_execution_context_t::passed_bmethod_me` and fix functions to pass the `me` directly. `passed_bmethod_me` was used to make bmethod (methods defined by `defined_method`). `rb_vm_invoke_bmethod` invoke `Proc` with `me` information as method frame (`lambda` frame, actually). If the proc call is not bmethod call, `passed_bmethod_me` should be NULL. However, there is a bug which passes wrong `me` for normal block call. http://ci.rvm.jp/results/trunk-asserts@silicon-docker/1449470 This is because wrong `me` was remained in `passed_bmethod_me` (and used incorrectly it after collected by GC). We need to clear `passed_bmethod_me` just after bmethod call, but clearing is not enough. To solve this issue, I removed `passed_bmethod_me` and pass `me` information as a function parameter of `rb_vm_invoke_bmethod`, `invoke_block_from_c_proc` and `invoke_iseq_block_from_c` in vm.c. * vm.c (invoke_iseq_block_from_c): the number of parameters is too long so that I try to specify `ALWAYS_INLINE`. * vm.c (invoke_block_from_c_proc): ditto. * vm_insnhelper.c (vm_yield_with_cfunc): now there are no pathes to use bmethod here. Modified files: trunk/vm.c trunk/vm_core.h trunk/vm_insnhelper.c Index: vm_core.h =================================================================== --- vm_core.h (revision 65635) +++ vm_core.h (revision 65636) @@ -858,7 +858,6 @@ typedef struct rb_execution_context_stru https://github.com/ruby/ruby/blob/trunk/vm_core.h#L858 /* temporary places */ VALUE errinfo; VALUE passed_block_handler; /* for rb_iterate */ - const rb_callable_method_entry_t *passed_bmethod_me; /* for bmethod */ uint8_t raised_flag; /* only 3 bits needed */ Index: vm_insnhelper.c =================================================================== --- vm_insnhelper.c (revision 65635) +++ vm_insnhelper.c (revision 65636) @@ -1943,9 +1943,8 @@ vm_call_bmethod_body(rb_execution_contex https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1943 VALUE val; /* control block frame */ - ec->passed_bmethod_me = cc->me; GetProcPtr(cc->me->def->body.proc, proc); - val = rb_vm_invoke_bmethod(ec, proc, calling->recv, calling->argc, argv, calling->block_handler); + val = rb_vm_invoke_bmethod(ec, proc, calling->recv, calling->argc, argv, calling->block_handler, cc->me); return val; } @@ -2517,8 +2516,6 @@ vm_yield_with_cfunc(rb_execution_context https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L2516 int is_lambda = FALSE; /* TODO */ VALUE val, arg, blockarg; const struct vm_ifunc *ifunc = captured->code.ifunc; - const rb_callable_method_entry_t *me = ec->passed_bmethod_me; - ec->passed_bmethod_me = NULL; if (is_lambda) { arg = rb_ary_new4(argc, argv); @@ -2536,7 +2533,7 @@ vm_yield_with_cfunc(rb_execution_context https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L2533 VM_FRAME_MAGIC_IFUNC | VM_FRAME_FLAG_CFRAME, self, VM_GUARDED_PREV_EP(captured->ep), - (VALUE)me, + Qfalse, 0, ec->cfp->sp, 0, 0); val = (*ifunc->func)(arg, ifunc->data, argc, argv, blockarg); rb_vm_pop_frame(ec); Index: vm.c =================================================================== --- vm.c (revision 65635) +++ vm.c (revision 65636) @@ -299,7 +299,9 @@ static void vm_collect_usage_register(in https://github.com/ruby/ruby/blob/trunk/vm.c#L299 #endif static VALUE vm_make_env_object(const rb_execution_context_t *ec, rb_control_frame_t *cfp); -extern VALUE rb_vm_invoke_bmethod(rb_execution_context_t *ec, rb_proc_t *proc, VALUE self, int argc, const VALUE *argv, VALUE block_handler); +extern VALUE rb_vm_invoke_bmethod(rb_execution_context_t *ec, rb_proc_t *proc, VALUE self, + int argc, const VALUE *argv, VALUE block_handler, + const rb_callable_method_entry_t *me); static VALUE vm_invoke_proc(rb_execution_context_t *ec, rb_proc_t *proc, VALUE self, int argc, const VALUE *argv, VALUE block_handler); static VALUE rb_block_param_proxy; @@ -1031,18 +1033,22 @@ invoke_bmethod(rb_execution_context_t *e https://github.com/ruby/ruby/blob/trunk/vm.c#L1033 return ret; } +ALWAYS_INLINE(static inline VALUE + invoke_iseq_block_from_c(rb_execution_context_t *ec, const struct rb_captured_block *captured, + VALUE self, int argc, const VALUE *argv, VALUE passed_block_handler, + const rb_cref_t *cref, int is_lambda, const rb_callable_method_entry_t *me)); + static inline VALUE invoke_iseq_block_from_c(rb_execution_context_t *ec, const struct rb_captured_block *captured, VALUE self, int argc, const VALUE *argv, VALUE passed_block_handler, - const rb_cref_t *cref, int is_lambda) + const rb_cref_t *cref, int is_lambda, const rb_callable_method_entry_t *me) { const rb_iseq_t *iseq = rb_iseq_check(captured->code.iseq); int i, opt_pc; VALUE type = VM_FRAME_MAGIC_BLOCK | (is_lambda ? VM_FRAME_FLAG_LAMBDA : 0); rb_control_frame_t *cfp = ec->cfp; VALUE *sp = cfp->sp; - const rb_callable_method_entry_t *me = ec->passed_bmethod_me; - ec->passed_bmethod_me = NULL; + stack_check(ec); CHECK_VM_STACK_OVERFLOW(cfp, argc); @@ -1076,7 +1082,7 @@ invoke_block_from_c_bh(rb_execution_cont https://github.com/ruby/ruby/blob/trunk/vm.c#L1082 const struct rb_captured_block *captured = VM_BH_TO_ISEQ_BLOCK(block_handler); return invoke_iseq_block_from_c(ec, captured, captured->self, argc, argv, passed_block_handler, - cref, is_lambda); + cref, is_lambda, NULL); } case block_handler_type_ifunc: return vm_yield_with_cfunc(ec, VM_BH_TO_IFUNC_BLOCK(block_handler), @@ -1139,17 +1145,24 @@ vm_yield_force_blockarg(rb_execution_con https://github.com/ruby/ruby/blob/trunk/vm.c#L1145 VM_BLOCK_HANDLER_NONE, NULL, FALSE, TRUE); } +ALWAYS_INLINE(static inline VALUE + invoke_block_from_c_proc(rb_execution_context_t *ec, const rb_proc_t *proc, + VALUE self, int argc, const VALUE *argv, + VALUE passed_block_handler, int is_lambda, + const rb_callable_method_entry_t *me)); + static inline VALUE invoke_block_from_c_proc(rb_execution_context_t *ec, const rb_proc_t *proc, VALUE self, int argc, const VALUE *argv, - VALUE passed_block_handler, int is_lambda) + VALUE passed_block_handler, int is_lambda, + const rb_callable_method_entry_t *me) { const struct rb_block *block = &proc->block; again: switch (vm_block_type(block)) { case block_type_iseq: - return invoke_iseq_block_from_c(ec, &block->as.captured, self, argc, argv, passed_block_handler, NULL, is_lambda); + return invoke_iseq_block_from_c(ec, &block->as.captured, self, argc, argv, passed_block_handler, NULL, is_lambda, me); case block_type_ifunc: return vm_yield_with_cfunc(ec, &block->as.captured, self, argc, argv, passed_block_handler); case block_type_symbol: @@ -1167,14 +1180,14 @@ static VALUE https://github.com/ruby/ruby/blob/trunk/vm.c#L1180 vm_invoke_proc(rb_execution_context_t *ec, rb_proc_t *proc, VALUE self, int argc, const VALUE *argv, VALUE passed_block_handler) { - return invoke_block_from_c_proc(ec, proc, self, argc, argv, passed_block_handler, proc->is_lambda); + return invoke_block_from_c_proc(ec, proc, self, argc, argv, passed_block_handler, proc->is_lambda, NULL); } MJIT_FUNC_EXPORTED VALUE rb_vm_invoke_bmethod(rb_execution_context_t *ec, rb_proc_t *proc, VALUE self, - int argc, const VALUE *argv, VALUE block_handler) + int argc, const VALUE *argv, VALUE block_handler, const rb_callable_method_entry_t *me) { - return invoke_block_from_c_proc(ec, proc, self, argc, argv, block_handler, TRUE); + return invoke_block_from_c_proc(ec, proc, self, argc, argv, block_handler, TRUE, me); } MJIT_FUNC_EXPORTED VALUE @@ -1185,7 +1198,7 @@ rb_vm_invoke_proc(rb_execution_context_t https://github.com/ruby/ruby/blob/trunk/vm.c#L1198 vm_block_handler_verify(passed_block_handler); if (proc->is_from_method) { - return rb_vm_invoke_bmethod(ec, proc, self, argc, argv, passed_block_handler); + return rb_vm_invoke_bmethod(ec, proc, self, argc, argv, passed_block_handler, NULL); } else { return vm_invoke_proc(ec, proc, self, argc, argv, passed_block_handler); -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/