From 904dd09f505193410d0a64299eaed9195a139437 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 09:03:45 +0200 Subject: [PATCH] align small malloc-allocations even less, and test that we do --- src/machine.rs | 4 +- src/shims/foreign_items.rs | 70 +++++++++++++++--------- tests/run-pass/{realloc.rs => malloc.rs} | 11 ++++ 3 files changed, 58 insertions(+), 27 deletions(-) rename tests/run-pass/{realloc.rs => malloc.rs} (70%) diff --git a/src/machine.rs b/src/machine.rs index 20fe2e055f..7ff9acca7f 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -25,6 +25,8 @@ pub enum MiriMemoryKind { Rust, /// `malloc` memory. C, + /// Windows `HeapAlloc` memory. + WinHeap, /// Part of env var emulation. Env, /// Statics. @@ -381,7 +383,7 @@ impl MayLeak for MiriMemoryKind { fn may_leak(self) -> bool { use self::MiriMemoryKind::*; match self { - Rust | C => false, + Rust | C | WinHeap => false, Env | Static => true, } } diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index aece4d3e1a..8a008d9093 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -10,8 +10,8 @@ use crate::*; impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { - /// Returns the minimum alignment for the target architecture. - fn min_align(&self) -> Align { + /// Returns the minimum alignment for the target architecture for allocations of the given size. + fn min_align(&self, size: u64, kind: MiriMemoryKind) -> Align { let this = self.eval_context_ref(); // List taken from `libstd/sys_common/alloc.rs`. let min_align = match this.tcx.tcx.sess.target.target.arch.as_str() { @@ -19,21 +19,39 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx "x86_64" | "aarch64" | "mips64" | "s390x" | "sparc64" => 16, arch => bug!("Unsupported target architecture: {}", arch), }; - Align::from_bytes(min_align).unwrap() + // Windows always aligns, even small allocations. + // Source: + // 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(); + } + // We have `size < min_align`. Round `size` *down* to the next power of two and use that. + fn prev_power_of_two(x: u64) -> u64 { + let next_pow2 = x.next_power_of_two(); + if next_pow2 == x { + // x *is* a power of two, just use that. + x + } else { + // x is between two powers, so next = 2*prev. + next_pow2 / 2 + } + } + Align::from_bytes(prev_power_of_two(size)).unwrap() } fn malloc( &mut self, size: u64, zero_init: bool, + kind: MiriMemoryKind, ) -> Scalar { let this = self.eval_context_mut(); let tcx = &{this.tcx.tcx}; if size == 0 { Scalar::from_int(0, this.pointer_size()) } else { - let align = this.min_align(); - let ptr = this.memory_mut().allocate(Size::from_bytes(size), align, MiriMemoryKind::C.into()); + let align = this.min_align(size, kind); + let ptr = this.memory_mut().allocate(Size::from_bytes(size), align, kind.into()); if zero_init { // We just allocated this, the access cannot fail this.memory_mut() @@ -47,13 +65,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx fn free( &mut self, ptr: Scalar, + kind: MiriMemoryKind, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); if !ptr.is_null_ptr(this) { this.memory_mut().deallocate( ptr.to_ptr()?, None, - MiriMemoryKind::C.into(), + kind.into(), )?; } Ok(()) @@ -63,39 +82,38 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx &mut self, old_ptr: Scalar, new_size: u64, + kind: MiriMemoryKind, ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let align = this.min_align(); + let new_align = this.min_align(new_size, kind); if old_ptr.is_null_ptr(this) { if new_size == 0 { Ok(Scalar::from_int(0, this.pointer_size())) } else { let new_ptr = this.memory_mut().allocate( Size::from_bytes(new_size), - align, - MiriMemoryKind::C.into() + new_align, + kind.into() ); Ok(Scalar::Ptr(new_ptr)) } } else { let old_ptr = old_ptr.to_ptr()?; let memory = this.memory_mut(); - let old_size = Size::from_bytes(memory.get(old_ptr.alloc_id)?.bytes.len() as u64); if new_size == 0 { memory.deallocate( old_ptr, - Some((old_size, align)), - MiriMemoryKind::C.into(), + None, + kind.into(), )?; Ok(Scalar::from_int(0, this.pointer_size())) } else { let new_ptr = memory.reallocate( old_ptr, - old_size, - align, + None, Size::from_bytes(new_size), - align, - MiriMemoryKind::C.into(), + new_align, + kind.into(), )?; Ok(Scalar::Ptr(new_ptr)) } @@ -144,14 +162,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx match link_name { "malloc" => { let size = this.read_scalar(args[0])?.to_usize(this)?; - let res = this.malloc(size, /*zero_init:*/ false); + let res = this.malloc(size, /*zero_init:*/ false, MiriMemoryKind::C); this.write_scalar(res, dest)?; } "calloc" => { let items = this.read_scalar(args[0])?.to_usize(this)?; let len = this.read_scalar(args[1])?.to_usize(this)?; let size = items.checked_mul(len).ok_or_else(|| InterpError::Overflow(mir::BinOp::Mul))?; - let res = this.malloc(size, /*zero_init:*/ true); + let res = this.malloc(size, /*zero_init:*/ true, MiriMemoryKind::C); this.write_scalar(res, dest)?; } "posix_memalign" => { @@ -186,12 +204,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } "free" => { let ptr = this.read_scalar(args[0])?.not_undef()?; - this.free(ptr)?; + this.free(ptr, MiriMemoryKind::C)?; } "realloc" => { let old_ptr = this.read_scalar(args[0])?.not_undef()?; let new_size = this.read_scalar(args[1])?.to_usize(this)?; - let res = this.realloc(old_ptr, new_size)?; + let res = this.realloc(old_ptr, new_size, MiriMemoryKind::C)?; this.write_scalar(res, dest)?; } @@ -259,12 +277,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx if !align.is_power_of_two() { return err!(HeapAllocNonPowerOfTwoAlignment(align)); } + let align = Align::from_bytes(align).unwrap(); let new_ptr = this.memory_mut().reallocate( ptr, - Size::from_bytes(old_size), - Align::from_bytes(align).unwrap(), + Some((Size::from_bytes(old_size), align)), Size::from_bytes(new_size), - Align::from_bytes(align).unwrap(), + align, MiriMemoryKind::Rust.into(), )?; this.write_scalar(Scalar::Ptr(new_ptr), dest)?; @@ -767,14 +785,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let flags = this.read_scalar(args[1])?.to_u32()?; let size = this.read_scalar(args[2])?.to_usize(this)?; let zero_init = (flags & 0x00000008) != 0; // HEAP_ZERO_MEMORY - let res = this.malloc(size, zero_init); + let res = this.malloc(size, zero_init, MiriMemoryKind::WinHeap); this.write_scalar(res, dest)?; } "HeapFree" => { let _handle = this.read_scalar(args[0])?.to_isize(this)?; let _flags = this.read_scalar(args[1])?.to_u32()?; let ptr = this.read_scalar(args[2])?.not_undef()?; - this.free(ptr)?; + this.free(ptr, MiriMemoryKind::WinHeap)?; this.write_scalar(Scalar::from_int(1, Size::from_bytes(4)), dest)?; } "HeapReAlloc" => { @@ -782,7 +800,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let _flags = this.read_scalar(args[1])?.to_u32()?; let ptr = this.read_scalar(args[2])?.not_undef()?; let size = this.read_scalar(args[3])?.to_usize(this)?; - let res = this.realloc(ptr, size)?; + let res = this.realloc(ptr, size, MiriMemoryKind::WinHeap)?; this.write_scalar(res, dest)?; } diff --git a/tests/run-pass/realloc.rs b/tests/run-pass/malloc.rs similarity index 70% rename from tests/run-pass/realloc.rs rename to tests/run-pass/malloc.rs index c23b3e645c..a50b3f3606 100644 --- a/tests/run-pass/realloc.rs +++ b/tests/run-pass/malloc.rs @@ -1,4 +1,5 @@ //ignore-windows: Uses POSIX APIs +//compile-flags: -Zmiri-seed= #![feature(rustc_private)] @@ -7,6 +8,16 @@ use core::{slice, ptr}; extern crate libc; fn main() { + // Test that small allocations sometimes *are* not very aligned. + let saw_unaligned = (0..64).any(|_| unsafe { + let p = libc::malloc(3); + let addr = p as usize; + let unaligned = addr % 4 != 0; // test that this is not 4-aligned + libc::free(p); // FIXME have to free *after* test; should allow ptr-to-int of dangling ptr. + unaligned + }); + assert!(saw_unaligned); + unsafe { // Use calloc for initialized memory let p1 = libc::calloc(20, 1);