Fix #322: a throw out of a called function halts the caller's remaining statements - #324
Merged
Merged
Conversation
…ng statements A `throw` that propagated out of a *called* function did not stop the CALLING function from running its subsequent statements — execution (including side-effecting builtin calls) continued until the nearest enclosing try, or the function returned. So a function that called a throwing helper kept doing work after the error, which could corrupt state, double-apply effects, or mask the failure point. (Surfaced while fixing #306: eigen.eigs's `print of <unbound>` printed null then raised, because the meta-interpreter's `return fn of arg` ran after arg-eval threw.) Root cause: CHECK_ERROR (the per-instruction error hook that DISPATCH runs before every opcode) handled only two cases — catch in the *throwing* frame, or halt when no handler exists anywhere in this vm_run — and had no branch for "error set, no handler in THIS frame, but a handler exists in an OUTER frame". That case fell through and kept executing the current frame. Fix: make CHECK_ERROR a loop. For the missing case it unwinds the current frame (drains its stack window, frees an owned env, restores its loop-stall globals, drops its chunk ref — mirroring vm_error_halt's per-frame drain), restores the caller as the current frame, and re-checks — repeating until it reaches the handler frame (catch) or runs out (halt). The unwind branch only runs when frame->try_count == 0, so g_try_depth needs no adjustment; it sets chunk alongside frame/ip so the eventual catch runs with a consistent chunk; vm_handler_in_range scans only [base_frame, top), so a re-entrant vm_run (sandbox_run) boundary is respected — host handlers below base_frame stay invisible. The control case (throw directly in a try body) and the uncaught case are unchanged; a callee that catches its own error does not over-unwind. Since DISPATCH runs CHECK_ERROR before every instruction, this covers the whole interpreter; JIT-hot throwing chains bail to the interpreter and propagate too. Regression: section [109] (test_throw_unwind) — multi-level nesting, throw-in-a-loop-in-a-function (loop-fresh env unwind), inner-catch (no over-unwind), and a 20k-iteration JIT-hot throwing chain. Release + ASan suites 2348/2348, leak floor 0; the frame-unwind path (heap locals through 20 frames x 5000) is ASan-clean. Closes #322. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
A
throwthat propagated out of a called function did not stop the calling function from running its subsequent statements — execution (including side-effecting builtin calls) continued until the nearest enclosingtry, or the function returned. Any function that called a throwing helper kept doing work after the error, which could corrupt state, double-apply effects, or mask the failure point.Surfaced while fixing #306: the meta-interpreter's
print of <unbound>printednullthen raised, becausereturn fn of argran after arg-eval threw. Pure-host reproduction:Before: prints
SHOULD-NOT-PRINTthencaught. After: justcaught. (Throw directly in atrybody already halted correctly — that control case is unchanged.)Root cause
CHECK_ERROR(the per-instruction hookDISPATCHruns before every opcode) handled only two cases — catch in the throwing frame, or halt when no handler exists anywhere in thisvm_run— with no branch for "error set, no handler in THIS frame, but a handler exists in an OUTER frame." That case fell through and kept executing the current frame.Fix
Make
CHECK_ERRORa loop. For the missing case it unwinds the current frame (drains its stack window, frees an owned env, restores its loop-stall globals, drops its chunk ref — mirroringvm_error_halt's per-frame drain), restores the caller as the current frame, and re-checks — repeating until it reaches the handler frame (catch) or runs out (halt).frame->try_count == 0, sog_try_depthneeds no adjustment.chunkalongsideframe/ip, so the eventual catch runs with a consistent chunk.vm_handler_in_rangescans only[base_frame, top), so a re-entrantvm_run(sandbox_run) boundary is respected — host handlers belowbase_framestay invisible.Since
DISPATCHrunsCHECK_ERRORbefore every instruction, this covers the whole interpreter; JIT-hot throwing chains bail to the interpreter and propagate too.Validation
test_throw_unwind) — multi-level nesting, throw-in-a-loop-in-a-function (loop-fresh env unwind), inner-catch (no over-unwind), and a 20k-iteration JIT-hot throwing chain.Closes #322.
🤖 Generated with Claude Code