k0kubun	2019-03-21 15:48:00 +0900 (Thu, 21 Mar 2019)

  New Revision: 67330

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=67330

  Log:
    Check argument_arity_error condition in inlinable_iseq_p
    
    to avoid inlining a method call when it becomes argument_arity_error,
    fixing a potential bug.

  Modified files:
    trunk/mjit_compile.c
    trunk/tool/ruby_vm/views/_mjit_compile_send.erb
Index: tool/ruby_vm/views/_mjit_compile_send.erb
===================================================================
--- tool/ruby_vm/views/_mjit_compile_send.erb	(revision 67329)
+++ tool/ruby_vm/views/_mjit_compile_send.erb	(revision 67330)
@@ -18,14 +18,14 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_send.erb#L18
 %
     if (has_valid_method_type(cc_copy)) {
         const rb_iseq_t *iseq;
-        unsigned int argc = ci->orig_argc; /* unlike `ci->orig_argc`, `argc` may include blockarg */
+        unsigned int argc = ci->orig_argc; // this `argc` variable is for calculating a value's position on stack considering `blockarg`.
 % if insn.name == 'send'
-        argc += ((ci->flag & VM_CALL_ARGS_BLOCKARG) ? 1 : 0);
+        argc += ((ci->flag & VM_CALL_ARGS_BLOCKARG) ? 1 : 0); // simulate `vm_caller_setup_arg_block`'s `--reg_cfp->sp`
 % end
 
-        if (!(ci->flag & VM_CALL_TAILCALL) && /* inlining non-tailcall path */
-            cc_copy->me->def->type == VM_METHOD_TYPE_ISEQ && inlinable_iseq_p(ci, cc_copy, iseq = def_iseq_ptr(cc_copy->me->def)) /* CC_SET_FASTPATH in vm_callee_setup_arg */) {
-            int param_size = iseq->body->param.size; /* TODO: check calling->argc for argument_arity_error */
+        if (!(ci->flag & VM_CALL_TAILCALL) // inlining non-tailcall path
+                && cc_copy->me->def->type == VM_METHOD_TYPE_ISEQ && inlinable_iseq_p(ci, cc_copy, iseq = def_iseq_ptr(cc_copy->me->def))) { // CC_SET_FASTPATH in vm_callee_setup_arg
+            int param_size = iseq->body->param.size;
 
             fprintf(f, "{\n");
 % # JIT: Declare stack_size to be used in some macro of _mjit_compile_insn_body.erb
Index: mjit_compile.c
===================================================================
--- mjit_compile.c	(revision 67329)
+++ mjit_compile.c	(revision 67330)
@@ -65,11 +65,12 @@ has_valid_method_type(CALL_CACHE cc) https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L65
 // Returns true if iseq is inlinable, otherwise NULL. This becomes true in the same condition
 // as CC_SET_FASTPATH (in vm_callee_setup_arg) is called from vm_call_iseq_setup.
 static bool
-inlinable_iseq_p(CALL_INFO ci, CALL_CACHE cc, const rb_iseq_t *iseq)
+inlinable_iseq_p(const CALL_INFO ci, const CALL_CACHE cc, const rb_iseq_t *iseq)
 {
     extern bool rb_simple_iseq_p(const rb_iseq_t *iseq);
     return iseq != NULL
         && !(ci->flag & VM_CALL_KW_SPLAT) && rb_simple_iseq_p(iseq) // Top of vm_callee_setup_arg. In this case, opt_pc is 0.
+        && ci->orig_argc == iseq->body->param.lead_num // exclude argument_arity_error (assumption: `calling->argc == ci->orig_argc` in send insns)
         && vm_call_iseq_optimizable_p(ci, cc); // CC_SET_FASTPATH condition
 }
 

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