Rollup merge of #122537 - RalfJung:interpret-allocation, r=oli-obk
interpret/allocation: fix aliasing issue in interpreter and refactor getters a bit That new raw getter will be needed to let Miri pass pointers to natively executed FFI code ("extern-so" mode). While doing that I realized our get_bytes_mut are named less scary than get_bytes_unchecked so I rectified that. Also I realized `mem_copy_repeatedly` would break if we called it for multiple overlapping copies so I made sure this does not happen. And I realized that we are actually [violating Stacked Borrows in the interpreter](https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/I.20think.20Miri.20violates.20Stacked.20Borrows.20.F0.9F.99.88).^^ That was introduced in https://github.com/rust-lang/rust/pull/87777. r? ```@oli-obk```
This commit is contained in:
commit
84e55be8da
2 changed files with 45 additions and 14 deletions
|
@ -37,9 +37,16 @@ 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<Self>;
|
||||
|
||||
/// Gives direct access to the raw underlying storage.
|
||||
///
|
||||
/// 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]>`.
|
||||
/// Default `bytes` for `Allocation` is a `Box<u8>`.
|
||||
impl AllocBytes for Box<[u8]> {
|
||||
fn from_bytes<'a>(slice: impl Into<Cow<'a, [u8]>>, _align: Align) -> Self {
|
||||
Box::<[u8]>::from(slice.into())
|
||||
|
@ -51,6 +58,11 @@ impl AllocBytes for Box<[u8]> {
|
|||
let bytes = unsafe { bytes.assume_init() };
|
||||
Some(bytes)
|
||||
}
|
||||
|
||||
fn as_mut_ptr(&mut self) -> *mut u8 {
|
||||
// Carefully avoiding any intermediate references.
|
||||
ptr::addr_of_mut!(**self).cast()
|
||||
}
|
||||
}
|
||||
|
||||
/// This type represents an Allocation in the Miri/CTFE core engine.
|
||||
|
@ -399,10 +411,6 @@ impl<Prov: Provenance, Extra, Bytes: AllocBytes> Allocation<Prov, Extra, Bytes>
|
|||
|
||||
/// Byte accessors.
|
||||
impl<Prov: Provenance, Extra, Bytes: AllocBytes> Allocation<Prov, Extra, Bytes> {
|
||||
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 +460,14 @@ impl<Prov: Provenance, Extra, Bytes: AllocBytes> Allocation<Prov, Extra, Bytes>
|
|||
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 +478,9 @@ impl<Prov: Provenance, Extra, Bytes: AllocBytes> Allocation<Prov, Extra, Bytes>
|
|||
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 +489,19 @@ 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
|
||||
// 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 `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.as_mut_ptr()
|
||||
}
|
||||
}
|
||||
|
||||
/// Reading and writing.
|
||||
|
@ -589,7 +608,8 @@ impl<Prov: Provenance, Extra, Bytes: AllocBytes> Allocation<Prov, Extra, Bytes>
|
|||
};
|
||||
|
||||
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.
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue