Skip to content

miri engine: keep around some information for dead allocations #54254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 46 additions & 5 deletions src/librustc_mir/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ pub struct Memory<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'mir, 'tcx>> {
/// a static creates a copy here, in the machine.
alloc_map: FxHashMap<AllocId, (MemoryKind<M::MemoryKinds>, Allocation)>,

/// To be able to compare pointers with NULL, and to check alignment for accesses
/// to ZSTs (where pointers may dangle), we keep track of the size even for allocations
/// that do not exist any more.
dead_alloc_map: FxHashMap<AllocId, (Size, Align)>,

pub tcx: TyCtxtAt<'a, 'tcx, 'tcx>,
}

Expand All @@ -74,6 +79,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
Memory {
data,
alloc_map: FxHashMap::default(),
dead_alloc_map: FxHashMap::default(),
tcx,
}
}
Expand Down Expand Up @@ -150,6 +156,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
size_and_align: Option<(Size, Align)>,
kind: MemoryKind<M::MemoryKinds>,
) -> EvalResult<'tcx> {
debug!("deallocating: {}", ptr.alloc_id);

if ptr.offset.bytes() != 0 {
return err!(DeallocateNonBasePtr);
}
Expand Down Expand Up @@ -189,23 +197,41 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
}
}

debug!("deallocated : {}", ptr.alloc_id);
// Don't forget to remember size and align of this now-dead allocation
let old = self.dead_alloc_map.insert(
ptr.alloc_id,
(Size::from_bytes(alloc.bytes.len() as u64), alloc.align)
);
if old.is_some() {
bug!("Nothing can be deallocated twice");
}

Ok(())
}

/// Check that the pointer is aligned AND non-NULL. This supports scalars
/// for the benefit of other parts of miri that need to check alignment even for ZST.
/// Check that the pointer is aligned AND non-NULL. This supports ZSTs in two ways:
/// You can pass a scalar, and a `Pointer` does not have to actually still be allocated.
pub fn check_align(&self, ptr: Scalar, required_align: Align) -> EvalResult<'tcx> {
// Check non-NULL/Undef, extract offset
let (offset, alloc_align) = match ptr {
Scalar::Ptr(ptr) => {
let alloc = self.get(ptr.alloc_id)?;
(ptr.offset.bytes(), alloc.align)
let (size, align) = self.get_size_and_align(ptr.alloc_id)?;
// check this is not NULL -- which we can ensure only if this is in-bounds
// of some (potentially dead) allocation.
if ptr.offset > size {
return err!(PointerOutOfBounds {
ptr,
access: true,
allocation_size: size,
});
};
// keep data for alignment check
(ptr.offset.bytes(), align)
}
Scalar::Bits { bits, size } => {
assert_eq!(size as u64, self.pointer_size().bytes());
assert!(bits < (1u128 << self.pointer_size().bits()));
// check this is not NULL
if bits == 0 {
return err!(InvalidNullPointerUsage);
}
Expand Down Expand Up @@ -306,6 +332,21 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
}
}

pub fn get_size_and_align(&self, id: AllocId) -> EvalResult<'tcx, (Size, Align)> {
Ok(match self.get(id) {
Ok(alloc) => (Size::from_bytes(alloc.bytes.len() as u64), alloc.align),
Err(err) => match err.kind {
EvalErrorKind::DanglingPointerDeref =>
// This should be in the dead allocation map
*self.dead_alloc_map.get(&id).expect(
"allocation missing in dead_alloc_map"
),
// E.g. a function ptr allocation
_ => return Err(err)
}
})
}

pub fn get_mut(
&mut self,
id: AllocId,
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
}
let (ptr, ptr_align) = mplace.to_scalar_ptr_align();

if mplace.layout.size.bytes() == 0 {
if mplace.layout.is_zst() {
// Not all ZSTs have a layout we would handle below, so just short-circuit them
// all here.
self.memory.check_align(ptr, ptr_align)?;
Expand Down Expand Up @@ -376,14 +376,14 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
})
}

// Take an operand, representing a pointer, and dereference it -- that
// Take an operand, representing a pointer, and dereference it to a place -- that
// will always be a MemPlace.
pub(super) fn deref_operand(
&self,
src: OpTy<'tcx>,
) -> EvalResult<'tcx, MPlaceTy<'tcx>> {
let val = self.read_value(src)?;
trace!("deref to {} on {:?}", val.layout.ty, val);
trace!("deref to {} on {:?}", val.layout.ty, *val);
Ok(self.ref_to_mplace(val)?)
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
// wrong type.

// Nothing to do for ZSTs, other than checking alignment
if dest.layout.size.bytes() == 0 {
if dest.layout.is_zst() {
self.memory.check_align(ptr, ptr_align)?;
return Ok(());
}
Expand Down