diff --git a/ISSUES.md b/ISSUES.md index 93ea4b9..f8274c1 100644 --- a/ISSUES.md +++ b/ISSUES.md @@ -12,9 +12,9 @@ Opening `/Test/example.txt` through QEMU `vvfat` could corrupt coroutine/guest s Opening a book panics **only in WASM** (`Invalid read ... reason: rejected` → Load access fault) inside `uzlib_uncompress` during the reader's font `prewarmCache` **if** compiled guest stores are enabled. Native QEMU from the same submodule renders fine. -Root cause (confirmed by full diff against pristine upstream): `wasm_tci_only_tb = true` in `tcg_out_qemu_st` is **upstream qemu-wasm's own design**. Upstream forces every store-containing TB to the TCI interpreter because its compiled restartable-block store path is not rewind-safe (a yielded softmmu helper replays the block; safe for loads, corrupting for stores). Font inflate is store-heavy, so that hot loop never JIT-promotes — a real but secondary perf ceiling. +Root cause (updated, E15/E16): the corruption is a **nondeterministic race**, not a deterministic miscompilation. Per-TB compiled store codegen is bit-identical to TCI (E10/E11, 300k+ comparisons). Runtime store-gating bisection showed the crash does not depend on any specific store TB's identity — only on enough compiled stores of *both* widths running — and the same fixed config alternates CRASH/PASS across runs. Prime suspect is shared-memory ordering / concurrency between the compiled store path and another agent touching guest RAM (IO-thread device models, dirty/notdirty handling, or the Asyncify store slow-path `DONE_FLAG`/`UNWINDING` re-entrancy). `wasm_tci_only_tb = true` in `tcg_out_qemu_st` (upstream qemu-wasm's blanket store-TCI) suppresses the race in practice: 0 corruption crashes over 4 clean runs (a 5th failure was an unrelated cold-boot watchdog flake). -Remaining fix is to make compiled stores rewind-safe and drop the blanket TCI — a substantial TCG-backend task, ~15 min per browser iteration, no native repro. Full analysis: `docs/wasm-inflate-panic.md`. +Remaining fix is to eliminate the race so compiled stores are safe and the blanket TCI can be dropped — a TCG-backend/threading task, ~15 min per browser iteration, no native repro. Opt-in debug tools: `?nostoretci` (all stores compiled, reproduces the race), `?sthash`, `?stcw/stcmask/stcval`. Full analysis: `docs/wasm-inflate-panic.md`. ## Resolved: `-icount` throttled book open ~4.7x diff --git a/docs/wasm-inflate-panic.md b/docs/wasm-inflate-panic.md index dd91bec..4b4e967 100644 --- a/docs/wasm-inflate-panic.md +++ b/docs/wasm-inflate-panic.md @@ -448,3 +448,56 @@ The narrowed fallback is a little faster when it works, but it is not reliable enough: one run failed before controls enabled with an interrupt watchdog. The default is restored to the upstream blanket store-TCI fallback. Keep the 32-bit-only result as a useful diagnostic, not a production fix. + +## E15/E16: the corruption is a nondeterministic race, not a miscompiled TB + +Added runtime-settable, opt-in store gating to isolate the culprit without +rebuilds: + +- `?sthash=1|2` — compile one hash-parity half of *all* store TBs (both widths + present in the compiled half), interpret the other. +- `?stcw=&stcmask=&stcval=` — keep every width compiled except the + selected width `stcw`, which compiles only when `(hash(pc) & m) == v`. Intended + for single-width binary search; logs `STBISECT compile pc=... w=...`. + +### Attempted bisection and what it actually showed +- `sthash=1` (compile odd-hash half) -> crash; `sthash=2` (even half) -> pass. + Initial read: a specific pair of TBs. +- Width bisection contradicted that: with all other widths compiled, **both** + 8-bit halves crash and **both** 32-bit halves crash. Identity of the compiled + store TBs does not matter; only that enough of them run. + +### Decisive test: determinism +Re-running a single fixed config exposes nondeterminism: + +```text +sthash=2 run 1 -> CRASH +sthash=2 run 2 -> PASS +sthash=2 run 3 -> CRASH +``` + +So the earlier single-run "pass" results (including the width fallbacks and the +first `sthash=2`) were probabilistic, not fixes. **The compiled-store corruption +is a nondeterministic race**, not a deterministic miscompilation of any specific +TB. E10/E11 (per-TB compiled == TCI, 300k+ comparisons) are fully consistent: +each TB is individually correct; the fault is emergent timing/ordering. + +### Reliability of the production default +Blanket store-TCI over 5 runs: 4 clean reader-opens, 1 failure — and that 1 was +a **cold-boot interrupt-watchdog timeout** (`MEPC 0x4219a37e`), a separate +host-scheduling boot flake, not the `0x4201e210` corruption. So blanket +store-TCI shows 0 corruption crashes here; it suppresses the race in practice +but the underlying race remains. + +### Revised suspect +Prime suspect is now shared-memory ordering / concurrency between the compiled +guest-store path and another agent touching guest RAM (IO/main thread device +models, dirty-bit/notdirty handling, or the Asyncify store slow-path +`DONE_FLAG`/`UNWINDING` re-entrancy flagged in E9), rather than store codegen. +The crash is a corrupted *pointer* (invalid read at `0x3FCB80xx`, region null), +consistent with a torn/stale store of a pointer-sized value under a race. + +### Instrumentation status +`?sthash` and `?stcw/stcmask/stcval` are opt-in debug tools; the default build +path is unchanged (blanket store-TCI). The narrowed 32-bit-only fallback remains +rejected as a default because it only lowers, not removes, the race probability. diff --git a/third_party/qemu b/third_party/qemu index f46ca3b..a18a299 160000 --- a/third_party/qemu +++ b/third_party/qemu @@ -1 +1 @@ -Subproject commit f46ca3b386d384c9048d0917985031a7bbae6ac1 +Subproject commit a18a2991c681fb967b096b895c51f0eff4da1ddc diff --git a/web/app.js b/web/app.js index 8865948..566c297 100644 --- a/web/app.js +++ b/web/app.js @@ -493,6 +493,8 @@ async function boot(file, variant, setStatus) { } else if (params.has('tcgdiff2')) options._wasm_diff_enable2(); else if (params.has('tcgdiff')) options._wasm_diff_enable(); if (params.has('nostoretci')) options._wasm_disable_store_tci(); + if (params.has('sthash')) options._wasm_set_store_tci_hash(Number(params.get('sthash')) || 1); + if (params.has('stcmask')) options._wasm_set_store_bisect(Number(params.get('stcw')) || 0, Number(params.get('stcmask')) >>> 0, (Number(params.get('stcval')) || 0) >>> 0); options.FS.mkdirTree('/bios'); options.FS.mkdirTree('/var/tmp'); options.FS.writeFile('/bios/esp32c3-rom.bin', rom);