Fix mmap soundness bug (not user-visible) (#199)

It was previously possible to create an uninitialized slice by
calling `Mmap::new(0).as_slice()`.

The `Mmap` type isn't exported to users, but it's the principle of the
thing!

Also, none of our unit tests actually wrote more than 4K to a single
`Mmap`, so I added a unit test to do that.

CHANGELOG.md

@@ -35,6 +35,7 @@
   the `GenericVmFunction<N>` type implemented both `Tape` and `Function`.
 - Add `vars()` to `Function` trait, because there are cases where we want to get
   the variable map without building a tape (and it must always be the same).
+- Fix soundness bug in `Mmap` (probably not user-visible)
 
 # 0.3.3
 - `Function` and evaluator types now produce multiple outputs

fidget/src/jit/mmap.rs

@@ -5,8 +5,16 @@ use windows::Win32::System::Memory::{
 };
 
 pub struct Mmap {
+    /// Pointer to a memory-mapped region, which may be uninitialized
     ptr: *mut std::ffi::c_void,
+
+    /// Total length of the region
     len: usize,
+
+    /// Number of bytes that have been initialized
+    ///
+    /// This value is conservative and assumes that bytes are written in order
+    written: usize,
 }
 
 // SAFETY: this is philosophically a `Vec<u8>`, so can be sent to other threads
@@ -23,6 +31,7 @@ impl Mmap {
         Self {
             ptr: std::ptr::null_mut::<std::ffi::c_void>(),
             len: 0,
+            written: 0,
         }
     }
 
@@ -48,7 +57,11 @@ impl Mmap {
         if ptr == libc::MAP_FAILED {
             Err(std::io::Error::last_os_error())
         } else {
-            Ok(Self { ptr, len })
+            Ok(Self {
+                ptr,
+                len,
+                written: 0,
+            })
         }
     }
 
@@ -68,18 +81,38 @@ impl Mmap {
         if ptr.is_null() {
             Err(std::io::Error::last_os_error())
         } else {
-            Ok(Self { ptr, len })
+            Ok(Self {
+                ptr,
+                len,
+                written: 0,
+            })
         }
     }
 
+    /// Returns the size of the allocation
     #[inline(always)]
     pub fn len(&self) -> usize {
         self.len
     }
 
+    /// Returns the number of bytes written
+    #[inline(always)]
+    pub fn written(&self) -> usize {
+        self.written
+    }
+
+    /// Sets our `written` length
+    ///
+    /// All bytes from `0..written` must be initialized when this is called
+    pub unsafe fn set_written(&mut self, written: usize) {
+        self.written = written;
+    }
+
     #[inline(always)]
     pub fn as_mut_slice(&mut self) -> &mut [u8] {
-        unsafe { std::slice::from_raw_parts_mut(self.ptr as *mut u8, self.len) }
+        unsafe {
+            std::slice::from_raw_parts_mut(self.ptr as *mut u8, self.written)
+        }
     }
 
     /// Writes to the given offset in the memory map
@@ -92,18 +125,28 @@ impl Mmap {
         unsafe {
             *(self.ptr as *mut u8).add(index) = byte;
         }
+        if index == self.written {
+            self.written += 1;
+        }
     }
 
     /// Treats the memory-mapped data as a slice
     #[inline(always)]
     pub fn as_slice(&self) -> &[u8] {
-        unsafe { std::slice::from_raw_parts(self.ptr as *const u8, self.len) }
+        unsafe {
+            std::slice::from_raw_parts(self.ptr as *const u8, self.written)
+        }
     }
 
     /// Returns the inner pointer
     pub fn as_ptr(&self) -> *const std::ffi::c_void {
         self.ptr
     }
+
+    /// Returns the inner pointer (mutably)
+    pub fn as_mut_ptr(&mut self) -> *mut std::ffi::c_void {
+        self.ptr
+    }
 }
 
 #[cfg(any(target_os = "linux", target_os = "windows"))]

fidget/src/jit/mod.rs

@@ -628,7 +628,14 @@ impl MmapAssembler {
     /// Doubles the size of the internal `Mmap` and copies over data
     fn expand_mmap(&mut self) {
         let mut next = Mmap::new(self.mmap.len() * 2).unwrap();
-        next.as_mut_slice()[0..self.len].copy_from_slice(self.mmap.as_slice());
+        unsafe {
+            std::ptr::copy_nonoverlapping(
+                self.mmap.as_ptr(),
+                next.as_mut_ptr(),
+                self.mmap.written(),
+            );
+            next.set_written(self.mmap.written());
+        }
         std::mem::swap(&mut self.mmap, &mut next);
     }
 }
@@ -653,7 +660,7 @@ fn build_asm_fn_with_storage<A: Assembler>(
     mut s: Mmap,
 ) -> Mmap {
     // This guard may be a unit value on some systems
-    #[cfg(target_os = "macos")]
+    #[cfg(target_os = "macos")] // XXX use #[expect(unused)] here?
     let _guard = Mmap::thread_mode_write();
 
     let size_estimate = t.len() * A::bytes_per_clause();
@@ -1329,4 +1336,26 @@ mod test {
     crate::interval_tests!(JitFunction);
     crate::float_slice_tests!(JitFunction);
     crate::point_tests!(JitFunction);
+
+    #[test]
+    fn test_mmap_expansion() {
+        let mmap = Mmap::new(0).unwrap();
+
+        let mut asm = MmapAssembler::from(mmap);
+        const COUNT: u32 = 23456; // larger than 1 page (4 KiB)
+
+        #[cfg(target_os = "macos")] // XXX use #[expect(unused)] here?
+        let _guard = Mmap::thread_mode_write();
+
+        for i in 0..COUNT {
+            asm.push_u32(i);
+        }
+        let mmap = asm.finalize().unwrap();
+        let slice = mmap.as_slice();
+        assert_eq!(slice.len(), COUNT as usize * 4);
+        for (i, c) in slice.chunks_exact(4).enumerate() {
+            let b = u32::from_le_bytes(c.try_into().unwrap());
+            assert_eq!(i as u32, b);
+        }
+    }
 }