Skip to content

Commit

Permalink
Merge tezos/tezos!14852: RISC-V: Remove BackendFull usage from test_d…
Browse files Browse the repository at this point in the history
…eterminism functionality

Co-authored-by: Ole Krüger <ole.kruger@trili.tech>

Approved-by: Victor Dumitrescu <victor.dumitrescu@nomadic-labs.com>
Approved-by: Felix Puscasu <felix.puscasu@trili.tech>
Approved-by: Emma Turner <1623821-emturner@users.noreply.gitlab.com>

See merge request https://gitlab.com/tezos/tezos/-/merge_requests/14852
  • Loading branch information
Marge Bot committed Sep 11, 2024
2 parents ee9da68 + 45bf059 commit f106158
Show file tree
Hide file tree
Showing 11 changed files with 257 additions and 127 deletions.
34 changes: 14 additions & 20 deletions src/riscv/lib/src/machine_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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::<F, MachineStateLayout<T1K, TestInstructionCacheLayout>, _>(|space| {
let mut machine: MachineState<
T1K,
TestInstructionCacheLayout,
ManagerFor<'_, F, MachineStateLayout<T1K, TestInstructionCacheLayout>>,
> = MachineState::bind(space);
#[test]
fn test_machine_state_reset() {
test_determinism::<MachineStateLayout<T1K, TestInstructionCacheLayout>, _>(|space| {
let mut machine: MachineState<T1K, TestInstructionCacheLayout, _> =
MachineState::bind(space);
machine.reset();
});
});
}

backend_test!(test_step, F, {
let mut backend = create_backend!(MachineStateLayout<T1K, TestInstructionCacheLayout>, F);
Expand Down Expand Up @@ -1267,16 +1263,14 @@ mod tests {
});
});

