Skip to content

Commit e4ceef6

Browse files
avrabeclaude
andauthored
fix(trap-semantics): unreachable traps on every backend (#665); rv32 rem_s drops the spurious INT_MIN/-1 guard (#666) (#668)
#665 — wasm `unreachable` compiled to a NO-OP on thumb-2 AND rv32 (falls through instead of trapping, WASM Core §4.4.5). Root cause was ONE decode drop: `convert_operator` returned `None` for `Unreachable` and `is_intentionally_ignored` whitelisted it alongside `Nop`, so no backend ever received the op — the selector trap arms (ARM `UDF #0`, RV32 `ebreak`) already existed but were dead code. Fix per path: - decoder (synth-core): `Unreachable` now decodes to `WasmOp::Unreachable`; only `Nop` stays intentionally ignorable. - ARM direct (`select_with_stack`) + `select_default`: existing `UDF #0` arms now fire (no change needed). - ARM optimized path (optimizer_bridge): previously lumped `Unreachable` in with Nop as an IR placeholder — now a typed loud-DECLINE to the direct selector (the bridge `Opcode` enum has no trap opcode; adding one would ripple through the #513 mirror-pinned reg_effect/rewrite_op machinery). Same decline-don't-drop pattern as #120 floats / #500 non-tail return. No new ArmOp on the optimized path, so the #511 estimator oracle is untouched (Udf was already covered by the div-zero guards). - RV32: existing `ebreak` arm now fires (no change needed). - aarch64: new `brk #0` encoder + selector arm (was a loud-decline). #666 — rv32 `i32.rem_s(INT_MIN,-1)` spuriously trapped: the selector shared div_s's INT_MIN/-1 overflow `ebreak` guard with rem_s via `bin_with_signed_div_traps`. WASM §4.3.2 defines irem_s(INT_MIN,-1) = 0 with NO trap, and RISC-V M-ext `rem` already returns 0 for the overflow case (unprivileged spec §7.2), so rem_s now takes plain `bin_with_zero_trap` — zero-divisor guard KEPT, bare `rem` is exactly wasm-correct. The pre-existing test `rv32_signed_rem_also_gets_overflow_guard` pinned the BUG; it is rewritten as the #633-twin fix-guard pins (`rv32_signed_rem_carries_only_zero_guard_666` + `rv32_signed_div_still_carries_both_guards_666`), mirroring ARM's `test_633_i64_rems_has_no_overflow_guard` and the existing i64 RV32 pin. Oracles (red on origin/main, green here; CI job trap-semantics-oracle): - scripts/repro/unreachable_665_differential.py — thumb2 + rv32 under unicorn vs wasmtime: bare `unreachable` traps, guarded `unreachable` taken traps, NOT taken returns normally (non-vacuity). Red on main: boom(7,9) "returned" 7 (arg fall-through) on both ISAs. Known gap kept visible: rv32 loud-declines the if/else-result-with-unreachable shape (#343 arity check) — contract-compliant (never falls through). - scripts/repro/rem_s_666_differential.py — rv32 trap table: rems(INT_MIN,-1)→0 no-trap (red on main: spurious ebreak), rems(INT_MIN,1)→0, rems(7,3)→1, rems(-7,3)→-1, rems(7,0) traps, divs(INT_MIN,-1) traps, divs(7,0) traps, divs(7,3)→2. Frozen anchors 10/10 bit-identical (no fixture contains `unreachable` — verified control_step/flight_seam/flight_seam_flat/signed_div_const). Workspace tests 108/108 suites green; fmt + clippy -D warnings clean. Fixes #665 Fixes #666 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent df982f1 commit e4ceef6

10 files changed

Lines changed: 500 additions & 13 deletions

File tree

.github/workflows/ci.yml

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,42 @@ jobs:
286286
- name: Run two-move execution oracle
287287
run: python scripts/repro/cmp_select_two_move_differential.py
288288

289+
trap-semantics-oracle:
290+
name: trap-semantics oracle (#665 unreachable + #666 rem_s)
291+
# #665: wasm `unreachable` must TRAP (WASM §4.4.5) — it was decoder-dropped
292+
# to a no-op on EVERY backend, falling through panic/abort guards. #666:
293+
# rv32 rem_s wrongly carried div_s's INT_MIN/-1 ebreak guard —
294+
# irem_s(INT_MIN,-1) = 0, no trap (§4.3.2). Both EXECUTION-validated under
295+
# unicorn (thumb2 + rv32) against wasmtime ground truth, including the
296+
# non-vacuity direction (a guarded `unreachable` NOT taken runs normally;
297+
# rem_s zero-divisor + div_s overflow traps are KEPT). Isolated job:
298+
# emulation deps pip-installed here ONLY.
299+
runs-on: ubuntu-latest
300+
steps:
301+
- uses: actions/checkout@v7
302+
- uses: dtolnay/rust-toolchain@stable
303+
- name: Cache Cargo dependencies
304+
uses: actions/cache@v6
305+
with:
306+
path: |
307+
~/.cargo/registry
308+
~/.cargo/git
309+
target/
310+
key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
311+
restore-keys: |
312+
${{ runner.os }}-cargo-
313+
- name: Build synth
314+
run: cargo build -p synth-cli
315+
- uses: actions/setup-python@v6
316+
with:
317+
python-version: "3.x"
318+
- name: Install emulation deps
319+
run: pip install wasmtime unicorn pyelftools
320+
- name: Run unreachable trap oracle (#665, thumb2 + rv32)
321+
run: SYNTH=./target/debug/synth python scripts/repro/unreachable_665_differential.py
322+
- name: Run rem_s trap-table oracle (#666, rv32)
323+
run: SYNTH=./target/debug/synth python scripts/repro/rem_s_666_differential.py
324+
289325
fact-spec-oracle:
290326
name: fact-spec elision oracle (#494 phases 2 + 2b)
291327
# VCR-PERF-002 Phase 2 (#494): the proof-carrying-specialization lever

crates/synth-backend-aarch64/src/encoder.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,13 @@ pub fn ret() -> u32 {
6767
0xD65F_03C0
6868
}
6969

70+
/// `brk #imm16` — A64 breakpoint/trap. Used for wasm `unreachable` (#665):
71+
/// WASM §4.4.5 requires an unconditional trap, the A64 analogue of Thumb-2
72+
/// `udf #0` / RV32 `ebreak`.
73+
pub fn brk(imm16: u16) -> u32 {
74+
0xD420_0000 | ((imm16 as u32) << 5)
75+
}
76+
7077
/// Materialize a 32-bit constant into `wd` with `movz` + optional `movk`.
7178
/// Returns 1 or 2 words.
7279
pub fn mov_imm32(rd: Reg, value: u32) -> Vec<u32> {

crates/synth-backend-aarch64/src/selector.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,12 @@ pub fn select(ops: &[WasmOp], num_params: u32) -> Result<Vec<u32>, SelectError>
7979
}
8080
stack.push(dst);
8181
}
82+
// #665: wasm `unreachable` traps unconditionally (WASM §4.4.5) —
83+
// emit `brk #0`, the A64 analogue of Thumb-2 `udf #0` / RV32
84+
// `ebreak`. It pushes nothing; everything after it is
85+
// wasm-validated dead code, and the trailing `End` epilogue after
86+
// the trap is harmless.
87+
WasmOp::Unreachable => words.push(enc::brk(0)),
8288
WasmOp::I32Add => binop(&mut words, &mut stack, enc::add)?,
8389
WasmOp::I32Sub => binop(&mut words, &mut stack, enc::sub)?,
8490
WasmOp::I32Mul => binop(&mut words, &mut stack, enc::mul)?,

crates/synth-backend-riscv/src/selector.rs

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1553,7 +1553,16 @@ impl Selector {
15531553
self.bin_with_zero_trap(op, |rd, rs1, rs2| RiscVOp::Divu { rd, rs1, rs2 })?
15541554
}
15551555
I32RemS => {
1556-
self.bin_with_signed_div_traps(op, |rd, rs1, rs2| RiscVOp::Rem { rd, rs1, rs2 })?
1556+
// #666: rem_s carries ONLY the zero-divisor guard. WASM §4.3.2
1557+
// defines irem_s(INT_MIN, -1) = 0 (no trap) — the INT_MIN/-1
1558+
// overflow trap belongs to div_s alone. The div_s guard was
1559+
// wrongly shared here, so rem_s(INT_MIN,-1) spuriously
1560+
// ebreak'd. RISC-V M-ext `rem` already returns 0 for the
1561+
// overflow case (RISC-V unprivileged spec §7.2, "the remainder
1562+
// of an overflowing signed division is zero"), so the bare
1563+
// instruction is exactly wasm-correct. Twin of the ARM #633
1564+
// fix-guard (test_633_i64_rems_has_no_overflow_guard).
1565+
self.bin_with_zero_trap(op, |rd, rs1, rs2| RiscVOp::Rem { rd, rs1, rs2 })?
15571566
}
15581567
I32RemU => {
15591568
self.bin_with_zero_trap(op, |rd, rs1, rs2| RiscVOp::Remu { rd, rs1, rs2 })?
@@ -2130,8 +2139,12 @@ impl Selector {
21302139
}
21312140

21322141
/// Variant of `bin_with_zero_trap` that also guards `INT_MIN / -1`, which
2133-
/// `div`/`rem` would silently return as `INT_MIN` / `0` respectively — WASM
2134-
/// semantics require a trap. See `docs/binary-safety-design.md` §3.3.
2142+
/// `div` would silently return as `INT_MIN` — WASM `idiv_s` semantics
2143+
/// require a trap. See `docs/binary-safety-design.md` §3.3.
2144+
///
2145+
/// `div_s` ONLY — never `rem_s` (#666): WASM `irem_s(INT_MIN, -1)` is
2146+
/// defined as 0 (no trap), and RISC-V `rem` already returns 0 for that
2147+
/// case, so `rem_s` takes plain [`Self::bin_with_zero_trap`].
21352148
///
21362149
/// Sequence:
21372150
/// ```text
@@ -6213,8 +6226,15 @@ mod tests {
62136226
assert_eq!(bne_count, 1, "only zero-divisor guard expected for div_u");
62146227
}
62156228

6229+
/// #666 fix-guard twin (RV32 analogue of the ARM #633 pin,
6230+
/// `test_633_i64_rems_has_no_overflow_guard`): i32 `rem_s` must carry ONLY
6231+
/// the zero-divisor guard — WASM §4.3.2 defines `irem_s(INT_MIN, -1) = 0`
6232+
/// (no trap), and RISC-V M-ext `rem` already returns 0 for that case, so
6233+
/// the INT_MIN/-1 `ebreak` belongs to `div_s` alone. This test previously
6234+
/// pinned the BUG (asserting rem_s got the overflow guard too, which made
6235+
/// `rem_s(INT_MIN, -1)` spuriously trap).
62166236
#[test]
6217-
fn rv32_signed_rem_also_gets_overflow_guard() {
6237+
fn rv32_signed_rem_carries_only_zero_guard_666() {
62186238
let opts = SelectorOptions::wasm_compliant();
62196239
let out = s_with_opts(
62206240
&[
@@ -6235,8 +6255,38 @@ mod tests {
62356255
}
62366256
)
62376257
});
6238-
assert!(bne_count >= 3);
6239-
assert!(count(&out, |op| matches!(op, RiscVOp::Rem { .. })) == 1);
6258+
assert_eq!(
6259+
bne_count, 1,
6260+
"rem_s must emit only the zero-divisor BNE (no INT_MIN/-1 guard): {out:?}"
6261+
);
6262+
assert_eq!(
6263+
count(&out, |op| matches!(op, RiscVOp::Ebreak)),
6264+
1,
6265+
"rem_s must emit exactly one ebreak (the zero-divisor trap): {out:?}"
6266+
);
6267+
assert_eq!(count(&out, |op| matches!(op, RiscVOp::Rem { .. })), 1);
6268+
}
6269+
6270+
/// #666 twin, other direction: `div_s` KEEPS both guards under
6271+
/// wasm-compliant options — removing the rem_s over-trap must not weaken
6272+
/// the div_s overflow trap.
6273+
#[test]
6274+
fn rv32_signed_div_still_carries_both_guards_666() {
6275+
let opts = SelectorOptions::wasm_compliant();
6276+
let out = s_with_opts(
6277+
&[
6278+
WasmOp::LocalGet(0),
6279+
WasmOp::LocalGet(1),
6280+
WasmOp::I32DivS,
6281+
WasmOp::End,
6282+
],
6283+
2,
6284+
opts,
6285+
);
6286+
assert!(
6287+
count(&out, |op| matches!(op, RiscVOp::Ebreak)) >= 2,
6288+
"div_s keeps the zero-divisor AND INT_MIN/-1 ebreaks: {out:?}"
6289+
);
62406290
}
62416291

62426292
/// Two-arg call: top-of-stack args move to a0 and a1 in source order.

crates/synth-core/src/wasm_decoder.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,7 +1042,7 @@ pub struct FunctionOps {
10421042
/// function (diagnostic + symbol absent → link error names it) instead —
10431043
/// the #180/#185 "unsupported op must Err, never silently continue"
10441044
/// contract. `None` once every op decoded or was intentionally ignorable
1045-
/// (Nop/Unreachable).
1045+
/// (Nop).
10461046
pub unsupported: Option<String>,
10471047
/// #509: blocktype arity side-table — `(param_count, result_count)` of the
10481048
/// k-th `Block`/`Loop`/`If` op in `ops`, in order of appearance.
@@ -1135,7 +1135,7 @@ fn decode_function_body(
11351135
op_offsets.push(offset as u32);
11361136
} else if unsupported.is_none() && !is_intentionally_ignored(&op) {
11371137
// The op was DROPPED by `convert_operator` (`_ => None`) and is not
1138-
// an intentional no-op (Nop/Unreachable) — record it so the
1138+
// an intentional no-op (Nop) — record it so the
11391139
// function is loud-skipped rather than silently miscompiled (#369).
11401140
unsupported = Some(format!("{op:?}"));
11411141
}
@@ -1148,9 +1148,13 @@ fn decode_function_body(
11481148
/// carry no value-affecting semantics for our backend, so dropping them is
11491149
/// correct (NOT a silent miscompile). Everything else that decodes to `None`
11501150
/// is an unsupported op that must loud-skip its function (#369).
1151+
///
1152+
/// #665: `Unreachable` is NOT on this list — it traps (WASM §4.4.5), so it
1153+
/// decodes to `WasmOp::Unreachable` and every backend lowers it to a trap
1154+
/// instruction (or loud-declines). Only `Nop` is genuinely ignorable.
11511155
fn is_intentionally_ignored(op: &wasmparser::Operator) -> bool {
11521156
use wasmparser::Operator::*;
1153-
matches!(op, Nop | Unreachable)
1157+
matches!(op, Nop)
11541158
}
11551159

11561160
/// Convert a wasmparser Operator to our WasmOp enum
@@ -1338,8 +1342,16 @@ fn convert_operator(op: &wasmparser::Operator) -> Option<WasmOp> {
13381342
// End is needed for control flow pattern matching
13391343
End => Some(WasmOp::End),
13401344

1341-
// Nop/Unreachable - skip these
1342-
Nop | Unreachable => None,
1345+
// #665: `unreachable` MUST reach the backends — WASM Core §4.4.5
1346+
// requires it to trap unconditionally. It was previously dropped here
1347+
// (treated like Nop), so every backend compiled it to a no-op and
1348+
// control FELL THROUGH panic!/abort/unreachable-default guards with
1349+
// undefined register state. The selector arms (ARM: UDF #0, RV32:
1350+
// ebreak) already existed; they just never received the op.
1351+
Unreachable => Some(WasmOp::Unreachable),
1352+
1353+
// Nop - skip (genuinely no semantics)
1354+
Nop => None,
13431355

13441356
// Drop is needed for br_if pattern matching
13451357
Drop => Some(WasmOp::Drop),

crates/synth-synthesis/src/optimizer_bridge.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2526,15 +2526,31 @@ impl OptimizerBridge {
25262526
continue;
25272527
}
25282528

2529+
// #665: `unreachable` must TRAP (WASM §4.4.5). The optimized
2530+
// path's local `Opcode` enum has no trap opcode, and adding
2531+
// one would ripple through the #513 mirror-pinned
2532+
// reg_effect/rewrite_op machinery — so decline to the direct
2533+
// selector, whose `Unreachable` arm emits `UDF #0`. Same
2534+
// decline-don't-drop pattern as #120 floats and the #500
2535+
// non-tail `return` (this arm previously lumped `unreachable`
2536+
// in with Nop as a placeholder — a silent fall-through).
2537+
WasmOp::Unreachable => {
2538+
return Err(synth_core::Error::validation(
2539+
"optimized lowering path does not support `unreachable` \
2540+
(no trap opcode in the bridge IR); the direct \
2541+
instruction selector lowers it to UDF #0 — issue #665",
2542+
));
2543+
}
2544+
25292545
// ===== Stack-neutral control-flow / no-op ops =====
25302546
//
2531-
// Nop / Unreachable / Return have no slot_stack effect at
2547+
// Nop / Return have no slot_stack effect at
25322548
// this layer (Return's value handling is done later by the
25332549
// function-epilogue codegen in `ir_to_arm`). They still
25342550
// emit an IR Nop placeholder so the IR length is in lockstep
25352551
// with the wasm op index for any downstream pass that
25362552
// relies on positional alignment.
2537-
WasmOp::Nop | WasmOp::Unreachable | WasmOp::Return => {
2553+
WasmOp::Nop | WasmOp::Return => {
25382554
// #500: `return` is representable here only as a Nop
25392555
// placeholder — the real epilogue is emitted once at the
25402556
// function end, and the callee-saved push/pop frame is

scripts/repro/rem_s_666.wat

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
;; #666 — rv32 i32.rem_s(INT_MIN, -1) spuriously trapped.
2+
;;
3+
;; WASM Core §4.3.2: irem_s(INT_MIN, -1) = 0, NO trap — only idiv_s traps on
4+
;; the INT_MIN/-1 overflow. The rv32 selector shared div_s's INT_MIN/-1
5+
;; ebreak guard with rem_s; RISC-V M-ext `rem` already returns 0 for the
6+
;; overflow case, so the guard was pure over-trap. Twin of the ARM #633
7+
;; fix-guard pin.
8+
(module
9+
(func (export "rems") (param i32 i32) (result i32)
10+
(i32.rem_s (local.get 0) (local.get 1)))
11+
(func (export "divs") (param i32 i32) (result i32)
12+
(i32.div_s (local.get 0) (local.get 1))))

0 commit comments

Comments
 (0)