Skip to content

Commit b614d67

Browse files
rust-timerrust-timer
authored
Unrolled build for rust-lang#131593
Rollup merge of rust-lang#131593 - RalfJung:alloc-no-clone, r=saethlin miri: avoid cloning AllocExtra We shouldn't be cloning Miri allocations, so make `AllocExtra::clone` panic instead, and adjust the one case where we *do* clone (the leak check) to avoid cloning. This is in preparation for rust-lang/miri#3966 where I am adding something to `AllocExtra` that cannot (easily) be cloned. r? ``@saethlin``
2 parents f6648f2 + bc4366b commit b614d67

File tree

5 files changed

+31
-18
lines changed

5 files changed

+31
-18
lines changed

‎compiler/rustc_const_eval/src/const_eval/machine.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ impl<K: Hash + Eq, V> interpret::AllocMap<K, V> for FxIndexMap<K, V> {
140140

141141
#[inline(always)]
142142
fn filter_map_collect<T>(&self, mut f: impl FnMut(&K, &V) -> Option<T>) -> Vec<T> {
143-
self.iter().filter_map(move |(k, v)| f(k, &*v)).collect()
143+
self.iter().filter_map(move |(k, v)| f(k, v)).collect()
144144
}
145145

146146
#[inline(always)]

‎compiler/rustc_const_eval/src/interpret/memory.rs

+18-13
Original file line numberDiff line numberDiff line change
@@ -993,11 +993,14 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
993993
bytes
994994
}
995995

996-
/// Find leaked allocations. Allocations reachable from `static_roots` or a `Global` allocation
997-
/// are not considered leaked, as well as leaks whose kind's `may_leak()` returns true.
998-
pub fn find_leaked_allocations(
999-
&self,
1000-
static_roots: &[AllocId],
996+
/// Find leaked allocations, remove them from memory and return them. Allocations reachable from
997+
/// `static_roots` or a `Global` allocation are not considered leaked, as well as leaks whose
998+
/// kind's `may_leak()` returns true.
999+
///
1000+
/// This is highly destructive, no more execution can happen after this!
1001+
pub fn take_leaked_allocations(
1002+
&mut self,
1003+
static_roots: impl FnOnce(&Self) -> &[AllocId],
10011004
) -> Vec<(AllocId, MemoryKind<M::MemoryKind>, Allocation<M::Provenance, M::AllocExtra, M::Bytes>)>
10021005
{
10031006
// Collect the set of allocations that are *reachable* from `Global` allocations.
@@ -1008,7 +1011,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
10081011
self.memory.alloc_map.filter_map_collect(move |&id, &(kind, _)| {
10091012
if Some(kind) == global_kind { Some(id) } else { None }
10101013
});
1011-
todo.extend(static_roots);
1014+
todo.extend(static_roots(self));
10121015
while let Some(id) = todo.pop() {
10131016
if reachable.insert(id) {
10141017
// This is a new allocation, add the allocation it points to `todo`.
@@ -1023,13 +1026,15 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
10231026
};
10241027

10251028
// All allocations that are *not* `reachable` and *not* `may_leak` are considered leaking.
1026-
self.memory.alloc_map.filter_map_collect(|id, (kind, alloc)| {
1027-
if kind.may_leak() || reachable.contains(id) {
1028-
None
1029-
} else {
1030-
Some((*id, *kind, alloc.clone()))
1031-
}
1032-
})
1029+
let leaked: Vec<_> = self.memory.alloc_map.filter_map_collect(|&id, &(kind, _)| {
1030+
if kind.may_leak() || reachable.contains(&id) { None } else { Some(id) }
1031+
});
1032+
let mut result = Vec::new();
1033+
for &id in leaked.iter() {
1034+
let (kind, alloc) = self.memory.alloc_map.remove(&id).unwrap();
1035+
result.push((id, kind, alloc));
1036+
}
1037+
result
10331038
}
10341039

10351040
/// Runs the closure in "validation" mode, which means the machine's memory read hooks will be

‎src/tools/miri/src/diagnostics.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -473,14 +473,14 @@ pub fn report_leaks<'tcx>(
473473
leaks: Vec<(AllocId, MemoryKind, Allocation<Provenance, AllocExtra<'tcx>, MiriAllocBytes>)>,
474474
) {
475475
let mut any_pruned = false;
476-
for (id, kind, mut alloc) in leaks {
476+
for (id, kind, alloc) in leaks {
477477
let mut title = format!(
478478
"memory leaked: {id:?} ({}, size: {:?}, align: {:?})",
479479
kind,
480480
alloc.size().bytes(),
481481
alloc.align.bytes()
482482
);
483-
let Some(backtrace) = alloc.extra.backtrace.take() else {
483+
let Some(backtrace) = alloc.extra.backtrace else {
484484
ecx.tcx.dcx().err(title);
485485
continue;
486486
};

‎src/tools/miri/src/eval.rs

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

‎src/tools/miri/src/machine.rs

+9-1
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ impl ProvenanceExtra {
321321
}
322322

323323
/// Extra per-allocation data
324-
#[derive(Debug, Clone)]
324+
#[derive(Debug)]
325325
pub struct AllocExtra<'tcx> {
326326
/// Global state of the borrow tracker, if enabled.
327327
pub borrow_tracker: Option<borrow_tracker::AllocState>,
@@ -338,6 +338,14 @@ pub struct AllocExtra<'tcx> {
338338
pub backtrace: Option<Vec<FrameInfo<'tcx>>>,
339339
}
340340

341+
// We need a `Clone` impl because the machine passes `Allocation` through `Cow`...
342+
// but that should never end up actually cloning our `AllocExtra`.
343+
impl<'tcx> Clone for AllocExtra<'tcx> {
344+
fn clone(&self) -> Self {
345+
panic!("our allocations should never be cloned");
346+
}
347+
}
348+
341349
impl VisitProvenance for AllocExtra<'_> {
342350
fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
343351
let AllocExtra { borrow_tracker, data_race, weak_memory, backtrace: _ } = self;

0 commit comments

Comments
 (0)