From 1fdef75378315be5058ad638d12444b84dc2077e Mon Sep 17 00:00:00 2001 From: tkshsbcue Date: Thu, 21 May 2026 16:42:06 +0530 Subject: [PATCH] fix(vm): reserve jump-table entry 0 for try/finally fallthrough MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The jump table emitted at the end of a `try/finally` block uses the `finally_throw_index` register to dispatch to a `break`/`continue`/`return` that was pending when control transferred into the finally. The index register is initialised to `0` and only updated by `HandleFinally`, so `0` represents "no jump record was selected — fall through past the table". #4852 made `JumpTable` index its address array directly and dropped the explicit default address. The same PR also removed the `+1` offset that `HandleFinally` used to apply to its index, so the initial value `0` now collides with the first registered jump record. Any `return`, `break`, or `continue` syntactically present (but not executed) inside a `try` block whose `finally` runs is then taken as soon as the finally completes, even though control should fall through to the code after the `try` statement. A minimal reproducer (which silently breaks React 19's `dispatchSetStateInternal`): function g(x) { try { if (x) return -1; } catch (e) {} finally {} return 42; } g(0); // returned `undefined`, should be `42` Restore the `+1` offset in `HandleFinally` and size the emitted jump table at `N + 1` entries: entry `0` is patched to point past all of the jump-record handlers, and entries `1..=N` continue to dispatch to the registered records. A jump emitted right after the table skips the record handlers in the fallthrough case so that the new entry `0` target is only reachable through the table. Fixes #5369. --- core/engine/src/bytecompiler/jump_control.rs | 24 +++++++---- core/engine/src/tests/control_flow/mod.rs | 42 ++++++++++++++++++++ 2 files changed, 58 insertions(+), 8 deletions(-) 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(