From 9a9605cbbcebf7d7ff99b09ed4abbf5bc44fadf1 Mon Sep 17 00:00:00 2001 From: Greg Colombo Date: Sat, 5 Oct 2024 03:41:49 +0000 Subject: [PATCH] proptest CpuidMap --- Cargo.lock | 72 +++++++++++++++++++++++ Cargo.toml | 1 + crates/cpuid-utils/Cargo.toml | 3 + crates/cpuid-utils/src/lib.rs | 107 +++++++++++++++++++++++++++++++++- 4 files changed, 180 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 22c1a53ae..04fd2b4c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -359,6 +359,21 @@ dependencies = [ "serde", ] +[[package]] +name = "bit-set" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0700ddab506f33b20a03b13996eccd309a48e5ff77d0d95926aa0210fb4e95f1" +dependencies = [ + "bit-vec", +] + +[[package]] +name = "bit-vec" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "349f9b6a179ed607305526ca489b34ad0a41aed5f7980fa90eb03160b69598fb" + [[package]] name = "bit_field" version = "0.10.2" @@ -760,6 +775,7 @@ dependencies = [ "bitflags 2.6.0", "propolis_api_types", "propolis_types", + "proptest", "thiserror", ] @@ -4456,6 +4472,26 @@ dependencies = [ "serde_test", ] +[[package]] +name = "proptest" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b4c2511913b88df1637da85cc8d96ec8e43a3f8bb8ccb71ee1ac240d6f3df58d" +dependencies = [ + "bit-set", + "bit-vec", + "bitflags 2.6.0", + "lazy_static", + "num-traits", + "rand", + "rand_chacha", + "rand_xorshift", + "regex-syntax 0.8.4", + "rusty-fork", + "tempfile", + "unarray", +] + [[package]] name = "quick-error" version = "1.2.3" @@ -4555,6 +4591,15 @@ dependencies = [ "getrandom", ] +[[package]] +name = "rand_xorshift" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d25bf25ec5ae4a3f1b92f929810509a2f53d7dca2f50b794ff57e3face536c8f" +dependencies = [ + "rand_core", +] + [[package]] name = "rayon" version = "1.10.0" @@ -5017,6 +5062,18 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "rusty-fork" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb3dcc6e454c328bb824492db107ab7c0ae8fcffe4ad210136ef014458c1bc4f" +dependencies = [ + "fnv", + "quick-error", + "tempfile", + "wait-timeout", +] + [[package]] name = "ryu" version = "1.0.18" @@ -6458,6 +6515,12 @@ version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed646292ffc8188ef8ea4d1e0e0150fb15a5c2e12ad9b8fc191ae7a8a7f3c4b9" +[[package]] +name = "unarray" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eaea85b334db583fe3274d12b4cd1880032beab409c0d774be044d4480ab9a94" + [[package]] name = "unicode-bidi" version = "0.3.15" @@ -6771,6 +6834,15 @@ dependencies = [ "utf8parse", ] +[[package]] +name = "wait-timeout" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f200f5b12eb75f8c1ed65abd4b2db8a6e1b138a20de009dacee265a2498f3f6" +dependencies = [ + "libc", +] + [[package]] name = "waitgroup" version = "0.1.2" diff --git a/Cargo.toml b/Cargo.toml index 16ba87acf..feb79ee40 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -133,6 +133,7 @@ pin-project-lite = "0.2.13" proc-macro2 = "1.0" proc-macro-error = "1" progenitor = "0.8.0" +proptest = "1.5.0" quote = "1.0" rand = "0.8" reqwest = { version = "0.12.0", default-features = false } diff --git a/crates/cpuid-utils/Cargo.toml b/crates/cpuid-utils/Cargo.toml index 4f73e9187..096eb5bbc 100644 --- a/crates/cpuid-utils/Cargo.toml +++ b/crates/cpuid-utils/Cargo.toml @@ -11,5 +11,8 @@ propolis_api_types = {workspace = true, optional = true} propolis_types.workspace = true thiserror.workspace = true +[dev-dependencies] +proptest.workspace = true + [features] instance-spec = ["propolis_api_types"] diff --git a/crates/cpuid-utils/src/lib.rs b/crates/cpuid-utils/src/lib.rs index 355feb953..a9f4ab39d 100644 --- a/crates/cpuid-utils/src/lib.rs +++ b/crates/cpuid-utils/src/lib.rs @@ -585,6 +585,8 @@ pub fn host_query_all() -> CpuidMap { #[cfg(test)] mod test { + use proptest::prelude::*; + use super::*; #[test] @@ -720,8 +722,107 @@ mod test { assert!(map.is_empty()); } - #[test] - fn map_iteration() { - todo!() + #[derive(Debug)] + enum MapEntry { + Leaf(u32, CpuidValues), + Subleaves(u32, Vec<(u32, CpuidValues)>), + } + + /// Produces a random CPUID leaf entry. Each entry has an leaf number in + /// [0..8) and one of the following values: + /// + /// - One of sixteen random [`CpuidValues`] values + /// - One subleaf with index [0..5) and one of two values + /// - Two such subleaves + /// - Three such subleaves + /// + /// There are (16 + 10 + 100 + 1000) = 1,126 possible values for each leaf, + /// for a total of 9,008 possible leaf entries. + fn map_entry_strategy() -> impl Strategy { + const MAX_LEAF: u32 = 8; + prop_oneof![ + (0..MAX_LEAF, prop::array::uniform4(0..2u32)).prop_map( + |(leaf, value_arr)| { MapEntry::Leaf(leaf, value_arr.into()) } + ), + ( + 0..MAX_LEAF, + prop::collection::vec(0..5u32, 1..=3), + proptest::bool::ANY + ) + .prop_map(|(leaf, subleaves, set_value)| { + let value = if set_value { + CpuidValues { eax: 1, ebx: 2, ecx: 3, edx: 4 } + } else { + CpuidValues::default() + }; + let subleaves = subleaves + .into_iter() + .map(|subleaf| (subleaf, value)) + .collect(); + MapEntry::Subleaves(leaf, subleaves) + }) + ] + } + + proptest! { + /// Verifies that a [`CpuidMapIterator`] visits all of the leaf and + /// subleaf entries in a map and does so in the expected order. + /// + /// proptest will generate a set of 3-8 leaves for each test according + /// to the strategy defined in [`map_entry_strategy`]. + #[test] + fn map_iteration_order( + entries in prop::collection::vec(map_entry_strategy(), 3..=8) + ) { + let mut map = CpuidMap::default(); + let mut _expected_len = 0; + + // Insert all of the entries into the map. The input array may have + // some duplicates and may assign both a no-subleaf and a subleaf + // value to a single leaf; ignore all of the resulting errors and + // substitutions. + for entry in entries { + match entry { + MapEntry::Leaf(leaf, values) => { + if let Ok(None) = map.insert( + CpuidIdent::leaf(leaf), + values + ) { + _expected_len += 1; + } + } + MapEntry::Subleaves(leaf, subleaves) => { + for (subleaf, values) in subleaves { + if let Ok(None) = map.insert( + CpuidIdent::subleaf(leaf, subleaf), + values + ) { + _expected_len += 1; + } + } + } + } + } + + assert_eq!(map.len(), _expected_len); + + // The iterator should visit leaves in order and return subleaves + // before the next leaf. This happens to be the ordering provided by + // `CpuidIdent`'s `Ord` implementation, so it suffices just to + // compare identifiers directly. + let mut _observed_len = 0; + let output: Vec<(CpuidIdent, CpuidValues)> = map.iter().collect(); + for (first, second) in + output.as_slice().windows(2).map(|sl| (sl[0].0, sl[1].0)) { + assert!(first < second, "first: {first:?}, second: {second:?}"); + _observed_len += 1; + } + + // The `windows(2)` iterator will not count the last entry (it has + // no successor), so the actual observed length is one more than the + // number of observed iterations. (Note that by construction the map + // is not empty, so there is always a last entry.) + assert_eq!(_observed_len + 1, _expected_len); + } } }