Bug #22225
closedYJIT regenerates a branch while a duplicate target still points to the invalidated block
Description
YJIT adds one incoming-list entry for each branch target that points to a block. When both targets point to the same block, that block's incoming list contains the same BranchRef twice.
invalidate_block_version processes those entries separately. For the first entry, it redirects one target and immediately calls regenerate_branch. The other target still references
the invalidated block.
The following assertion fails immediately before that call:
diff --git a/yjit/src/core.rs b/yjit/src/core.rs
index d08cd1fb26..4555081ae2 100644
--- a/yjit/src/core.rs
+++ b/yjit/src/core.rs
@@ -4282,6 +4282,18 @@ pub fn invalidate_block_version(blockref: &BlockRef) {
}
}
+ assert!(
+ branch.targets.iter().all(|target| unsafe {
+ // SAFETY: no mutation.
+ target
+ .ref_unchecked()
+ .as_ref()
+ .and_then(|target| target.get_block())
+ != Some(*blockref)
+ }),
+ "regeneration started while a branch target still referenced the invalidated block"
+ );
+
// Rewrite the branch with the new jump target address
let old_branch_size = branch.code_size();
regenerate_branch(cb, branch);
This Ruby script triggers the assertion:
# duplicate_target_invalidation.rb
module DuplicateTargetInvalidation
TARGET = 1
def self.call(condition)
if condition
1
else
2
end
TARGET
end
end
3.times do |index|
DuplicateTargetInvalidation.call(index.even?)
end
DuplicateTargetInvalidation.send(:remove_const, :TARGET)
puts "completed"
Save the patch as assert-no-invalidated-target-before-regeneration.patch and the script as duplicate_target_invalidation.rb. From CRuby ruby 4.1.0dev:
git apply assert-no-invalidated-target-before-regeneration.patch
./autogen.sh
mkdir build
cd build
../configure --enable-yjit=dev
make -j"$(nproc)"
./ruby --yjit-call-threshold=1 ../duplicate_target_invalidation.rb
The process aborts before printing completed:
[BUG] YJIT: panicked at yjit/src/core.rs:4285:9:
regeneration started while a branch target still referenced the invalidated block
regenerate_branch runs with one target redirected to a stub and the other still pointing at the invalidated block. That can leave generated code jumping back into code YJIT just invalidated.
Updated by cosgroveb (Brian Cosgrove) 3 days ago
ยท Edited
Apologies for the rather in the weeds report here but:
- Is this expected behavior?
- This does result in an infinite loop/self-jump in a much more elaborate repro and in my CI environment (a large test suite > 100k examples often re-defining things.
My aim is to keep this report laser-focused and reduced to the smallest example possible of the state that produces such conditions, in the hope that a succinct report aids maintainers in triage.
Updated by luke-gru (Luke Gruber) 3 days ago
- Assignee set to jit
Thank you for the report and the great repro. This does look like a bug to me. The YJIT folks will take a look at it soon.
Updated by luke-gru (Luke Gruber) 2 days ago
I think this is one of those instances I should have used an LLM, sorry! It has me thinking that this is not a bug after all. The JIT team will still take a look at it, but this may not be the repro to your bug that you were hoping for.
Updated by cosgroveb (Brian Cosgrove) 2 days ago
luke-gru (Luke Gruber) wrote in #note-3:
I think this is one of those instances I should have used an LLM, sorry! It has me thinking that this is not a bug after all. The JIT team will still take a look at it, but this may not be the repro to your bug that you were hoping for.
Thanks for taking an early look, nevertheless, if that ends up being so. This is a hairy thing. We'll see.
Updated by alanwu (Alan Wu) 2 days ago
- Status changed from Open to Feedback
That can leave generated code jumping back into code YJIT just invalidated.
This alone isn't problematic since invalidate_block_version patches in an exit at the invalidated block.
This does result in an infinite loop/self-jump in a much more elaborate repro and in my CI environment (a large test suite > 100k examples often re-defining things.
This symptom sounds familiar to a bug we've fixed. Are you running into this issue with the latest release?
Updated by cosgroveb (Brian Cosgrove) 2 days ago
luke-gru (Luke Gruber) wrote in #note-3:
I think this is one of those instances I should have used an LLM, sorry! It has me thinking that this is not a bug after all. The JIT team will still take a look at it, but this may not be the repro to your bug that you were hoping for.
I should be clear that this script doesn't reproduce the self-jump itself. Calling it a "repro" is maybe a bit strong.
Obviously this intermediate state happens inside the VM lock. I simply want to make sure that it's intentional that the branch is rewritten after updating only one of its' targets.
Updated by cosgroveb (Brian Cosgrove) 2 days ago
alanwu (Alan Wu) wrote in #note-5:
That can leave generated code jumping back into code YJIT just invalidated.
This alone isn't problematic since
invalidate_block_versionpatches in an exit at the invalidated block.This does result in an infinite loop/self-jump in a much more elaborate repro and in my CI environment (a large test suite > 100k examples often re-defining things.
This symptom sounds familiar to a bug we've fixed. Are you running into this issue with the latest release?
Not the latest release in CI. I'm willing to backport this bug fix to the release we are using (privately) in our CI to see if it resolves our issue if you can direct me to a commit?
Updated by cosgroveb (Brian Cosgrove) 2 days ago
alanwu (Alan Wu) wrote in #note-5:
That can leave generated code jumping back into code YJIT just invalidated.
This alone isn't problematic since
invalidate_block_versionpatches in an exit at the invalidated block.This does result in an infinite loop/self-jump in a much more elaborate repro and in my CI environment (a large test suite > 100k examples often re-defining things.
This symptom sounds familiar to a bug we've fixed. Are you running into this issue with the latest release?
This feels very suspiciously like https://bugs.ruby-lang.org/issues/21257 to me. I'll rule it in or out and update here when I have.