From 1740f8ba2410e24f8100568e3bd1a77254808a8e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 14 Sep 2024 10:28:17 +0200 Subject: [PATCH] also use compute_size_in_bytes for relevant multiplications in Miri --- src/concurrency/thread.rs | 2 +- src/intrinsics/mod.rs | 17 ++---------- src/shims/foreign_items.rs | 48 ++++++++++++++++++++++++--------- src/shims/unix/foreign_items.rs | 4 +-- tests/pass-dep/libc/libc-mem.rs | 13 ++++++++- 5 files changed, 53 insertions(+), 31 deletions(-) diff --git a/src/concurrency/thread.rs b/src/concurrency/thread.rs index 306245a843..6d13b34e93 100644 --- a/src/concurrency/thread.rs +++ b/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 --git a/src/intrinsics/mod.rs b/src/intrinsics/mod.rs index 0ab1b9dfb6..7acc99a8af 100644 --- a/src/intrinsics/mod.rs +++ b/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 --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 88ab012f97..67dc147617 100644 --- a/src/shims/foreign_items.rs +++ b/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 --git a/src/shims/unix/foreign_items.rs b/src/shims/unix/foreign_items.rs index 273a99b311..73f933264f 100644 --- a/src/shims/unix/foreign_items.rs +++ b/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 --git a/tests/pass-dep/libc/libc-mem.rs b/tests/pass-dep/libc/libc-mem.rs index aa383a99bc..c399616b9a 100644 --- a/tests/pass-dep/libc/libc-mem.rs +++ b/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()); } }