Skip to content

Commit ac03075

Browse files
authored
x64: fix misaligned load fault with sunk load when AVX is disabled (#10417)
In [#10408], the new assembler re-opened an old issue related to unaligned loads with SSE instructions. SSE instructions expect 128-bit aligned loads when using the `m128` operand and fault if that is not the case. This had been fixed previously by disallowing load-sinking for `XmmMem` ([#4891]) but more recently we had adopted the use of `XmmMemAligned` in `cranelift-codegen`. Since [#10316] had no knowledge of `XmmMemAligned` (only `XmmMem`), it caused the same kind fault--an OOB trap that was in fact an unaligned load. Why didn't CI catch this? Since all the CI machines have AVX and we do not explicitly test the SSE-only case, these unaligned, sunk loads would use the AVX lowering in CI. AVX loads handle unaligned accesses without a fault. This was only discovered during fuzzing when AVX was disabled (i.e., `--target x86_64-unknown-linux-gnu`). To fix this, this change adopts the `XmmMemAligned` type in the generated assembler code. This is temporary, though: a more lasting fix should pass along an "alignment required" bit from the assembler AST. In the meantime, this closes #10408. [#10408]: #10408 [#4891]: #4891 [#10316]: #10316
1 parent 0a29578 commit ac03075

File tree

2 files changed

+6
-3
lines changed

2 files changed

+6
-3
lines changed

cranelift/codegen/meta/src/gen_asm.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ pub fn rust_param_raw(op: &Operand) -> String {
1515
}
1616
}
1717
OperandKind::RegMem(rm) => match rm.bits() {
18-
128 => "&XmmMem".to_string(),
18+
128 => "&XmmMemAligned".to_string(),
1919
_ => "&GprMem".to_string(),
2020
},
2121
OperandKind::Reg(r) => match r.bits() {
@@ -204,7 +204,7 @@ pub fn isle_param_raw(op: &Operand) -> String {
204204
},
205205
OperandKind::FixedReg(_) => "Gpr".to_string(),
206206
OperandKind::RegMem(rm) => match rm.bits() {
207-
128 => "XmmMem".to_string(),
207+
128 => "XmmMemAligned".to_string(),
208208
_ => "GprMem".to_string(),
209209
},
210210
}

cranelift/codegen/src/isa/x64/lower/isle.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -1058,7 +1058,10 @@ impl IsleContext<'_, '_, MInst, X64Backend> {
10581058
}
10591059

10601060
/// Helper used by code generated by the `cranelift-assembler-x64` crate.
1061-
fn convert_xmm_mem_to_assembler_read_xmm_mem(&self, read: &XmmMem) -> asm::XmmMem<Xmm, Gpr> {
1061+
fn convert_xmm_mem_to_assembler_read_xmm_mem(
1062+
&self,
1063+
read: &XmmMemAligned,
1064+
) -> asm::XmmMem<Xmm, Gpr> {
10621065
match read.clone().into() {
10631066
RegMem::Reg { reg } => asm::XmmMem::Xmm(Xmm::new(reg).unwrap()),
10641067
RegMem::Mem { addr } => asm::XmmMem::Mem(addr.into()),

0 commit comments

Comments
 (0)