Skip to content

fix: #346 output validation + data count section, and #347 memory-op bound - #349

Merged
avrabe merged 2 commits into
mainfrom
fix/346-output-validation
Aug 25, 2026
Merged

fix: #346 output validation + data count section, and #347 memory-op bound#349
avrabe merged 2 commits into
mainfrom
fix/346-output-validation

Conversation

@avrabe

@avrabe avrabe commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Scope note: this PR now carries two fixes. #350 (the #347 memory-op bound) auto-merged into this branch, so #346 and #347 land together. Both are v1.4.1 scope, both have their own rivet artifact, and CI is green on the combined head. #350's own description carries the full #347 write-up.

Closes #346 · closes the shipping blocker in #347 · closes the CLI half of #345.


Part 1 — #346: never write a module loom has not validated

Reproduces from 79 bytes

(module (memory 1) (data $d "hello")
  (func (export "init") (param i32)
    local.get 0  i32.const 0  i32.const 5
    memory.init $d
    data.drop $d))
$ loom optimize dc.wasm -o out.wasm
✅ Optimization complete!     $ echo $? -> 0
$ wasm-tools validate out.wasm
error: data count section required (at offset 0x2d)

The section was never emitted at all

Not "dropped": there was no reference to the data count section anywhere in loom-core. The parser read MemoryInit/DataDrop; nothing ever wrote the section they require. Now emitted (id 12) between element and code whenever those instructions survive.

The guarantee was unreachable

The #257 MANDATORY AUTHORITATIVE OUTPUT-VALIDATION BACKSTOP, whose comment states it is "the systemic guarantee that loom can NEVER emit structurally invalid wasm", lives inside optimize_module. The CLI does not call it — it drives the passes directly (#345). Grepping loom-cli/src/main.rs for validate: three hits, all comments.

All three write paths — serial, islands, component — now pass through one gate. On failure the original is written instead, but only after the original is itself proven valid, and the exit code is non-zero.

Confirmed discriminating twice, by reverting each half: without the encoder fix the memory.init test fails on "data count section required"; without the gate the CLI writes the invalid module and exits 0.


Part 2 — #347: bound the quantity that actually drives solver cost

463 KB fused module: >455 s (killed) → 45 s, exit 0, output valid.

Bisected to one pass, confirmed it was the verification not the transform (LOOM_Z3_MAX_INSTRUCTIONS=1 → 1s), then profiled:

13725  TranslationValidator::verify -> Z3_solver_check      97%
 5290   smt::theory_array_base::propagate                   40%
 3849    assert_store_axiom2_core

The solver's array theory instantiating store axioms pairwise — quadratic in memory accesses per body, which inlining multiplies by concatenating callees. LOOM_Z3_MAX_INSTRUCTIONS bounds the wrong quantity.

