miri: avoid cloning AllocExtra

This commit is contained in:
Ralf Jung 2024-10-12 12:14:28 +02:00
parent fb20e4d3b9
commit bc4366b099
5 changed files with 31 additions and 18 deletions

View File

@ -140,7 +140,7 @@ impl<K: Hash + Eq, V> interpret::AllocMap<K, V> for FxIndexMap<K, V> {
#[inline(always)] #[inline(always)]
fn filter_map_collect<T>(&self, mut f: impl FnMut(&K, &V) -> Option<T>) -> Vec<T> { fn filter_map_collect<T>(&self, mut f: impl FnMut(&K, &V) -> Option<T>) -> Vec<T> {
self.iter().filter_map(move |(k, v)| f(k, &*v)).collect() self.iter().filter_map(move |(k, v)| f(k, v)).collect()
} }
#[inline(always)] #[inline(always)]

View File

@ -993,11 +993,14 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
bytes bytes
} }
/// Find leaked allocations. Allocations reachable from `static_roots` or a `Global` allocation /// Find leaked allocations, remove them from memory and return them. Allocations reachable from
/// are not considered leaked, as well as leaks whose kind's `may_leak()` returns true. /// `static_roots` or a `Global` allocation are not considered leaked, as well as leaks whose
pub fn find_leaked_allocations( /// kind's `may_leak()` returns true.
&self, ///
static_roots: &[AllocId], /// This is highly destructive, no more execution can happen after this!
pub fn take_leaked_allocations(
&mut self,
static_roots: impl FnOnce(&Self) -> &[AllocId],
) -> Vec<(AllocId, MemoryKind<M::MemoryKind>, Allocation<M::Provenance, M::AllocExtra, M::Bytes>)> ) -> Vec<(AllocId, MemoryKind<M::MemoryKind>, Allocation<M::Provenance, M::AllocExtra, M::Bytes>)>
{ {
// Collect the set of allocations that are *reachable* from `Global` allocations. // Collect the set of allocations that are *reachable* from `Global` allocations.
@ -1008,7 +1011,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
self.memory.alloc_map.filter_map_collect(move |&id, &(kind, _)| { self.memory.alloc_map.filter_map_collect(move |&id, &(kind, _)| {
if Some(kind) == global_kind { Some(id) } else { None } if Some(kind) == global_kind { Some(id) } else { None }
}); });
todo.extend(static_roots); todo.extend(static_roots(self));
while let Some(id) = todo.pop() { while let Some(id) = todo.pop() {
if reachable.insert(id) { if reachable.insert(id) {
// This is a new allocation, add the allocation it points to `todo`. // This is a new allocation, add the allocation it points to `todo`.
@ -1023,13 +1026,15 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
}; };
// All allocations that are *not* `reachable` and *not* `may_leak` are considered leaking. // All allocations that are *not* `reachable` and *not* `may_leak` are considered leaking.
self.memory.alloc_map.filter_map_collect(|id, (kind, alloc)| { let leaked: Vec<_> = self.memory.alloc_map.filter_map_collect(|&id, &(kind, _)| {
if kind.may_leak() || reachable.contains(id) { if kind.may_leak() || reachable.contains(&id) { None } else { Some(id) }
None });
} else { let mut result = Vec::new();
Some((*id, *kind, alloc.clone())) for &id in leaked.iter() {
} let (kind, alloc) = self.memory.alloc_map.remove(&id).unwrap();
}) result.push((id, kind, alloc));
}
result
} }
/// Runs the closure in "validation" mode, which means the machine's memory read hooks will be /// Runs the closure in "validation" mode, which means the machine's memory read hooks will be

View File

@ -473,14 +473,14 @@ pub fn report_leaks<'tcx>(
leaks: Vec<(AllocId, MemoryKind, Allocation<Provenance, AllocExtra<'tcx>, MiriAllocBytes>)>, leaks: Vec<(AllocId, MemoryKind, Allocation<Provenance, AllocExtra<'tcx>, MiriAllocBytes>)>,
) { ) {
let mut any_pruned = false; let mut any_pruned = false;
for (id, kind, mut alloc) in leaks { for (id, kind, alloc) in leaks {
let mut title = format!( let mut title = format!(
"memory leaked: {id:?} ({}, size: {:?}, align: {:?})", "memory leaked: {id:?} ({}, size: {:?}, align: {:?})",
kind, kind,
alloc.size().bytes(), alloc.size().bytes(),
alloc.align.bytes() alloc.align.bytes()
); );
let Some(backtrace) = alloc.extra.backtrace.take() else { let Some(backtrace) = alloc.extra.backtrace else {
ecx.tcx.dcx().err(title); ecx.tcx.dcx().err(title);
continue; continue;
}; };

View File

@ -476,7 +476,7 @@ pub fn eval_entry<'tcx>(
} }
// Check for memory leaks. // Check for memory leaks.
info!("Additional static roots: {:?}", ecx.machine.static_roots); info!("Additional static roots: {:?}", ecx.machine.static_roots);
let leaks = ecx.find_leaked_allocations(&ecx.machine.static_roots); let leaks = ecx.take_leaked_allocations(|ecx| &ecx.machine.static_roots);
if !leaks.is_empty() { if !leaks.is_empty() {
report_leaks(&ecx, leaks); report_leaks(&ecx, leaks);
tcx.dcx().note("set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check"); tcx.dcx().note("set `MIRIFLAGS=-Zmiri-ignore-leaks` to disable this check");

View File

@ -321,7 +321,7 @@ impl ProvenanceExtra {
} }
/// Extra per-allocation data /// Extra per-allocation data
#[derive(Debug, Clone)] #[derive(Debug)]
pub struct AllocExtra<'tcx> { pub struct AllocExtra<'tcx> {
/// Global state of the borrow tracker, if enabled. /// Global state of the borrow tracker, if enabled.
pub borrow_tracker: Option<borrow_tracker::AllocState>, pub borrow_tracker: Option<borrow_tracker::AllocState>,
@ -338,6 +338,14 @@ pub struct AllocExtra<'tcx> {
pub backtrace: Option<Vec<FrameInfo<'tcx>>>, pub backtrace: Option<Vec<FrameInfo<'tcx>>>,
} }
// We need a `Clone` impl because the machine passes `Allocation` through `Cow`...
// but that should never end up actually cloning our `AllocExtra`.
impl<'tcx> Clone for AllocExtra<'tcx> {
fn clone(&self) -> Self {
panic!("our allocations should never be cloned");
}
}
impl VisitProvenance for AllocExtra<'_> { impl VisitProvenance for AllocExtra<'_> {
fn visit_provenance(&self, visit: &mut VisitWith<'_>) { fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
let AllocExtra { borrow_tracker, data_race, weak_memory, backtrace: _ } = self; let AllocExtra { borrow_tracker, data_race, weak_memory, backtrace: _ } = self;