From 745f384219bd27ae14e901ecc9505dfae300c85b Mon Sep 17 00:00:00 2001
From: Ralf Anton Beier <ralf_beier@me.com>
Date: Fri, 15 May 2026 19:21:44 +0200
Subject: [PATCH] =?UTF-8?q?fix(fuzz):=20carve=20out=20return-value-placeme?=
 =?UTF-8?q?nt=20dead=20stores=20=E2=80=94=20closes=20#112?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The exploration fuzz harness `i64_lowering_doesnt_clobber_params` was
flagging `Movw R0, 0` writes as AAPCS param-clobber bugs when the very
next ARM op was a `Mov R0, _` overwriting the same register. The two-
instruction sequence appears in the function-final i32 return-value
placement — the selector zeros R0 and then immediately overwrites with
the actual return value. The Movw R0, 0 is a redundant dead store, not a
real clobber: the param is already preserved at this point and the
immediately-following Mov overrides the zero before any observer sees it.

This is the "option (b) harness-side" fix per the issue's investigation
comment — cheaper than the underlying peephole (option a, deferred for a
future codegen-quality PR) and lets the exploration harness make
forward progress toward gating-status promotion (#31).

## Carve-out shape

For each ARM instruction in the param-protection window, check whether
the immediately-following instruction also writes the same param reg.
If so, the current write is dead and gets skipped. Indexed iteration
keeps the lookup O(1).

## Soundness

The carve-out is local: it only suppresses writes whose result is
*provably* dead at the next instruction. A real mid-computation clobber
would either:
  (a) have non-overwriting next-op → still flagged.
  (b) be part of a chain where the *final* write overwrites R{p} →
      still flagged on the final write (its next op is typically Pop,
      which doesn't write the param reg).

So the carve-out does not hide real AAPCS clobbers; it only suppresses
the duplicate report on the first write of a write-then-overwrite pair.

## What this enables

After this lands and a couple of green fuzz-smoke cycles confirm the
harness stays quiet, it can be promoted from `gating: false` to
`gating: true` in `.github/workflows/fuzz-smoke.yml` — tracked as
task #31.

Issue: #112
---
 .../i64_lowering_doesnt_clobber_params.rs     | 30 ++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/fuzz/fuzz_targets/i64_lowering_doesnt_clobber_params.rs b/fuzz/fuzz_targets/i64_lowering_doesnt_clobber_params.rs
index f95e995..70148b4 100644
--- a/fuzz/fuzz_targets/i64_lowering_doesnt_clobber_params.rs
+++ b/fuzz/fuzz_targets/i64_lowering_doesnt_clobber_params.rs
@@ -109,7 +109,7 @@ fuzz_target!(|input: FuzzInput| {
             3 => Reg::R3,
             _ => continue,
         };
-        for instr in &arm_instrs {
+        for (instr_idx, instr) in arm_instrs.iter().enumerate() {
             // The function prologue (Push, Sub from SP) has source_line None.
             // We only care about instructions that flow from user-level wasm ops.
             let line = match instr.source_line {
@@ -131,6 +131,34 @@ fuzz_target!(|input: FuzzInput| {
             {
                 continue;
             }
+            // Skip return-value-placement dead stores: when the very next ARM
+            // op also writes R{p}, the current write's value is dead and
+            // overwritten before any observer can read it. This pattern
+            // appears in the function-final return-value sequence where the
+            // selector emits e.g. `Movw R0, 0` followed by `Mov R0, R8` to
+            // place an i32 return value (the Movw is a redundant zero-init
+            // that the second Mov immediately overwrites). The lowering is
+            // suboptimal — see issue #112 option (a) for a peephole fix —
+            // but the param IS already preserved at this point (the LocalGet
+            // we're protecting reads from R{p} earlier in the function), so
+            // this is not a real AAPCS clobber.
+            //
+            // Soundness note: this carve-out is safe because:
+            //  1. The next-op-overwrites-same-reg condition is *local* — we
+            //     don't carve out arbitrary writes, only ones whose result is
+            //     provably dead at the next instruction.
+            //  2. If a real bug were to emit `Movw R0, _; Mov R0, R8` in the
+            //     middle of computation (not return-value placement), the
+            //     param's value is already gone anyway — both writes overwrite
+            //     it. The carve-out doesn't *hide* a clobber, it just suppresses
+            //     a duplicate report. The single Mov R0, R8 still gets flagged
+            //     if it precedes any LocalGet(0) — except its own next op is
+            //     usually Pop, which doesn't write R0.
+            if let Some(next) = arm_instrs.get(instr_idx + 1)
+                && writes(&next.op).contains(&param_reg)
+            {
+                continue;
+            }
             for w in writes(&instr.op) {
                 assert_ne!(
                     w,