about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-05-24 08:11:36 +0000
committerbors <bors@rust-lang.org>2024-05-24 08:11:36 +0000
commit7fc41d1bdf4cd9c04f52d3342983c6b1964b0a42 (patch)
tree32160cb1f70c008465c33343f85cf48b781f9fc7
parent88d519f718bf4c44302aac2127ab97cd3010573d (diff)
parentb84620ff17b386969052d26b2868aa7e2557be79 (diff)
downloadrust-7fc41d1bdf4cd9c04f52d3342983c6b1964b0a42.tar.gz
rust-7fc41d1bdf4cd9c04f52d3342983c6b1964b0a42.zip
Auto merge of #3625 - Strophox:miri-allocation-fix, r=RalfJung
Bugfix `MiriAllocBytes` to guarantee different addresses

Fix in `alloc_bytes.rs` following https://github.com/rust-lang/miri/pull/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.
-rw-r--r--src/tools/miri/src/alloc_bytes.rs46
-rw-r--r--src/tools/miri/src/lib.rs1
2 files changed, 24 insertions, 23 deletions
diff --git a/src/tools/miri/src/alloc_bytes.rs b/src/tools/miri/src/alloc_bytes.rs
index 7952abdf9f4..97841a05cde 100644
--- 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
index e4879f2f531..d23a44c376b 100644
--- 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,