Load/store imm12 encoders truncate offset & 0xFFF with no bounds check — silent wrong-address miscompile
Continuing the encoder audit that surfaced #255 (cmp/adds/subs ThumbExpandImm). The thumb32 load/store immediate-offset encoders in crates/synth-backend/src/arm_encoder.rs all mask the offset to 12 bits and emit unconditionally:
let hw2: u16 = ((rd_bits << 12) | (offset & 0xFFF)) as u16; // no check that offset <= 0xFFF
Ok(bytes)
Affected (all the _imm forms): encode_thumb32_ldr (L6256), str (L6270), ldrb_imm (L6320), ldrsb_imm, ldrh_imm (L6370), ldrsh_imm (L6395), strb_imm, strh_imm (L6445). A grep for return Err/bounds checks across L6256–6470 returns 0.
The bug
LDR.W/STR.W Rd, [Rn, #imm12] (T3/T4) supports imm12 ∈ 0..=4095. For offset >= 0x1000, offset & 0xFFF silently wraps → the access targets the wrong address (offset mod 4096). Same silent-truncation class as #253 (add sp,#256 → #0), but on a memory access — so the failure mode is a wrong load/store (corruption or fault) rather than wrong arithmetic.
Live vs. latent
Likely latent for the current gale workload (host-pointer struct fields and wasm frame offsets are all < 4096), exactly like cmp_imm was before #256. But it bites for: a struct/local frame field at offset ≥ 4096, a static/global placed high in linear memory, or any larger wasm module. Given #251/#253/#256 were all fixed defensively even while latent, this one is worth the same guard — and the consequence here is worse (silent memory corruption).
Suggested fix (mirrors #253)
Guard each: if offset > 0xFFF { return Err(...) } so the selector is forced to materialize the offset and use the register-offset form (encode_thumb32_ldr_reg/str_reg, which already exist right below). A shared helper would cover all eight _imm encoders. (ARM also defines an LDR/STR T4 imm8 index form for a small negative/writeback range, but the clean, uniform fix is the Err + register-offset fallback.)
Found via the wasm-cross-LTO silicon work; happy to add an on-target test vector (a [Rn, #4096] load round-trip on the G474RE) once a fix lands. Close as dup/wontfix if the selector already guarantees offset < 4096 upstream — but a defensive Err still seems warranted given the class history.
Load/store imm12 encoders truncate
offset & 0xFFFwith no bounds check — silent wrong-address miscompileContinuing the encoder audit that surfaced #255 (cmp/adds/subs ThumbExpandImm). The thumb32 load/store immediate-offset encoders in
crates/synth-backend/src/arm_encoder.rsall mask the offset to 12 bits and emit unconditionally:Affected (all the
_immforms):encode_thumb32_ldr(L6256),str(L6270),ldrb_imm(L6320),ldrsb_imm,ldrh_imm(L6370),ldrsh_imm(L6395),strb_imm,strh_imm(L6445). A grep forreturn Err/bounds checks across L6256–6470 returns 0.The bug
LDR.W/STR.W Rd, [Rn, #imm12](T3/T4) supportsimm12 ∈ 0..=4095. Foroffset >= 0x1000,offset & 0xFFFsilently wraps → the access targets the wrong address (offset mod 4096). Same silent-truncation class as #253 (add sp,#256 → #0), but on a memory access — so the failure mode is a wrong load/store (corruption or fault) rather than wrong arithmetic.Live vs. latent
Likely latent for the current gale workload (host-pointer struct fields and wasm frame offsets are all < 4096), exactly like cmp_imm was before #256. But it bites for: a struct/local frame field at offset ≥ 4096, a static/global placed high in linear memory, or any larger wasm module. Given #251/#253/#256 were all fixed defensively even while latent, this one is worth the same guard — and the consequence here is worse (silent memory corruption).
Suggested fix (mirrors #253)
Guard each:
if offset > 0xFFF { return Err(...) }so the selector is forced to materialize the offset and use the register-offset form (encode_thumb32_ldr_reg/str_reg, which already exist right below). A shared helper would cover all eight_immencoders. (ARM also defines an LDR/STR T4imm8index form for a small negative/writeback range, but the clean, uniform fix is theErr+ register-offset fallback.)Found via the wasm-cross-LTO silicon work; happy to add an on-target test vector (a
[Rn, #4096]load round-trip on the G474RE) once a fix lands. Close as dup/wontfix if the selector already guaranteesoffset < 4096upstream — but a defensiveErrstill seems warranted given the class history.