Skip to content

Commit 606ca4d

Browse files
saethlinsaethlin
committed
Report a backtrace for memory leaks under Miri
1 parent fd57c6b commit 606ca4d

File tree

13 files changed

+145
-60
lines changed

13 files changed

+145
-60
lines changed

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,10 @@ pub struct Frame<'mir, 'tcx, Prov: Provenance = AllocId, Extra = ()> {
132132
}
133133

134134
/// What we store about a frame in an interpreter backtrace.
135-
#[derive(Debug)]
135+
#[derive(Clone, Debug)]
136136
pub struct FrameInfo<'tcx> {
137137
pub instance: ty::Instance<'tcx>,
138138
pub span: Span,
139-
pub lint_root: Option<hir::HirId>,
140139
}
141140

142141
#[derive(Clone, Copy, Eq, PartialEq, Debug)] // Miri debug-prints these
@@ -947,10 +946,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
947946
// This deliberately does *not* honor `requires_caller_location` since it is used for much
948947
// more than just panics.
949948
for frame in stack.iter().rev() {
950-
let lint_root = frame.lint_root();
951949
let span = frame.current_span();
952-
953-
frames.push(FrameInfo { span, instance: frame.instance, lint_root });
950+
frames.push(FrameInfo { span, instance: frame.instance });
954951
}
955952
trace!("generate stacktrace: {:#?}", frames);
956953
frames

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
104104
type FrameExtra;
105105

106106
/// Extra data stored in every allocation.
107-
type AllocExtra: Debug + Clone + 'static;
107+
type AllocExtra: Debug + Clone + 'tcx;
108108

109109
/// Type for the bytes of the allocation.
110110
type Bytes: AllocBytes + 'static;

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

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
215215
self.allocate_raw_ptr(alloc, kind)
216216
}
217217

218-
/// This can fail only of `alloc` contains provenance.
218+
/// This can fail only if `alloc` contains provenance.
219219
pub fn allocate_raw_ptr(
220220
&mut self,
221221
alloc: Allocation,
@@ -807,9 +807,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
807807
DumpAllocs { ecx: self, allocs }
808808
}
809809

810-
/// Print leaked memory. Allocations reachable from `static_roots` or a `Global` allocation
811-
/// are not considered leaked. Leaks whose kind `may_leak()` returns true are not reported.
812-
pub fn leak_report(&self, static_roots: &[AllocId]) -> usize {
810+
/// Find leaked allocations. Allocations reachable from `static_roots` or a `Global` allocation
811+
/// are not considered leaked, as well as leaks whose kind's `may_leak()` returns true.
812+
pub fn find_leaked_allocations(
813+
&self,
814+
static_roots: &[AllocId],
815+
) -> Vec<(AllocId, MemoryKind<M::MemoryKind>, Allocation<M::Provenance, M::AllocExtra, M::Bytes>)>
816+
{
813817
// Collect the set of allocations that are *reachable* from `Global` allocations.
814818
let reachable = {
815819
let mut reachable = FxHashSet::default();
@@ -833,14 +837,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
833837
};
834838

835839
// All allocations that are *not* `reachable` and *not* `may_leak` are considered leaking.
836-
let leaks: Vec<_> = self.memory.alloc_map.filter_map_collect(|&id, &(kind, _)| {
837-
if kind.may_leak() || reachable.contains(&id) { None } else { Some(id) }
838-
});
839-
let n = leaks.len();
840-
if n > 0 {
841-
eprintln!("The following memory was leaked: {:?}", self.dump_allocs(leaks));
842-
}
843-
n
840+
self.memory.alloc_map.filter_map_collect(|id, (kind, alloc)| {
841+
if kind.may_leak() || reachable.contains(id) {
842+
None
843+
} else {
844+
Some((*id, *kind, alloc.clone()))
845+
}
846+
})
844847
}
845848
}
846849

‎src/tools/miri/src/borrow_tracker/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ pub enum AllocState {
352352
TreeBorrows(Box<RefCell<tree_borrows::AllocState>>),
353353
}
354354

