ruby-changes:69870
From: Alan <ko1@a...>
Date: Tue, 23 Nov 2021 08:23:45 +0900 (JST)
Subject: [ruby-changes:69870] 13d1ded253 (master): YJIT: Make block invalidation more robust
https://git.ruby-lang.org/ruby.git/commit/?id=13d1ded253 From 13d1ded253940585a993e92648ab9f77d355586d Mon Sep 17 00:00:00 2001 From: Alan Wu <XrXr@u...> Date: Thu, 4 Nov 2021 12:30:30 -0400 Subject: YJIT: Make block invalidation more robust This commit adds an entry_exit field to block_t for use in invalidate_block_version(). By patching the start of the block while invalidating it, invalidate_block_version() can function correctly while there is no executable memory left for new branch stubs. This change additionally fixes correctness for situations where we cannot patch incoming jumps to the invalidated block. In situations such as Shopify/yjit#226, the address to the start of the block is saved and used later, possibly after the block is invalidated. The assume_* family of function now generate block->entry_exit before remembering blocks for invalidation. RubyVM::YJIT.simulate_oom! is introduced for testing out of memory conditions. The test for it is disabled for now because OOM triggers other failure conditions not addressed by this commit. Fixes Shopify/yjit#226 --- bootstraptest/test_yjit.rb | 74 +++++++++++++++++++++++++++++++ yjit.rb | 4 ++ yjit_codegen.c | 108 ++++++++++++++++++++++++++++----------------- yjit_codegen.h | 2 + yjit_core.c | 53 +++++++++++++++++++--- yjit_core.h | 8 +++- yjit_iface.c | 39 ++++++++++++---- 7 files changed, 231 insertions(+), 57 deletions(-) diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index f1900f9f3f8..8286be74e7a 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -2381,3 +2381,77 @@ assert_equal '{:foo=>2}', %q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_yjit.rb#L2381 foo foo } + +# block invalidation edge case +assert_equal 'undef', %q{ + class A + def foo(arg) + arg.times { A.remove_method(:bar) } + self + end + + def bar + 4 + end + + def use(arg) + # two consecutive sends. When bar is removed, the return address + # for calling it is already on foo's control frame + foo(arg).bar + rescue NoMethodError + :undef + end + end + + A.new.use 0 + A.new.use 0 + A.new.use 1 +} + +# block invalidation edge case +assert_equal 'ok', %q{ + class A + Good = :ng + def foo(arg) + arg.times { A.const_set(:Good, :ok) } + self + end + + def id(arg) + arg + end + + def use(arg) + # send followed by an opt_getinlinecache. + # The return address remains on the control frame + # when opt_getinlinecache is invalidated. + foo(arg).id(Good) + end + end + + A.new.use 0 + A.new.use 0 + A.new.use 1 +} + +# block invalidation while out of memory +assert_equal 'new', %q{ + def foo + :old + end + + def test + foo + end + + test + test + + RubyVM::YJIT.simulate_oom! if defined?(RubyVM::YJIT) + + def foo + :new + end + + test +} if false # disabled for now since OOM crashes in the test harness diff --git a/yjit.rb b/yjit.rb index 3592e20a7fd..c555fd27cc4 100644 --- a/yjit.rb +++ b/yjit.rb @@ -149,6 +149,10 @@ module RubyVM::YJIT https://github.com/ruby/ruby/blob/trunk/yjit.rb#L149 Primitive.cexpr! 'rb_yjit_enabled_p() ? Qtrue : Qfalse' end + def self.simulate_oom! + Primitive.simulate_oom_bang + end + # Avoid calling a method here to not interfere with compilation tests if Primitive.yjit_stats_enabled_p at_exit { _print_stats } diff --git a/yjit_codegen.c b/yjit_codegen.c index 60ddf489e10..c34c56a972d 100644 --- a/yjit_codegen.c +++ b/yjit_codegen.c @@ -405,6 +405,26 @@ yjit_side_exit(jitstate_t *jit, ctx_t *ctx) https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L405 return jit->side_exit_for_pc; } +// Ensure that there is an exit for the start of the block being compiled. +// Block invalidation uses this exit. +static void +jit_ensure_block_entry_exit(jitstate_t *jit) +{ + block_t *block = jit->block; + if (block->entry_exit) return; + + if (jit->insn_idx == block->blockid.idx) { + // We are compiling the first instruction in the block. + // Generate the exit with the cache in jitstate. + block->entry_exit = yjit_side_exit(jit, &block->ctx); + } + else { + VALUE *pc = yjit_iseq_pc_at_idx(block->blockid.iseq, block->blockid.idx); + uint32_t pos = yjit_gen_exit(pc, &block->ctx, ocb); + block->entry_exit = cb_get_ptr(ocb, pos); + } +} + // Generate a runtime guard that ensures the PC is at the start of the iseq, // otherwise take a side exit. This is to handle the situation of optional // parameters. When a function with optional parameters is called, the entry @@ -630,7 +650,7 @@ yjit_gen_block(block_t *block, rb_execution_context_t *ec) https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L650 RUBY_ASSERT(opcode >= 0 && opcode < VM_INSTRUCTION_SIZE); // opt_getinlinecache wants to be in a block all on its own. Cut the block short - // if we run into it. See gen_opt_getinlinecache for details. + // if we run into it. See gen_opt_getinlinecache() for details. if (opcode == BIN(opt_getinlinecache) && insn_idx > starting_insn_idx) { jit_jump_to_next_insn(&jit, ctx); break; @@ -657,32 +677,24 @@ yjit_gen_block(block_t *block, rb_execution_context_t *ec) https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L677 // Lookup the codegen function for this instruction codegen_fn gen_fn = gen_fns[opcode]; - if (!gen_fn) { - // If we reach an unknown instruction, - // exit to the interpreter and stop compiling - yjit_gen_exit(jit.pc, ctx, cb); - break; - } - - if (0) { - fprintf(stderr, "compiling %d: %s\n", insn_idx, insn_name(opcode)); - print_str(cb, insn_name(opcode)); - } - - // :count-placement: - // Count bytecode instructions that execute in generated code. - // Note that the increment happens even when the output takes side exit. - GEN_COUNTER_INC(cb, exec_instruction); + codegen_status_t status = YJIT_CANT_COMPILE; + if (gen_fn) { + if (0) { + fprintf(stderr, "compiling %d: %s\n", insn_idx, insn_name(opcode)); + print_str(cb, insn_name(opcode)); + } - // Add a comment for the name of the YARV instruction - ADD_COMMENT(cb, insn_name(opcode)); + // :count-placement: + // Count bytecode instructions that execute in generated code. + // Note that the increment happens even when the output takes side exit. + GEN_COUNTER_INC(cb, exec_instruction); - // Call the code generation function - codegen_status_t status = gen_fn(&jit, ctx, cb); + // Add a comment for the name of the YARV instruction + ADD_COMMENT(cb, insn_name(opcode)); - // For now, reset the chain depth after each instruction as only the - // first instruction in the block can concern itself with the depth. - ctx->chain_depth = 0; + // Call the code generation function + status = gen_fn(&jit, ctx, cb); + } // If we can't compile this instruction // exit to the interpreter and stop compiling @@ -690,10 +702,20 @@ yjit_gen_block(block_t *block, rb_execution_context_t *ec) https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L702 // TODO: if the codegen function makes changes to ctx and then return YJIT_CANT_COMPILE, // the exit this generates would be wrong. We could save a copy of the entry context // and assert that ctx is the same here. - yjit_gen_exit(jit.pc, ctx, cb); + uint32_t exit_off = yjit_gen_exit(jit.pc, ctx, cb); + + // If this is the first instruction in the block, then we can use + // the exit for block->entry_exit. + if (insn_idx == block->blockid.idx) { + block->entry_exit = cb_get_ptr(cb, exit_off); + } break; } + // For now, reset the chain depth after each instruction as only the + // first instruction in the block can concern itself with the depth. + ctx->chain_depth = 0; + // Move to the next instruction to compile insn_idx += insn_len(opcode); @@ -1971,7 +1993,7 @@ gen_fixnum_cmp(jitstate_t *jit, ctx_t *ctx, cmov_fn cmov_op) https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L1993 // Note: we generate the side-exit before popping operands from the stack uint8_t *side_exit = yjit_side_exit(jit, ctx); - if (!assume_bop_not_redefined(jit->block, INTEGER_REDEFINED_OP_FLAG, BOP_LT)) { + if (!assume_bop_not_redefined(jit, INTEGER_REDEFINED_OP_FLAG, BOP_LT)) { return YJIT_CANT_COMPILE; } @@ -2036,7 +2058,7 @@ gen_equality_specialized(jitstate_t *jit, ctx_t *ctx, uint8_t *side_exit) https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L2058 x86opnd_t b_opnd = ctx_stack_opnd(ctx, 0); if (FIXNUM_P(comptime_a) && FIXNUM_P(comptime_b)) { - if (!assume_bop_not_redefined(jit->block, INTEGER_REDEFINED_OP_FLAG, BOP_EQ)) { + if (!assume_bop_not_redefined(jit, INTEGER_REDEFINED_OP_FLAG, BOP_EQ)) { // if overridden, emit the generic version return false; } @@ -2059,7 +2081,7 @@ gen_equality_specialized(jitstate_t *jit, ctx_t *ctx, uint8_t *side_exit) https://github.com/ruby/ruby/blob/trunk/yjit_codegen.c#L2081 } else if (CLASS_OF(comptime_a) == rb_cString && CLASS_OF(comptime_b) == rb_cString) { - if (!assume_bop_not_redefined(jit->block, STRING_REDEFINED_OP_FLAG, BOP_EQ)) { + if (!assume_bop_not_redefined(jit, STRING_REDEFINED_OP_FLAG, BOP_EQ)) { // if overridde (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/