fix(fuzz): carve out return-value-placement dead stores — closes #112#118
Open
avrabe wants to merge 1 commit into
Open
fix(fuzz): carve out return-value-placement dead stores — closes #112#118avrabe wants to merge 1 commit into
avrabe wants to merge 1 commit into
Conversation
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
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #112 — the exploration fuzz harness `i64_lowering_doesnt_clobber_params` was false-positiving on the `Movw R0, 0; Mov R0, R8` return-value-placement pair. The Movw zeros R0 and is then immediately overwritten by the Mov — a redundant dead store, not a real AAPCS clobber. The param IS already preserved at that point in the function.
This is the option (b) harness-side fix per the issue's investigation comment, deliberately cheaper than the underlying codegen peephole (option a, deferred to a future codegen-quality PR).
Carve-out
For each ARM instruction in the param-protection window, check the immediately-following instruction: if it also writes the same param reg, the current write is provably dead and gets skipped. Indexed iteration via `arm_instrs.get(instr_idx + 1)` keeps the lookup O(1) and side-steps the prior `std::ptr::eq` hack-shape.
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:
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
Once green for a couple of fuzz-smoke cycles, this harness can be promoted from `gating: false` to `gating: true` in `.github/workflows/fuzz-smoke.yml` — tracked as a follow-up.
Test plan
Refs: #112 (will be auto-closed by this PR's merge).
🤖 Generated with Claude Code