From 0d925911d21ea51422fbc2447b24478e570c4441 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Thu, 14 Nov 2024 11:54:34 -0600 Subject: [PATCH] Sort the jump tables based on new values 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 #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 #8421 --- .../jruby/ir/instructions/BSwitchInstr.java | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/jruby/ir/instructions/BSwitchInstr.java b/core/src/main/java/org/jruby/ir/instructions/BSwitchInstr.java index 135b3668c83..e6415cedcd0 100644 --- a/core/src/main/java/org/jruby/ir/instructions/BSwitchInstr.java +++ b/core/src/main/java/org/jruby/ir/instructions/BSwitchInstr.java @@ -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"; @@ -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);