Skip to content

Commit

Permalink
YJIT: Fix shrinking block with assumption too much (#10585)
Browse files Browse the repository at this point in the history
* YJIT: Fix shrinking block with assumption too much

Under the very specific circumstances, discovered by a test case in
`ruby/spec`, an `expandarray` block can contain just a branch and carry
a method lookup assumption. Previously, when we regenerated the branch,
we allowed it to shrink to empty, since we put the code at the jump
target immediately after it. That was incorrect and caused a crash while
the block is invalidated, since that left no room to patch in an exit.

When regenerating a branch that makes up a block entirely, and the block
could be invalidated, we need to ensure there is room for invalidation.
When there is code before the branch, they should act as padding, so we
don't need to worry about those cases.

* skip on RJIT
  • Loading branch information
XrXr committed Apr 22, 2024
1 parent aa5b53d commit 1bb7638
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
13 changes: 13 additions & 0 deletions bootstraptest/test_yjit.rb
Expand Up @@ -4770,6 +4770,19 @@ def tests
tests
}

# regression test for invalidating an empty block
assert_equal '0', %q{
def foo = (* = 1).pred
foo # compile it
class Integer
def to_ary = [] # invalidate
end
foo # try again
} unless rjit_enabled? # doesn't work on RJIT

# test integer left shift with constant rhs
assert_equal [0x80000000000, 'a+', :ok].inspect, %q{
def shift(val) = val << 43
Expand Down
6 changes: 6 additions & 0 deletions yjit/src/core.rs
Expand Up @@ -2643,6 +2643,12 @@ fn regenerate_branch(cb: &mut CodeBlock, branch: &Branch) {
branch.get_target_address(1).map(|addr| Target::CodePtr(addr)),
);

// If the entire block is the branch and the block could be invalidated,
// we need to pad to ensure there is room for invalidation patching.
if branch.start_addr == block.start_addr && branch_terminates_block && block.entry_exit.is_some() {
asm.pad_inval_patch();
}

// Rewrite the branch
let old_write_pos = cb.get_write_pos();
let old_dropped_bytes = cb.has_dropped_bytes();
Expand Down

0 comments on commit 1bb7638

Please sign in to comment.