Merge pull request #2401 from sporksmith/miri

Run tests under Miri in CI

Author: sporksmith

.github/workflows/sanitizers.yml

@@ -0,0 +1,43 @@
+# Syntax reference:
+# https://help.github.com/en/actions/automating-your-workflow-with-github-actions/workflow-syntax-for-github-actions
+
+name: Rust Sanitizers
+permissions: read-all
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    types: [opened, synchronize]
+
+
+jobs:
+  miri:
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v3
+      with:
+        persist-credentials: false
+        # Run on PR head instead of merge result. Running on the merge
+        # result can give confusing results, and we require PR to be up to
+        # date with target branch before merging, anyway.
+        # See https://github.com/shadow/shadow/issues/2166
+        ref: ${{ github.event.pull_request.head.sha }}
+
+    # From https://github.com/rust-lang/miri#running-miri-on-ci
+    - name: Install miri
+      run: |
+          rustup toolchain install nightly --component miri
+          rustup override set nightly
+          cargo miri setup
+
+    - name: Test
+      run: |
+        cd src
+        # Disable isolation for some tests that use the current time (Instant::now).
+        # 
+        # Disable leak-checking for now. Some tests intentionally panic, causing leaks.
+        export MIRIFLAGS="-Zmiri-disable-isolation -Zmiri-ignore-leaks"
+
+        cargo miri test --workspace

src/lib/syscall-logger/src/lib.rs

@@ -1,27 +1,44 @@
 use proc_macro::*;
 use quote::ToTokens;
 
-/// This macro wraps a syscall handler function by renaming the original function and making a new
+// FIXME: These doctests are effectively disabled via `compile_fail`.
+// For them to work, this crate would need to depend on shadow_rs. We can't do that though,
+// since shadow_rs depends on this crate. This could perhaps be fixed by moving the parts
+// of shadow_rs neede by this crate out to a separate crate to break the cycle.
+/// This macro wraps a syscall handler by renaming the original function and making a new
 /// function with the original name that calls the original function. When the syscall handler
 /// function is called, it will log the syscall if syscall logging is enabled in Shadow.
 ///
 /// For example,
 ///
-/// ```
-/// #[log_syscall(/* rv */ libc::c_int, /* fd */ libc::c_int)]
-/// pub fn close(ctx: &mut ThreadContext, args: &SysCallArgs) -> SyscallResult {}
+/// ```compile_fail
+/// # use syscall_logger::log_syscall;
+/// # use shadow_rs::host::context::ThreadContext;
+/// # use shadow_rs::host::syscall_types::{SysCallArgs, SyscallResult};
+/// struct MyHandler {}
+///
+/// impl MyHandler {
+///     #[log_syscall(/* rv */ libc::c_int, /* fd */ libc::c_int)]
+///     pub fn close(ctx: &mut ThreadContext, args: &SysCallArgs) -> SyscallResult {}
+/// }
 /// ```
 ///
 /// will become,
 ///
-/// ```
-/// pub fn close(ctx: &mut ThreadContext, args: &SysCallArgs) -> SyscallResult {
-///     ...
-///     let rv = close_original(ctx, args);
-///     ...
-///     rv
-/// }
-/// fn close_original(ctx: &mut ThreadContext, args: &SysCallArgs) -> SyscallResult {
+/// ```compile_fail
+/// # use shadow_rs::host::context::ThreadContext;
+/// # use shadow_rs::host::syscall_types::{SysCallArgs, SyscallResult};
+/// struct MyHandler {}
+///
+/// impl MyHandler {
+///     pub fn close(ctx: &mut ThreadContext, args: &SysCallArgs) -> SyscallResult {
+///         // ...
+///         let rv = close_original(ctx, args);
+///         // ...
+///         rv
+///     }
+///     fn close_original(ctx: &mut ThreadContext, args: &SysCallArgs) -> SyscallResult {
+///     }
 /// }
 /// ```
 #[proc_macro_attribute]

src/main/core/support/configuration.rs

