Repository navigation
Don't load the context twice for __ENCODING__ - #9831
Merged
Merged
Conversation
GetEncodingInstr loaded the ThreadContext before calling pushRubyEncoding, which loads it again itself. The extra context was left on the operand stack. That went unnoticed in straight-line code, but where branches merge the stack depths differed, so ASM failed to compute frames and the method couldn't be compiled. Fixes jruby#9825
sferik
force-pushed
the
jit-encoding-stack
branch
from
October 19, 2026 23:49
2a64917 to
e31e3fa
Compare
headius
approved these changes
Oct 20, 2026
headius
left a comment
Member
There was a problem hiding this comment.
Lots of word salad for a simple double push, but obviously this is an easy and obvious fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #9825.
GetEncodingInstrloaded theThreadContextand then calledpushRubyEncoding, which loads it again itself (its doc says "Stack required: none"). The extra context sat on the operand stack unused. In straight-line code nothing noticed, but anywhere branches merge, likev ? __ENCODING__ : 1orv && __ENCODING__, the two paths reached the merge with different stack depths, and ASM failed to compute frames. The fix is to drop the extraloadContext().The new spec covers both of those forms. It fails in JIT and AOT mode before this change and passes in all four modes after it. With it,
test/mri/ruby/test_pattern_matching.rbcompiles completely, and the JIT compiles the methods from the issue and returns the right encodings.While writing the spec, I noticed that array patterns like
[1] in [1]fail in the IR persistence mode with an unrelatedNullPointerException, so the spec sticks to the non-pattern forms. That turned out to be a bug in reading persisted IR, which also breaks jrubyc-compiled files with array patterns, and is now #9834.