Skip to content

fix(opt): ir_to_arm returns Result — panic-free optimized path, fuzz re-gated#132

Merged
avrabe merged 4 commits into
mainfrom
fix/ir-to-arm-panic-free
May 23, 2026
Merged

fix(opt): ir_to_arm returns Result — panic-free optimized path, fuzz re-gated#132
avrabe merged 4 commits into
mainfrom
fix/ir-to-arm-panic-free

Conversation

@avrabe

@avrabe avrabe commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Makes synth's optimized ARM lowering path (ir_to_arm) panic-free on malformed input — the last remaining panic site in that pipeline — and re-promotes the wasm_ops_lower_or_error fuzz harness to gating. Closes debt open since v0.3.1.

Background

wasm_to_ir was hardened across v0.3.1/v0.4.0 (returns Result, pre-flight wasm_stack_check). But ir_to_arm's get_arm_reg closure still panic!'d on an unmapped vreg (the PR #101 defensive panic). Because of that, wasm_ops_lower_or_error was demoted to gating: false in PR #117 and never re-promoted.

Part 1 — ir_to_arm panic-free

The defensive check is not removed — it's converted panic→Err. A malformed-input case and a genuine internal bug both now yield a typed Err with the rich diagnostic. The harness contract ("lower or Err, never panic") is satisfied.

Part 2 — fuzz re-gated

.github/workflows/fuzz-smoke.yml: wasm_ops_lower_or_error flipped gating: falsegating: true; the issue-#121/PR-#117 demote comment replaced with a note that ir_to_arm now returns Result.

Part 3 — regression test

New crates/synth-synthesis/tests/regression_ir_to_arm_panic_free.rs, 5 tests: mixed i32 arith/shift, type-mismatched i64, i64-extend-into-shift, well-formed-program-lowers-to-Ok, and a hand-built IR instruction (Add with unproduced src vregs) that reaches get_arm_reg's defensive branch directly and confirms it yields Err (with a diagnostic-content assertion) rather than panicking.

Note: the optimize_full pre-flight catches WASM-op-level malformed shapes before ir_to_arm; the hand-built-IR test is what exercises the now-Err path directly.

Validation

  • cargo test --workspace --exclude synth-verify — 0 regressions (+5 tests).
  • cargo clippy --workspace --exclude synth-verify --all-targets -- -D warnings — clean.
  • cargo fmt --check — clean.

Test plan

  • CI green, including the now-gating wasm_ops_lower_or_error fuzz harness
  • No regression in AAPCS / i64 / optimized-path suites

🤖 Generated with Claude Code

avrabe and others added 2 commits May 22, 2026 20:13
`ir_to_arm`'s `get_arm_reg` closure panicked ("synth internal compiler
error: vreg vN has no assigned ARM register...") when an IR vreg had no
mapping and no spill slot. PR #101 introduced that panic — correct then
(a loud crash beats a silent R0-fallback miscompilation), but it was the
last remaining panic site in the optimized lowering path.

Convert the `panic!` into `Err(Error::synthesis(...))` carrying the same
issue-#93 diagnostic, and change `ir_to_arm`'s signature from
`Vec<ArmOp>` to `Result<Vec<ArmOp>>`. All 166 `get_arm_reg` call sites
propagate via `?`. The defensive check is preserved — a genuine
`wasm_to_ir` bug still surfaces as a rich typed error, it just no longer
kills the process.

Callers updated: `arm_backend.rs` folds the `ir_to_arm` step into the
existing `optimize_full` fallback so an `Err` falls back to the direct
selector (honest degradation, same as the issue-#120 float path); the
`wasm_ops_lower_or_error` fuzz harness skips on `Err`. New regression
test `regression_ir_to_arm_panic_free` confirms a hand-built IR with an
unmapped src vreg now yields `Err` (not a panic) and that the error
keeps its issue-#93 diagnostic content.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The harness was demoted to `gating: false` in PR #117 pending the
issue-#121 slot_stack refactor. That refactor landed, and the last panic
site in the optimized path — `ir_to_arm`'s defensive `get_arm_reg` — now
returns `Result` instead of `panic!`-ing. The whole `optimize_full` +
`ir_to_arm` path is panic-free, so the harness (contract: "lower or Err,
never panic") gates again. Update the matrix comment accordingly.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@codecov

codecov Bot commented May 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.61692% with 47 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/synth-synthesis/src/optimizer_bridge.rs 75.39% 47 Missing ⚠️

📢 Thoughts on this report? Let us know!

avrabe added 2 commits May 23, 2026 06:39
Sibling fix to the panic-free `ir_to_arm` work in this PR: the
`pop_i32_unary!` macro inside `wasm_to_ir` still used `.expect(...)`,
which the earlier slot_stack Result conversion missed because the
`.expect(...)` ended with `,` (the macro pattern) rather than `;` (the
statement pattern the original sed targeted).

The re-gated `wasm_ops_lower_or_error` fuzz harness — re-promoted to
gating in this PR — found a malformed input that slips past the
pre-flight check and reaches an i32 unary op via `pop_i32_unary!`,
triggering the leftover `.expect()` panic. Converted to
`.ok_or_else(...)?` matching `pop_i32_binary!`'s shape so the
function-level `Result` propagates instead of panicking.

This is exactly the bug class re-gating the harness exists to catch —
the gating fuzz did its job here.
The catch-all comment still described downstream consumers failing
'loudly via slot_stack.pop().expect(...)'. After the Result conversion
and the pop_i32_unary! macro fix in this PR, no .expect() remains;
unknown-op back-references now surface as a typed Err via
.ok_or_else(...)?. Same bug-finder semantics, different failure mode.
The comment now matches the code.
@avrabe

avrabe commented May 23, 2026

Copy link
Copy Markdown
Contributor Author

Two follow-ups pushed: b47eaae fixes the leftover pop_i32_unary! macro .expect(...) (my earlier Result conversion missed it because the macro body's .expect ended with , not ;); the re-gated fuzz caught it on the first run — exactly the bug class re-gating exists to find. c16e314 updates a stale catch-all comment to match the new Err propagation. Polling the rerun.

@avrabe avrabe merged commit a9adbc7 into main May 23, 2026
10 of 13 checks passed
@avrabe avrabe deleted the fix/ir-to-arm-panic-free branch May 23, 2026 08:30
avrabe added a commit that referenced this pull request May 23, 2026
v0.5.0 — verification & robustness:
- #133 validator pattern to full i32 + i64 surface (#76)
- #131 RV32 i64 div/rem (Phase 3 completes i64 integer)
- #132 panic-free ir_to_arm + macro fix + gating fuzz restored

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
avrabe added a commit that referenced this pull request May 30, 2026
#185)

Two fuzz/lint fixes surfaced while landing #180:

- #185: `encoder_no_panic` found a pre-existing panic — feeding PC (R15) as a
  data operand to a Thumb-2 op guarded by `verify_reg_bits` aborts under
  `-Cdebug-assertions`. Convert the 11 `verify_reg_bits` debug-assert sites to a
  fallible `reg_bits_checked() -> Result<()>` that returns a typed Err, matching
  the established "return Err, not panic" pattern (#101/#120/#132). Add a
  deterministic unit test + seed the crash input into the fuzz corpus.

- Clippy (CI rust 1.96.0) flagged a collapsible `if let { if }` in the #167
  wast_compile test; rewrite as a let-chain.

The encoder has further pre-existing totality gaps (arithmetic underflow in
ARM32 BCond offset, etc.) tracked in #186 — out of scope for this bugfix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
avrabe added a commit that referenced this pull request May 30, 2026
…cause #180, re-enable optimized memory path (#183)

* fix(encoder): high-register Thumb ADD/ADDS/SUBS use 32-bit .W form (#180)

Root-cause of the optimized linear-memory miscompilation (#178, mitigated in
#179 by declining memory ops). The bug was in the Thumb *encoder*, not the
optimizer lowering: `ArmOp::Add` (and `Adds`/`Subs`) reg-forms unconditionally
emitted the 16-bit encoding, whose 3-bit register fields overflow for high
registers. The MemLoad/MemStore base scratch is R12, so `add ip, ip, r0` was
emitted as the corrupt `adds r4, r5, r1` (0x186C) — silently dropping the
address operand, making every optimized pointer-deref read/write a fixed
address. (i64 `Adds`/`Subs` low-word ops hit the same class via R8-R11 pairs.)

Fix: guard the 16-bit form on rd/rn/rm < 8 and fall back to 32-bit
ADD.W/ADDS.W/SUBS.W for high registers, exactly as the Sub handler already did.

- arm_encoder.rs: high-reg guards + encode_thumb32_adds/subs_reg_raw helpers
- optimizer_bridge.rs: remove the #179 decline + is_linear_memory_op (obsolete)
- byte-level encoder regression tests (EB0C 0C00 / EB1A 0A08 / EBBA 0A08)
- re-enable the 4 #[ignore]d issue_104 memory CSE tests
- audit_optimized_aapcs: re-assert optimized memory codegen (not the decline)
- wast_compile: rewrite the #178 CLI test to verify the address operand is used
  (no corrupt 6C 18; contains add.w ip, ip, rN) instead of byte-equality

Verified: optimized `i32.load`/`i32.store` of a pointer param now lowers to
`movw/movt ip; add.w ip, ip, r0; ldr/str [ip,#off]` (byte-checked via objdump),
matching --no-optimize semantics. Full suite: 1282 passed, 0 failed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(release): v0.11.4 — workspace pin sweep + CHANGELOG (#180)

Bump [workspace.package] version and every path-dep version pin 0.11.3 → 0.11.4
in lockstep (Cargo.toml ×11 + MODULE.bazel + Cargo.lock), per the mandatory
pre-tag pin-sweep. CHANGELOG [0.11.4] with falsification statement.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(encoder): return Err (not panic) on PC operand; fix collapsible-if (#185)

Two fuzz/lint fixes surfaced while landing #180:

- #185: `encoder_no_panic` found a pre-existing panic — feeding PC (R15) as a
  data operand to a Thumb-2 op guarded by `verify_reg_bits` aborts under
  `-Cdebug-assertions`. Convert the 11 `verify_reg_bits` debug-assert sites to a
  fallible `reg_bits_checked() -> Result<()>` that returns a typed Err, matching
  the established "return Err, not panic" pattern (#101/#120/#132). Add a
  deterministic unit test + seed the crash input into the fuzz corpus.

- Clippy (CI rust 1.96.0) flagged a collapsible `if let { if }` in the #167
  wast_compile test; rewrite as a let-chain.

The encoder has further pre-existing totality gaps (arithmetic underflow in
ARM32 BCond offset, etc.) tracked in #186 — out of scope for this bugfix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* style(backend): fix doc_lazy_continuation in #167 test doc comment

CI clippy (rust 1.96.0) flags `-D clippy::doc-lazy-continuation`: the doc line
starting with `>= num_imports` reads as a malformed blockquote. Wrap it in a
code span so no doc line begins with `>`. Pre-existing on main; unblocks the
v0.11.4 Clippy gate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant