engine: move wasm sim live-state ownership into the blob#630
Merged
Conversation
The @simlin/engine wasm backend was reconstructing the VM's live-state semantics in TypeScript by poking the blob's curr chunk directly -- a host-side zero-fill on reset and an override mirror on setValue. Each divergence from the VM surfaced as a separate review finding, and the zero-fill and mirror even contradicted each other (reset clobbered the override it had just mirrored). Make the blob own the live curr chunk's presentation end-to-end so the host needs no shadow writes: - set_value mirrors the override into curr (like the VM's set_value_now) and marks the slot in a new const_override_set region. - reset zeroes curr and reapplies the explicitly-overridden constants (matching libsimlin's recreate-and-reapply reset). The marker region is needed because a freshly-created VM leaves an unoverridden constant at 0 until initials run -- reapplying mere defaults would diverge. - run_to breaks at the top of its loop when saved >= n_chunks, so a resume on an already-complete slab (a second runToEnd, or scrubbing that stays at the end) is a no-op rather than writing one results row past the n_chunks-row region and corrupting the snapshot/GF regions after it. The DirectBackend wasm path drops the zero-fill and the mirror; it now calls the blob exports 1:1, structurally matching the VM path. Regression tests cover both review findings: the out-of-bounds write on a full slab (run_to on a full slab is a memory no-op) and the reset-clobbers-override divergence (setValue/reset present the override in curr, matching the VM).
Review — no blocking issues foundI reviewed the diff (the Verification notes:
Both #628 findings (the OOB write on a resumed full slab, and reset clobbering a mirrored override) are addressed, each with a TDD regression test on both the Rust blob and the TS host. Overall correctness: correct. No findings. |
This was referenced May 23, 2026
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.
Summary
Follow-up to #628. The
@simlin/enginewasm backend was reconstructing the bytecode VM's live-state semantics in TypeScript by poking the blob'scurrchunk directly -- a host-side zero-fill onresetand an override mirror onsetValue. Every nuance of the VM's live-state presentation had to be re-derived in the host, and each gap surfaced as a separate review finding (the "whack-a-mole" #628 kept hitting). Two of those even contradicted each other:reset's zero-fill clobbered the override thatsetValuehad just mirrored intocurr.This makes the blob own the live
currchunk's presentation end-to-end, so the host needs no shadow writes and theDirectBackendwasm path calls the blob exports 1:1 -- structurally identical to the VM-via-libsimlin path.What changed (the blob,
wasmgen/module.rs)set_valuenow mirrors the override into the livecurrchunk (matching the VM'sset_value_now) and marks the slot in a newconst_override_setmarker region.resetnow zeroescurrand reapplies the explicitly-overridden constants, matching libsimlin's recreate-and-reapplysimlin_sim_reset. The marker region is required: a freshly-created VM leaves an unoverridden constant at0until initials run, so reapplying the mere compiled defaults would diverge (the validity region alone can't express "was explicitly set").run_tobreaks at the top of its loop whensaved >= n_chunks, so a resume on an already-complete slab is a no-op.clear_valuesalso drops the override marks (a cleared override is no longer reapplied on the next reset).The blob's export set and the
WasmLayoutwire format are unchanged (the marker region is internal). TheDirectBackendwasm path drops the host-side zero-fill (simReset) and the host-side override mirror (simSetValue).Fixes the two newest #628 review findings
run_topast a full slab (a secondrunToEnd, or interactive scrubbing that stays at the end) re-entered the stepping loop and wrote one results row past then_chunks-row region, silently corrupting the snapshot/GF regions immediately after it (each re-trigger added another row). Now it is a complete no-op.setValue+reset,getValueof the overridden constant returned0instead of the preserved override, because the host zero-fill cleared the mirrored value. Nowresetreapplies the override intocurr, so the read matches the VM.Tests (TDD)
New regression tests, each watched fail first:
wasmgen/module.rs:run_to_on_full_slab_is_a_noop(linear memory is byte-identical after a resumedrun_toon a full slab) andset_value_writes_curr_and_reset_reapplies_override(set_value writes curr; reset zeroes curr and reapplies the override; clear_values drops it).src/engine/tests/wasm-backend.test.ts:runToEnd twice leaves the step count and series unchanged (matches VM)andreset preserves an override in the live curr state (getValue matches VM).Full pre-commit gate passes:
cargo test --workspace,pnpm -C src/engine test(412), libsimlintests/wasm.rsresumable-ABI vs VM, clippy, tsc, pysimlin.