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

From ea30dd702512ff9df34fe8c71c825f8f901bf5b1 Mon Sep 17 00:00:00 2001
From: Takashi Kokubun <takashikkbn@g...>
Date: Tue, 2 Jul 2019 23:32:09 +0900
Subject: Avoid corrupting VM stack on inlined setlocal

setlocal relies on cfp->ep, and frame-omitted method inlining introduced
in Ruby 2.7 kept it wrong.

This change might slow down frame-omitted method inlining for cfp->ep
manipulation, and it obviously complicates the implementaion more. By
introducing an optimization that changes Ruby's local variable to C
local variable, we could optimize it and simplify the cfp->ep
manipulation later.

[Bug #15971]

diff --git a/test/ruby/test_jit.rb b/test/ruby/test_jit.rb
index 6636073..cd7ac23 100644
--- a/test/ruby/test_jit.rb
+++ b/test/ruby/test_jit.rb
@@ -11,6 +11,7 @@ class TestJIT < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_jit.rb#L11
 
   IGNORABLE_PATTERNS = [
     /\AJIT recompile: .+\n\z/,
+    /\AJIT inline: .+\n\z/,
     /\ASuccessful MJIT finish\n\z/,
   ]
 
@@ -841,6 +842,19 @@ class TestJIT < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_jit.rb#L842
     end;
   end
 
+  def test_block_handler_with_frame_omitted_inlining
+    assert_eval_with_jit("#{<<~"begin;"}\n#{<<~"end;"}", stdout: "70.0\n70.0\n70.0\n", success_count: 1, min_calls: 2)
+    begin;
+      def multiply(a, b)
+        a *= b
+      end
+
+      3.times do
+        p multiply(7.0, 10.0)
+      end
+    end;
+  end
+
   def test_program_counter_with_regexpmatch
     assert_eval_with_jit("#{<<~"begin;"}\n#{<<~"end;"}", stdout: "aa", success_count: 1)
     begin;
diff --git a/tool/ruby_vm/views/_mjit_compile_send.erb b/tool/ruby_vm/views/_mjit_compile_send.erb
index da7e965..3db1386 100644
--- a/tool/ruby_vm/views/_mjit_compile_send.erb
+++ b/tool/ruby_vm/views/_mjit_compile_send.erb
@@ -46,10 +46,15 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_send.erb#L46
 
 % # JIT: If ISeq is inlinable, call the inlined method without pushing a frame.
             if (status->inlined_iseqs != NULL && status->inlined_iseqs[pos] == iseq->body) {
+                // TODO: consider using C variables for Ruby variables to simplify cfp->ep manipulation
                 fprintf(f, "    {\n");
                 fprintf(f, "        VALUE orig_self = reg_cfp->self;\n");
+                fprintf(f, "        VALUE orig_ep = reg_cfp->ep;\n");
                 fprintf(f, "        reg_cfp->self = stack[%d];\n", b->stack_size - argc - 1);
+                fprintf(f, "        reg_cfp->ep = reg_cfp->sp + %d + 2;\n", iseq->body->local_table_size - iseq->body->param.size); // simulate `vm_push_frame`'s local_size and 2 increments
+                fprintf(f, "        ((VALUE *)reg_cfp->ep)[VM_ENV_DATA_INDEX_ENV] = ((VALUE *)orig_ep)[VM_ENV_DATA_INDEX_ENV];\n"); // `vm_env_write_slowpath` checks this value
                 fprintf(f, "        stack[%d] = _mjit_inlined_%d(ec, reg_cfp, orig_self, original_iseq);\n", b->stack_size - argc - 1, pos);
+                fprintf(f, "        reg_cfp->ep = orig_ep;\n");
                 fprintf(f, "        reg_cfp->self = orig_self;\n");
                 fprintf(f, "    }\n");
             }
-- 
cgit v0.10.2


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