Skip to content

Commit

Permalink
Auto merge of rust-lang#136035 - SpecificProtagonist:miri-zeroed-allo…
Browse files Browse the repository at this point in the history
…c, r=<try>

miri: optimize zeroed alloc

When allocating zero-initialized memory in MIR interpretation, rustc allocates zeroed memory, marks it as initialized and then re-zeroes it. Remove the last step.

I don't expect this to have much of an effect on performance normally, but in my case in which I'm creating a large allocation via mmap miri is unusable without this.

There's probably a better way – with less code duplication – to implement this. Maybe adding a zero_init flag to the relevant methods, but then `Allocation::uninit` & co need a new name :)
  • Loading branch information
bors committed Jan 25, 2025
2 parents 6365178 + bd28faf commit 837b710
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 39 deletions.
14 changes: 14 additions & 0 deletions compiler/rustc_const_eval/src/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,20 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
self.insert_allocation(alloc, kind)
}

pub fn allocate_zeroed_ptr(
&mut self,
size: Size,
align: Align,
kind: MemoryKind<M::MemoryKind>,
) -> InterpResult<'tcx, Pointer<M::Provenance>> {
let alloc = if M::PANIC_ON_ALLOC_FAIL {
Allocation::zeroed(size, align)
} else {
Allocation::try_zeroed(size, align)?
};
self.insert_allocation(alloc, kind)
}

pub fn insert_allocation(
&mut self,
alloc: Allocation<M::Provenance, (), M::Bytes>,
Expand Down
43 changes: 43 additions & 0 deletions compiler/rustc_middle/src/mir/interpret/allocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,49 @@ impl<Prov: Provenance, Bytes: AllocBytes> Allocation<Prov, (), Bytes> {
}
}

fn zeroed_inner<R>(size: Size, align: Align, fail: impl FnOnce() -> R) -> Result<Self, R> {
// We raise an error if we cannot create the allocation on the host.
// This results in an error that can happen non-deterministically, since the memory
// available to the compiler can change between runs. Normally queries are always
// deterministic. However, we can be non-deterministic here because all uses of const
// evaluation (including ConstProp!) will make compilation fail (via hard error
// or ICE) upon encountering a `MemoryExhausted` error.
let bytes = Bytes::zeroed(size, align).ok_or_else(fail)?;

Ok(Allocation {
bytes,
provenance: ProvenanceMap::new(),
init_mask: InitMask::new(size, true),
align,
mutability: Mutability::Mut,
extra: (),
})
}

/// Try to create an Allocation of `size` zero-initialized bytes, failing if there is not enough memory
/// available to the compiler to do so.
pub fn try_zeroed<'tcx>(size: Size, align: Align) -> InterpResult<'tcx, Self> {
Self::zeroed_inner(size, align, || {
ty::tls::with(|tcx| tcx.dcx().delayed_bug("exhausted memory during interpretation"));
InterpErrorKind::ResourceExhaustion(ResourceExhaustionInfo::MemoryExhausted)
})
.into()
}

/// Try to create an Allocation of `size` zero-initialized bytes, panics if there is not enough memory
/// available to the compiler to do so.
pub fn zeroed(size: Size, align: Align) -> Self {
match Self::zeroed_inner(size, align, || {
panic!(
"interpreter ran out of memory: cannot create allocation of {} bytes",
size.bytes()
);
}) {
Ok(x) => x,
Err(x) => x,
}
}

/// Add the extra.
pub fn with_extra<Extra>(self, extra: Extra) -> Allocation<Prov, Extra, Bytes> {
Allocation {
Expand Down
16 changes: 5 additions & 11 deletions src/tools/miri/src/shims/alloc.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::iter;

use rustc_abi::{Align, Size};
use rustc_ast::expand::allocator::AllocatorKind;

Expand Down Expand Up @@ -83,15 +81,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
fn malloc(&mut self, size: u64, zero_init: bool) -> InterpResult<'tcx, Pointer> {
let this = self.eval_context_mut();
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(
ptr.into(),
iter::repeat(0u8).take(usize::try_from(size).unwrap()),
)
.unwrap();
}
let ptr = if zero_init {
this.allocate_zeroed_ptr(Size::from_bytes(size), align, MiriMemoryKind::C.into())?
} else {
this.allocate_ptr(Size::from_bytes(size), align, MiriMemoryKind::C.into())?
};
interp_ok(ptr.into())
}

Expand Down
10 changes: 1 addition & 9 deletions src/tools/miri/src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::collections::hash_map::Entry;
use std::io::Write;
use std::iter;
use std::path::Path;

use rustc_abi::{Align, AlignFromBytesError, Size};
Expand Down Expand Up @@ -533,18 +532,11 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {

this.check_rustc_alloc_request(size, align)?;

let ptr = this.allocate_ptr(
let ptr = this.allocate_zeroed_ptr(
Size::from_bytes(size),
Align::from_bytes(align).unwrap(),
MiriMemoryKind::Rust.into(),
)?;

// We just allocated this, the access is definitely in-bounds.
this.write_bytes_ptr(
ptr.into(),
iter::repeat(0u8).take(usize::try_from(size).unwrap()),
)
.unwrap();
this.write_pointer(ptr, dest)
});
}
Expand Down
13 changes: 5 additions & 8 deletions src/tools/miri/src/shims/unix/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
return interp_ok(this.eval_libc("MAP_FAILED"));
}

let ptr =
this.allocate_ptr(Size::from_bytes(map_length), align, MiriMemoryKind::Mmap.into())?;
// We just allocated this, the access is definitely in-bounds and fits into our address space.
// mmap guarantees new mappings are zero-init.
this.write_bytes_ptr(
ptr.into(),
std::iter::repeat(0u8).take(usize::try_from(map_length).unwrap()),
)
.unwrap();
let ptr = this.allocate_zeroed_ptr(
Size::from_bytes(map_length),
align,
MiriMemoryKind::Mmap.into(),
)?;

interp_ok(Scalar::from_pointer(ptr, this))
}
Expand Down
24 changes: 13 additions & 11 deletions src/tools/miri/src/shims/windows/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,17 +258,19 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// 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()),
)?;
}
let ptr = if zero_init {
this.allocate_zeroed_ptr(
Size::from_bytes(size),
Align::from_bytes(align).unwrap(),
MiriMemoryKind::WinHeap.into(),
)?
} else {
this.allocate_ptr(
Size::from_bytes(size),
Align::from_bytes(align).unwrap(),
MiriMemoryKind::WinHeap.into(),
)?
};
this.write_pointer(ptr, dest)?;
}
"HeapFree" => {
Expand Down

0 comments on commit 837b710

Please sign in to comment.