show proper UB when making a too large allocation request

Author: RalfJung

src/alloc_bytes.rs

@@ -67,14 +67,14 @@ impl MiriAllocBytes {
         size: usize,
         align: usize,
         alloc_fn: impl FnOnce(Layout) -> *mut u8,
-    ) -> Result<MiriAllocBytes, Layout> {
-        let layout = Layout::from_size_align(size, align).unwrap();
+    ) -> Result<MiriAllocBytes, ()> {
+        let Ok(layout) = Layout::from_size_align(size, align) else { return Err(()) };
         // 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)
+            Err(())
         } else {
             // SAFETY: All `MiriAllocBytes` invariants are fulfilled.
             Ok(Self { ptr, layout })
@@ -89,8 +89,9 @@ impl AllocBytes for MiriAllocBytes {
         let align = align.bytes_usize();
         // 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));
+        let alloc_bytes = MiriAllocBytes::alloc_with(size, align, alloc_fn).unwrap_or_else(|()| {
+            panic!("Miri ran out of memory: cannot create allocation of {size} bytes")
+        });
         // SAFETY: `alloc_bytes.ptr` and `slice.as_ptr()` are non-null, properly aligned
         // and valid for the `size`-many bytes to be copied.
         unsafe { alloc_bytes.ptr.copy_from(slice.as_ptr(), size) };

src/shims/alloc.rs

@@ -5,18 +5,6 @@ use rustc_target::abi::{Align, Size};
 
 use crate::*;
 
-/// Check some basic requirements for this allocation request:
-/// non-zero size, power-of-two alignment.
-pub(super) fn check_alloc_request<'tcx>(size: u64, align: u64) -> InterpResult<'tcx> {
-    if size == 0 {
-        throw_ub_format!("creating allocation with size 0");
-    }
-    if !align.is_power_of_two() {
-        throw_ub_format!("creating allocation with non-power-of-two alignment {}", align);
-    }
-    Ok(())
-}
-
 impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
 pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
     /// Returns the alignment that `malloc` would guarantee for requests of the given size.

src/shims/foreign_items.rs

@@ -12,7 +12,7 @@ use rustc_target::{
     spec::abi::Abi,
 };
 
-use super::alloc::{check_alloc_request, EvalContextExt as _};
+use super::alloc::EvalContextExt as _;
 use super::backtrace::EvalContextExt as _;
 use crate::*;
 use helpers::{ToHost, ToSoft};
@@ -204,6 +204,22 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
 impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {}
 trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
+    /// Check some basic requirements for this allocation request:
+    /// non-zero size, power-of-two alignment.
+    fn check_rustc_alloc_request(&self, size: u64, align: u64) -> InterpResult<'tcx> {
+        let this = self.eval_context_ref();
+        if size == 0 {
+            throw_ub_format!("creating allocation with size 0");
+        }
+        if i128::from(size) > this.tcx.data_layout.pointer_size.signed_int_max() {
+            throw_ub_format!("creating an allocation larger than half the address space");
+        }
+        if !align.is_power_of_two() {
+            throw_ub_format!("creating allocation with non-power-of-two alignment {}", align);
+        }
+        Ok(())
+    }
+
     fn emulate_foreign_item_inner(
         &mut self,
         link_name: Symbol,
@@ -462,7 +478,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     let size = this.read_target_usize(size)?;
                     let align = this.read_target_usize(align)?;
 
-                    check_alloc_request(size, align)?;
+                    this.check_rustc_alloc_request(size, align)?;
 
                     let memory_kind = match link_name.as_str() {
                         "__rust_alloc" => MiriMemoryKind::Rust,
@@ -496,7 +512,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     let size = this.read_target_usize(size)?;
                     let align = this.read_target_usize(align)?;
 
-                    check_alloc_request(size, align)?;
+                    this.check_rustc_alloc_request(size, align)?;
 
                     let ptr = this.allocate_ptr(
                         Size::from_bytes(size),
@@ -560,7 +576,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     let new_size = this.read_target_usize(new_size)?;
                     // No need to check old_size; we anyway check that they match the allocation.
 
-                    check_alloc_request(new_size, align)?;
+                    this.check_rustc_alloc_request(new_size, align)?;
 
                     let align = Align::from_bytes(align).unwrap();
                     let new_ptr = this.reallocate_ptr(

tests/fail/alloc/too_large.rs

@@ -0,0 +1,10 @@
+extern "Rust" {
+    fn __rust_alloc(size: usize, align: usize) -> *mut u8;
+}
+
+fn main() {
+    let bytes = isize::MAX as usize + 1;
+    unsafe {
+        __rust_alloc(bytes, 1); //~ERROR: larger than half the address space
+    }
+}

tests/fail/alloc/too_large.stderr

@@ -0,0 +1,15 @@
+error: Undefined Behavior: creating an allocation larger than half the address space
+  --> $DIR/too_large.rs:LL:CC
+   |
+LL |         __rust_alloc(bytes, 1);
+   |         ^^^^^^^^^^^^^^^^^^^^^^ creating an allocation larger than half the address space
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `main` at $DIR/too_large.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+