355-
impl machine::AllocExtra {
355+
impl machine::AllocExtra<'_> {
356356
#[track_caller]
357357
pub fn borrow_tracker_sb(&self) -> &RefCell<stacked_borrows::AllocState> {
358358
match self.borrow_tracker {

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

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ pub enum NonHaltingDiagnostic {
105105
}
106106

107107
/// Level of Miri specific diagnostics
108-
enum DiagLevel {
108+
pub enum DiagLevel {
109109
Error,
110110
Warning,
111111
Note,
@@ -114,7 +114,7 @@ enum DiagLevel {
114114
/// Attempts to prune a stacktrace to omit the Rust runtime, and returns a bool indicating if any
115115
/// frames were pruned. If the stacktrace does not have any local frames, we conclude that it must
116116
/// be pointing to a problem in the Rust runtime itself, and do not prune it at all.
117-
fn prune_stacktrace<'tcx>(
117+
pub fn prune_stacktrace<'tcx>(
118118
mut stacktrace: Vec<FrameInfo<'tcx>>,
119119
machine: &MiriMachine<'_, 'tcx>,
120120
) -> (Vec<FrameInfo<'tcx>>, bool) {
@@ -338,12 +338,45 @@ pub fn report_error<'tcx, 'mir>(
338338
None
339339
}
340340

341+
pub fn report_leaks<'mir, 'tcx>(
342+
ecx: &InterpCx<'mir, 'tcx, MiriMachine<'mir, 'tcx>>,
343+
leaks: Vec<(AllocId, MemoryKind<MiriMemoryKind>, Allocation<Provenance, AllocExtra<'tcx>>)>,
344+
) {
345+
let mut any_pruned = false;
346+
for (id, kind, mut alloc) in leaks {
347+
let Some(backtrace) = alloc.extra.backtrace.take() else {
348+
continue;
349+
};
350+
let (backtrace, pruned) = prune_stacktrace(backtrace, &ecx.machine);
351+
any_pruned |= pruned;
352+
report_msg(
353+
DiagLevel::Error,
354+
&format!(
355+
"memory leaked: {id:?} ({}, size: {:?}, align: {:?}), allocated here:",
356+
kind,
357+
alloc.size().bytes(),
358+
alloc.align.bytes()
359+
),
360+
vec![],
361+
vec![],
362+
vec![],
363+
&backtrace,
364+
&ecx.machine,
365+
);
366+
}
367+
if any_pruned {
368+
ecx.tcx.sess.diagnostic().note_without_error(
369+
"some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace",
370+
);
371+
}
372+
}
373+
341374
/// Report an error or note (depending on the `error` argument) with the given stacktrace.
342375
/// Also emits a full stacktrace of the interpreter stack.
343376
/// We want to present a multi-line span message for some errors. Diagnostics do not support this
344377
/// directly, so we pass the lines as a `Vec<String>` and display each line after the first with an
345378
/// additional `span_label` or `note` call.
346-
fn report_msg<'tcx>(
379+
pub fn report_msg<'tcx>(
347380
diag_level: DiagLevel,
348381
title: &str,
349382
span_msg: Vec<String>,

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use std::thread;
1010
use log::info;
1111

1212
use crate::borrow_tracker::RetagFields;
13+
use crate::diagnostics::report_leaks;
1314
use rustc_data_structures::fx::FxHashSet;
1415
use rustc_hir::def::Namespace;
1516
use rustc_hir::def_id::DefId;
@@ -457,10 +458,12 @@ pub fn eval_entry<'tcx>(
457458
}
458459
// Check for memory leaks.
459460
info!("Additonal static roots: {:?}", ecx.machine.static_roots);
460-
let leaks = ecx.leak_report(&ecx.machine.static_roots);
461-
if leaks != 0 {
462-
tcx.sess.err("the evaluated program leaked memory");
463-
tcx.sess.note_without_error("pass `-Zmiri-ignore-leaks` to disable this check");
461+
let leaks = ecx.find_leaked_allocations(&ecx.machine.static_roots);
462+
if !leaks.is_empty() {
463+
report_leaks(&ecx, leaks);
464+
tcx.sess.note_without_error(
465+
"the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check",
466+
);
464467
// Ignore the provided return code - let the reported error
465468
// determine the return code.
466469
return None;

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

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -253,20 +253,24 @@ impl ProvenanceExtra {
253253

254254
/// Extra per-allocation data
255255
#[derive(Debug, Clone)]
256-
pub struct AllocExtra {
256+
pub struct AllocExtra<'tcx> {
257257
/// Global state of the borrow tracker, if enabled.
258258
pub borrow_tracker: Option<borrow_tracker::AllocState>,
259-
/// Data race detection via the use of a vector-clock,
260-
/// this is only added if it is enabled.
259+
/// Data race detection via the use of a vector-clock.
260+
/// This is only added if it is enabled.
261261
pub data_race: Option<data_race::AllocState>,
262-
/// Weak memory emulation via the use of store buffers,
263-
/// this is only added if it is enabled.
262+
/// Weak memory emulation via the use of store buffers.
263+
/// This is only added if it is enabled.
264264
pub weak_memory: Option<weak_memory::AllocState>,
265+
/// A backtrace to where this allocation was allocated.
266+
/// As this is recorded for leak reports, it only exists
267+
/// if this allocation is leakable.
268+
pub backtrace: Option<Vec<FrameInfo<'tcx>>>,
265269
}
266270

267-
impl VisitTags for AllocExtra {
271+
impl VisitTags for AllocExtra<'_> {
268272
fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
269-
let AllocExtra { borrow_tracker, data_race, weak_memory } = self;
273+
let AllocExtra { borrow_tracker, data_race, weak_memory, backtrace: _ } = self;
270274

271275
borrow_tracker.visit_tags(visit);
272276
data_race.visit_tags(visit);
@@ -773,7 +777,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
773777
type ExtraFnVal = Dlsym;
774778

775779
type FrameExtra = FrameExtra<'tcx>;
776-
type AllocExtra = AllocExtra;
780+
type AllocExtra = AllocExtra<'tcx>;
777781

778782
type Provenance = Provenance;
779783
type ProvenanceExtra = ProvenanceExtra;
@@ -967,9 +971,20 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
967971
)
968972
});
969973
let buffer_alloc = ecx.machine.weak_memory.then(weak_memory::AllocState::new_allocation);
974+
975+
// If an allocation is leaked, we want to report a backtrace to indicate where it was
976+
// allocated. We don't need to record a backtrace for allocations which are allowed to
977+
// leak.
978+
let backtrace = if kind.may_leak() { None } else { Some(ecx.generate_stacktrace()) };
979+
970980
let alloc: Allocation<Provenance, Self::AllocExtra> = alloc.adjust_from_tcx(
971981
&ecx.tcx,
972-
AllocExtra { borrow_tracker, data_race: race_alloc, weak_memory: buffer_alloc },
982+
AllocExtra {
983+
borrow_tracker,
984+
data_race: race_alloc,
985+
weak_memory: buffer_alloc,
986+
backtrace,
987+
},
973988
|ptr| ecx.global_base_pointer(ptr),
974989
)?;
975990
Ok(Cow::Owned(alloc))
@@ -1049,7 +1064,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
10491064
fn before_memory_read(
10501065
_tcx: TyCtxt<'tcx>,
10511066
machine: &Self,
1052-
alloc_extra: &AllocExtra,
1067+
alloc_extra: &AllocExtra<'tcx>,
10531068
(alloc_id, prov_extra): (AllocId, Self::ProvenanceExtra),
10541069
range: AllocRange,
10551070
) -> InterpResult<'tcx> {
@@ -1069,7 +1084,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
10691084
fn before_memory_write(
10701085
_tcx: TyCtxt<'tcx>,
10711086
machine: &mut Self,
1072-
alloc_extra: &mut AllocExtra,
1087+
alloc_extra: &mut AllocExtra<'tcx>,
10731088
(alloc_id, prov_extra): (AllocId, Self::ProvenanceExtra),
10741089
range: AllocRange,
10751090
) -> InterpResult<'tcx> {
@@ -1089,7 +1104,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
10891104
fn before_memory_deallocation(
10901105
_tcx: TyCtxt<'tcx>,
10911106
machine: &mut Self,
1092-
alloc_extra: &mut AllocExtra,
1107+
alloc_extra: &mut AllocExtra<'tcx>,
10931108
(alloc_id, prove_extra): (AllocId, Self::ProvenanceExtra),
10941109
range: AllocRange,
10951110
) -> InterpResult<'tcx> {

‎src/tools/miri/src/tag_gc.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ impl VisitTags for Operand<Provenance> {
125125
}
126126
}
127127

128-
impl VisitTags for Allocation<Provenance, AllocExtra> {
128+
impl VisitTags for Allocation<Provenance, AllocExtra<'_>> {
129129
fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
130130
for prov in self.provenance().provenances() {
131131
prov.visit_tags(visit);

‎src/tools/miri/tests/fail/memleak.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//@error-pattern: the evaluated program leaked memory
1+
//@error-pattern: memory leaked
22
//@normalize-stderr-test: ".*│.*" -> "$$stripped$$"
33

44
fn main() {

‎src/tools/miri/tests/fail/memleak.stderr

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,21 @@
1-
The following memory was leaked: ALLOC (Rust heap, size: 4, align: 4) {
2-
$stripped$
3-
}
1+
error: memory leaked: ALLOC (Rust heap, size: 4, align: 4), allocated here:
2+
--> RUSTLIB/alloc/src/alloc.rs:LL:CC
3+
|
4+
LL | unsafe { __rust_alloc(layout.size(), layout.align()) }
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: inside `std::alloc::alloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
8+
= note: inside `std::alloc::Global::alloc_impl` at RUSTLIB/alloc/src/alloc.rs:LL:CC
9+
= note: inside `<std::alloc::Global as std::alloc::Allocator>::allocate` at RUSTLIB/alloc/src/alloc.rs:LL:CC
10+
= note: inside `alloc::alloc::exchange_malloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
11+
= note: inside `std::boxed::Box::<i32>::new` at RUSTLIB/alloc/src/boxed.rs:LL:CC
12+
note: inside `main`
13+
--> $DIR/memleak.rs:LL:CC
14+
|
15+
LL | std::mem::forget(Box::new(42));
16+
| ^^^^^^^^^^^^
417

5-
error: the evaluated program leaked memory
6-
7-
note: pass `-Zmiri-ignore-leaks` to disable this check
18+
note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check
819

920
error: aborting due to previous error
1021

‎src/tools/miri/tests/fail/memleak_rc.32bit.stderr

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,22 @@
1-
The following memory was leaked: ALLOC (Rust heap, size: 16, align: 4) {
2-
$stripped$
3-
}
1+
error: memory leaked: ALLOC (Rust heap, size: 16, align: 4), allocated here:
2+
--> RUSTLIB/alloc/src/alloc.rs:LL:CC
3+
|
4+
LL | unsafe { __rust_alloc(layout.size(), layout.align()) }
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: inside `std::alloc::alloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
8+
= note: inside `std::alloc::Global::alloc_impl` at RUSTLIB/alloc/src/alloc.rs:LL:CC
9+
= note: inside `<std::alloc::Global as std::alloc::Allocator>::allocate` at RUSTLIB/alloc/src/alloc.rs:LL:CC
10+
= note: inside `alloc::alloc::exchange_malloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
11+
= note: inside `std::boxed::Box::<std::rc::RcBox<std::cell::RefCell<std::option::Option<Dummy>>>>::new` at RUSTLIB/alloc/src/boxed.rs:LL:CC
12+
= note: inside `std::rc::Rc::<std::cell::RefCell<std::option::Option<Dummy>>>::new` at RUSTLIB/alloc/src/rc.rs:LL:CC
13+
note: inside `main`
14+
--> $DIR/memleak_rc.rs:LL:CC
15+
|
16+
LL | let x = Dummy(Rc::new(RefCell::new(None)));
17+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
418

5-
error: the evaluated program leaked memory
6-
7-
note: pass `-Zmiri-ignore-leaks` to disable this check
19+
note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check
820

921
error: aborting due to previous error
1022

‎src/tools/miri/tests/fail/memleak_rc.64bit.stderr

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,22 @@
1-
The following memory was leaked: ALLOC (Rust heap, size: 32, align: 8) {
2-
$stripped$
3-
$stripped$
4-
}
1+
error: memory leaked: ALLOC (Rust heap, size: 32, align: 8), allocated here:
2+
--> RUSTLIB/alloc/src/alloc.rs:LL:CC
3+
|
4+
LL | unsafe { __rust_alloc(layout.size(), layout.align()) }
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: inside `std::alloc::alloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
8+
= note: inside `std::alloc::Global::alloc_impl` at RUSTLIB/alloc/src/alloc.rs:LL:CC
9+
= note: inside `<std::alloc::Global as std::alloc::Allocator>::allocate` at RUSTLIB/alloc/src/alloc.rs:LL:CC
10+
= note: inside `alloc::alloc::exchange_malloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
11+
= note: inside `std::boxed::Box::<std::rc::RcBox<std::cell::RefCell<std::option::Option<Dummy>>>>::new` at RUSTLIB/alloc/src/boxed.rs:LL:CC
12+
= note: inside `std::rc::Rc::<std::cell::RefCell<std::option::Option<Dummy>>>::new` at RUSTLIB/alloc/src/rc.rs:LL:CC
13+
note: inside `main`
14+
--> $DIR/memleak_rc.rs:LL:CC
15+
|
16+
LL | let x = Dummy(Rc::new(RefCell::new(None)));
17+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
518

6-
error: the evaluated program leaked memory
7-
8-
note: pass `-Zmiri-ignore-leaks` to disable this check
19+
note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check
920

1021
error: aborting due to previous error
1122

‎src/tools/miri/tests/fail/memleak_rc.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//@error-pattern: the evaluated program leaked memory
1+
//@error-pattern: memory leaked
22
//@stderr-per-bitwidth
33
//@normalize-stderr-test: ".*│.*" -> "$$stripped$$"
44

0 commit comments

Comments
 (0)