Get test suite ready for miri

This commit brings the test suite into a state where "cargo miri test
--features backend-mmap" reports all tests as passing. Some tests have
been marked with `#[cfg(not(miri))]` due to exercising operations that
miri does not support, and which cannot be reasonably mocked out (such
as creating a file-backed mmap).

Calls to `mmap` with `MAP_ANONYMOUS` have been mocked out with calls to
`std::allow::alloc_zeroed`, due to miri not supporting `mmap`.

Some tests ran into alignment issues that are not present on production
systems, presumable because the system allocator always aligns
sufficiently. Miri will not do that, and happily align everything to a
1-byte boundary. However, miri only exercises a _single_ execution path,
so it is possible that on that single execution path, everything
happened to be aligned sufficiently for a test to pass. That is why the
adjustments made in this commit might not be sufficient.

Signed-off-by: Patrick Roy <[email protected]>

Author: roypat

src/bytes.rs

@@ -26,7 +26,9 @@ use crate::volatile_memory::VolatileSlice;
 ///
 /// A type `T` is `ByteValued` if and only if it can be initialized by reading its contents from a
 /// byte array.  This is generally true for all plain-old-data structs.  It is notably not true for
-/// any type that includes a reference.
+/// any type that includes a reference. It is generally also not safe for non-packed structs, as
+/// compiler-inserted padding is considered uninitialized memory, and thus reads/writing it will
+/// cause undefined behavior.
 ///
 /// Implementing this trait guarantees that it is safe to instantiate the struct with random data.
 pub unsafe trait ByteValued: Copy + Default + Send + Sync {

src/guest_memory.rs

@@ -1057,6 +1057,7 @@ mod tests {
     // has the same value (and thus it did not do a non-atomic access). The test succeeds if
     // no mismatch is detected after performing accesses for a pre-determined amount of time.
     #[cfg(feature = "backend-mmap")]
+    #[cfg(not(miri))] // This test simulates a race condition between guest and vmm
     fn non_atomic_access_helper<T>()
     where
         T: ByteValued
@@ -1074,13 +1075,16 @@ mod tests {
         #[derive(Clone, Copy, Debug, Default, PartialEq)]
         struct Data<T> {
             val: T,
-            some_bytes: [u8; 7],
+            some_bytes: [u8; 8],
         }
 
         // Some sanity checks.
         assert_eq!(mem::align_of::<T>(), mem::align_of::<Data<T>>());
         assert_eq!(mem::size_of::<T>(), mem::align_of::<T>());
 
+        // There must be no padding bytes, as otherwise implementing ByteValued is UB
+        assert_eq!(mem::size_of::<Data<T>>(), mem::size_of::<T>() + 8);
+
         unsafe impl<T: ByteValued> ByteValued for Data<T> {}
 
         // Start of first guest memory region.
@@ -1099,7 +1103,7 @@ mod tests {
         // Need to clone this and move it into the new thread we create.
         let mem2 = mem.clone();
         // Just some bytes.
-        let some_bytes = [1u8, 2, 4, 16, 32, 64, 128];
+        let some_bytes = [1u8, 2, 4, 16, 32, 64, 128, 255];
 
         let mut data = Data {
             val: T::from(0u8),
@@ -1146,6 +1150,7 @@ mod tests {
 
     #[cfg(feature = "backend-mmap")]
     #[test]
+    #[cfg(not(miri))]
     fn test_non_atomic_access() {
         non_atomic_access_helper::<u16>()
     }

src/mmap.rs

@@ -892,6 +892,7 @@ mod tests {
     }
 
     #[test]
+    #[cfg(not(miri))] // Miri cannot mmap files
     fn mapped_file_read() {
         let mut f = TempFile::new().unwrap().into_file();
         let sample_buf = &[1, 2, 3, 4, 5];

src/mmap_unix.rs

@@ -142,6 +142,7 @@ impl<B: Bitmap> MmapRegionBuilder<B> {
             (-1, 0)
         };
 
+        #[cfg(not(miri))]
         // SAFETY: This is safe because we're not allowing MAP_FIXED, and invalid parameters
         // cannot break Rust safety guarantees (things may change if we're mapping /dev/mem or
         // some wacky file).
@@ -156,10 +157,22 @@ impl<B: Bitmap> MmapRegionBuilder<B> {
             )
         };
 
+        #[cfg(not(miri))]
         if addr == libc::MAP_FAILED {
             return Err(Error::Mmap(io::Error::last_os_error()));
         }
 
+        #[cfg(miri)]
+        if self.size == 0 {
+            return Err(Error::Mmap(io::Error::from_raw_os_error(libc::EINVAL)));
+        }
+
+        // Miri does not support the mmap syscall, so we use rust's allocator for miri tests
+        #[cfg(miri)]
+        let addr = unsafe {
+            std::alloc::alloc_zeroed(std::alloc::Layout::from_size_align(self.size, 8).unwrap())
+        };
+
         Ok(MmapRegion {
             addr: addr as *mut u8,
             size: self.size,
@@ -412,7 +425,14 @@ impl<B> Drop for MmapRegion<B> {
             // SAFETY: This is safe because we mmap the area at addr ourselves, and nobody
             // else is holding a reference to it.
             unsafe {
+                #[cfg(not(miri))]
                 libc::munmap(self.addr as *mut libc::c_void, self.size);
+
+                #[cfg(miri)]
+                std::alloc::dealloc(
+                    self.addr,
+                    std::alloc::Layout::from_size_align(self.size, 8).unwrap(),
+                );
             }
         }
     }
@@ -499,6 +519,7 @@ mod tests {
     }
 
     #[test]
+    #[cfg(not(miri))] // Miri cannot mmap files
     fn test_mmap_region_from_file() {
         let mut f = TempFile::new().unwrap().into_file();
         let offset: usize = 0;
@@ -517,6 +538,7 @@ mod tests {
     }
 
     #[test]
+    #[cfg(not(miri))] // Miri cannot mmap files
     fn test_mmap_region_build() {
         let a = Arc::new(TempFile::new().unwrap().into_file());
 
@@ -587,6 +609,7 @@ mod tests {
     }
 
     #[test]
+    #[cfg(not(miri))] // Causes warnings due to the pointer casts
     fn test_mmap_region_build_raw() {
         let addr = 0;
         let size = unsafe { libc::sysconf(libc::_SC_PAGESIZE) as usize };
@@ -605,6 +628,7 @@ mod tests {
     }
 
     #[test]
+    #[cfg(not(miri))] // Miri cannot mmap files
     fn test_mmap_region_fds_overlap() {
         let a = Arc::new(TempFile::new().unwrap().into_file());
         assert_eq!(unsafe { libc::ftruncate(a.as_raw_fd(), 1024 * 10) }, 0);