also use compute_size_in_bytes for relevant multiplications in Miri

Author: RalfJung

Diff for: compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -216,7 +216,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 self.copy_intrinsic(&args[0], &args[1], &args[2], /*nonoverlapping*/ false)?;
             }
             sym::write_bytes => {
-                self.write_bytes_intrinsic(&args[0], &args[1], &args[2])?;
+                self.write_bytes_intrinsic(&args[0], &args[1], &args[2], "write_bytes")?;
             }
             sym::compare_bytes => {
                 let result = self.compare_bytes_intrinsic(&args[0], &args[1], &args[2])?;
@@ -634,11 +634,12 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         Ok(())
     }
 
-    pub(crate) fn write_bytes_intrinsic(
+    pub fn write_bytes_intrinsic(
         &mut self,
         dst: &OpTy<'tcx, <M as Machine<'tcx>>::Provenance>,
         byte: &OpTy<'tcx, <M as Machine<'tcx>>::Provenance>,
         count: &OpTy<'tcx, <M as Machine<'tcx>>::Provenance>,
+        name: &'static str,
     ) -> InterpResult<'tcx> {
         let layout = self.layout_of(dst.layout.ty.builtin_deref(true).unwrap())?;
 
@@ -648,9 +649,9 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
 
         // `checked_mul` enforces a too small bound (the correct one would probably be target_isize_max),
         // but no actual allocation can be big enough for the difference to be noticeable.
-        let len = self.compute_size_in_bytes(layout.size, count).ok_or_else(|| {
-            err_ub_custom!(fluent::const_eval_size_overflow, name = "write_bytes")
-        })?;
+        let len = self
+            .compute_size_in_bytes(layout.size, count)
+            .ok_or_else(|| err_ub_custom!(fluent::const_eval_size_overflow, name = name))?;
 
         let bytes = std::iter::repeat(byte).take(len.bytes_usize());
         self.write_bytes_ptr(dst, bytes)

Diff for: compiler/rustc_const_eval/src/interpret/memory.rs

@@ -222,7 +222,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         } else {
             Allocation::try_uninit(size, align)?
         };
-        self.allocate_raw_ptr(alloc, kind)
+        self.insert_allocation(alloc, kind)
     }
 
     pub fn allocate_bytes_ptr(
@@ -233,14 +233,15 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         mutability: Mutability,
     ) -> InterpResult<'tcx, Pointer<M::Provenance>> {
         let alloc = Allocation::from_bytes(bytes, align, mutability);
-        self.allocate_raw_ptr(alloc, kind)
+        self.insert_allocation(alloc, kind)
     }
 
