Skip to content

Commit 80a1a1b

Browse files
authored
YJIT: Fix potential infinite loop when OOM (rubyGH-13186)
Avoid generating an infinite loop in the case where: 1. Block `first` is adjacent to block `second`, and the branch from `first` to `second` is a fallthrough, and 2. Block `second` immediately exits to the interpreter, and 3. Block `second` is invalidated and YJIT is OOM While pondering how to fix this, I think I've stumbled on another related edge case: 1. Block `incoming_one` and `incoming_two` both branch to block `second`. Block `incoming_one` has a fallthrough 2. Block `second` immediately exits to the interpreter (so it starts with its exit) 3. When Block `second` is invalidated, the incoming fallthrough branch from `incoming_one` might be rewritten first, which overwrites the start of block `second` with a jump to a new branch stub. 4. YJIT runs of out memory 5. The incoming branch from `incoming_two` is then rewritten, but because we're OOM we can't generate a new stub, so we use `second`'s exit as the branch target. However `second`'s exit was already overwritten with a jump to the branch stub for `incoming_one`, so `incoming_two` will end up jumping to `incoming_one`'s branch stub. Fixes [Bug #21257]
1 parent 37db51b commit 80a1a1b

File tree

2 files changed

+102
-5
lines changed

2 files changed

+102
-5
lines changed

bootstraptest/test_yjit.rb

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3667,6 +3667,74 @@ def foo
36673667
test
36683668
}
36693669

3670+
# Bug #21257 (infinite jmp)
3671+
assert_equal 'ok', %q{
3672+
Good = :ok
3673+
3674+
def first
3675+
second
3676+
end
3677+
3678+
def second
3679+
::Good
3680+
end
3681+
3682+
# Make `second` side exit on its first instruction
3683+
trace = TracePoint.new(:line) { }
3684+
trace.enable(target: method(:second))
3685+
3686+
first
3687+
# Recompile now that the constant cache is populated, so we get a fallthrough from `first` to `second`
3688+
# (this is need to reproduce with --yjit-call-threshold=1)
3689+
RubyVM::YJIT.code_gc if defined?(RubyVM::YJIT)
3690+
first
3691+
3692+
# Trigger a constant cache miss in rb_vm_opt_getconstant_path (in `second`) next time it's called
3693+
module InvalidateConstantCache
3694+
Good = nil
3695+
end
3696+
3697+
RubyVM::YJIT.simulate_oom! if defined?(RubyVM::YJIT)
3698+
3699+
first
3700+
first
3701+
}
3702+
3703+
assert_equal 'ok', %q{
3704+
# Multiple incoming branches into second
3705+
Good = :ok
3706+
3707+
def incoming_one
3708+
second
3709+
end
3710+
3711+
def incoming_two
3712+
second
3713+
end
3714+
3715+
def second
3716+
::Good
3717+
end
3718+
3719+
# Make `second` side exit on its first instruction
3720+
trace = TracePoint.new(:line) { }
3721+
trace.enable(target: method(:second))
3722+
3723+
incoming_one
3724+
# Recompile now that the constant cache is populated, so we get a fallthrough from `incoming_one` to `second`
3725+
# (this is need to reproduce with --yjit-call-threshold=1)
3726+
RubyVM::YJIT.code_gc if defined?(RubyVM::YJIT)
3727+
incoming_one
3728+
incoming_two
3729+
3730+
# Trigger a constant cache miss in rb_vm_opt_getconstant_path (in `second`) next time it's called
3731+
module InvalidateConstantCache
3732+
Good = nil
3733+
end
3734+
3735+
incoming_one
3736+
}
3737+
36703738
assert_equal 'ok', %q{
36713739
# Try to compile new method while OOM
36723740
def foo

yjit/src/core.rs

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4158,7 +4158,23 @@ pub fn invalidate_block_version(blockref: &BlockRef) {
41584158
}
41594159

41604160
// For each incoming branch
4161-
for branchref in block.incoming.0.take().iter() {
4161+
let mut incoming_branches = block.incoming.0.take();
4162+
4163+
// An adjacent branch will write into the start of the block being invalidated, possibly
4164+
// overwriting the block's exit. If we run out of memory after doing this, any subsequent
4165+
// incoming branches we rewrite won't be able use the block's exit as a fallback when they
4166+
// are unable to generate a stub. To avoid this, if there's an incoming branch that's
4167+
// adjacent to the invalidated block, make sure we process it last.
4168+
let adjacent_branch_idx = incoming_branches.iter().position(|branchref| {
4169+
let branch = unsafe { branchref.as_ref() };
4170+
let target_next = block.start_addr == branch.end_addr.get();
4171+
target_next
4172+
});
4173+
if let Some(adjacent_branch_idx) = adjacent_branch_idx {
4174+
incoming_branches.swap(adjacent_branch_idx, incoming_branches.len() - 1)
4175+
}
4176+
4177+
for (i, branchref) in incoming_branches.iter().enumerate() {
41624178
let branch = unsafe { branchref.as_ref() };
41634179
let target_idx = if branch.get_target_address(0) == Some(block_start) {
41644180
0
@@ -4198,10 +4214,18 @@ pub fn invalidate_block_version(blockref: &BlockRef) {
41984214
let target_next = block.start_addr == branch.end_addr.get();
41994215

42004216
if target_next {
4201-
// The new block will no longer be adjacent.
4202-
// Note that we could be enlarging the branch and writing into the
4203-
// start of the block being invalidated.
4204-
branch.gen_fn.set_shape(BranchShape::Default);
4217+
if stub_addr != block.start_addr {
4218+
// The new block will no longer be adjacent.
4219+
// Note that we could be enlarging the branch and writing into the
4220+
// start of the block being invalidated.
4221+
branch.gen_fn.set_shape(BranchShape::Default);
4222+
} else {
4223+
// The branch target is still adjacent, so the branch must remain
4224+
// a fallthrough so we don't overwrite the target with a jump.
4225+
//
4226+
// This can happen if we're unable to generate a stub and the
4227+
// target block also exits on entry (block_start == block_entry_exit).
4228+
}
42054229
}
42064230

42074231
// Rewrite the branch with the new jump target address
@@ -4211,6 +4235,11 @@ pub fn invalidate_block_version(blockref: &BlockRef) {
42114235
if target_next && branch.end_addr > block.end_addr {
42124236
panic!("yjit invalidate rewrote branch past end of invalidated block: {:?} (code_size: {})", branch, block.code_size());
42134237
}
4238+
let is_last_incoming_branch = i == incoming_branches.len() - 1;
4239+
if target_next && branch.end_addr.get() > block_entry_exit && !is_last_incoming_branch {
4240+
// We might still need to jump to this exit if we run out of memory when rewriting another incoming branch.
4241+
panic!("yjit invalidate rewrote branch over exit of invalidated block: {:?}", branch);
4242+
}
42144243
if !target_next && branch.code_size() > old_branch_size {
42154244
panic!(
42164245
"invalidated branch grew in size (start_addr: {:?}, old_size: {}, new_size: {})",

0 commit comments

Comments
 (0)