Skip to content

Commit

Permalink
Sort the jump tables based on new values
Browse files Browse the repository at this point in the history
When deserializing a BSwitch we recalculate the values for the
switch based on the new runtime, which mostly means getting new
Symbol IDs. This was fixed in jruby#8209. However we need to ensure
that the jump table is sorted or the binary search for the switch
will sometimes fail.

This patch sorts the related tables based on the new switch values.

Fixes jruby#8421
  • Loading branch information
headius committed Nov 14, 2024
1 parent 6f9df83 commit 0d92591
Showing 1 changed file with 27 additions and 7 deletions.
34 changes: 27 additions & 7 deletions core/src/main/java/org/jruby/ir/instructions/BSwitchInstr.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public BSwitchInstr(int[] jumps, Operand[] jumpOperands, Operand operand, Label
super(Operation.B_SWITCH);

// We depend on the jump table being sorted, so ensure that's the case here
assert jumpsAreSorted(jumps);
assert jumpsAreSorted(jumps) : "jump table must be sorted";

// Switch cases must not have an empty "case" value (GH-6440)
assert operand != null : "Switch cases must not have an empty \"case\" value";
Expand Down Expand Up @@ -108,17 +108,37 @@ public void encode(IRWriterEncoder e) {
public static BSwitchInstr decode(IRReaderDecoder d) {
try {
Operand[] jumpOperands = d.decodeOperandArray();
Operand operand = d.decodeOperand();
Label rubyCase = d.decodeLabel();
Label[] targets = d.decodeLabelArray();
Label elseTarget = d.decodeLabel();
Class<?> expectedClass = Class.forName(d.decodeString());

int[] jumps = new int[jumpOperands.length];
for (int i = 0; i < jumps.length; i++) {
Operand operand = jumpOperands[i];
if (operand instanceof Symbol) {
jumps[i] = ((Symbol) operand).getSymbol().getId();
} else if (operand instanceof Fixnum) {
jumps[i] = (int) ((Fixnum) operand).getValue();
Operand jumpOperand = jumpOperands[i];
if (jumpOperand instanceof Symbol) {
jumps[i] = ((Symbol) jumpOperand).getSymbol().getId();
} else if (jumpOperand instanceof Fixnum) {
jumps[i] = (int) ((Fixnum) jumpOperand).getValue();
}
}

return new BSwitchInstr(jumps, jumpOperands, d.decodeOperand(), d.decodeLabel(), d.decodeLabelArray(), d.decodeLabel(), Class.forName(d.decodeString()));
int[] sortedJumps = jumps.clone();
Arrays.sort(sortedJumps);

Operand[] sortedJumpOperands = jumpOperands.clone();
Label[] sortedTargets = targets.clone();

for (int i = 0; i < jumps.length; i++) {
int oldJump = jumps[i];
int newIndex = Arrays.binarySearch(sortedJumps, oldJump);

sortedJumpOperands[newIndex] = jumpOperands[i];
sortedTargets[newIndex] = targets[i];
}

return new BSwitchInstr(sortedJumps, sortedJumpOperands, operand, rubyCase, sortedTargets, elseTarget, expectedClass);
} catch (Exception e) {
// should never happen unless encode was corrupted
Helpers.throwException(e);
Expand Down

0 comments on commit 0d92591

Please sign in to comment.