-    pub fn allocate_raw_ptr(
+    pub fn insert_allocation(
         &mut self,
         alloc: Allocation<M::Provenance, (), M::Bytes>,
         kind: MemoryKind<M::MemoryKind>,
     ) -> InterpResult<'tcx, Pointer<M::Provenance>> {
+        assert!(alloc.size() <= self.max_size_of_val());
         let id = self.tcx.reserve_alloc_id();
         debug_assert_ne!(
             Some(kind),

Diff for: compiler/rustc_const_eval/src/interpret/operator.rs

@@ -290,7 +290,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
 
     /// Computes the total size of this access, `count * elem_size`,
     /// checking for overflow beyond isize::MAX.
-    pub(super) fn compute_size_in_bytes(&self, elem_size: Size, count: u64) -> Option<Size> {
+    pub fn compute_size_in_bytes(&self, elem_size: Size, count: u64) -> Option<Size> {
         // `checked_mul` applies `u64` limits independent of the target pointer size... but the
         // subsequent check for `max_size_of_val` means we also handle 32bit targets correctly.
         // (We cannot use `Size::checked_mul` as that enforces `obj_size_bound` as the limit, which

Diff for: src/tools/miri/src/concurrency/thread.rs

@@ -898,7 +898,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             // This allocation will be deallocated when the thread dies, so it is not in read-only memory.
             alloc.mutability = Mutability::Mut;
             // Create a fresh allocation with this content.
-            let ptr = this.allocate_raw_ptr(alloc, MiriMemoryKind::Tls.into())?;
+            let ptr = this.insert_allocation(alloc, MiriMemoryKind::Tls.into())?;
             this.machine.threads.set_thread_local_alloc(def_id, ptr);
             Ok(ptr)
         }

Diff for: src/tools/miri/src/intrinsics/mod.rs

@@ -3,11 +3,8 @@
 mod atomic;
 mod simd;
 
-use std::iter;
-
 use rand::Rng;
 use rustc_apfloat::{Float, Round};
-use rustc_middle::ty::layout::LayoutOf;
 use rustc_middle::{
     mir,
     ty::{self, FloatTy},
@@ -119,19 +116,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 this.copy_op(dest, &place)?;
             }
 
-            "write_bytes" | "volatile_set_memory" => {
+            "volatile_set_memory" => {
                 let [ptr, val_byte, count] = check_arg_count(args)?;
-                let ty = ptr.layout.ty.builtin_deref(true).unwrap();
-                let ty_layout = this.layout_of(ty)?;
-                let val_byte = this.read_scalar(val_byte)?.to_u8()?;
-                let ptr = this.read_pointer(ptr)?;
-                let count = this.read_target_usize(count)?;
-                // `checked_mul` enforces a too small bound (the correct one would probably be target_isize_max),
-                // but no actual allocation can be big enough for the difference to be noticeable.
-                let byte_count = ty_layout.size.checked_mul(count, this).ok_or_else(|| {
-                    err_ub_format!("overflow computing total size of `{intrinsic_name}`")
-                })?;
-                this.write_bytes_ptr(ptr, iter::repeat(val_byte).take(byte_count.bytes_usize()))?;
+                this.write_bytes_intrinsic(ptr, val_byte, count, "volatile_set_memory")?;
             }
 
             // Memory model / provenance manipulation

Diff for: src/tools/miri/src/shims/foreign_items.rs

@@ -196,7 +196,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
         if size == 0 {
             throw_ub_format!("creating allocation with size 0");
         }
-        if i128::from(size) > this.tcx.data_layout.pointer_size.signed_int_max() {
+        if size > this.max_size_of_val().bytes() {
             throw_ub_format!("creating an allocation larger than half the address space");
         }
         if let Err(e) = Align::from_bytes(align) {
@@ -441,19 +441,34 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
             "malloc" => {
                 let [size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
                 let size = this.read_target_usize(size)?;
-                let res = this.malloc(size, /*zero_init:*/ false)?;
-                this.write_pointer(res, dest)?;
+                if size <= this.max_size_of_val().bytes() {
+                    let res = this.malloc(size, /*zero_init:*/ false)?;
+                    this.write_pointer(res, dest)?;
+                } else {
+                    // If this does not fit in an isize, return null and, on Unix, set errno.
+                    if this.target_os_is_unix() {
+                        let einval = this.eval_libc("ENOMEM");
+                        this.set_last_error(einval)?;
+                    }
+                    this.write_null(dest)?;
+                }
             }
             "calloc" => {
-                let [items, len] =
+                let [items, elem_size] =
                     this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
                 let items = this.read_target_usize(items)?;
-                let len = this.read_target_usize(len)?;
-                let size = items
-                    .checked_mul(len)
-                    .ok_or_else(|| err_ub_format!("overflow during calloc size computation"))?;
-                let res = this.malloc(size, /*zero_init:*/ true)?;
-                this.write_pointer(res, dest)?;
+                let elem_size = this.read_target_usize(elem_size)?;
+                if let Some(size) = this.compute_size_in_bytes(Size::from_bytes(elem_size), items) {
+                    let res = this.malloc(size.bytes(), /*zero_init:*/ true)?;
+                    this.write_pointer(res, dest)?;
+                } else {
+                    // On size overflow, return null and, on Unix, set errno.
+                    if this.target_os_is_unix() {
+                        let einval = this.eval_libc("ENOMEM");
+                        this.set_last_error(einval)?;
+                    }
+                    this.write_null(dest)?;
+                }
             }
             "free" => {
                 let [ptr] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
@@ -465,8 +480,17 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
                 let old_ptr = this.read_pointer(old_ptr)?;
                 let new_size = this.read_target_usize(new_size)?;
-                let res = this.realloc(old_ptr, new_size)?;
-                this.write_pointer(res, dest)?;
+                if new_size <= this.max_size_of_val().bytes() {
+                    let res = this.realloc(old_ptr, new_size)?;
+                    this.write_pointer(res, dest)?;
+                } else {
+                    // If this does not fit in an isize, return null and, on Unix, set errno.
+                    if this.target_os_is_unix() {
+                        let einval = this.eval_libc("ENOMEM");
+                        this.set_last_error(einval)?;
+                    }
+                    this.write_null(dest)?;
+                }
             }
 
             // Rust allocation

Diff for: src/tools/miri/src/shims/unix/foreign_items.rs

@@ -363,14 +363,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 //
                 // Linux: https://www.unix.com/man-page/linux/3/reallocarray/
                 // FreeBSD: https://man.freebsd.org/cgi/man.cgi?query=reallocarray
-                match nmemb.checked_mul(size) {
+                match this.compute_size_in_bytes(Size::from_bytes(size), nmemb) {
                     None => {
                         let einval = this.eval_libc("ENOMEM");
                         this.set_last_error(einval)?;
                         this.write_null(dest)?;
                     }
                     Some(len) => {
-                        let res = this.realloc(ptr, len)?;
+                        let res = this.realloc(ptr, len.bytes())?;
                         this.write_pointer(res, dest)?;
                     }
                 }

Diff for: src/tools/miri/tests/pass-dep/libc/libc-mem.rs

@@ -101,6 +101,10 @@ fn test_malloc() {
         let slice = slice::from_raw_parts(p3 as *const u8, 20);
         assert_eq!(&slice, &[0_u8; 20]);
 
+        // new size way too big (so this doesn't actually realloc).
+        let p_too_big = libc::realloc(p3, usize::MAX);
+        assert!(p_too_big.is_null());
+
         // old size > new size
         let p4 = libc::realloc(p3, 10);
         let slice = slice::from_raw_parts(p4 as *const u8, 10);
@@ -119,9 +123,13 @@ fn test_malloc() {
     unsafe {
         let p1 = libc::realloc(ptr::null_mut(), 20);
         assert!(!p1.is_null());
-
         libc::free(p1);
     }
+
+    unsafe {
+        let p_too_big = libc::malloc(usize::MAX);
+        assert!(p_too_big.is_null());
+    }
 }
 
 fn test_calloc() {
@@ -143,6 +151,9 @@ fn test_calloc() {
         let slice = slice::from_raw_parts(p4 as *const u8, 4 * 8);
         assert_eq!(&slice, &[0_u8; 4 * 8]);
         libc::free(p4);
+
+        let p_too_big = libc::calloc(usize::MAX / 4, 4);
+        assert!(p_too_big.is_null());
     }
 }