@@ -1089,6 +1089,8 @@ mod tests {
     use super::*;
 
     #[test]
+    // can't call foreign function: process_parseArgStr
+    #[cfg_attr(miri, ignore)]
     fn test_parse_args() {
         let arg_str = r#"the quick brown fox "jumped over" the "\"lazy\" dog""#;
         let expected_args = &[
@@ -1108,6 +1110,8 @@ mod tests {
     }
 
     #[test]
+    // can't call foreign function: process_parseArgStr
+    #[cfg_attr(miri, ignore)]
     fn test_parse_args_empty() {
         let arg_str = "";
         let expected_args: &[&str] = &[];
@@ -1119,6 +1123,8 @@ mod tests {
     }
 
     #[test]
+    // can't call foreign function: process_parseArgStr
+    #[cfg_attr(miri, ignore)]
     fn test_parse_args_error() {
         let arg_str = r#"hello "world"#;
 
@@ -1129,6 +1135,8 @@ mod tests {
     }
 
     #[test]
+    // can't call foreign function: process_parseArgStr
+    #[cfg_attr(miri, ignore)]
     fn test_nullable_option() {
         // format the yaml with an optional general option
         let yaml_fmt_fn = |option| {

src/main/host/descriptor/mod.rs

@@ -996,6 +996,8 @@ mod tests {
     use crate::host::syscall_types::{Blocked, Failed, SyscallError};
 
     #[test]
+    // can't call foreign function: syscallcondition_new
+    #[cfg_attr(miri, ignore)]
     fn test_syscallresult_roundtrip() {
         for val in vec![
             Ok(1.into()),
@@ -1032,6 +1034,8 @@ mod tests {
     }
 
     #[test]
+    // can't call foreign function: syscallcondition_new
+    #[cfg_attr(miri, ignore)]
     fn test_syscallreturn_roundtrip() {
         let condition = SysCallCondition::new(Trigger::from(c::Trigger {
             type_: 1,

src/main/host/syscall/format.rs

@@ -614,6 +614,8 @@ mod test {
     use std::process::Command;
 
     #[test]
+    // can't call foreign function: gnu_get_libc_version
+    #[cfg_attr(miri, ignore)]
     fn test_no_args() {
         let args = SysCallArgs {
             number: 100,

src/main/utility/childpid_watcher.rs

@@ -359,6 +359,8 @@ mod tests {
     }
 
     #[test]
+    // can't call foreign function: pipe
+    #[cfg_attr(miri, ignore)]
     fn register_before_exit() {
         let notifier = nix::sys::eventfd::eventfd(0, nix::sys::eventfd::EfdFlags::empty()).unwrap();
 
@@ -412,6 +414,8 @@ mod tests {
     }
 
     #[test]
+    // can't call foreign function: pipe
+    #[cfg_attr(miri, ignore)]
     fn register_after_exit() {
         let (read_fd, write_fd) = nix::unistd::pipe().unwrap();
         let child = match unsafe { nix::unistd::fork() }.unwrap() {
@@ -463,6 +467,8 @@ mod tests {
     }
 
     #[test]
+    // can't call foreign function: pipe
+    #[cfg_attr(miri, ignore)]
     fn register_multiple() {
         let cb1_ran = Arc::new((Mutex::new(false), Condvar::new()));
         let cb2_ran = Arc::new((Mutex::new(false), Condvar::new()));
@@ -502,6 +508,8 @@ mod tests {
     }
 
     #[test]
+    // can't call foreign function: pipe
+    #[cfg_attr(miri, ignore)]
     fn unregister_one() {
         let cb1_ran = Arc::new((Mutex::new(false), Condvar::new()));
         let cb2_ran = Arc::new((Mutex::new(false), Condvar::new()));

src/main/utility/interval_map.rs

@@ -346,6 +346,8 @@ mod tests {
     }
 
     #[test]
+    // Too slow for miri
+    #[cfg_attr(miri, ignore)]
     fn test_insert_random() {
         use rand::Rng;
         use rand_core::SeedableRng;
@@ -576,6 +578,8 @@ mod tests {
     }
 
     #[test]
+    // Too slow for miri
+    #[cfg_attr(miri, ignore)]
     fn test_clear_random() {
         use rand::Rng;
         use rand_core::SeedableRng;

src/main/utility/proc_maps.rs

@@ -515,6 +515,9 @@ mod tests {
     }
 
     #[test]
+    // Hangs in miri. Not sure why, but also not surpring in general that this
+    // test would be incompatible.
+    #[cfg_attr(miri, ignore)]
     fn test_mappings_for_pid() {
         // Difficult to write a precise test here; just try to read our own mappings and validate
         // that it parses and is non-empty.

src/test/test_utils.rs

@@ -247,6 +247,7 @@ where
 /// Similar to the `vec!` macro, `set!` will create a `HashSet` with the given elements.
 ///
 /// ```
+/// # use test_utils::*;
 /// let s = set![1, 2, 3, 1];
 /// assert_eq!(s.len(), 3);
 /// ```
@@ -357,15 +358,21 @@ pub fn nop_sig_handler() -> nix::sys::signal::SigHandler {
 /// Example:
 ///
 /// ```
-/// let x = 2;
-/// let y = 3;
-/// ensure_ord!(x, >, y);
-/// ```
+/// # use test_utils::*;
+/// # use anyhow::anyhow;
+/// fn fn1() -> Result<(), anyhow::Error> {
+///     let x = 2;
+///     let y = 3;
+///     ensure_ord!(x, >, y);
+///     Ok(())
+/// }
 ///
-/// Will evaluate to the same error as:
+/// fn fn2() -> Result<(), anyhow::Error> {
+///     return Err(anyhow!("!(2 > 3)"));
+/// }
 ///
-/// ```
-/// anyhow!("!(3 > 2)");
+/// assert_eq!(format!("{}", fn1().unwrap_err()),
+///            format!("{}", fn2().unwrap_err()));
 /// ```
 #[macro_export]
 macro_rules! ensure_ord {