Skip to content

fix(#929): DECLINE a 64-bit call argument instead of miscompiling it - #943

Merged
avrabe merged 2 commits into
mainfrom
fix/i64-call-arg-929-clean
Aug 12, 2026
Merged

fix(#929): DECLINE a 64-bit call argument instead of miscompiling it#943
avrabe merged 2 commits into
mainfrom
fix/i64-call-arg-929-clean

Conversation

@avrabe

@avrabe avrabe commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Closes #929. v0.56 lane RQ-56-I64CALL.

The defect

(func $g (param i64 i32) (result i32) (local.get 1))
f(7)  -> wasmtime 7,  thumb-2 287454020
f(42) -> wasmtime 42, thumb-2 287454020   (= 0x11223344, the i64's HIGH half)

emit_arg_moves maps one argument to one register. AAPCS gives a 64-bit argument an even-aligned pair, so the high half was never placed and every later argument shifted down.

The part worth reading

Its docstring has always said it cannot do this:

"i64/f64 arguments — which AAPCS passes in register pairs — are NOT marshalled"

…and it marshalled them anyway. A documented non-capability with nothing enforcing it is not a limitation; it's a silent miscompile. pop_call_args already refused this exact shape for stack argument positions (#503) — register positions never got the equivalent. This adds it.

The refusal is wider than the reported shape, deliberately

@avrabe recorded (i32, i64) as correct. It isn't. AAPCS places that i64 in r2:r3; synth puts its low half in r1. It looked right only because the callee under test returned the i32, so the wrong i64 was never read.

Refusing every i64 register argument is the honest boundary. Refusing only "i64 not last" would leave an accidental correctness in place to break later.

Outcome

The caller is omitted with a named #929 diagnostic (1 of 2 functions were skipped). No wrong bytes ship; a consumer gets a link error instead of a wrong answer.

Follow-up, with its oracle already in place

Real AAPCS pair marshalling. The #928 conformance gate declines 118 assertions as i64-pair — that is precisely this capability, waiting to accept the implementation.

Red-first

Removing the refusal fails both decline tests while the all-i32 regression guard stays green — so the refusal is provably not a blanket one.

fmt 0 · clippy 0 · 135 test suites.

@codecov

codecov Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/synth-synthesis/src/instruction_selector.rs 92.85% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

avrabe and others added 2 commits August 12, 2026 00:22
On thumb-2, a call whose parameter list contains an i64 marshalled arguments
into the wrong registers and exited 0 with no warning. gale measured, executed
under qemu against wasmtime:

    (func $g (param i64 i32) (result i32) (local.get 1))
    f(7)  -> wasmtime 7,  thumb-2 287454020
    f(42) -> wasmtime 42, thumb-2 287454020      (= 0x11223344, the i64 HIGH half)

ROOT CAUSE: emit_arg_moves maps ONE argument to ONE register
(arg_srcs[i] -> ARG_REGS[i]). AAPCS gives a 64-bit argument an EVEN-ALIGNED
register PAIR, so the high half was never placed and every later argument
shifted down one register.

Its docstring has always said it cannot do this — 'i64/f64 arguments, which
AAPCS passes in register *pairs*, are NOT marshalled' — and it marshalled them
anyway. A documented non-capability with nothing enforcing it is not a
limitation; it is a silent miscompile. pop_call_args already REFUSED the same
shape for STACK argument positions (#503); register positions never got the
equivalent. This adds it.

THE REFUSAL IS WIDER THAN THE REPORTED SHAPE, deliberately. gale recorded
(i32, i64) as correct. It is not: AAPCS places that i64 in r2:r3 while synth
puts its low half in r1. It looked right only because the callee under test
returned the i32, so the wrong i64 was never read. Refusing every i64 register
argument is the honest boundary; refusing only 'i64 not last' would leave an
accidental correctness in place to break later.

Outcome on the repro: the caller is OMITTED with a named #929 diagnostic
('1 of 2 functions were skipped'). No wrong bytes ship, and a consumer gets a
link error rather than a wrong answer.

FOLLOW-UP (named, and its oracle already exists): real AAPCS pair marshalling.
The #928 conformance gate declines 118 assertions as `i64-pair` — that is
exactly this capability, waiting to accept the implementation.

Red-first: removing the refusal fails both decline tests while the all-i32
regression guard stays GREEN, so the refusal is provably not a blanket one.

fmt 0, clippy 0, 135 test suites.

Refs #929, RQ-56-I64CALL
This branch's only failing check was **VCR-VER-004 instrument independence**,
and the oracle inside it PASSED — the failing step was its cleanup assert:

    ##[group]Run git diff --exit-code
    diff --git a/Cargo.lock b/Cargo.lock
    ##[error]Process completed with exit code 1

Not caused by anything in this PR. `main`'s wit-component 0.254->0.255 bump
left an orphaned `wasm-encoder 0.254.0` entry in the lock, so the job's first
cargo invocation regenerates it and the tree is no longer clean. `main` itself
has been red on this job since that bump.

Note the branch was clean LOCALLY before the rebase: it predated the bump, so
its lock matched its own manifest. CI checks out the branch MERGED with main,
where the new manifest meets the old lock. Rebasing onto main reproduces it,
and one `cargo metadata` fixes it — the same 21 deletions seen on a pristine
`origin/main` worktree and in #947.

Fixing the gate rather than merging past it: the check is not in main's
required nine, so this could have been waved through, but a red gate whose
cause is understood is a gate you can turn green.

Refs #929, #924

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YJK5LZZEkV5smCY1jKn18L
@avrabe
avrabe force-pushed the fix/i64-call-arg-929-clean branch from 7d556e7 to 314420b Compare August 11, 2026 22:22
avrabe added a commit that referenced this pull request Aug 11, 2026
Same cause as #947 and #943: main's wit-component 0.254->0.255 bump left an
orphaned `wasm-encoder 0.254.0` entry, so this job's first cargo invocation
regenerates the lock and its `git diff --exit-code` cleanup assert fails. The
oracle inside the job PASSES; only the tree-restore check is red.

Refs #928, #924

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YJK5LZZEkV5smCY1jKn18L
avrabe added a commit that referenced this pull request Aug 12, 2026
)

* RQ-56-CONF (#928): the wast assertions are EXECUTED, not discarded

tests/wast/ carries 381 assert_return and 2 assert_trap. Until now NONE were
checked: the only CI path over those files asserted that `synth compile` exits 0
and DISCARDED the expected values. The backend's primary correctness suite
graded 'did an ELF come out', not 'is it right' — the exit-0 vacuity class (#911,
the v0.54 sret_decide gate printing MISMATCH and exiting 0) sitting on the
correctness suite itself, and why three silent miscompiles (#929/#930/#931)
shipped and were found from outside.

The runner that CAN compare values (crates/synth-test) drives Renode over
telnet, so it cannot run on a stock runner and never has. This uses the
mechanism the differentials already use: compile, execute under unicorn, compare
against the .wast's OWN expected literal — the spec's answer, which is what
makes it conformance rather than a second opinion.

    #928 CHECKS=240/240 executed assertions over 25 wast files; 141 declined
      declined: i64-pair=118, memory-unmapped=23

From ZERO executed to 240. Declines are counted and NAMED, never silent; the 118
i64-pair declines are exactly where #929 lives, so enabling that is the
follow-up that would have caught it.

THE FIRST RUN FOUND A WRONG ASSERTION — ours, not synth's. large_compute(1):

    wasmtime   = 3538945
    synth      = 3538945   (agree)
    tests/wast = 287       (WRONG)

synth is correct; the hand-written expectation had drifted for free because it
had never once been executed. Corrected against wasmtime with the reasoning
recorded in the fixture. 239 of 240 hand-written expectations were right — but
nobody could have known which.

Non-vacuity in two places that cannot go quiet alone: the script FAILS on zero
executed assertions, and the CI step greps a three-digit CHECKS count.

Totals move to 137 oracles / 295,621 entries on all three pinned surfaces.
(Caught before push: `printf "%'d"` emitted the EUROPEAN separator, so the docs
briefly read 295.621 — and claim_check passed 43/43 anyway, because it pins
consistency ACROSS surfaces, not format correctness.)

fmt 0, clippy 0, 134 test suites, claim 43/43, wiring 161 scripts / 0 UNDECLARED.

Refs #928, RQ-56-CONF

* RQ-56-CONF: give the conformance gate a floor on the number that means something

Review catch on this PR: `# ci-checks: emulations >= 200` against a measured
263 let the executed-assertion count fall 240 -> 205 and still pass green. A
gate with that much slack does not detect the regression it exists to detect.

Two changes, because the obvious one alone is not enough:

1. `emulations >= 200` -> `>= 263` (the measured value; ratchet direction up).

2. A REAL floor inside the script, on `checked`. The `emulations` floor does
   NOT cover the executed count: `emulations` counts EMULATOR ENTRIES, and 23
   of the 263 are assertions that fault and then DECLINE (unmapped memory). So
   240 -> 205 executed could happen with `emulations` still at 263 and BOTH
   gates green. The old `if checked == 0` non-vacuity check is far too weak to
   catch it.

   The floor now lives on `checked`, asserted where it is computed:
   `min_executed = 240`.

Verified in both directions with true exit codes (not through a pipe, which
hides them): green leg exit 0; floor mutated to 999 -> exit 1 with the FAIL
message. The mutation was asserted to have applied before the run.

Ledger bumped with the floor: `--min-emulation-floor` 295621 -> 295684 in both
ci.yml and claims.yaml. `claim_check.py` 43/43; `oracle_wiring_check.py` exit 0.

Refs #928

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YJK5LZZEkV5smCY1jKn18L

* chore: regenerate Cargo.lock after rebase (un-reds VCR-VER-004)

Same cause as #947 and #943: main's wit-component 0.254->0.255 bump left an
orphaned `wasm-encoder 0.254.0` entry, so this job's first cargo invocation
regenerates the lock and its `git diff --exit-code` cleanup assert fails. The
oracle inside the job PASSES; only the tree-restore check is red.

Refs #928, #924

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YJK5LZZEkV5smCY1jKn18L

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@avrabe
avrabe enabled auto-merge (squash) August 12, 2026 04:13
@avrabe
avrabe merged commit 2d51a91 into main Aug 12, 2026
56 checks passed
@avrabe
avrabe deleted the fix/i64-call-arg-929-clean branch August 12, 2026 04:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CRITICAL] thumb-2 silently miscompiles any call with an i64 parameter that is not last — args land in the wrong registers, exit 0, no warning

1 participant