Skip to content

fix(vm): reserve jump-table entry 0 for try/finally fallthrough #5381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tkshsbcue wants to merge 2 commits into
base: main
Choose a base branch
from
+144 −8
Open

fix(vm): reserve jump-table entry 0 for try/finally fallthrough #5381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1fdef75
fix(vm): reserve jump-table entry 0 for try/finally fallthrough
tkshsbcue May 21, 2026
35b4d55
test(insta-bytecode): cover try/finally jump-table fallthrough
tkshsbcue May 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions core/engine/src/bytecompiler/jump_control.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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::<u32>() 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);
}
Expand Down
42 changes: 42 additions & 0 deletions core/engine/src/tests/control_flow/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
16 changes: 16 additions & 0 deletions tests/insta-bytecode/scripts/try-finally.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
70 changes: 70 additions & 0 deletions tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
---
source: tests/insta-bytecode/src/lib.rs
expression: output
input_file: tests/insta-bytecode/scripts/try-finally.js
---
-------------------------- Compiled Output: '<main>' ---------------------------
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)
Loading