diff --git a/crates/synth-cli/tests/wast_compile.rs b/crates/synth-cli/tests/wast_compile.rs index 3776ef0..39ff5bf 100644 --- a/crates/synth-cli/tests/wast_compile.rs +++ b/crates/synth-cli/tests/wast_compile.rs @@ -533,3 +533,71 @@ fn compile_import_call_uses_field_name_173() { let _ = std::fs::remove_file(&output); } + +/// Regression test for #178: the optimized (default) path miscompiled +/// linear-memory access — a pointer-param `i32.load` lowered to a load from a +/// FIXED address (`0x20000100`), dropping the operand. The fix declines memory +/// modules to `select_with_stack`, so the default and `--no-optimize` outputs +/// must now be byte-identical (and both correct: `ldr [fp, r0]`). If the +/// optimizer ever stops declining and re-introduces the bug, the two diverge. +#[test] +fn pointer_deref_optimized_matches_no_optimize_178() { + use object::{Object, ObjectSection}; + + let wat = workspace_root() + .join("tests") + .join("integration") + .join("ptr_deref.wat"); + assert!(wat.exists(), "ptr_deref.wat not found: {}", wat.display()); + + let text_of = |extra: &[&str]| -> Vec { + let out = std::env::temp_dir().join(format!("synth_ptr_{}.o", extra.len())); + let mut args = vec![ + "compile", + wat.to_str().unwrap(), + "--target", + "cortex-m4f", + "--all-exports", + "--relocatable", + "-o", + out.to_str().unwrap(), + ]; + args.extend_from_slice(extra); + let result = Command::new(synth_binary()) + .args(&args) + .output() + .expect("run synth"); + assert!( + result.status.success(), + "compile failed: {}", + String::from_utf8_lossy(&result.stderr) + ); + let data = std::fs::read(&out).unwrap(); + let elf = object::File::parse(&*data).expect("parse ELF"); + let text = elf + .section_by_name(".text") + .and_then(|s| s.data().ok()) + .expect(".text") + .to_vec(); + let _ = std::fs::remove_file(&out); + text + }; + + let optimized = text_of(&[]); + let no_optimize = text_of(&["--no-optimize"]); + + // The default (optimized) path must fall back to the correct select_with_stack + // lowering for memory access — so its .text equals the --no-optimize .text. + assert_eq!( + optimized, no_optimize, + "optimized pointer-deref output must match --no-optimize (memory path declined, #178)" + ); + + // And neither must contain the bug signature `movw ip, #0x100` (f240 1c00), + // the fixed-address load that ignored the pointer operand. + let bug_sig = [0x40, 0xf2, 0x00, 0x1c]; + assert!( + !optimized.windows(4).any(|w| w == bug_sig), + "compiled .text must not contain the fixed-0x100-address load (#178)" + ); +} diff --git a/crates/synth-synthesis/src/optimizer_bridge.rs b/crates/synth-synthesis/src/optimizer_bridge.rs index c6c7935..7a7206a 100644 --- a/crates/synth-synthesis/src/optimizer_bridge.rs +++ b/crates/synth-synthesis/src/optimizer_bridge.rs @@ -381,6 +381,44 @@ impl OptimizerBridge { /// (VFP/FPU). This includes float→int / int→float conversion ops and float /// loads/stores, since they too produce or consume float-typed values the /// optimized path can neither map to a vreg nor lower to VFP. + /// Linear-memory load/store ops the optimized path miscompiles (#178). + /// + /// The optimized `wasm_to_ir` → `ir_to_arm` lowering of `MemLoad`/`MemStore` + /// drops the address operand: it emits the linear-memory base (0x20000100) + /// but the `ADD base, base, Raddr` comes out with garbage registers, so the + /// load/store hits a fixed address regardless of the operand — both for a + /// dynamic (pointer-param) address AND a constant one. Any function that + /// dereferences memory therefore reads/writes the wrong location. + /// + /// Until the optimized memory path is repaired (and re-enabled — tracked + /// separately), decline any module using linear memory and fall back to + /// `InstructionSelector::select_with_stack`, which lowers these correctly + /// (`ldr [fp, Raddr]`). Correct-but-unoptimized beats fast-but-wrong. + fn is_linear_memory_op(op: &WasmOp) -> bool { + matches!( + op, + WasmOp::I32Load { .. } + | WasmOp::I32Store { .. } + | WasmOp::I32Load8S { .. } + | WasmOp::I32Load8U { .. } + | WasmOp::I32Load16S { .. } + | WasmOp::I32Load16U { .. } + | WasmOp::I32Store8 { .. } + | WasmOp::I32Store16 { .. } + | WasmOp::I64Load { .. } + | WasmOp::I64Store { .. } + | WasmOp::I64Load8S { .. } + | WasmOp::I64Load8U { .. } + | WasmOp::I64Load16S { .. } + | WasmOp::I64Load16U { .. } + | WasmOp::I64Load32S { .. } + | WasmOp::I64Load32U { .. } + | WasmOp::I64Store8 { .. } + | WasmOp::I64Store16 { .. } + | WasmOp::I64Store32 { .. } + ) + } + fn is_unsupported_float_op(op: &WasmOp) -> bool { matches!( op, @@ -2043,6 +2081,19 @@ impl OptimizerBridge { ))); } + // Issue #178: the optimized path miscompiles linear-memory load/store — + // the address operand is dropped, so the access hits a fixed address + // regardless of the (dynamic or constant) operand. Decline the module so + // the backend falls back to `select_with_stack`, which lowers memory + // access correctly (`ldr [fp, Raddr]`). Re-enable once the optimized + // memory path is repaired. + if let Some(mem_op) = wasm_ops.iter().find(|op| Self::is_linear_memory_op(op)) { + return Err(Error::UnsupportedInstruction(format!( + "optimized lowering path miscompiles linear-memory access ({mem_op:?}) — \ + the address operand is dropped; the non-optimized selector is correct — issue #178" + ))); + } + // Preprocess: convert if-else patterns to select let preprocessed = self.preprocess_wasm_ops(wasm_ops); diff --git a/crates/synth-synthesis/tests/audit_optimized_aapcs.rs b/crates/synth-synthesis/tests/audit_optimized_aapcs.rs index a85c761..afe0271 100644 --- a/crates/synth-synthesis/tests/audit_optimized_aapcs.rs +++ b/crates/synth-synthesis/tests/audit_optimized_aapcs.rs @@ -147,41 +147,45 @@ fn optimized_i32_mul_4params_does_not_clobber_r3() { assert_no_clobber_before_epilogue(&arm, 4, "i32_mul_4params"); } +// #178: the optimized path miscompiled linear-memory access (the address +// operand was dropped), so it now DECLINES modules using memory and falls +// back to `select_with_stack` (which lowers `i32.load` correctly and has its +// own AAPCS audit, issue #103). These two tests therefore assert the decline +// rather than auditing the (removed) optimized memory codegen. #[test] -fn optimized_i32_load_4params_does_not_clobber_r3() { - // The MemLoad handler also used to hardcode `rd = Reg::R3`. +fn optimized_i32_load_declines_to_select_with_stack_178() { + let bridge = OptimizerBridge::new(); let wasm = vec![ WasmOp::LocalGet(0), WasmOp::I32Load { offset: 0, align: 0, }, - WasmOp::LocalGet(2), - WasmOp::Drop, - WasmOp::LocalGet(3), - WasmOp::Drop, - WasmOp::Drop, ]; - let arm = compile_optimized(&wasm, 4); - assert_no_clobber_before_epilogue(&arm, 4, "i32_load_4params"); + let err = bridge + .optimize_full(&wasm) + .expect_err("optimized path must decline linear-memory access (#178)"); + let msg = err.to_string(); + assert!( + msg.contains("178") || msg.to_lowercase().contains("linear-memory"), + "decline error should reference the memory issue, got: {msg}" + ); } #[test] -fn optimized_i32_load8u_4params_does_not_clobber_r3() { +fn optimized_i32_load8u_declines_to_select_with_stack_178() { + let bridge = OptimizerBridge::new(); let wasm = vec![ WasmOp::LocalGet(0), WasmOp::I32Load8U { offset: 0, align: 0, }, - WasmOp::LocalGet(2), - WasmOp::Drop, - WasmOp::LocalGet(3), - WasmOp::Drop, - WasmOp::Drop, ]; - let arm = compile_optimized(&wasm, 4); - assert_no_clobber_before_epilogue(&arm, 4, "i32_load8u_4params"); + assert!( + bridge.optimize_full(&wasm).is_err(), + "optimized path must decline sub-word linear-memory access (#178)" + ); } #[test] diff --git a/crates/synth-synthesis/tests/audit_subword_memops.rs b/crates/synth-synthesis/tests/audit_subword_memops.rs index 9266e78..c456a8b 100644 --- a/crates/synth-synthesis/tests/audit_subword_memops.rs +++ b/crates/synth-synthesis/tests/audit_subword_memops.rs @@ -11,9 +11,16 @@ use synth_synthesis::{OptimizerBridge, WasmOp}; fn compile(wasm: &[WasmOp], num_params: usize) { let bridge = OptimizerBridge::new(); - let (ir, _cfg, _stats) = bridge.optimize_full(wasm).expect("optimize_full"); - // ir_to_arm is the panic site for unmapped-vreg consumers. - let _arm = bridge.ir_to_arm(&ir, num_params); + // #178: the optimized path now DECLINES linear-memory ops (typed Err → + // backend falls back to the correct select_with_stack). A decline is the + // expected, panic-free outcome for these memory-op modules. If a module + // does optimize, ir_to_arm must still not panic (the original audit intent). + match bridge.optimize_full(wasm) { + Ok((ir, _cfg, _stats)) => { + let _arm = bridge.ir_to_arm(&ir, num_params); + } + Err(_) => { /* declined to select_with_stack — panic-free, fine (#178) */ } + } } #[test] diff --git a/crates/synth-synthesis/tests/issue_104_i32_loadstore_cse.rs b/crates/synth-synthesis/tests/issue_104_i32_loadstore_cse.rs index d5ed486..e151134 100644 --- a/crates/synth-synthesis/tests/issue_104_i32_loadstore_cse.rs +++ b/crates/synth-synthesis/tests/issue_104_i32_loadstore_cse.rs @@ -293,6 +293,7 @@ fn execute_store_load_param1_addr(addr: u32, value: u32) -> u32 { /// surviving `v0` (the first `Opcode::Load { dest: v0, addr: 1 }` /// for local 1), which maps to R1 — correct. #[test] +#[ignore = "optimized memory codegen declined pending #178 repair; re-enable when the optimized path handles linear memory correctly"] fn issue_104_store_load_roundtrip_optimized() { // Pick a value far enough from 0 that a R0-fallback bug would point // at a sparse-memory address that doesn't contain `value` (so we'd @@ -317,6 +318,7 @@ fn issue_104_store_load_roundtrip_optimized() { /// uninitialized sparse memory → 0, or some other address that doesn't /// hold `value`). #[test] +#[ignore = "optimized memory codegen declined pending #178 repair; re-enable when the optimized path handles linear memory correctly"] fn issue_104_store_load_table_driven() { let cases: &[(u32, u32)] = &[ // (addr, value) — value chosen distinct from addr. @@ -342,6 +344,7 @@ fn issue_104_store_load_table_driven() { /// it would fire on either the natural or swapped layout if the bug /// caused the load to compute a different scratch. #[test] +#[ignore = "optimized memory codegen declined pending #178 repair; re-enable when the optimized path handles linear memory correctly"] fn issue_104_load_and_store_use_consistent_base_register() { let arm = compile_optimized(&store_load_wasm_ops_param0_addr()); @@ -382,6 +385,7 @@ fn issue_104_load_and_store_use_consistent_base_register() { /// `Str` (i.e. the load uses a base computation involving the /// address param, not the value param). #[test] +#[ignore = "optimized memory codegen declined pending #178 repair; re-enable when the optimized path handles linear memory correctly"] fn issue_104_load_addr_does_not_use_value_register() { let arm = compile_optimized(&store_load_wasm_ops_param1_addr()); diff --git a/crates/synth-synthesis/tests/regression_issue_121_slot_stack.rs b/crates/synth-synthesis/tests/regression_issue_121_slot_stack.rs index bc98806..d4ca203 100644 --- a/crates/synth-synthesis/tests/regression_issue_121_slot_stack.rs +++ b/crates/synth-synthesis/tests/regression_issue_121_slot_stack.rs @@ -62,13 +62,16 @@ fn i64_binary_on_partial_stack_does_not_panic() { fn compile_through_optimized(ops: &[WasmOp]) { let bridge = OptimizerBridge::new(); - let (instrs, _cfg, _stats) = bridge - .optimize_full(ops) - .expect("optimize_full should succeed for valid wasm"); - // The ir_to_arm step is where the pre-fix defensive panic fires when a - // back-reference lands on an unmapped vreg. We don't assert on the - // ARM output here — just that we reach this line without panicking. - let _arm = bridge.ir_to_arm(&instrs, /* num_params = */ 4); + // The point of this audit is "no panic". #178: the optimized path now + // declines linear-memory ops (typed Err → backend falls back to + // select_with_stack), a panic-free outcome. If it does optimize, the + // ir_to_arm step must still not panic on the back-reference. + match bridge.optimize_full(ops) { + Ok((instrs, _cfg, _stats)) => { + let _arm = bridge.ir_to_arm(&instrs, /* num_params = */ 4); + } + Err(_) => { /* declined (e.g. linear memory, #178) — panic-free, fine */ } + } } /// Round-6 fuzz-harness shape from PR #117: a Drop sits between the diff --git a/tests/integration/ptr_deref.wat b/tests/integration/ptr_deref.wat new file mode 100644 index 0000000..b0c491e --- /dev/null +++ b/tests/integration/ptr_deref.wat @@ -0,0 +1,5 @@ +(module + (memory 1) + (func $load_field (export "load_field") (param i32) (result i32) + local.get 0 + i32.load offset=0))