fix(vm): reserve jump-table entry 0 for try/finally fallthrough#5381
Open
tkshsbcue wants to merge 1 commit into
Open
fix(vm): reserve jump-table entry 0 for try/finally fallthrough#5381tkshsbcue wants to merge 1 commit into
tkshsbcue wants to merge 1 commit into
Conversation
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". boa-dev#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 boa-dev#5369.
Test262 conformance changes
Tested main commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5381 +/- ##
===========================================
+ Coverage 47.24% 60.04% +12.80%
===========================================
Files 476 566 +90
Lines 46892 63027 +16135
===========================================
+ Hits 22154 37846 +15692
- Misses 24738 25181 +443 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The jump table emitted at the end of a
try/finallyblock uses thefinally_throw_indexregister to dispatch to abreak/continue/returnthat was pending when control transferred into the finally. The index register is initialised to0and only updated byHandleFinally, so0represents "no jump record was selected — fall through past the table".#4852 made
JumpTableindex its address array directly and dropped the explicit default address. The same PR also removed the+1offset thatHandleFinallyused to apply to its index, so the initial value0now collides with the first registered jump record. Anyreturn,break, orcontinuesyntactically present (but not executed) inside atryblock whosefinallyruns is then taken as soon as the finally completes, even though control should fall through to the code after thetrystatement.A minimal reproducer (which silently breaks React 19's
dispatchSetStateInternal):Restore the
+1offset inHandleFinallyand size the emitted jump table atN + 1entries: entry0is patched to point past all of the jump-record handlers, and entries1..=Ncontinue 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 entry0target is only reachable through the table.Fixes #5369 as well!