Skip to content

Commit

Permalink
store allocation bytes in Vec to avoid aliasing trouble
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Mar 15, 2024
1 parent fb3e79d commit e36e63e
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 24 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {

type AllocExtra = ();
type FrameExtra = ();
type Bytes = Box<[u8]>;
type Bytes = Vec<u8>;

#[inline(always)]
fn ignore_optional_overflow_checks(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,15 @@ pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> {
/// A reference to some allocation that was already bounds-checked for the given region
/// and had the on-access machine hooks run.
#[derive(Copy, Clone)]
pub struct AllocRef<'a, 'tcx, Prov: Provenance, Extra, Bytes: AllocBytes = Box<[u8]>> {
pub struct AllocRef<'a, 'tcx, Prov: Provenance, Extra, Bytes: AllocBytes = Vec<u8>> {
alloc: &'a Allocation<Prov, Extra, Bytes>,
range: AllocRange,
tcx: TyCtxt<'tcx>,
alloc_id: AllocId,
}
/// A reference to some allocation that was already bounds-checked for the given region
/// and had the on-access machine hooks run.
pub struct AllocRefMut<'a, 'tcx, Prov: Provenance, Extra, Bytes: AllocBytes = Box<[u8]>> {
pub struct AllocRefMut<'a, 'tcx, Prov: Provenance, Extra, Bytes: AllocBytes = Vec<u8>> {
alloc: &'a mut Allocation<Prov, Extra, Bytes>,
range: AllocRange,
tcx: TyCtxt<'tcx>,
Expand Down
36 changes: 16 additions & 20 deletions compiler/rustc_middle/src/mir/interpret/allocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,32 +38,29 @@ pub trait AllocBytes:
/// Returns `None` if we ran out of memory on the host.
fn zeroed(size: Size, _align: Align) -> Option<Self>;

/// Gives direct access to the raw underlying storage. Must be aligned according to the `align`
/// passed during construction.
/// Gives direct access to the raw underlying storage.
///
/// 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;
/// Crucially this pointer is compatible with:
/// - other pointers retunred by this method, and
/// - references returned from `deref()`, as long as there was no write.
fn as_mut_ptr(&mut self) -> *mut u8;
}

// Default `bytes` for `Allocation` is a `Box<[u8]>`.
impl AllocBytes for Box<[u8]> {
// Default `bytes` for `Allocation` is a `Vec<u8>`.
impl AllocBytes for Vec<u8> {
fn from_bytes<'a>(slice: impl Into<Cow<'a, [u8]>>, _align: Align) -> Self {
Box::<[u8]>::from(slice.into())
slice.into().into_owned()
}

fn zeroed(size: Size, _align: Align) -> Option<Self> {
let bytes = Box::<[u8]>::try_new_zeroed_slice(size.bytes_usize()).ok()?;
// SAFETY: the box was zero-allocated, which is a valid initial value for Box<[u8]>
let bytes = unsafe { bytes.assume_init() };
Some(bytes)
Some(bytes.into())
}

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");
fn as_mut_ptr(&mut self) -> *mut u8 {
Vec::as_mut_ptr(self)
}
}

Expand All @@ -76,7 +73,7 @@ impl AllocBytes for Box<[u8]> {
// hashed. (see the `Hash` impl below for more details), so the impl is not derived.
#[derive(Clone, Eq, PartialEq, TyEncodable, TyDecodable)]
#[derive(HashStable)]
pub struct Allocation<Prov: Provenance = CtfeProvenance, Extra = (), Bytes = Box<[u8]>> {
pub struct Allocation<Prov: Provenance = CtfeProvenance, Extra = (), Bytes = Vec<u8>> {
/// The actual bytes of the allocation.
/// Note that the bytes of a pointer represent the offset of the pointer.
bytes: Bytes,
Expand Down Expand Up @@ -491,19 +488,18 @@ impl<Prov: Provenance, Extra, Bytes: AllocBytes> Allocation<Prov, Extra, Bytes>
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...
// Cruciall, we go via `AllocBytes::as_mut_ptr`, not `AllocBytes::deref_mut`.
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.
/// without reseting anything. Directly exposes `AllocBytes::as_mut_ptr`. 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()
self.bytes.as_mut_ptr()
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {

type Provenance = Provenance;
type ProvenanceExtra = ProvenanceExtra;
type Bytes = Box<[u8]>;
type Bytes = Vec<u8>;

type MemoryMap = MonoHashMap<
AllocId,
Expand Down

0 comments on commit e36e63e

Please sign in to comment.