I went in with a different hypothesis (the #219 acyclic executor, which runs only for inline_functions and fit the evidence). The profile disproved it.

The bound REVERTS rather than keeps — unlike the instruction bound, whose keep-behaviour is left untouched and the inconsistency stated rather than quietly harmonised.

Tiny bodies are exempt, and that is not a nicety. The naive bound silently broke #219's silicon-validated seam dissolution: seams need bound ≥4, #347 needs ≤2 — irreconcilable. The exemption resolves it, with better coverage (49.8% vs 45.2%).

What it is not: not a solution (the obligations are discharged only by #313 slice 5), and not a timeout — the solver's timeout is wired and at 100 ms the pass still exceeded 120 s. A wall-clock budget is ruled out by REQ-14.

Measured cost on a normal module: 0.47% larger output, 21% of the optimization gain foregone.


Verification

loom-core --lib 539/539 (default and LOOM_VERIFY_BACKEND=both) · 410/410 --no-default-features · --test verification 47/47 · loom-cli 21/21 · fmt + clippy clean.

Verifies TEST-346-OUTPUT-IS-VALIDATED-BEFORE-IT-IS-WRITTEN, TEST-347-MEMORY-OP-BOUND

@avrabe
avrabe force-pushed the feat/331-honest-coverage-stats branch from 0580280 to 0e5538d Compare August 21, 2026 17:00
@avrabe
avrabe force-pushed the fix/346-output-validation branch from 07eaa24 to 3c51744 Compare August 21, 2026 17:01
Base automatically changed from feat/331-honest-coverage-stats to main August 21, 2026 18:02
…ata count section

`loom optimize` dropped the data count section while keeping the
`memory.init` instructions that require it, then printed
"✅ Optimization complete!" and exited 0 over a module no validator
accepts. Reproduces from a 79-byte hand-written module, not just the
fused one in the report.

Silent invalid output is the worst shape this can take: the next tool in
the chain reports the failure against ITS OWN input, so the blame lands
downstream of the tool that actually broke the module.

--- the narrow half: the section was never emitted at all ---

Not "dropped" — there was no reference to the data count section anywhere
in loom-core. The parser read MemoryInit/DataDrop; nothing ever wrote the
section they require. The spec makes it mandatory precisely so those
instructions can be validated without scanning the data section.

Now emitted (id 12) between element and code whenever those instructions
survive into the output. The count is the number of data segments — the
size of the index space they index into, not the number of references.

--- the systemic half: the guarantee was unreachable ---

The #257 MANDATORY AUTHORITATIVE OUTPUT-VALIDATION BACKSTOP, whose
comment states it is the systemic guarantee that loom can NEVER emit
structurally invalid wasm, lives inside `optimize_module`. The CLI does
not call it — it drives the passes directly (#345). Grepping
loom-cli/src/main.rs for "validate" returned three hits, all comments.
The binary everyone runs had no output validation whatsoever, so ANY
encoder bug shipped silently as success. This one did.

All three CLI write paths — serial pipeline, islands, component — now go
through one gate that validates before writing, using the same
authoritative check via a newly exposed `encode::validate_output_bytes`
so the CLI cannot drift to a weaker feature set than the parser.

On failure the ORIGINAL input is written instead, but only after the
original is itself proven valid — a .wat input, or one that was already
invalid, must not be copied over the output path and presented as a
module. Exit is non-zero either way: a caller that checks learns
something broke, and a caller that ignores exit codes still gets a module
that loads.

--- tests ---

The load-bearing assertion is that the emitted module carries a data
count SECTION, not merely that it validates — validation alone would also
pass if a future change simply deleted the instructions.

The unit tests on the gate are what would fail if the gate were removed;
the fixture sweep ("exit 0 implies the artifact validates") holds
trivially while the encoder is correct and so cannot catch that. Paired
with a positive control, since a gate that refused everything would pass
the refusal test.

Confirmed discriminating twice, by reverting each half: without the
encoder fix the memory.init test fails on "data count section required";
without the gate the CLI writes the invalid module and exits 0.

loom-cli 21/21, loom-core --lib 534/534 (default AND both-mode),
--test verification 47/47. fmt clean; clippy unchanged (the three
remaining warnings are pre-existing in the feature-gated
maybe_differential_gate).

Verifies TEST-346-OUTPUT-IS-VALIDATED-BEFORE-IT-IS-WRITTEN
Fixes #346
Refs #345, #257, #289

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RZof1M5HMBSYVZPeoT4MEn
@avrabe
avrabe force-pushed the fix/346-output-validation branch from 3c51744 to 25e2628 Compare August 21, 2026 18:02
…what cannot be verified (#350)

97 of 100 real-world components exceeded a 60 s budget. One 463 KB
meld-fused module ran past 455 s and was killed, while a structurally
DENSER 1.13 MB ordinary module finished in 39 s — so it is not size,
functions, blocks, loops or br_table, all of which the control exceeds.

Root-caused by profiling, after bisecting the hang to a single pass:

    14172  loom_core::optimize::inline_functions
    13725   TranslationValidator::verify -> Z3_solver_check      97%
     5290    smt::theory_array_base::propagate                   40%
     3849     assert_store_axiom2_core

The cost is the solver's ARRAY theory — the memory model — instantiating
store axioms PAIRWISE, so it grows quadratically in memory accesses per
body. Only the inliner triggers it, for a structural reason: it is the
one pass that concatenates callee bodies into a caller, and so the one
pass that multiplies memory accesses per function.

`LOOM_Z3_MAX_INSTRUCTIONS` bounds instruction count, which does not track
that quantity at all. This bounds the one that does.

I had a different hypothesis going in — the acyclic-CF executor from
#219, which runs ONLY for inline_functions and fit the evidence
perfectly. The profile disproved it. Worth recording, because a fix for
that would have shipped and changed nothing.

--- exceeding the bound REVERTS, unlike the instruction bound ---

Returning Ok would accept a transform nothing verified, and at this bound
that ships thousands of unproven transforms per module — the wrong
direction for a charter whose rule is to skip rather than risk. Refusing
costs optimization and costs nothing in safety, which is what makes a
bound this aggressive defensible. The instruction bound still keeps;
that inconsistency is left visible rather than quietly harmonised, since
changing long-shipped behaviour belongs in its own change.

Reverts are attributed (`<pass>/memory-ops-over-threshold`) and recorded
in ONE place — recording at the refusal site as well would count each
revert twice, which is the same class of defect as the mislabelled
counter this branch series just fixed.

--- tiny bodies are exempt, and that is not a nicety ---

The naive bound broke #219. Measured: the seam-dissolution tests need a
bound >= 4; the 463 KB module needs <= 2. Irreconcilable — and the first
implementation silently regressed a shipped, silicon-validated
capability, caught only because those tests exist.

Exempting bodies at or below an instruction floor resolves it, because
the seam inlines are tiny and the pathological ones are inlined-large.
With the exemption: 45 s (from >455 s killed), seams intact, and
slightly BETTER coverage than without it (49.8% vs 45.2%).

--- what this is not ---

Not a solution. It buys termination by declining the hardest
obligations; they are discharged only when the memory model stops being
the incumbent's array theory (#313 slice 5).

Not a timeout, and not for want of trying: the solver's timeout IS wired
(LOOM_Z3_TIMEOUT_MS, default 5000) and measurably does not bound this —
at 100 ms the pass still exceeded 120 s, because the time goes into
axiom instantiation, not the search a timeout guards. A wall-clock
budget is also ruled out on principle: it would make which functions got
verified depend on machine speed, so the same input could produce
different output (REQ-14).

Measured cost on a normal module: 0.47% larger output, 21% of the
optimization gain foregone.

loom-core --lib 539/539 (default AND both-mode), 410/410
--no-default-features, --test verification 47/47, loom-cli 21/21.

Verifies TEST-347-MEMORY-OP-BOUND
Refs #347, #219, #313, #331


Claude-Session: https://claude.ai/code/session_01RZof1M5HMBSYVZPeoT4MEn

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@avrabe avrabe changed the title fix: #346 never write a module loom has not validated, and emit the data count section fix: #346 output validation + data count section, and #347 memory-op bound Aug 25, 2026
@avrabe
avrabe merged commit d628ef9 into main Aug 25, 2026
23 of 25 checks passed
@avrabe
avrabe deleted the fix/346-output-validation branch August 25, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant