From e80e655b519518ad83608043313c8108005691c9 Mon Sep 17 00:00:00 2001 From: giles Date: Wed, 6 May 2026 21:45:44 +0000 Subject: [PATCH] =?UTF-8?q?sx:=20step=202=20=E2=80=94=20restore=20frame=20?= =?UTF-8?q?locals=20on=20browser=20VmSuspension=20resume?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In `resume_vm`'s `restore_reuse`, the saved sp captured by `call_closure_reuse` was ignored when restoring the caller frame after the async callback finished. The suspended callee's locals/temps stayed on the value stack above saved_sp, so subsequent LOCAL_GET/SET in the caller frame (e.g. letrec sibling bindings waiting on the suspending call) read stale callee data instead of their own slots. Sibling bindings appeared nil after a perform/resume cycle on the JIT path used by the WASM browser kernel. Fix: after popping the callback result and restoring saved_frames, reset `vm.sp <- saved_sp` (when sp is above), then push the callback result. Mirrors the OP_RETURN+sp-reset discipline that sync `call_closure_reuse` already follows. New tests in `spec/tests/test-letrec-resume.sx` cover single binding, sibling bindings, mutual recursion siblings, and nested letrec — all four pass. Full OCaml run_tests: 4529/5868 (was 4525/5864), zero regressions. Co-Authored-By: Claude Opus 4.7 (1M context) --- hosts/ocaml/lib/sx_vm.ml | 10 +++++++- plans/sx-improvements.md | 2 +- spec/tests/test-letrec-resume.sx | 44 ++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 spec/tests/test-letrec-resume.sx diff --git a/hosts/ocaml/lib/sx_vm.ml b/hosts/ocaml/lib/sx_vm.ml index 4bfb456a..ce65ef15 100644 --- a/hosts/ocaml/lib/sx_vm.ml +++ b/hosts/ocaml/lib/sx_vm.ml @@ -896,9 +896,17 @@ let resume_vm vm result = let rec restore_reuse pending = match pending with | [] -> () - | (saved_frames, _saved_sp) :: rest -> + | (saved_frames, saved_sp) :: rest -> let callback_result = pop vm in vm.frames <- saved_frames; + (* Restore sp to the value captured before the suspended callee was + pushed. The callee's locals/temps may still be on the stack above + saved_sp; without this reset, subsequent LOCAL_GET/SET in the + caller frame (e.g. letrec sibling bindings waiting on the call) + see stale callee data instead of their own slots. Mirrors the + OP_RETURN+sp-reset semantics that sync `call_closure_reuse` + relies on for clean caller-frame state. *) + if saved_sp < vm.sp then vm.sp <- saved_sp; push vm callback_result; (try run vm; diff --git a/plans/sx-improvements.md b/plans/sx-improvements.md index d4bb64ea..701883b3 100644 --- a/plans/sx-improvements.md +++ b/plans/sx-improvements.md @@ -182,7 +182,7 @@ these when operands are known numbers/lists. | Step | Status | Commit | |------|--------|--------| | 1 — JIT combinator bug | [x] | 882a4b76 | -| 2 — letrec+resume | [ ] | — | +| 2 — letrec+resume | [x] | (pending) | | 3 — tokenizer :end/:line | [ ] | — | | 4 — parser spans complete | [ ] | — | | 5 — OCaml AdtValue + define-type + match | [ ] | — | diff --git a/spec/tests/test-letrec-resume.sx b/spec/tests/test-letrec-resume.sx new file mode 100644 index 00000000..07639546 --- /dev/null +++ b/spec/tests/test-letrec-resume.sx @@ -0,0 +1,44 @@ +;; Letrec + perform/resume regression tests — Step 2 +;; Verifies sibling bindings survive across an IO suspension when the +;; suspended call goes through call_closure_reuse (JIT path). +;; The browser/WASM kernel reuses the host VM via call_closure_reuse; +;; if restore_reuse drops the caller's saved sp, sibling letrec bindings +;; come back as nil after resume. +(defsuite + "letrec-resume" + (deftest + "single binding survives perform/resume" + (let + ((state (cek-step-loop (make-cek-state (quote (letrec ((f (fn () (perform {:op "io"})))) (f))) (make-env) (list))))) + (assert (cek-suspended? state)) + (let + ((final (cek-resume state 7))) + (assert (cek-terminal? final)) + (assert= (cek-value final) 7)))) + (deftest + "sibling bindings survive perform/resume" + (let + ((state (cek-step-loop (make-cek-state (quote (letrec ((g (fn () 100)) (f (fn () (perform {:op "io"})))) (+ (f) (g)))) (make-env) (list))))) + (assert (cek-suspended? state)) + (let + ((final (cek-resume state 5))) + (assert (cek-terminal? final)) + (assert= (cek-value final) 105)))) + (deftest + "mutual recursion sibling preserved across resume" + (let + ((state (cek-step-loop (make-cek-state (quote (letrec ((even? (fn (n) (if (= n 0) true (odd? (- n 1))))) (odd? (fn (n) (if (= n 0) false (even? (- n 1))))) (fetch (fn () (perform {:op "io"})))) (let ((x (fetch))) (even? x)))) (make-env) (list))))) + (assert (cek-suspended? state)) + (let + ((final (cek-resume state 4))) + (assert (cek-terminal? final)) + (assert= (cek-value final) true)))) + (deftest + "nested letrec — outer sibling survives inner perform" + (let + ((state (cek-step-loop (make-cek-state (quote (letrec ((outer-val (fn () 99)) (inner-call (fn () (letrec ((suspend-fn (fn () (perform {:op "io"})))) (suspend-fn))))) (+ (inner-call) (outer-val)))) (make-env) (list))))) + (assert (cek-suspended? state)) + (let + ((final (cek-resume state 1))) + (assert (cek-terminal? final)) + (assert= (cek-value final) 100)))))