Skip to content

YJIT: Fix potential infinite loop when OOM #13186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 28, 2025

Conversation

rianmcguire
Copy link
Contributor

@rianmcguire rianmcguire commented Apr 27, 2025

Fixes https://bugs.ruby-lang.org/issues/21257 [Bug #21257]

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.

I'm not sure what the consequences of calling the wrong branch stub are, but the comments in invalidate_block_version suggest the stubs are supposed to be unique.

I've attempted to address both of these cases with this change, but I can split it up if that would be preferable.

This comment has been minimized.

@rianmcguire rianmcguire marked this pull request as ready for review April 27, 2025 04:16
@matzbot matzbot requested a review from a team April 27, 2025 04:17
@XrXr XrXr merged commit 80a1a1b into ruby:master Apr 28, 2025
83 of 86 checks passed
@XrXr
Copy link
Member

XrXr commented Apr 28, 2025

Thank you so much for the investigation and the thorough fix! 🙏🏼
(and sorry for not getting back to you quicker!)

XrXr pushed a commit to XrXr/ruby that referenced this pull request Apr 28, 2025
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.

Backport [Bug #21257]
k0kubun pushed a commit that referenced this pull request Apr 28, 2025
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.

Backport [Bug #21257]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants