fix(#273,#274): preserve div/rem traps — no const-fold of INT_MIN/-1, div/rem not dead-eliminable#276
Open
avrabe wants to merge 1 commit into
Open
fix(#273,#274): preserve div/rem traps — no const-fold of INT_MIN/-1, div/rem not dead-eliminable#276avrabe wants to merge 1 commit into
avrabe wants to merge 1 commit into
Conversation
…div/rem not dead-eliminable) Integer div_s/div_u/rem_s/rem_u carry mandatory wasm traps: divide-by-zero (any op) and signed overflow INT_MIN / -1 (div_s). Two optimizations were silently dropping these traps, returning a value where the original traps. #273 — constant folding dropped the overflow trap. The signed const-fold arms in loom-shared used wrapping_div/wrapping_rem guarded only on divisor != 0, so i32/i64 div_s(INT_MIN,-1) folded to the constant INT_MIN instead of staying a trapping div_s. Fixed: skip folding div_s when dividend == INT_MIN && divisor == -1 (leave the op to trap at runtime). rem_s(INT_MIN,-1) does NOT trap (result 0) — folded explicitly to 0, avoiding wrapping_rem's panic path. div_u/rem_u by-zero was already unfoldable (guarded on divisor != 0); unsigned has no overflow case. #274 — dead-value/carrier forwarding deleted trapping div/rem. The #219 carrier-forwarding pass classified div/rem as pure via is_simple_pure_instr; its dead-carrier path (forward_carrier_in_body) DELETES the carrier's RHS run outright, so an unused div/rem with a runtime-variable divisor was removed, dropping its potential trap. Fixed: div/rem removed from is_simple_pure_instr (potentially-trapping ⇒ not dead-eliminable). This is conservative — the #219 seam carrier is extend/shift/or/and, never div/rem — so no perf regression. Pure non-trapping ops stay eliminable. Regression tests (i32+i64, all four ops): #273 div_s(INT_MIN,-1) not folded; rem_s(INT_MIN,-1) folds to 0; normal div still folds. #274 dead-result div_s/div_u/rem_s/rem_u not eliminated through the full pipeline and through forward_carrier_locals directly; dead pure i32.add still eliminated. Confirmed via wasmtime that the optimized modules trap identically to the originals (integer overflow / integer divide by zero) rather than returning. Fixes: #273 Fixes: #274 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Fixes #273. Fixes #274. Two trap-elimination MISCOMPILES (optimized code returned a value where the original TRAPS). Both pre-existed on 1.1.18. wasmtime-verified: optimized modules now trap identically to the original.
#273 — constant-folding dropped the overflow trap
loom-shared/src/lib.rs(rewrite_pure_impl, theDiv/Rem Sarms): the signed const-fold usedwrapping_div/wrapping_remguarded only ondivisor != 0, sodiv_s(INT_MIN, -1)wrapped toINT_MIN— dropping the mandatoryinteger overflowtrap. Added guard!(l == INT_MIN && r == -1)to the div_s fold (i32 + i64) so it stays a trapping op.rem_s(INT_MIN,-1)does NOT trap in wasm (→ 0), so it's folded explicitly to 0. Unsigned already sound.#274 — DCE/carrier-forwarding deleted trapping div/rem
loom-core/src/lib.rs(is_simple_pure_instr): div/rem were listed as pure, so the #219 carrier-forwarding dead-path deleted an unuseddiv_s/div_u/rem_s/rem_u(variable divisor), dropping its div-by-zero / overflow trap. Removed all 8 div/rem variants from the purity predicate — they are potentially-trapping, hence not dead-eliminable. Conservative + lossless for #219 (the seam carrier is extend/shift/or/and, never div/rem).Tests
13 regression tests: div_s(INT_MIN,-1) not folded (i32+i64); rem_s(INT_MIN,-1)→0; dead-result div/rem survive the full pipeline (all 4 ops); dead pure add still eliminated (no over-suppression). wasmtime confirms trap-equivalence.
cargo testgreen on loom-core/shared/isle/cli; fmt+clippy clean.Folds into v1.2.0 (the correctness release). 🤖 Generated with Claude Code