From a2beacd288d343ba96f53be3b8705ed2b67a7430 Mon Sep 17 00:00:00 2001 From: Jan-Justin van Tonder Date: Tue, 7 Mar 2023 01:06:19 +0100 Subject: [PATCH] cranelift-interpreter: Add trap on misaligned memory accesses (#5921) * Add checks to `InterpreterState::checked_{load,store}` to trap on misaligned memory accesses where `aligned` memory flag is set. * Alter `stack_{load,store}` instructions to now rely on `MemFlags::new()` instead of `MemFlags::trusted` since `InterpreterState::checked_{load,store}` is only able to deduce type alignment and not stack slot alignment. --- cranelift/interpreter/src/interpreter.rs | 61 ++++++++++++++++++++++++ cranelift/interpreter/src/state.rs | 4 ++ cranelift/interpreter/src/step.rs | 6 ++- 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/cranelift/interpreter/src/interpreter.rs b/cranelift/interpreter/src/interpreter.rs index aa44f8e3ba..eeab50f517 100644 --- a/cranelift/interpreter/src/interpreter.rs +++ b/cranelift/interpreter/src/interpreter.rs @@ -336,6 +336,11 @@ impl<'a> State<'a, DataValue> for InterpreterState<'a> { _ => unimplemented!(), }; + // Aligned flag is set and address is not aligned for the given type + if mem_flags.aligned() && addr_start % load_size != 0 { + return Err(MemoryError::MisalignedLoad { addr, load_size }); + } + Ok(match mem_flags.endianness(self.native_endianness) { Endianness::Big => DataValue::read_from_slice_be(src, ty), Endianness::Little => DataValue::read_from_slice_le(src, ty), @@ -363,6 +368,11 @@ impl<'a> State<'a, DataValue> for InterpreterState<'a> { _ => unimplemented!(), }; + // Aligned flag is set and address is not aligned for the given type + if mem_flags.aligned() && addr_start % store_size != 0 { + return Err(MemoryError::MisalignedStore { addr, store_size }); + } + Ok(match mem_flags.endianness(self.native_endianness) { Endianness::Big => v.write_to_slice_be(dst), Endianness::Little => v.write_to_slice_le(dst), @@ -965,4 +975,55 @@ mod tests { assert_eq!(result, vec![DataValue::F32(Ieee32::with_float(1.0))]) } + + #[test] + fn misaligned_store_traps() { + let code = " + function %test() { + ss0 = explicit_slot 16 + + block0: + v0 = stack_addr.i64 ss0 + v1 = iconst.i64 1 + store.i64 aligned v1, v0+2 + return + }"; + + let func = parse_functions(code).unwrap().into_iter().next().unwrap(); + let mut env = FunctionStore::default(); + env.add(func.name.to_string(), &func); + let state = InterpreterState::default().with_function_store(env); + let trap = Interpreter::new(state) + .call_by_name("%test", &[]) + .unwrap() + .unwrap_trap(); + + assert_eq!(trap, CraneliftTrap::User(TrapCode::HeapMisaligned)); + } + + #[test] + fn misaligned_load_traps() { + let code = " + function %test() { + ss0 = explicit_slot 16 + + block0: + v0 = stack_addr.i64 ss0 + v1 = iconst.i64 1 + store.i64 aligned v1, v0 + v2 = load.i64 aligned v0+2 + return + }"; + + let func = parse_functions(code).unwrap().into_iter().next().unwrap(); + let mut env = FunctionStore::default(); + env.add(func.name.to_string(), &func); + let state = InterpreterState::default().with_function_store(env); + let trap = Interpreter::new(state) + .call_by_name("%test", &[]) + .unwrap() + .unwrap_trap(); + + assert_eq!(trap, CraneliftTrap::User(TrapCode::HeapMisaligned)); + } } diff --git a/cranelift/interpreter/src/state.rs b/cranelift/interpreter/src/state.rs index c797bea7eb..5232bc9259 100644 --- a/cranelift/interpreter/src/state.rs +++ b/cranelift/interpreter/src/state.rs @@ -142,6 +142,10 @@ pub enum MemoryError { OutOfBoundsLoad { addr: Address, load_size: usize }, #[error("Store of {store_size} bytes is larger than available size at address {addr:?}")] OutOfBoundsStore { addr: Address, store_size: usize }, + #[error("Load of {load_size} bytes is misaligned at address {addr:?}")] + MisalignedLoad { addr: Address, load_size: usize }, + #[error("Store of {store_size} bytes is misaligned at address {addr:?}")] + MisalignedStore { addr: Address, store_size: usize }, } /// This dummy state allows interpretation over an immutable mapping of values in a single frame. diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index d0362e93d7..155e5dd7d1 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -173,6 +173,8 @@ where MemoryError::InvalidEntry { .. } => TrapCode::HeapOutOfBounds, MemoryError::OutOfBoundsStore { .. } => TrapCode::HeapOutOfBounds, MemoryError::OutOfBoundsLoad { .. } => TrapCode::HeapOutOfBounds, + MemoryError::MisalignedLoad { .. } => TrapCode::HeapMisaligned, + MemoryError::MisalignedStore { .. } => TrapCode::HeapMisaligned, }; // Assigns or traps depending on the value of the result @@ -522,7 +524,7 @@ where let load_ty = inst_context.controlling_type().unwrap(); let slot = inst.stack_slot().unwrap(); let offset = sum(imm(), args()?)? as u64; - let mem_flags = MemFlags::trusted(); + let mem_flags = MemFlags::new(); assign_or_memtrap({ state .stack_address(AddressSize::_64, slot, offset) @@ -533,7 +535,7 @@ where let arg = arg(0)?; let slot = inst.stack_slot().unwrap(); let offset = sum(imm(), args_range(1..)?)? as u64; - let mem_flags = MemFlags::trusted(); + let mem_flags = MemFlags::new(); continue_or_memtrap({ state .stack_address(AddressSize::_64, slot, offset)