From ea30dd702512ff9df34fe8c71c825f8f901bf5b1 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 2 Jul 2019 23:32:09 +0900 Subject: [PATCH] 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] --- test/ruby/test_jit.rb | 14 ++++++++++++++ tool/ruby_vm/views/_mjit_compile_send.erb | 5 +++++ 2 files changed, 19 insertions(+) diff --git a/test/ruby/test_jit.rb b/test/ruby/test_jit.rb index 66360731eb..cd7ac23697 100644 --- a/test/ruby/test_jit.rb +++ b/test/ruby/test_jit.rb @@ -11,6 +11,7 @@ class TestJIT < Test::Unit::TestCase IGNORABLE_PATTERNS = [ /\AJIT recompile: .+\n\z/, + /\AJIT inline: .+\n\z/, /\ASuccessful MJIT finish\n\z/, ] @@ -841,6 +842,19 @@ class TestJIT < Test::Unit::TestCase 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 da7e96581b..3db1386aca 100644 --- a/tool/ruby_vm/views/_mjit_compile_send.erb +++ b/tool/ruby_vm/views/_mjit_compile_send.erb @@ -46,10 +46,15 @@ % # 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"); }