Skip to content

YJIT: Fix potential infinite loop when OOM#13186

Merged
XrXr merged 5 commits intoruby:masterfrom
rianmcguire:21257-yjit-oom-infinite-jmp
Apr 28, 2025
Merged

YJIT: Fix potential infinite loop when OOM#13186
XrXr merged 5 commits intoruby:masterfrom
rianmcguire:21257-yjit-oom-infinite-jmp

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.

@launchable-app

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]
matzbot pushed a commit that referenced this pull request May 18, 2025
	YJIT: Fix potential infinite loop when OOM (GH-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]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants