mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #3598 - RalfJung:heap, r=RalfJung

Browse Source 
alloc: update comments around malloc() alignment

Also separate the C heap shims form the Windows heap shims; their guarantees aren't quite the same.
This commit is contained in:
bors 2024-05-11 08:53:42 +00:00
commit 79a85d4e99
5 changed files with 74 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/tools/miri/src/lib.rs
View File

@ -12,6 +12,7 @@
#![feature(let_chains)]
#![feature(lint_reasons)]
#![feature(trait_upcasting)]
#![feature(strict_overflow_ops)]
// Configure clippy and other lints
#![allow(
clippy::collapsible_else_if,

51
src/tools/miri/src/shims/alloc.rs
View File

@ -19,23 +19,34 @@ pub(super) fn check_alloc_request<'tcx>(size: u64, align: u64) -> InterpResult<'
impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
/// Returns the minimum alignment for the target architecture for allocations of the given size.
fn min_align(&self, size: u64, kind: MiriMemoryKind) -> Align {
/// Returns the alignment that `malloc` would guarantee for requests of the given size.
fn malloc_align(&self, size: u64) -> Align {
let this = self.eval_context_ref();
// List taken from `library/std/src/sys/pal/common/alloc.rs`.
// This list should be kept in sync with the one from libstd.
let min_align = match this.tcx.sess.target.arch.as_ref() {
// The C standard says: "The pointer returned if the allocation succeeds is suitably aligned
// so that it may be assigned to a pointer to any type of object with a fundamental
// alignment requirement and size less than or equal to the size requested."
// So first we need to figure out what the limits are for "fundamental alignment".
// This is given by `alignof(max_align_t)`. The following list is taken from
// `library/std/src/sys/pal/common/alloc.rs` (where this is called `MIN_ALIGN`) and should
// be kept in sync.
let max_fundamental_align = match this.tcx.sess.target.arch.as_ref() {
"x86" | "arm" | "mips" | "mips32r6" | "powerpc" | "powerpc64" | "wasm32" => 8,
"x86_64" | "aarch64" | "mips64" | "mips64r6" | "s390x" | "sparc64" | "loongarch64" =>
16,
arch => bug!("unsupported target architecture for malloc: `{}`", arch),
};
// Windows always aligns, even small allocations.
// Source: <https://support.microsoft.com/en-us/help/286470/how-to-use-pageheap-exe-in-windows-xp-windows-2000-and-windows-server>
// But jemalloc does not, so for the C heap we only align if the allocation is sufficiently big.
if kind == MiriMemoryKind::WinHeap || size >= min_align {
return Align::from_bytes(min_align).unwrap();
// The C standard only requires sufficient alignment for any *type* with size less than or
// equal to the size requested. Types one can define in standard C seem to never have an alignment
// bigger than their size. So if the size is 2, then only alignment 2 is guaranteed, even if
// `max_fundamental_align` is bigger.
// This matches what some real-world implementations do, see e.g.
// - https://github.com/jemalloc/jemalloc/issues/1533
// - https://github.com/llvm/llvm-project/issues/53540
// - https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2293.htm
if size >= max_fundamental_align {
return Align::from_bytes(max_fundamental_align).unwrap();
}
// C doesn't have zero-sized types, so presumably nothing is guaranteed here.
if size == 0 {
return Align::ONE;
}
@ -85,11 +96,10 @@ fn malloc(
&mut self,
size: u64,
zero_init: bool,
kind: MiriMemoryKind,
) -> InterpResult<'tcx, Pointer<Option<Provenance>>> {
let this = self.eval_context_mut();
let align = this.min_align(size, kind);
let ptr = this.allocate_ptr(Size::from_bytes(size), align, kind.into())?;
let align = this.malloc_align(size);
let ptr = this.allocate_ptr(Size::from_bytes(size), align, MiriMemoryKind::C.into())?;
if zero_init {
// We just allocated this, the access is definitely in-bounds and fits into our address space.
this.write_bytes_ptr(
@ -101,14 +111,10 @@ fn malloc(
Ok(ptr.into())
}
fn free(
&mut self,
ptr: Pointer<Option<Provenance>>,
kind: MiriMemoryKind,
) -> InterpResult<'tcx> {
fn free(&mut self, ptr: Pointer<Option<Provenance>>) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
if !this.ptr_is_null(ptr)? {
this.deallocate_ptr(ptr, None, kind.into())?;
this.deallocate_ptr(ptr, None, MiriMemoryKind::C.into())?;
}
Ok(())
}
@ -117,13 +123,12 @@ fn realloc(
&mut self,
old_ptr: Pointer<Option<Provenance>>,
new_size: u64,
kind: MiriMemoryKind,
) -> InterpResult<'tcx, Pointer<Option<Provenance>>> {
let this = self.eval_context_mut();
let new_align = this.min_align(new_size, kind);
let new_align = this.malloc_align(new_size);
if this.ptr_is_null(old_ptr)? {
// Here we must behave like `malloc`.
self.malloc(new_size, /*zero_init*/ false, kind)
self.malloc(new_size, /*zero_init*/ false)
} else {
if new_size == 0 {
// C, in their infinite wisdom, made this UB.
@ -135,7 +140,7 @@ fn realloc(
None,
Size::from_bytes(new_size),
new_align,
kind.into(),
MiriMemoryKind::C.into(),
)?;
Ok(new_ptr.into())
}

8
src/tools/miri/src/shims/foreign_items.rs
View File

@ -421,7 +421,7 @@ fn emulate_foreign_item_inner(
"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, MiriMemoryKind::C)?;
let res = this.malloc(size, /*zero_init:*/ false)?;
this.write_pointer(res, dest)?;
}
"calloc" => {
@ -432,20 +432,20 @@ fn emulate_foreign_item_inner(
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, MiriMemoryKind::C)?;
let res = this.malloc(size, /*zero_init:*/ true)?;
this.write_pointer(res, dest)?;
}
"free" => {
let [ptr] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
let ptr = this.read_pointer(ptr)?;
this.free(ptr, MiriMemoryKind::C)?;
this.free(ptr)?;
}
"realloc" => {
let [old_ptr, new_size] =
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, MiriMemoryKind::C)?;
let res = this.realloc(old_ptr, new_size)?;
this.write_pointer(res, dest)?;
}

2
src/tools/miri/src/shims/unix/foreign_items.rs
View File

@ -310,7 +310,7 @@ fn emulate_foreign_item_inner(
this.write_null(dest)?;
}
Some(len) => {
let res = this.realloc(ptr, len, MiriMemoryKind::C)?;
let res = this.realloc(ptr, len)?;
this.write_pointer(res, dest)?;
}
}

50
src/tools/miri/src/shims/windows/foreign_items.rs
View File

@ -5,10 +5,9 @@
use std::str;
use rustc_span::Symbol;
use rustc_target::abi::Size;
use rustc_target::abi::{Align, Size};
use rustc_target::spec::abi::Abi;
use crate::shims::alloc::EvalContextExt as _;
use crate::shims::os_str::bytes_to_os_str;
use crate::shims::windows::*;
use crate::*;
@ -248,8 +247,21 @@ fn emulate_foreign_item_inner(
let size = this.read_target_usize(size)?;
let heap_zero_memory = 0x00000008; // HEAP_ZERO_MEMORY
let zero_init = (flags & heap_zero_memory) == heap_zero_memory;
let res = this.malloc(size, zero_init, MiriMemoryKind::WinHeap)?;
this.write_pointer(res, dest)?;
// Alignment is twice the pointer size.
// Source: <https://learn.microsoft.com/en-us/windows/win32/api/heapapi/nf-heapapi-heapalloc>
let align = this.tcx.pointer_size().bytes().strict_mul(2);
let ptr = this.allocate_ptr(
Size::from_bytes(size),
Align::from_bytes(align).unwrap(),
MiriMemoryKind::WinHeap.into(),
)?;
if zero_init {
this.write_bytes_ptr(
ptr.into(),
iter::repeat(0u8).take(usize::try_from(size).unwrap()),
)?;
}
this.write_pointer(ptr, dest)?;
}
"HeapFree" => {
let [handle, flags, ptr] =
@ -257,23 +269,41 @@ fn emulate_foreign_item_inner(
this.read_target_isize(handle)?;
this.read_scalar(flags)?.to_u32()?;
let ptr = this.read_pointer(ptr)?;
this.free(ptr, MiriMemoryKind::WinHeap)?;
// "This pointer can be NULL." It doesn't say what happens then, but presumably nothing.
// (https://learn.microsoft.com/en-us/windows/win32/api/heapapi/nf-heapapi-heapfree)
if !this.ptr_is_null(ptr)? {
this.deallocate_ptr(ptr, None, MiriMemoryKind::WinHeap.into())?;
}
this.write_scalar(Scalar::from_i32(1), dest)?;
}
"HeapReAlloc" => {
let [handle, flags, ptr, size] =
let [handle, flags, old_ptr, size] =
this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
this.read_target_isize(handle)?;
this.read_scalar(flags)?.to_u32()?;
let ptr = this.read_pointer(ptr)?;
let old_ptr = this.read_pointer(old_ptr)?;
let size = this.read_target_usize(size)?;
let res = this.realloc(ptr, size, MiriMemoryKind::WinHeap)?;
this.write_pointer(res, dest)?;
let align = this.tcx.pointer_size().bytes().strict_mul(2); // same as above
// The docs say that `old_ptr` must come from an earlier HeapAlloc or HeapReAlloc,
// so unlike C `realloc` we do *not* allow a NULL here.
// (https://learn.microsoft.com/en-us/windows/win32/api/heapapi/nf-heapapi-heaprealloc)
let new_ptr = this.reallocate_ptr(
old_ptr,
None,
Size::from_bytes(size),
Align::from_bytes(align).unwrap(),
MiriMemoryKind::WinHeap.into(),
)?;
this.write_pointer(new_ptr, dest)?;
}
"LocalFree" => {
let [ptr] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?;
let ptr = this.read_pointer(ptr)?;
this.free(ptr, MiriMemoryKind::WinLocal)?;
// "If the hMem parameter is NULL, LocalFree ignores the parameter and returns NULL."
// (https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-localfree)
if !this.ptr_is_null(ptr)? {
this.deallocate_ptr(ptr, None, MiriMemoryKind::WinLocal.into())?;
}
this.write_null(dest)?;
}