From 45bf0598fc03185a0f09bc195bcff180461c74f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20Kr=C3=BCger?= Date: Sat, 7 Sep 2024 21:51:43 +0100 Subject: [PATCH] RISC-V: Remove BackendFull usage from test_determinism functionality --- src/riscv/lib/src/machine_state.rs | 34 ++-- src/riscv/lib/src/machine_state/bus.rs | 14 +- .../lib/src/machine_state/bus/devices.rs | 14 +- .../lib/src/machine_state/bus/main_memory.rs | 7 +- .../lib/src/machine_state/csregisters.rs | 10 +- src/riscv/lib/src/machine_state/hart_state.rs | 8 +- src/riscv/lib/src/machine_state/mode.rs | 7 +- src/riscv/lib/src/machine_state/registers.rs | 20 +-- src/riscv/lib/src/pvm/common.rs | 20 +-- src/riscv/lib/src/state_backend.rs | 85 ++++----- .../lib/src/state_backend/random_backend.rs | 165 ++++++++++++++++++ 11 files changed, 257 insertions(+), 127 deletions(-) create mode 100644 src/riscv/lib/src/state_backend/random_backend.rs diff --git a/src/riscv/lib/src/machine_state.rs b/src/riscv/lib/src/machine_state.rs index 4e93f29090..be5035ee36 100644 --- a/src/riscv/lib/src/machine_state.rs +++ b/src/riscv/lib/src/machine_state.rs @@ -925,10 +925,8 @@ pub enum MachineError { #[cfg(test)] mod tests { use super::{ - backend::tests::{test_determinism, ManagerFor}, - bus, - bus::main_memory::tests::T1K, - MachineState, MachineStateLayout, + backend::tests::test_determinism, bus, bus::main_memory::tests::T1K, MachineState, + MachineStateLayout, }; use crate::{ backend_test, @@ -955,16 +953,14 @@ mod tests { use crate::{bits::u64, machine_state::bus::main_memory::M1K}; use proptest::{prop_assert_eq, proptest}; - backend_test!(test_machine_state_reset, F, { - test_determinism::, _>(|space| { - let mut machine: MachineState< - T1K, - TestInstructionCacheLayout, - ManagerFor<'_, F, MachineStateLayout>, - > = MachineState::bind(space); + #[test] + fn test_machine_state_reset() { + test_determinism::, _>(|space| { + let mut machine: MachineState = + MachineState::bind(space); machine.reset(); }); - }); + } backend_test!(test_step, F, { let mut backend = create_backend!(MachineStateLayout, F); @@ -1267,16 +1263,14 @@ mod tests { }); }); - backend_test!(test_reset, F, { - test_determinism::, _>(|space| { - let mut machine_state: MachineState< - M1K, - TestInstructionCacheLayout, - ManagerFor<'_, F, MachineStateLayout>, - > = MachineState::bind(space); + #[test] + fn test_reset() { + test_determinism::, _>(|space| { + let mut machine_state: MachineState = + MachineState::bind(space); machine_state.reset(); }); - }); + } // Test that the machine state does not behave differently when potential ephermeral state is // reset that may impact instruction caching. diff --git a/src/riscv/lib/src/machine_state/bus.rs b/src/riscv/lib/src/machine_state/bus.rs index 339a9484b8..09d07cc415 100644 --- a/src/riscv/lib/src/machine_state/bus.rs +++ b/src/riscv/lib/src/machine_state/bus.rs @@ -201,18 +201,16 @@ where #[cfg(test)] mod tests { use super::{main_memory::tests::T1K, AddressSpace, Bus, BusLayout}; - use crate::{ - backend_test, - machine_state::backend::tests::{test_determinism, ManagerFor}, - }; + use crate::machine_state::backend::tests::test_determinism; use strum::IntoEnumIterator; - backend_test!(test_reset, F, { - test_determinism::, _>(|space| { - let mut bus: Bus>> = Bus::bind(space); + #[test] + fn test_reset() { + test_determinism::, _>(|space| { + let mut bus: Bus = Bus::bind(space); bus.reset(); }); - }); + } #[test] fn test_addr_spaces_start() { diff --git a/src/riscv/lib/src/machine_state/bus/devices.rs b/src/riscv/lib/src/machine_state/bus/devices.rs index 1be27cf137..268df85ba6 100644 --- a/src/riscv/lib/src/machine_state/bus/devices.rs +++ b/src/riscv/lib/src/machine_state/bus/devices.rs @@ -71,15 +71,13 @@ impl AddressableWrite for Devices< #[cfg(test)] mod tests { use super::{Devices, DevicesLayout}; - use crate::{ - backend_test, - machine_state::backend::tests::{test_determinism, ManagerFor}, - }; + use crate::machine_state::backend::tests::test_determinism; - backend_test!(test_reset, F, { - test_determinism::(|space| { - let mut devices: Devices> = Devices::bind(space); + #[test] + fn test_reset() { + test_determinism::(|space| { + let mut devices: Devices<_> = Devices::bind(space); devices.reset(); }); - }); + } } diff --git a/src/riscv/lib/src/machine_state/bus/main_memory.rs b/src/riscv/lib/src/machine_state/bus/main_memory.rs index b50631a1dd..2035ddc3fc 100644 --- a/src/riscv/lib/src/machine_state/bus/main_memory.rs +++ b/src/riscv/lib/src/machine_state/bus/main_memory.rs @@ -269,9 +269,10 @@ pub mod tests { check_address!(u8, 7, 0x11); }); - backend_test!(test_reset, F, { - test_determinism::(|mut memory| { + #[test] + fn test_reset() { + test_determinism::(|mut memory| { memory.reset(); }); - }); + } } diff --git a/src/riscv/lib/src/machine_state/csregisters.rs b/src/riscv/lib/src/machine_state/csregisters.rs index 07ef3cf771..8407a53df2 100644 --- a/src/riscv/lib/src/machine_state/csregisters.rs +++ b/src/riscv/lib/src/machine_state/csregisters.rs @@ -1997,13 +1997,13 @@ mod tests { assert_eq!(csrs.read::(CSRegister::sie), stip | seip); }); - backend_test!(test_reset, F, { - test_determinism::(|space| { - let mut csregs: CSRegisters> = - CSRegisters::bind(space); + #[test] + fn test_reset() { + test_determinism::(|space| { + let mut csregs: CSRegisters<_> = CSRegisters::bind(space); csregs.reset(); }); - }); + } backend_test!(test_fcsr, F, { let mut backend = create_backend!(CSRegistersLayout, F); diff --git a/src/riscv/lib/src/machine_state/hart_state.rs b/src/riscv/lib/src/machine_state/hart_state.rs index ed68f5fcba..cf62b2478a 100644 --- a/src/riscv/lib/src/machine_state/hart_state.rs +++ b/src/riscv/lib/src/machine_state/hart_state.rs @@ -246,17 +246,17 @@ impl HartState { #[cfg(test)] mod tests { use crate::{ - backend_test, machine_state::hart_state::{HartState, HartStateLayout}, state_backend::tests::test_determinism, }; - backend_test!(test_hart_state_reset, F, { + #[test] + fn test_hart_state_reset() { proptest::proptest!(|(pc: u64)| { - test_determinism::(|space| { + test_determinism::(|space| { let mut hart = HartState::bind(space); hart.reset(pc); }); }); - }); + } } diff --git a/src/riscv/lib/src/machine_state/mode.rs b/src/riscv/lib/src/machine_state/mode.rs index 183fbede4a..b3459f40d4 100644 --- a/src/riscv/lib/src/machine_state/mode.rs +++ b/src/riscv/lib/src/machine_state/mode.rs @@ -88,12 +88,13 @@ mod tests { }; use strum::IntoEnumIterator; - backend_test!(test_mode_reset, F, { - test_determinism::(|space| { + #[test] + fn test_mode_reset() { + test_determinism::(|space| { let mut mode1 = ModeCell::bind(space); mode1.reset(); }); - }); + } backend_test!(test_mode_read_write, F, { let mut backend = create_backend!(ModeLayout, F); diff --git a/src/riscv/lib/src/machine_state/registers.rs b/src/riscv/lib/src/machine_state/registers.rs index 073822f86c..e4eec482e6 100644 --- a/src/riscv/lib/src/machine_state/registers.rs +++ b/src/riscv/lib/src/machine_state/registers.rs @@ -464,21 +464,21 @@ mod tests { } }); - backend_test!(test_xregs_reset, F, { - test_determinism::(|space| { - let mut registers: XRegisters> = - XRegisters::bind(space); + #[test] + fn test_xregs_reset() { + test_determinism::(|space| { + let mut registers: XRegisters<_> = XRegisters::bind(space); registers.reset(); }); - }); + } - backend_test!(test_fregs_reset, F, { - test_determinism::(|space| { - let mut registers: FRegisters> = - FRegisters::bind(space); + #[test] + fn test_fregs_reset() { + test_determinism::(|space| { + let mut registers: FRegisters<_> = FRegisters::bind(space); registers.reset(); }); - }); + } #[test] fn parse_xregister_zero_to_x0() { diff --git a/src/riscv/lib/src/pvm/common.rs b/src/riscv/lib/src/pvm/common.rs index 7beaa26008..cf5bede62a 100644 --- a/src/riscv/lib/src/pvm/common.rs +++ b/src/riscv/lib/src/pvm/common.rs @@ -251,7 +251,6 @@ impl< mod tests { use super::*; use crate::{ - backend_test, machine_state::{ bus::{ main_memory::{M1K, M1M}, @@ -260,11 +259,7 @@ mod tests { instruction_cache::TestInstructionCacheLayout, registers::{a0, a1, a2, a3, a6, a7}, }, - state_backend::{ - memory_backend::InMemoryBackend, - tests::{test_determinism, ManagerFor}, - Backend, - }, + state_backend::{memory_backend::InMemoryBackend, tests::test_determinism, Backend}, }; use rand::{thread_rng, Fill}; use std::mem; @@ -393,18 +388,15 @@ mod tests { assert_eq!(written, buffer); } - backend_test!(test_reset, F, { - test_determinism::, _>(|mut space| { + #[test] + fn test_reset() { + test_determinism::, _>(|mut space| { // The [Pvm] type won't bind unless the version cell is set to its initial value. // TODO: RV-46 might change this constraint in the future. space.0.write(INITIAL_VERSION); - let mut machine_state: Pvm< - M1K, - TestInstructionCacheLayout, - ManagerFor<'_, F, PvmLayout>, - > = Pvm::bind(space); + let mut machine_state: Pvm = Pvm::bind(space); machine_state.reset(); }); - }); + } } diff --git a/src/riscv/lib/src/state_backend.rs b/src/riscv/lib/src/state_backend.rs index d7a7f5e8a9..c23b7054ac 100644 --- a/src/riscv/lib/src/state_backend.rs +++ b/src/riscv/lib/src/state_backend.rs @@ -64,6 +64,9 @@ pub mod memory_backend; pub mod owned_backend; mod region; +#[cfg(test)] +pub(crate) mod random_backend; + pub use alloc::*; pub use effects::*; pub use elems::*; @@ -245,71 +248,49 @@ pub mod test_helpers { #[cfg(test)] pub mod tests { use super::{test_helpers::TestBackendFactory, *}; - use crate::backend_test; - use rand::Fill; - use std::marker::PhantomData; - - /// Fill the backend with random data. - pub fn randomise_backend(backend: &mut B) { - struct Randomiser<'a, B> { - backend: &'a mut B, - } - - impl<'a, B> ManagerBase for Randomiser<'a, B> { - type Region = DummyRegion; - - type DynRegion = DummyRegion; - } - - impl<'a, B: BackendFull> ManagerAlloc for Randomiser<'a, B> { - fn allocate_region( - &mut self, - loc: Location<[E; LEN]>, - ) -> Self::Region { - let region = self.backend.region_mut(&loc); - region.try_fill(&mut rand::thread_rng()).unwrap(); - DummyRegion(PhantomData) - } - - fn allocate_dyn_region( - &mut self, - loc: Location<[u8; LEN]>, - ) -> Self::DynRegion { - self.allocate_region(loc) - } - } - - B::Layout::allocate( - &mut Randomiser { backend }, - B::Layout::placed().into_location(), - ); - } + use crate::{backend_test, state_backend::random_backend::Randomised}; + use std::{collections::hash_map::RandomState, hash::BuildHasher}; /// Construct the manager for a given backend lifetime `'a`, a test backend /// factory `F` and a specific layout `L`. pub type ManagerFor<'a, F, L> = <::Backend as BackendManagement>::Manager<'a>; - /// Dummy region that does nothing - struct DummyRegion(PhantomData); - /// Run `f` twice against two different randomised backends and see if the /// resulting backend state is the same afterwards. - pub fn test_determinism(f: T) + pub fn test_determinism(f: T) where - T: Fn(AllocatedOf>), + L: Layout, + T: Fn(AllocatedOf), { - let mut backend1 = crate::create_backend!(L, F); - randomise_backend(&mut backend1); + let hash_state = RandomState::default(); - let mut backend2 = crate::create_backend!(L, F); - randomise_backend(&mut backend2); + let (start1, snapshot1) = { + let placed = L::placed().into_location(); + let mut manager = Randomised::default(); - // Run the procedure against both backends. - f(backend1.allocate(L::placed().into_location())); - f(backend2.allocate(L::placed().into_location())); + let space = L::allocate(&mut manager, placed); + let initial_checksum = hash_state.hash_one(manager.snapshot_regions()); + + f(space); + + (initial_checksum, manager.snapshot_regions()) + }; + + let (start2, snapshot2) = { + let placed = L::placed().into_location(); + let mut manager = Randomised::default(); + + let space = L::allocate(&mut manager, placed); + let initial_checksum = hash_state.hash_one(manager.snapshot_regions()); + + f(space); + + (initial_checksum, manager.snapshot_regions()) + }; - assert_eq!(backend1, backend2); + assert_ne!(start1, start2); + assert_eq!(snapshot1, snapshot2); } /// Given a `StateLayout` and a [`TestBackendFactory`] type, diff --git a/src/riscv/lib/src/state_backend/random_backend.rs b/src/riscv/lib/src/state_backend/random_backend.rs new file mode 100644 index 0000000000..63c580509d --- /dev/null +++ b/src/riscv/lib/src/state_backend/random_backend.rs @@ -0,0 +1,165 @@ +// SPDX-FileCopyrightText: 2024 TriliTech +// +// SPDX-License-Identifier: MIT + +use super::{ + owned_backend::Owned, Elem, Location, ManagerAlloc, ManagerBase, ManagerRead, ManagerReadWrite, + ManagerWrite, +}; +use rand::Fill; +use std::{cell, mem, rc, slice}; + +/// Backend manager that randomises the allocated regions +#[derive(Default)] +pub struct Randomised { + rng: rand::rngs::OsRng, + snapshoters: Vec Vec>>, +} + +impl Randomised { + /// Collect snapshots of all allocated regions. + pub fn snapshot_regions(&self) -> Vec> { + self.snapshoters.iter().map(|f| f()).collect() + } +} + +impl ManagerBase for Randomised { + type Region = rc::Rc>; + + type DynRegion = rc::Rc>>; +} + +impl ManagerAlloc for Randomised { + fn allocate_region( + &mut self, + _loc: Location<[E; LEN]>, + ) -> Self::Region { + let region = unsafe { + let mut zeroed: [E; LEN] = mem::zeroed(); + + slice::from_raw_parts_mut(zeroed.as_mut_ptr().cast::(), mem::size_of_val(&zeroed)) + .try_fill(&mut self.rng) + .unwrap(); + + zeroed + }; + + let region = rc::Rc::new(cell::RefCell::new(region)); + + let region_clone = region.clone(); + self.snapshoters.push(Box::new(move || { + let region = region_clone.borrow(); + let slice = unsafe { + slice::from_raw_parts(region.as_ptr().cast::(), mem::size_of::<[E; LEN]>()) + }; + slice.to_vec() + })); + + region + } + + fn allocate_dyn_region( + &mut self, + _loc: Location<[u8; LEN]>, + ) -> Self::DynRegion { + let mut boxed: Box<[u8; LEN]> = unsafe { + let layout = std::alloc::Layout::new::<[u8; LEN]>(); + let alloc = std::alloc::alloc_zeroed(layout); + Box::from_raw(alloc.cast()) + }; + + boxed.try_fill(&mut self.rng).unwrap(); + + let region = rc::Rc::new(cell::RefCell::new(boxed)); + + let region_clone = region.clone(); + self.snapshoters.push(Box::new(move || { + let region = region_clone.borrow(); + let slice = unsafe { &*region.as_ptr().cast::<[u8; LEN]>() }; + slice.to_vec() + })); + + region + } +} + +impl ManagerRead for Randomised { + fn region_read(region: &Self::Region, index: usize) -> E { + Owned::region_read(®ion.borrow(), index) + } + + fn region_read_all(region: &Self::Region) -> Vec { + Owned::region_read_all(®ion.borrow()) + } + + fn region_read_some( + region: &Self::Region, + offset: usize, + buffer: &mut [E], + ) { + Owned::region_read_some(®ion.borrow(), offset, buffer) + } + + fn dyn_region_read( + region: &Self::DynRegion, + address: usize, + ) -> E { + Owned::dyn_region_read(®ion.borrow(), address) + } + + fn dyn_region_read_all( + region: &Self::DynRegion, + address: usize, + values: &mut [E], + ) { + Owned::dyn_region_read_all(®ion.borrow(), address, values) + } +} + +impl ManagerWrite for Randomised { + fn region_write( + region: &mut Self::Region, + index: usize, + value: E, + ) { + Owned::region_write(&mut region.borrow_mut(), index, value) + } + + fn region_write_all(region: &mut Self::Region, value: &[E]) { + Owned::region_write_all(&mut region.borrow_mut(), value) + } + + fn region_write_some( + region: &mut Self::Region, + index: usize, + buffer: &[E], + ) { + Owned::region_write_some(&mut region.borrow_mut(), index, buffer) + } + + fn dyn_region_write( + region: &mut Self::DynRegion, + address: usize, + value: E, + ) { + Owned::dyn_region_write(&mut region.borrow_mut(), address, value) + } + + fn dyn_region_write_all( + region: &mut Self::DynRegion, + address: usize, + values: &[E], + ) { + Owned::dyn_region_write_all(&mut region.borrow_mut(), address, values) + } +} + +impl ManagerReadWrite for Randomised { + fn region_replace( + region: &mut Self::Region, + index: usize, + value: E, + ) -> E { + Owned::region_replace(&mut region.borrow_mut(), index, value) + } +}