ReviewOS

zig-utils/zig-js

Wasm: four corpus modules are skipped because the packing generator emits invalid wat

#881
Closed glennmichael123 opened this 11 days ago ·
11 days ago

Found while auditing the 479 skips for #880. Four modules in the pinned wg-1.0 corpus are never executed — not by design, but because the packing step produced wat that wast2json/parseWat rejects.

data.wast:315  parseWat failed (data.wast:1:32: unexpected token (, expected ).) in generated wat: (module (memor…
data.wast:323  parseWat failed (data.wast:1:40: unexpected token (, expected ).) in generated wat: (module (memor…
elem.wast:281  parseWat failed (elem.wast:1:39: unexpected token (, expected ).) in generated wat: (module (table…
elem.wast:289  parseWat failed (elem.wast:1:47: unexpected token (, expected ).) in generated wat: (module (table…

All four are skipped_module, so every directive in them is skipped too. The pattern is consistent: a (module (memory … or (module (table … where the generator's reconstruction of an inline declaration loses a parenthesis, so the emitted single-line wat is unbalanced.

This is distinct from the other 475 skips, which are legitimate:

countreasonstatus
430.wat text parsing is outside the binary runtimeby design
40NaN payload is implementation-defined at the JS API boundarynot observable through JS Number
5Core 1.0 vs Core 2.0 rule differences (engine implements 2.0, matching V8)deliberate, each locked by a named test
4generator emitted invalid watthis issue

Those 475 are declared with explicit reasons in the inventory, so they satisfy #486's "zero hidden skips" contract. These 4 do not: they are untested surface with no semantic justification.

Reproduce the inventory with:

zig build wasm-spec -Doptimize=ReleaseFast -Dwasm-spec-out=/tmp/inv.json

then read files[].skips[] where kind == "skipped_module".

Fixing it means correcting the generator that produced tests/wasm/spec/{manifest.json,modules.bin} and repacking. Small coverage win (4 modules), but it is the only category of skip that is a defect rather than a decision.

11 days ago

Fixed in 02ace82.

Cause: printWat inlined the contents of an (offset ...) form instead of printing it. That is only sound for a single folded instruction — an abstract offset may hold several, so (data (offset (nop) (i32.const 0))) flattened to (data (nop) (i32.const 0)), which is two folded instructions where the text grammar admits one. wat2wasm rejected it while parsing, so four assert_invalid modules that should merely fail validation with "constant expression required" produced no bytes and were dropped whole. (offset ...) is canonical and never needed rewriting.

Regenerated from the unchanged pin (977f97014c962f7bd1291fcc6d28b41a924882bf) with checksum-verified WABT 1.0.39, per docs/wasm-spec.md. The manifest diff is exactly four directives — skipped_module → assert_invalid at data.wast:315/323 and elem.wast:281/289 — with the same 73 files and same 19,270 total directives.

Corpus: 18,795 pass / 0 fail / 475 skip / 0 crash in both Debug and ReleaseFast, from 18,791 / 0 / 479 / 0. The four recovered modules pass, so the engine does reject them as invalid.

Every remaining skip is now a declared decision rather than a defect:

countreason
430.wat text parsing — outside the binary runtime by design
42NaN payloads — implementation-defined at the JS API boundary
3deliberate Core 2.0 rule choices, each locked by a named unit test

That closes the last item from #880's skip audit.

Sign in to comment on this issue.