Auto merge of #3625 - Strophox:miri-allocation-fix, r=RalfJung

Bugfix `MiriAllocBytes` to guarantee different addresses Fix in `alloc_bytes.rs` following rust-lang/miri#3526 Currently when an allocation of `size == 0` is requested we return a `std::ptr::without_provenance_mut(align)`, but this means returned `ptr`s may overlap, which breaks things.
rust-lang · May 24, 2024 · 7fc41d1 · 7fc41d1
2 parents 88d519f b84620f
commit 7fc41d1
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 23 deletions.
diff --git a/src/tools/miri/src/alloc_bytes.rs b/src/tools/miri/src/alloc_bytes.rs
@@ -14,8  14,7 @@ pub struct MiriAllocBytes {
     layout: alloc::Layout,
     /// Pointer to the allocation contents.
     /// Invariant:
-    /// * If `self.layout.size() == 0`, then `self.ptr` is some suitably aligned pointer
-    ///   without provenance (and no actual memory was allocated).
     /// * If `self.layout.size() == 0`, then `self.ptr` was allocated with the equivalent layout with size 1.
     /// * Otherwise, `self.ptr` points to memory allocated with `self.layout`.
     ptr: *mut u8,
 }
@@ -30,10  29,15 @@ impl Clone for MiriAllocBytes {
 
 impl Drop for MiriAllocBytes {
     fn drop(&mut self) {
-        if self.layout.size() != 0 {
-            // SAFETY: Invariant, `self.ptr` points to memory allocated with `self.layout`.
-            unsafe { alloc::dealloc(self.ptr, self.layout) }
-        }
         // We have to reconstruct the actual layout used for allocation.
         // (`Deref` relies on `size` so we can't just always set it to at least 1.)
         let alloc_layout = if self.layout.size() == 0 {
             Layout::from_size_align(1, self.layout.align()).unwrap()
         } else {
             self.layout
         };
         // SAFETY: Invariant, `self.ptr` points to memory allocated with `self.layout`.
         unsafe { alloc::dealloc(self.ptr, alloc_layout) }
     }
 }
 
@@ -56,27  60,25 @@ impl std::ops::DerefMut for MiriAllocBytes {
 }
 
 impl MiriAllocBytes {
-    /// This method factors out how a `MiriAllocBytes` object is allocated,
-    /// specifically given an allocation function `alloc_fn`.
-    /// `alloc_fn` is only used if `size != 0`.
-    /// Returns `Err(layout)` if the allocation function returns a `ptr` that is `ptr.is_null()`.
     /// This method factors out how a `MiriAllocBytes` object is allocated, given a specific allocation function.
     /// If `size == 0` we allocate using a different `alloc_layout` with `size = 1`, to ensure each allocation has a unique address.
     /// Returns `Err(alloc_layout)` if the allocation function returns a `ptr` where `ptr.is_null()`.
     fn alloc_with(
         size: usize,
         align: usize,
         alloc_fn: impl FnOnce(Layout) -> *mut u8,
     ) -> Result<MiriAllocBytes, Layout> {
         let layout = Layout::from_size_align(size, align).unwrap();
-        let ptr = if size == 0 {
-            std::ptr::without_provenance_mut(align)
         // When size is 0 we allocate 1 byte anyway, to ensure each allocation has a unique address.
         let alloc_layout =
             if size == 0 { Layout::from_size_align(1, align).unwrap() } else { layout };
         let ptr = alloc_fn(alloc_layout);
         if ptr.is_null() {
             Err(alloc_layout)
         } else {
-            let ptr = alloc_fn(layout);
-            if ptr.is_null() {
-                return Err(layout);
-            }
-            ptr
-        };
-        // SAFETY: All `MiriAllocBytes` invariants are fulfilled.
-        Ok(Self { ptr, layout })
             // SAFETY: All `MiriAllocBytes` invariants are fulfilled.
             Ok(Self { ptr, layout })
         }
     }
 }
 
@@ -85,7  87,7 @@ impl AllocBytes for MiriAllocBytes {
         let slice = slice.into();
         let size = slice.len();
         let align = align.bytes_usize();
-        // SAFETY: `alloc_fn` will only be used if `size != 0`.
         // SAFETY: `alloc_fn` will only be used with `size != 0`.
         let alloc_fn = |layout| unsafe { alloc::alloc(layout) };
         let alloc_bytes = MiriAllocBytes::alloc_with(size, align, alloc_fn)
             .unwrap_or_else(|layout| alloc::handle_alloc_error(layout));
@@ -98,7  100,7 @@ impl AllocBytes for MiriAllocBytes {
     fn zeroed(size: Size, align: Align) -> Option<Self> {
         let size = size.bytes_usize();
         let align = align.bytes_usize();
-        // SAFETY: `alloc_fn` will only be used if `size != 0`.
         // SAFETY: `alloc_fn` will only be used with `size != 0`.
         let alloc_fn = |layout| unsafe { alloc::alloc_zeroed(layout) };
         MiriAllocBytes::alloc_with(size, align, alloc_fn).ok()
     }

diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs
@@ -13,7  13,6 @@
 #![feature(lint_reasons)]
 #![feature(trait_upcasting)]
 #![feature(strict_overflow_ops)]
-#![feature(strict_provenance)]
 // Configure clippy and other lints
 #![allow(
     clippy::collapsible_else_if,