diff --git a/core/engine/src/bytecompiler/jump_control.rs b/core/engine/src/bytecompiler/jump_control.rs index a6e87e826a2..4fc1020a306 100644 --- a/core/engine/src/bytecompiler/jump_control.rs +++ b/core/engine/src/bytecompiler/jump_control.rs @@ -117,7 +117,9 @@ impl JumpRecord { finally_throw_flag, finally_throw_index, } => { - let index = value as i32; + // Note: +1 because 0 is reserved for the fallthrough entry of the + // jump table emitted in `pop_try_with_finally_control_info`. + let index = value as i32 + 1; compiler .bytecode .emit_store_false(finally_throw_flag.into()); @@ -608,27 +610,33 @@ impl ByteCompiler<'_> { self.patch_jump_with_target(*label, finally_start); } + // The jump table holds `info.jumps.len() + 1` entries. Entry 0 is the + // fallthrough target, taken when no `break`, `continue`, or `return` + // inside the protected region selected a jump record (the index + // register keeps its initial value of `0`). Entries `1..=N` correspond + // to the registered jump records and are selected by `HandleFinally`. // NOTE: +4 to jump past the index operand. let jump_table_index = self.next_opcode_location() + size_of::() as u32; self.bytecode.emit_jump_table( finally_throw_index, - thin_vec![Self::DUMMY_ADDRESS; info.jumps.len()], + thin_vec![Self::DUMMY_ADDRESS; info.jumps.len() + 1], ); - // We are assuming any indices outside our jump table will fallback - // to executing the next available op. Since we kinda control the jump - // table index here, this doesn't matter too much, but we _could_ also - // throw a PanicError on the next instruction. + // Skip the jump-record handlers when falling through. + let fallthrough = self.jump(); - let mut patch_jumps = Vec::with_capacity(info.jumps.len()); + let mut patch_jumps = vec![Self::DUMMY_ADDRESS; info.jumps.len() + 1]; // Handle breaks/continue/returns in a finally block for i in 0..info.jumps.len() { - patch_jumps.push(self.next_opcode_location()); + patch_jumps[i + 1] = self.next_opcode_location(); let jump_record = info.jumps[i].clone(); jump_record.perform_actions(Self::DUMMY_ADDRESS, self); } + self.patch_jump(fallthrough); + patch_jumps[0] = self.next_opcode_location(); + self.bytecode .patch_jump_table(jump_table_index, &patch_jumps); } diff --git a/core/engine/src/tests/control_flow/mod.rs b/core/engine/src/tests/control_flow/mod.rs index a9aa769df69..13b4b7f79b7 100644 --- a/core/engine/src/tests/control_flow/mod.rs +++ b/core/engine/src/tests/control_flow/mod.rs @@ -195,6 +195,48 @@ fn catch_binding_finally() { )]); } +#[test] +fn finally_fallthrough_with_untaken_return() { + run_test_actions([ + TestAction::assert_eq( + indoc! {r#" + function g(x) { + try { + if (x) return -1; + } catch (e) {} finally {} + return 42; + } + g(0); + "#}, + 42, + ), + TestAction::assert_eq( + indoc! {r#" + function g(x) { + try { + if (x) return -1; + } catch (e) {} finally {} + return 42; + } + g(1); + "#}, + -1, + ), + TestAction::assert_eq( + indoc! {r#" + function h(x) { + try { + if (x) return -1; + } finally {} + return 42; + } + h(0); + "#}, + 42, + ), + ]); +} + #[test] fn finally_with_loop_break() { run_test_actions([TestAction::assert_eq( diff --git a/tests/insta-bytecode/scripts/try-finally.js b/tests/insta-bytecode/scripts/try-finally.js new file mode 100644 index 00000000000..52a32e6e686 --- /dev/null +++ b/tests/insta-bytecode/scripts/try-finally.js @@ -0,0 +1,16 @@ +// Regression test for the try/finally jump table (see PR #5381 / issue #5369). +// A `break`/`continue` that is syntactically present but not executed inside a +// `try` whose `finally` runs must fall through past the jump table instead of +// being taken once the finally completes. The emitted table reserves entry 0 +// for this fallthrough case. +let total = 0; +for (let i = 0; i < 5; ++i) { + try { + if (i === 2) continue; + if (i === 4) break; + } finally { + total += i; + } + total += 100; +} +total; diff --git a/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap new file mode 100644 index 00000000000..2972605ee3b --- /dev/null +++ b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap @@ -0,0 +1,70 @@ +--- +source: tests/insta-bytecode/src/lib.rs +expression: output +input_file: tests/insta-bytecode/scripts/try-finally.js +--- +-------------------------- Compiled Output: '
' --------------------------- +Location Handler Opcode Operands + 000000 StoreZero dst:r01 + 000005 PutLexicalValue src:r01, binding_index:0 + 00000e StoreZero dst:r02 + 000013 StoreInt8 value:5, dst:r03 + 000019 Jump address:000028 + 00001e IncrementLoopIteration + 00001f Inc src:r02, dst:r02 + 000028 JumpIfNotLessThan lhs:r02, rhs:r03, address:000147 + 000035 StoreTrue dst:r04 + 00003a StoreZero dst:r05 + 00003f > 0: 0000aa StoreInt8 value:2, dst:r07 + 000045 0: 0000aa StrictEq lhs:r02, rhs:r07, dst:r06 + 000052 0: 0000aa JumpIfFalse value:r06, address:00006f + 00005b 0: 0000aa StoreFalse dst:r04 + 000060 0: 0000aa StoreOne dst:r05 + 000065 0: 0000aa Jump address:0000b9 + 00006a 0: 0000aa Jump address:00006f + 00006f 0: 0000aa StoreInt8 value:4, dst:r07 + 000075 0: 0000aa StrictEq lhs:r02, rhs:r07, dst:r06 + 000082 0: 0000aa JumpIfFalse value:r06, address:0000a0 + 00008b 0: 0000aa StoreFalse dst:r04 + 000090 0: 0000aa StoreInt8 value:2, dst:r05 + 000096 0: 0000aa Jump address:0000b9 + 00009b 0: 0000aa Jump address:0000a0 + 0000a0 0: 0000aa StoreFalse dst:r04 + 0000a5 < 0: 0000aa Jump address:0000b9 + 0000aa > 1: 0000b9 Exception dst:r06 + 0000af 1: 0000b9 StoreTrue dst:r04 + 0000b4 < 1: 0000b9 Jump address:0000b9 + 0000b9 SetRegisterFromAccumulator dst:r07 + 0000be GetName dst:r08, binding_index:0 + 0000c7 Move src:r02, dst:r09 + 0000d0 Add lhs:r08, rhs:r09, dst:r08 + 0000dd SetName src:r08, binding_index:0 + 0000e6 SetAccumulator src:r07 + 0000eb JumpIfFalse value:r04, address:0000f9 + 0000f4 Throw src:r06 + 0000f9 JumpTable index:5, jump_table:(00011d, 000113, 000118) + 00010e Jump address:00011d + 000113 Jump address:00001e + 000118 Jump address:000147 + 00011d GetName dst:r04, binding_index:0 + 000126 StoreInt8 value:100, dst:r05 + 00012c Add lhs:r04, rhs:r05, dst:r04 + 000139 SetName src:r04, binding_index:0 + 000142 Jump address:00001e + 000147 GetName dst:r03, binding_index:0 + 000150 SetAccumulator src:r03 + 000155 CheckReturn + 000156 Return + +Register Count: 10, Flags: CodeBlockFlags(HAS_PROTOTYPE_PROPERTY) +Constants: + 0000: [STRING] "total" +Bindings: + 0000: total, scope: GlobalDeclarative +Handlers: + 0000: Range: [00003f, 0000aa): Handler: 0000aa, Environment: 00 + 0001: Range: [0000aa, 0000b9): Handler: 0000b9, Environment: 00 +Source Map: + 0000: 31..190: (7, 24) + 0001: 190..285: (12, 5) + 0002: 285..322: (14, 3)