Auto merge of #130148 - RalfJung:interpret-get_ptr_alloc_mut, r=saethlin

interpret: get_ptr_alloc_mut: lookup allocation only once

I don't think this will make a big perf difference, but it makes this function symmetric with `get_ptr_alloc` -- and it's always nice to successfully solve a borrow checker puzzle like this. ;)
This commit is contained in:
bors 2024-09-15 08:59:26 +00:00
commit 1ae268816c

View File

@ -7,7 +7,7 @@
//! short-circuiting the empty case! //! short-circuiting the empty case!
use std::assert_matches::assert_matches; use std::assert_matches::assert_matches;
use std::borrow::Cow; use std::borrow::{Borrow, Cow};
use std::collections::VecDeque; use std::collections::VecDeque;
use std::{fmt, mem, ptr}; use std::{fmt, mem, ptr};
@ -386,12 +386,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
size: Size, size: Size,
) -> InterpResult<'tcx, Option<(AllocId, Size, M::ProvenanceExtra)>> { ) -> InterpResult<'tcx, Option<(AllocId, Size, M::ProvenanceExtra)>> {
let size = i64::try_from(size.bytes()).unwrap(); // it would be an error to even ask for more than isize::MAX bytes let size = i64::try_from(size.bytes()).unwrap(); // it would be an error to even ask for more than isize::MAX bytes
self.check_and_deref_ptr( Self::check_and_deref_ptr(
self,
ptr, ptr,
size, size,
CheckInAllocMsg::MemoryAccessTest, CheckInAllocMsg::MemoryAccessTest,
|alloc_id, offset, prov| { |this, alloc_id, offset, prov| {
let (size, align) = self let (size, align) = this
.get_live_alloc_size_and_align(alloc_id, CheckInAllocMsg::MemoryAccessTest)?; .get_live_alloc_size_and_align(alloc_id, CheckInAllocMsg::MemoryAccessTest)?;
Ok((size, align, (alloc_id, offset, prov))) Ok((size, align, (alloc_id, offset, prov)))
}, },
@ -408,8 +409,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
msg: CheckInAllocMsg, msg: CheckInAllocMsg,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
let size = i64::try_from(size.bytes()).unwrap(); // it would be an error to even ask for more than isize::MAX bytes let size = i64::try_from(size.bytes()).unwrap(); // it would be an error to even ask for more than isize::MAX bytes
self.check_and_deref_ptr(ptr, size, msg, |alloc_id, _, _| { Self::check_and_deref_ptr(self, ptr, size, msg, |this, alloc_id, _, _| {
let (size, align) = self.get_live_alloc_size_and_align(alloc_id, msg)?; let (size, align) = this.get_live_alloc_size_and_align(alloc_id, msg)?;
Ok((size, align, ())) Ok((size, align, ()))
})?; })?;
Ok(()) Ok(())
@ -424,8 +425,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
size: i64, size: i64,
msg: CheckInAllocMsg, msg: CheckInAllocMsg,
) -> InterpResult<'tcx> { ) -> InterpResult<'tcx> {
self.check_and_deref_ptr(ptr, size, msg, |alloc_id, _, _| { Self::check_and_deref_ptr(self, ptr, size, msg, |this, alloc_id, _, _| {
let (size, align) = self.get_live_alloc_size_and_align(alloc_id, msg)?; let (size, align) = this.get_live_alloc_size_and_align(alloc_id, msg)?;
Ok((size, align, ())) Ok((size, align, ()))
})?; })?;
Ok(()) Ok(())
@ -439,12 +440,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
/// `alloc_size` will only get called for non-zero-sized accesses. /// `alloc_size` will only get called for non-zero-sized accesses.
/// ///
/// Returns `None` if and only if the size is 0. /// Returns `None` if and only if the size is 0.
fn check_and_deref_ptr<T>( fn check_and_deref_ptr<T, R: Borrow<Self>>(
&self, this: R,
ptr: Pointer<Option<M::Provenance>>, ptr: Pointer<Option<M::Provenance>>,
size: i64, size: i64,
msg: CheckInAllocMsg, msg: CheckInAllocMsg,
alloc_size: impl FnOnce( alloc_size: impl FnOnce(
R,
AllocId, AllocId,
Size, Size,
M::ProvenanceExtra, M::ProvenanceExtra,
@ -455,13 +457,14 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
return Ok(None); return Ok(None);
} }
Ok(match self.ptr_try_get_alloc_id(ptr, size) { Ok(match this.borrow().ptr_try_get_alloc_id(ptr, size) {
Err(addr) => { Err(addr) => {
// We couldn't get a proper allocation. // We couldn't get a proper allocation.
throw_ub!(DanglingIntPointer { addr, inbounds_size: size, msg }); throw_ub!(DanglingIntPointer { addr, inbounds_size: size, msg });
} }
Ok((alloc_id, offset, prov)) => { Ok((alloc_id, offset, prov)) => {
let (alloc_size, _alloc_align, ret_val) = alloc_size(alloc_id, offset, prov)?; let tcx = this.borrow().tcx;
let (alloc_size, _alloc_align, ret_val) = alloc_size(this, alloc_id, offset, prov)?;
let offset = offset.bytes(); let offset = offset.bytes();
// Compute absolute begin and end of the range. // Compute absolute begin and end of the range.
let (begin, end) = if size >= 0 { let (begin, end) = if size >= 0 {
@ -475,7 +478,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
throw_ub!(PointerOutOfBounds { throw_ub!(PointerOutOfBounds {
alloc_id, alloc_id,
alloc_size, alloc_size,
ptr_offset: self.sign_extend_to_target_isize(offset), ptr_offset: tcx.sign_extend_to_target_isize(offset),
inbounds_size: size, inbounds_size: size,
msg, msg,
}) })
@ -669,12 +672,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
) -> InterpResult<'tcx, Option<AllocRef<'a, 'tcx, M::Provenance, M::AllocExtra, M::Bytes>>> ) -> InterpResult<'tcx, Option<AllocRef<'a, 'tcx, M::Provenance, M::AllocExtra, M::Bytes>>>
{ {
let size_i64 = i64::try_from(size.bytes()).unwrap(); // it would be an error to even ask for more than isize::MAX bytes let size_i64 = i64::try_from(size.bytes()).unwrap(); // it would be an error to even ask for more than isize::MAX bytes
let ptr_and_alloc = self.check_and_deref_ptr( let ptr_and_alloc = Self::check_and_deref_ptr(
self,
ptr, ptr,
size_i64, size_i64,
CheckInAllocMsg::MemoryAccessTest, CheckInAllocMsg::MemoryAccessTest,
|alloc_id, offset, prov| { |this, alloc_id, offset, prov| {
let alloc = self.get_alloc_raw(alloc_id)?; let alloc = this.get_alloc_raw(alloc_id)?;
Ok((alloc.size(), alloc.align, (alloc_id, offset, prov, alloc))) Ok((alloc.size(), alloc.align, (alloc_id, offset, prov, alloc)))
}, },
)?; )?;
@ -726,7 +730,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// We have "NLL problem case #3" here, which cannot be worked around without loss of // We have "NLL problem case #3" here, which cannot be worked around without loss of
// efficiency even for the common case where the key is in the map. // efficiency even for the common case where the key is in the map.
// <https://rust-lang.github.io/rfcs/2094-nll.html#problem-case-3-conditional-control-flow-across-functions> // <https://rust-lang.github.io/rfcs/2094-nll.html#problem-case-3-conditional-control-flow-across-functions>
// (Cannot use `get_mut_or` since `get_global_alloc` needs `&self`.) // (Cannot use `get_mut_or` since `get_global_alloc` needs `&self`, and that boils down to
// Miri's `adjust_alloc_root_pointer` needing to look up the size of the allocation.
// It could be avoided with a totally separate codepath in Miri for handling the absolute address
// of global allocations, but that's not worth it.)
if self.memory.alloc_map.get_mut(id).is_none() { if self.memory.alloc_map.get_mut(id).is_none() {
// Slow path. // Slow path.
// Allocation not found locally, go look global. // Allocation not found locally, go look global.
@ -762,13 +769,22 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
size: Size, size: Size,
) -> InterpResult<'tcx, Option<AllocRefMut<'a, 'tcx, M::Provenance, M::AllocExtra, M::Bytes>>> ) -> InterpResult<'tcx, Option<AllocRefMut<'a, 'tcx, M::Provenance, M::AllocExtra, M::Bytes>>>
{ {
let parts = self.get_ptr_access(ptr, size)?; let tcx = self.tcx;
if let Some((alloc_id, offset, prov)) = parts { let validation_in_progress = self.memory.validation_in_progress;
let tcx = self.tcx;
let validation_in_progress = self.memory.validation_in_progress; let size_i64 = i64::try_from(size.bytes()).unwrap(); // it would be an error to even ask for more than isize::MAX bytes
// FIXME: can we somehow avoid looking up the allocation twice here? let ptr_and_alloc = Self::check_and_deref_ptr(
// We cannot call `get_raw_mut` inside `check_and_deref_ptr` as that would duplicate `&mut self`. self,
let (alloc, machine) = self.get_alloc_raw_mut(alloc_id)?; ptr,
size_i64,
CheckInAllocMsg::MemoryAccessTest,
|this, alloc_id, offset, prov| {
let (alloc, machine) = this.get_alloc_raw_mut(alloc_id)?;
Ok((alloc.size(), alloc.align, (alloc_id, offset, prov, alloc, machine)))
},
)?;
if let Some((alloc_id, offset, prov, alloc, machine)) = ptr_and_alloc {
let range = alloc_range(offset, size); let range = alloc_range(offset, size);
if !validation_in_progress { if !validation_in_progress {
M::before_memory_write(tcx, machine, &mut alloc.extra, (alloc_id, prov), range)?; M::before_memory_write(tcx, machine, &mut alloc.extra, (alloc_id, prov), range)?;