test-clock

.semgrep/rules/no-clock-read-in-drop.yaml

@@ -0,0 +1,27 @@
+rules:
+  - id: rust-no-clock-read-in-drop
+    languages: [rust]
+    severity: ERROR
+    message: |
+      Do not read the clock from a `Drop` implementation.
+
+      Once anything in the process has paused the virtual clock, tokio routes
+      every read through the calling thread's runtime context.  A `Drop` that
+      runs during thread-local teardown may find that context already
+      destroyed, and tokio's response is a panic inside a destructor -- which
+      aborts the process rather than failing the test.
+
+      Take the reading before the value is dropped and pass it in, or record
+      the instant when the value is created.
+    paths:
+      exclude:
+        - .codeql/tests/
+        - clock/src/
+    patterns:
+      - pattern-inside: |
+          fn drop(&mut self) {
+            ...
+          }
+      - pattern-either:
+          - pattern: clock::now()
+          - pattern: clock::system_now()

Cargo.toml

@@ -256,7 +256,7 @@ overflow-checks = false
 codegen-units = 1
 rpath = true
 
-[profile.fuzz]
+[profile.checked]
 inherits = "release"
 opt-level = 2
 debug-assertions = true

acl/tests/property_predicate.rs

@@ -211,100 +211,102 @@ where
 }
 const MIN_ASSERTED_HITS: u64 = 20;
 const MIN_ASSERTED_MISSES: u64 = 20;
-fn run_property<A, T>(
-    name_prefix: &str,
-    install_dpdk: impl Fn(String, &FiveTupleRule<A>) -> T + core::panic::RefUnwindSafe,
-) where
-    A: KeyAddr,
-    PrefixSpec<A>: FieldHit<A> + FieldMiss<A> + IsUniversal,
-    T: Lookup<FiveTuple<A>, Verdict>,
-    RawRule<A>: TypeGenerator,
-{
-    let asserted_hits = AtomicU64::new(0);
-    let asserted_misses = AtomicU64::new(0);
+macro_rules! run_property {
+    ($a:ty, $name_prefix:expr, $install_dpdk:expr) => {{
+        let asserted_hits = AtomicU64::new(0);
+        let asserted_misses = AtomicU64::new(0);
 
-    bolero::check!()
-        .with_type::<(RawRule<A>, Box<[u8]>, Box<[u8]>)>()
-        .for_each(|(raw, hit_bytes, miss_bytes)| {
-            let rule = build_rule(raw);
-            let dpdk = install_dpdk(unique_name(name_prefix), &rule);
-            let reference = ReferenceTable::<FiveTuple<A>, Verdict>::new(vec![RefRule::new(
-                rule.into_backend_fields::<Erased>(),
-                Verdict::Drop,
-            )]);
-
-            let hits = HitsGen { rule };
-            let n_hits = sweep(&hits, hit_bytes, |k| {
-                assert!(rule.accepts(k), "hits gen produced a rejected key: {k:?}");
-                assert_eq!(reference.lookup(k), Some(&Verdict::Drop));
-                assert_eq!(dpdk.lookup(k), Some(&Verdict::Drop));
-            });
-            asserted_hits.fetch_add(n_hits, Ordering::Relaxed);
+        bolero::check!()
+            .with_type::<(RawRule<$a>, Box<[u8]>, Box<[u8]>)>()
+            .for_each(|(raw, hit_bytes, miss_bytes)| {
+                let rule = build_rule(raw);
+                let dpdk = $install_dpdk(unique_name($name_prefix), &rule);
+                let reference = ReferenceTable::<FiveTuple<$a>, Verdict>::new(vec![RefRule::new(
+                    rule.into_backend_fields::<Erased>(),
+                    Verdict::Drop,
+                )]);
 
-            if !rule.is_universal() {
-                let misses = MissesGen { rule };
-                let n_misses = sweep(&misses, miss_bytes, |k| {
-                    assert!(
-                        !rule.accepts(k),
-                        "misses gen produced an accepted key: {k:?}",
-                    );
-                    assert_eq!(reference.lookup(k), None);
-                    assert_eq!(dpdk.lookup(k), None);
+                let hits = HitsGen { rule };
+                let n_hits = sweep(&hits, hit_bytes, |k| {
+                    assert!(rule.accepts(k), "hits gen produced a rejected key: {k:?}");
+                    assert_eq!(reference.lookup(k), Some(&Verdict::Drop));
+                    assert_eq!(dpdk.lookup(k), Some(&Verdict::Drop));
                 });
-                asserted_misses.fetch_add(n_misses, Ordering::Relaxed);
-            }
-        });
+                asserted_hits.fetch_add(n_hits, Ordering::Relaxed);
 
-    let h = asserted_hits.load(Ordering::Relaxed);
-    let m = asserted_misses.load(Ordering::Relaxed);
-    assert!(
-        h >= MIN_ASSERTED_HITS,
-        "asserted only {h} hits (< {MIN_ASSERTED_HITS}); generator may have gone inert",
-    );
-    assert!(
-        m >= MIN_ASSERTED_MISSES,
-        "asserted only {m} misses (< {MIN_ASSERTED_MISSES}); generator may have gone inert",
-    );
+                if !rule.is_universal() {
+                    let misses = MissesGen { rule };
+                    let n_misses = sweep(&misses, miss_bytes, |k| {
+                        assert!(
+                            !rule.accepts(k),
+                            "misses gen produced an accepted key: {k:?}",
+                        );
+                        assert_eq!(reference.lookup(k), None);
+                        assert_eq!(dpdk.lookup(k), None);
+                    });
+                    asserted_misses.fetch_add(n_misses, Ordering::Relaxed);
+                }
+            });
+
+        let h = asserted_hits.load(Ordering::Relaxed);
+        let m = asserted_misses.load(Ordering::Relaxed);
+        assert!(
+            h >= MIN_ASSERTED_HITS,
+            "asserted only {h} hits (< {MIN_ASSERTED_HITS}); generator may have gone inert",
+        );
+        assert!(
+            m >= MIN_ASSERTED_MISSES,
+            "asserted only {m} misses (< {MIN_ASSERTED_MISSES}); generator may have gone inert",
+        );
+    }};
 }
 
 #[test]
 #[dpdk::with_eal]
 fn property_v4() {
-    run_property::<Ipv4Addr, FiveTupleTableV4<Verdict>>("prop_v4", |name, rule| {
-        install_table(
-            &name,
-            NonZero::new(2).expect("nonzero"),
-            vec![
-                RuleSpec::<FiveTuple<Ipv4Addr>, Verdict>::new(
-                    Priority::new(1).expect("nonzero priority"),
-                    CategoryMask::new(1).expect("nonzero mask"),
-                    rule.into_backend_fields::<Dpdk>(),
-                    Verdict::Drop,
-                )
-                .expect("RuleSpec"),
-            ],
-        )
-        .expect("install_table")
-    });
+    run_property!(
+        Ipv4Addr,
+        "prop_v4",
+        |name: String, rule: &FiveTupleRule<Ipv4Addr>| {
+            install_table(
+                &name,
+                NonZero::new(2).expect("nonzero"),
+                vec![
+                    RuleSpec::<FiveTuple<Ipv4Addr>, Verdict>::new(
+                        Priority::new(1).expect("nonzero priority"),
+                        CategoryMask::new(1).expect("nonzero mask"),
+                        rule.into_backend_fields::<Dpdk>(),
+                        Verdict::Drop,
+                    )
+                    .expect("RuleSpec"),
+                ],
+            )
+            .expect("install_table")
+        }
+    );
 }
 
 #[test]
 #[dpdk::with_eal]
 fn property_v6() {
-    run_property::<Ipv6Addr, FiveTupleTableV6<Verdict>>("prop_v6", |name, rule| {
-        install_table(
-            &name,
-            NonZero::new(2).expect("nonzero"),
-            vec![
-                RuleSpec::<FiveTuple<Ipv6Addr>, Verdict>::new(
-                    Priority::new(1).expect("nonzero priority"),
-                    CategoryMask::new(1).expect("nonzero mask"),
-                    rule.into_backend_fields::<Dpdk>(),
-                    Verdict::Drop,
-                )
-                .expect("RuleSpec"),
-            ],
-        )
-        .expect("install_table")
-    });
+    run_property!(
+        Ipv6Addr,
+        "prop_v6",
+        |name: String, rule: &FiveTupleRule<Ipv6Addr>| {
+            install_table(
+                &name,
+                NonZero::new(2).expect("nonzero"),
+                vec![
+                    RuleSpec::<FiveTuple<Ipv6Addr>, Verdict>::new(
+                        Priority::new(1).expect("nonzero priority"),
+                        CategoryMask::new(1).expect("nonzero mask"),
+                        rule.into_backend_fields::<Dpdk>(),
+                        Verdict::Drop,
+                    )
+                    .expect("RuleSpec"),
+                ],
+            )
+            .expect("install_table")
+        }
+    );
 }

clock/Cargo.toml

@@ -11,3 +11,9 @@ virtual = ["dep:tokio"]
 
 [dependencies]
 tokio = { workspace = true, optional = true, features = ["test-util", "time"] }
+
+[dev-dependencies]
+concurrency = { workspace = true }
+
+[lints.rust]
+unexpected_cfgs = { level = "warn", check-cfg = ['cfg(wall_clock)'] }

clock/build.rs

@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright Open Network Fabric Authors
+
+use std::process::Command;
+use std::{env, fs, path::PathBuf};
+
+fn main() {
+    println!("cargo::rerun-if-env-changed=RUSTC_BOOTSTRAP");
+    println!("cargo::rustc-check-cfg=cfg(has_spawn_hook)");
+
+    let out = PathBuf::from(env::var_os("OUT_DIR").expect("cargo sets OUT_DIR"));
+    let probe = out.join("spawn_hook_probe.rs");
+    if fs::write(
+        &probe,
+        "#![feature(thread_spawn_hook)]\n\
+         pub fn probe() { std::thread::add_spawn_hook(|_| || {}); }\n",
+    )
+    .is_err()
+    {
+        return;
+    }
+
+    let rustc = env::var_os("RUSTC").unwrap_or_else(|| "rustc".into());
+    let accepted = Command::new(rustc)
+        .args(["--crate-type=lib", "--emit=metadata", "-o"])
+        .arg(out.join("spawn_hook_probe.rmeta"))
+        .arg(&probe)
+        .status()
+        .is_ok_and(|status| status.success());
+
+    if accepted {
+        println!("cargo::rustc-cfg=has_spawn_hook");
+    } else {
+        println!(
+            "cargo::warning=thread_spawn_hook is unavailable, so a test that drives the clock \
+             cannot check the threads it spawns. Set RUSTC_BOOTSTRAP=1 (the dev shell does)."
+        );
+    }
+}

clock/src/lib.rs

@@ -1,42 +1,91 @@
 // SPDX-License-Identifier: Apache-2.0
 // Copyright Open Network Fabric Authors
 
+#![cfg_attr(
+    all(has_spawn_hook, feature = "virtual", not(wall_clock)),
+    feature(thread_spawn_hook)
+)]
 #![deny(clippy::all, clippy::pedantic)]
 #![deny(rustdoc::all)]
 #![deny(unsafe_code)]
 
 pub use std::time::{Duration, Instant, SystemTime, SystemTimeError, TryFromFloatSecsError};
 
+#[cfg(feature = "virtual")]
+pub mod virtual_time;
+
 #[must_use]
 pub fn now() -> Instant {
-    #[cfg(feature = "virtual")]
+    #[cfg(all(feature = "virtual", not(wall_clock)))]
     {
-        tokio::time::Instant::now().into_std()
+        checked_now().unwrap_or_else(|| virtual_time::refuse())
     }
-    #[cfg(not(feature = "virtual"))]
+    #[cfg(not(all(feature = "virtual", not(wall_clock))))]
     {
         Instant::now()
     }
 }
 
+#[must_use]
+pub fn checked_now() -> Option<Instant> {
+    #[cfg(all(feature = "virtual", not(wall_clock)))]
+    {
+        if virtual_time::armed() && tokio::runtime::Handle::try_current().is_err() {
+            return None;
+        }
+        Some(tokio::time::Instant::now().into_std())
+    }
+    #[cfg(not(all(feature = "virtual", not(wall_clock))))]
+    {
+        Some(Instant::now())
+    }
+}
+
+#[must_use]
+pub const fn is_routed() -> bool {
+    cfg!(all(feature = "virtual", not(wall_clock)))
+}
+
+#[must_use]
+pub fn elapsed_since_first_reading() -> Option<(bool, Duration)> {
+    // nosemgrep: rust-no-direct-std-sync-import
+    static ORIGIN: std::sync::OnceLock<Instant> = std::sync::OnceLock::new();
+    let reading = checked_now()?;
+    let origin = *ORIGIN.get_or_init(|| reading);
+    Some(if reading >= origin {
+        (false, reading.saturating_duration_since(origin))
+    } else {
+        (true, origin.saturating_duration_since(reading))
+    })
+}
+
 #[must_use]
 pub fn system_now() -> SystemTime {
     SystemTime::now()
 }
 
+#[cfg(test)]
+pub(crate) fn serially() -> concurrency::sync::MutexGuard<'static, ()> {
+    static SERIAL: concurrency::sync::Mutex<()> = concurrency::sync::Mutex::new(());
+    SERIAL.lock()
+}
+
 #[cfg(test)]
 mod tests {
+    use super::serially;
     use super::{Duration, now, system_now};
 
     #[test]
     fn now_is_monotonic() {
+        let _serial = serially();
         let first = now();
         let second = now();
         assert!(second >= first, "the monotonic clock went backwards");
     }
 
     #[test]
     fn now_works_with_no_runtime() {
+        let _serial = serially();
         let _ = now();
         let _ = system_now();
     }