From d42d8263495a67bd53ad2fe2f30325947b993c3a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 15 Mar 2024 11:59:38 +0100 Subject: [PATCH] interpret/allocation: rename mutating functions to be more scary; add a new raw bytes getter --- .../rustc_const_eval/src/interpret/memory.rs | 17 ++++++-- .../src/mir/interpret/allocation.rs | 43 ++++++++++++++----- 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 86aad2e1642d6..35c678f6d4c6b 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -1157,11 +1157,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; // Side-step AllocRef and directly access the underlying bytes more efficiently. - // (We are staying inside the bounds here so all is good.) + // (We are staying inside the bounds here and all bytes do get overwritten so all is good.) let alloc_id = alloc_ref.alloc_id; let bytes = alloc_ref .alloc - .get_bytes_mut(&alloc_ref.tcx, alloc_ref.range) + .get_bytes_unchecked_for_overwrite(&alloc_ref.tcx, alloc_ref.range) .map_err(move |e| e.to_interp_error(alloc_id))?; // `zip` would stop when the first iterator ends; we want to definitely // cover all of `bytes`. @@ -1182,6 +1182,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.mem_copy_repeatedly(src, dest, size, 1, nonoverlapping) } + /// Performs `num_copies` many copies of `size` many bytes from `src` to `dest + i*size` (where + /// `i` is the index of the copy). + /// + /// Either `nonoverlapping` must be true or `num_copies` must be 1; doing repeated copies that + /// may overlap is not supported. pub fn mem_copy_repeatedly( &mut self, src: Pointer>, @@ -1243,8 +1248,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { (dest_alloc_id, dest_prov), dest_range, )?; + // Yes we do overwrite all bytes in `dest_bytes`. let dest_bytes = dest_alloc - .get_bytes_mut_ptr(&tcx, dest_range) + .get_bytes_unchecked_for_overwrite_ptr(&tcx, dest_range) .map_err(|e| e.to_interp_error(dest_alloc_id))? .as_mut_ptr(); @@ -1278,6 +1284,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } } + if num_copies > 0 { + assert!(nonoverlapping, "multi-copy only supported in non-overlapping mode"); + } let size_in_bytes = size.bytes_usize(); // For particularly large arrays (where this is perf-sensitive) it's common that @@ -1290,6 +1299,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } else if src_alloc_id == dest_alloc_id { let mut dest_ptr = dest_bytes; for _ in 0..num_copies { + // Here we rely on `src` and `dest` being non-overlapping if there is more than + // one copy. ptr::copy(src_bytes, dest_ptr, size_in_bytes); dest_ptr = dest_ptr.add(size_in_bytes); } diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs index 4047891d7697b..87ca8e5074ee4 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -37,6 +37,13 @@ pub trait AllocBytes: /// Create a zeroed `AllocBytes` of the specified size and alignment. /// Returns `None` if we ran out of memory on the host. fn zeroed(size: Size, _align: Align) -> Option; + + /// Gives direct access to the raw underlying storage. Must be aligned according to the `align` + /// passed during construction. + /// + /// Crucially this pointer will *not* be invalidated by `deref`/`deref_mut` calls! + /// In Tree Borrows terms, this pointer is a parent of the references returned there. + fn get_bytes_unchecked_raw_mut(&mut self) -> *mut u8; } // Default `bytes` for `Allocation` is a `Box<[u8]>`. @@ -51,6 +58,13 @@ impl AllocBytes for Box<[u8]> { let bytes = unsafe { bytes.assume_init() }; Some(bytes) } + + fn get_bytes_unchecked_raw_mut(&mut self) -> *mut u8 { + // We didn't actually put the memory at the right alignment, so we can't support this. + // (That's okay, this function is only needed in Miri which implements this trait for its + // own type.) + panic!("raw bytes access not supported"); + } } /// This type represents an Allocation in the Miri/CTFE core engine. @@ -399,10 +413,6 @@ impl Allocation /// Byte accessors. impl Allocation { - pub fn base_addr(&self) -> *const u8 { - self.bytes.as_ptr() - } - /// This is the entirely abstraction-violating way to just grab the raw bytes without /// caring about provenance or initialization. /// @@ -452,13 +462,14 @@ impl Allocation Ok(self.get_bytes_unchecked(range)) } - /// Just calling this already marks everything as defined and removes provenance, - /// so be sure to actually put data there! + /// This is the entirely abstraction-violating way to just get mutable access to the raw bytes. + /// Just calling this already marks everything as defined and removes provenance, so be sure to + /// actually overwrite all the data there! /// /// It is the caller's responsibility to check bounds and alignment beforehand. /// Most likely, you want to use the `PlaceTy` and `OperandTy`-based methods /// on `InterpCx` instead. - pub fn get_bytes_mut( + pub fn get_bytes_unchecked_for_overwrite( &mut self, cx: &impl HasDataLayout, range: AllocRange, @@ -469,8 +480,9 @@ impl Allocation Ok(&mut self.bytes[range.start.bytes_usize()..range.end().bytes_usize()]) } - /// A raw pointer variant of `get_bytes_mut` that avoids invalidating existing aliases into this memory. - pub fn get_bytes_mut_ptr( + /// A raw pointer variant of `get_bytes_unchecked_for_overwrite` that avoids invalidating existing immutable aliases + /// into this memory. + pub fn get_bytes_unchecked_for_overwrite_ptr( &mut self, cx: &impl HasDataLayout, range: AllocRange, @@ -479,10 +491,20 @@ impl Allocation self.provenance.clear(range, cx)?; assert!(range.end().bytes_usize() <= self.bytes.len()); // need to do our own bounds-check + // FIXME: actually now that `self.bytes` is a `Box`, this *does* invalidate existing + // immutable aliases at least under Stacked Borrows... let begin_ptr = self.bytes.as_mut_ptr().wrapping_add(range.start.bytes_usize()); let len = range.end().bytes_usize() - range.start.bytes_usize(); Ok(ptr::slice_from_raw_parts_mut(begin_ptr, len)) } + + /// This gives direct mutable access to the entire buffer, just exposing their internal state + /// without reseting anything. Directly exposes the `AllocBytes` method of the same name. Only + /// works if `OFFSET_IS_ADDR` is true. + pub fn get_bytes_unchecked_raw_mut(&mut self) -> *mut u8 { + assert!(Prov::OFFSET_IS_ADDR); + self.bytes.get_bytes_unchecked_raw_mut() + } } /// Reading and writing. @@ -589,7 +611,8 @@ impl Allocation }; let endian = cx.data_layout().endian; - let dst = self.get_bytes_mut(cx, range)?; + // Yes we do overwrite all the bytes in `dst`. + let dst = self.get_bytes_unchecked_for_overwrite(cx, range)?; write_target_uint(endian, dst, bytes).unwrap(); // See if we have to also store some provenance.