backend_test!(test_reset, F, {
test_determinism::<F, MachineStateLayout<M1K, TestInstructionCacheLayout>, _>(|space| {
let mut machine_state: MachineState<
M1K,
TestInstructionCacheLayout,
ManagerFor<'_, F, MachineStateLayout<M1K, TestInstructionCacheLayout>>,
> = MachineState::bind(space);
#[test]
fn test_reset() {
test_determinism::<MachineStateLayout<M1K, TestInstructionCacheLayout>, _>(|space| {
let mut machine_state: MachineState<M1K, TestInstructionCacheLayout, _> =
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.
Expand Down
14 changes: 6 additions & 8 deletions src/riscv/lib/src/machine_state/bus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<F, BusLayout<T1K>, _>(|space| {
let mut bus: Bus<T1K, ManagerFor<'_, F, BusLayout<T1K>>> = Bus::bind(space);
#[test]
fn test_reset() {
test_determinism::<BusLayout<T1K>, _>(|space| {
let mut bus: Bus<T1K, _> = Bus::bind(space);
bus.reset();
});
});
}

#[test]
fn test_addr_spaces_start() {
Expand Down
14 changes: 6 additions & 8 deletions src/riscv/lib/src/machine_state/bus/devices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,13 @@ impl<E: backend::Elem, M: backend::ManagerBase> AddressableWrite<E> 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::<F, DevicesLayout, _>(|space| {
let mut devices: Devices<ManagerFor<'_, F, DevicesLayout>> = Devices::bind(space);
#[test]
fn test_reset() {
test_determinism::<DevicesLayout, _>(|space| {
let mut devices: Devices<_> = Devices::bind(space);
devices.reset();
});
});
}
}
7 changes: 4 additions & 3 deletions src/riscv/lib/src/machine_state/bus/main_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,10 @@ pub mod tests {
check_address!(u8, 7, 0x11);
});

backend_test!(test_reset, F, {
test_determinism::<F, T1K, _>(|mut memory| {
#[test]
fn test_reset() {
test_determinism::<T1K, _>(|mut memory| {
memory.reset();
});
});
}
}
10 changes: 5 additions & 5 deletions src/riscv/lib/src/machine_state/csregisters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1997,13 +1997,13 @@ mod tests {
assert_eq!(csrs.read::<CSRRepr>(CSRegister::sie), stip | seip);
});

backend_test!(test_reset, F, {
test_determinism::<F, CSRegistersLayout, _>(|space| {
let mut csregs: CSRegisters<ManagerFor<'_, F, CSRegistersLayout>> =
CSRegisters::bind(space);
#[test]
fn test_reset() {
test_determinism::<CSRegistersLayout, _>(|space| {
let mut csregs: CSRegisters<_> = CSRegisters::bind(space);
csregs.reset();
});
});
}

backend_test!(test_fcsr, F, {
let mut backend = create_backend!(CSRegistersLayout, F);
Expand Down
8 changes: 4 additions & 4 deletions src/riscv/lib/src/machine_state/hart_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,17 +246,17 @@ impl<M: backend::ManagerBase> HartState<M> {
#[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::<F, HartStateLayout, _>(|space| {
test_determinism::<HartStateLayout, _>(|space| {
let mut hart = HartState::bind(space);
hart.reset(pc);
});
});
});
}
}
7 changes: 4 additions & 3 deletions src/riscv/lib/src/machine_state/mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,13 @@ mod tests {
};
use strum::IntoEnumIterator;

backend_test!(test_mode_reset, F, {
test_determinism::<F, ModeLayout, _>(|space| {
#[test]
fn test_mode_reset() {
test_determinism::<ModeLayout, _>(|space| {
let mut mode1 = ModeCell::bind(space);
mode1.reset();
});
});
}

backend_test!(test_mode_read_write, F, {
let mut backend = create_backend!(ModeLayout, F);
Expand Down
20 changes: 10 additions & 10 deletions src/riscv/lib/src/machine_state/registers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,21 +464,21 @@ mod tests {
}
});

backend_test!(test_xregs_reset, F, {
test_determinism::<F, XRegistersLayout, _>(|space| {
let mut registers: XRegisters<ManagerFor<'_, F, XRegistersLayout>> =
XRegisters::bind(space);
#[test]
fn test_xregs_reset() {
test_determinism::<XRegistersLayout, _>(|space| {
let mut registers: XRegisters<_> = XRegisters::bind(space);
registers.reset();
});
});
}

backend_test!(test_fregs_reset, F, {
test_determinism::<F, FRegistersLayout, _>(|space| {
let mut registers: FRegisters<ManagerFor<'_, F, FRegistersLayout>> =
FRegisters::bind(space);
#[test]
fn test_fregs_reset() {
test_determinism::<FRegistersLayout, _>(|space| {
let mut registers: FRegisters<_> = FRegisters::bind(space);
registers.reset();
});
});
}

#[test]
fn parse_xregister_zero_to_x0() {
Expand Down
20 changes: 6 additions & 14 deletions src/riscv/lib/src/pvm/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@ impl<
mod tests {
use super::*;
use crate::{
backend_test,
machine_state::{
bus::{
main_memory::{M1K, M1M},
Expand All @@ -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;
Expand Down Expand Up @@ -393,18 +388,15 @@ mod tests {
assert_eq!(written, buffer);
}

backend_test!(test_reset, F, {
test_determinism::<F, PvmLayout<M1K, TestInstructionCacheLayout>, _>(|mut space| {
#[test]
fn test_reset() {
test_determinism::<PvmLayout<M1K, TestInstructionCacheLayout>, _>(|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<M1K, TestInstructionCacheLayout>>,
> = Pvm::bind(space);
let mut machine_state: Pvm<M1K, TestInstructionCacheLayout, _> = Pvm::bind(space);
machine_state.reset();
});
});
}
}
85 changes: 33 additions & 52 deletions src/riscv/lib/src/state_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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<B: BackendFull>(backend: &mut B) {
struct Randomiser<'a, B> {
backend: &'a mut B,
}

impl<'a, B> ManagerBase for Randomiser<'a, B> {
type Region<E: Elem, const LEN: usize> = DummyRegion<E>;

type DynRegion<const LEN: usize> = DummyRegion<u8>;
}

impl<'a, B: BackendFull> ManagerAlloc for Randomiser<'a, B> {
fn allocate_region<E: Elem, const LEN: usize>(
&mut self,
loc: Location<[E; LEN]>,
) -> Self::Region<E, LEN> {
let region = self.backend.region_mut(&loc);
region.try_fill(&mut rand::thread_rng()).unwrap();
DummyRegion(PhantomData)
}

fn allocate_dyn_region<const LEN: usize>(
&mut self,
loc: Location<[u8; LEN]>,
) -> Self::DynRegion<LEN> {
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> =
<<F as TestBackendFactory>::Backend<L> as BackendManagement>::Manager<'a>;

/// Dummy region that does nothing
struct DummyRegion<E>(PhantomData<E>);

/// Run `f` twice against two different randomised backends and see if the
/// resulting backend state is the same afterwards.
pub fn test_determinism<F: TestBackendFactory, L: Layout, T>(f: T)
pub fn test_determinism<L, T>(f: T)
where
T: Fn(AllocatedOf<L, ManagerFor<'_, F, L>>),
L: Layout,
T: Fn(AllocatedOf<L, Randomised>),
{
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,
Expand Down
Loading

0 comments on commit f106158

Please sign in to comment.