Report a backtrace for memory leaks under Miri

saethlin · saethlin · commit 606ca4da7ea3 · 2023-04-16T08:52:00.000-04:00
diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs
@@ -132,11 +132,10 @@ pub struct Frame<'mir, 'tcx, Prov: Provenance = AllocId, Extra = ()> {
 }
 
 /// What we store about a frame in an interpreter backtrace.
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub struct FrameInfo<'tcx> {
     pub instance: ty::Instance<'tcx>,
     pub span: Span,
-    pub lint_root: Option<hir::HirId>,
 }
 
 #[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> {
         // This deliberately does *not* honor `requires_caller_location` since it is used for much
         // more than just panics.
         for frame in stack.iter().rev() {
-            let lint_root = frame.lint_root();
             let span = frame.current_span();
-
-            frames.push(FrameInfo { span, instance: frame.instance, lint_root });
+            frames.push(FrameInfo { span, instance: frame.instance });
         }
         trace!("generate stacktrace: {:#?}", frames);
         frames
diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs
@@ -104,7 +104,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
     type FrameExtra;
 
     /// Extra data stored in every allocation.
-    type AllocExtra: Debug + Clone + 'static;
+    type AllocExtra: Debug + Clone + 'tcx;
 
     /// Type for the bytes of the allocation.
     type Bytes: AllocBytes + 'static;
diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs
@@ -215,7 +215,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         self.allocate_raw_ptr(alloc, kind)
     }
 
-    /// This can fail only of `alloc` contains provenance.
+    /// This can fail only if `alloc` contains provenance.
     pub fn allocate_raw_ptr(
         &mut self,
         alloc: Allocation,
@@ -807,9 +807,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         DumpAllocs { ecx: self, allocs }
     }
 
-    /// Print leaked memory. Allocations reachable from `static_roots` or a `Global` allocation
-    /// are not considered leaked. Leaks whose kind `may_leak()` returns true are not reported.
-    pub fn leak_report(&self, static_roots: &[AllocId]) -> usize {
+    /// Find leaked allocations. Allocations reachable from `static_roots` or a `Global` allocation
+    /// are not considered leaked, as well as leaks whose kind's `may_leak()` returns true.
+    pub fn find_leaked_allocations(
+        &self,
+        static_roots: &[AllocId],
+    ) -> Vec<(AllocId, MemoryKind<M::MemoryKind>, Allocation<M::Provenance, M::AllocExtra, M::Bytes>)>
+    {
         // Collect the set of allocations that are *reachable* from `Global` allocations.
         let reachable = {
             let mut reachable = FxHashSet::default();
@@ -833,14 +837,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         };
 
         // All allocations that are *not* `reachable` and *not* `may_leak` are considered leaking.
-        let leaks: Vec<_> = self.memory.alloc_map.filter_map_collect(|&id, &(kind, _)| {
-            if kind.may_leak() || reachable.contains(&id) { None } else { Some(id) }
-        });
-        let n = leaks.len();
-        if n > 0 {
-            eprintln!("The following memory was leaked: {:?}", self.dump_allocs(leaks));
-        }
-        n
+        self.memory.alloc_map.filter_map_collect(|id, (kind, alloc)| {
+            if kind.may_leak() || reachable.contains(id) {
+                None
+            } else {
+                Some((*id, *kind, alloc.clone()))
+            }
+        })
     }
 }
 
diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs
@@ -352,7 +352,7 @@ pub enum AllocState {
     TreeBorrows(Box<RefCell<tree_borrows::AllocState>>),
 }
 
-impl machine::AllocExtra {
+impl machine::AllocExtra<'_> {
     #[track_caller]
     pub fn borrow_tracker_sb(&self) -> &RefCell<stacked_borrows::AllocState> {
         match self.borrow_tracker {
diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs
@@ -105,7 +105,7 @@ pub enum NonHaltingDiagnostic {
 }
 
 /// Level of Miri specific diagnostics
-enum DiagLevel {
+pub enum DiagLevel {
     Error,
     Warning,
     Note,
@@ -114,7 +114,7 @@ enum DiagLevel {
 /// Attempts to prune a stacktrace to omit the Rust runtime, and returns a bool indicating if any
 /// frames were pruned. If the stacktrace does not have any local frames, we conclude that it must
 /// be pointing to a problem in the Rust runtime itself, and do not prune it at all.
-fn prune_stacktrace<'tcx>(
+pub fn prune_stacktrace<'tcx>(
     mut stacktrace: Vec<FrameInfo<'tcx>>,
     machine: &MiriMachine<'_, 'tcx>,
 ) -> (Vec<FrameInfo<'tcx>>, bool) {
@@ -338,12 +338,45 @@ pub fn report_error<'tcx, 'mir>(
     None
 }
 
+pub fn report_leaks<'mir, 'tcx>(
+    ecx: &InterpCx<'mir, 'tcx, MiriMachine<'mir, 'tcx>>,
+    leaks: Vec<(AllocId, MemoryKind<MiriMemoryKind>, Allocation<Provenance, AllocExtra<'tcx>>)>,
+) {
+    let mut any_pruned = false;
+    for (id, kind, mut alloc) in leaks {
+        let Some(backtrace) = alloc.extra.backtrace.take() else {
+            continue;
+        };
+        let (backtrace, pruned) = prune_stacktrace(backtrace, &ecx.machine);
+        any_pruned |= pruned;
+        report_msg(
+            DiagLevel::Error,
+            &format!(
+                "memory leaked: {id:?} ({}, size: {:?}, align: {:?}), allocated here:",
+                kind,
+                alloc.size().bytes(),
+                alloc.align.bytes()
+            ),
+            vec![],
+            vec![],
+            vec![],
+            &backtrace,
+            &ecx.machine,
+        );
+    }
+    if any_pruned {
+        ecx.tcx.sess.diagnostic().note_without_error(
+            "some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace",
+        );
+    }
+}
+
 /// Report an error or note (depending on the `error` argument) with the given stacktrace.
 /// Also emits a full stacktrace of the interpreter stack.
 /// We want to present a multi-line span message for some errors. Diagnostics do not support this
 /// directly, so we pass the lines as a `Vec<String>` and display each line after the first with an
 /// additional `span_label` or `note` call.
-fn report_msg<'tcx>(
+pub fn report_msg<'tcx>(
     diag_level: DiagLevel,
     title: &str,
     span_msg: Vec<String>,
diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs
@@ -10,6 +10,7 @@ use std::thread;
 use log::info;
 
 use crate::borrow_tracker::RetagFields;
+use crate::diagnostics::report_leaks;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir::def::Namespace;
 use rustc_hir::def_id::DefId;
@@ -457,10 +458,12 @@ pub fn eval_entry<'tcx>(
         }
         // Check for memory leaks.
         info!("Additonal static roots: {:?}", ecx.machine.static_roots);
-        let leaks = ecx.leak_report(&ecx.machine.static_roots);
-        if leaks != 0 {
-            tcx.sess.err("the evaluated program leaked memory");
-            tcx.sess.note_without_error("pass `-Zmiri-ignore-leaks` to disable this check");
+        let leaks = ecx.find_leaked_allocations(&ecx.machine.static_roots);
+        if !leaks.is_empty() {
+            report_leaks(&ecx, leaks);
+            tcx.sess.note_without_error(
+                "the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check",
+            );
             // Ignore the provided return code - let the reported error
             // determine the return code.
             return None;
diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs
@@ -253,20 +253,24 @@ impl ProvenanceExtra {
 
 /// Extra per-allocation data
 #[derive(Debug, Clone)]
-pub struct AllocExtra {
+pub struct AllocExtra<'tcx> {
     /// Global state of the borrow tracker, if enabled.
     pub borrow_tracker: Option<borrow_tracker::AllocState>,
-    /// Data race detection via the use of a vector-clock,
-    ///  this is only added if it is enabled.
+    /// Data race detection via the use of a vector-clock.
+    /// This is only added if it is enabled.
     pub data_race: Option<data_race::AllocState>,
-    /// Weak memory emulation via the use of store buffers,
-    ///  this is only added if it is enabled.
+    /// Weak memory emulation via the use of store buffers.
+    /// This is only added if it is enabled.
     pub weak_memory: Option<weak_memory::AllocState>,
+    /// A backtrace to where this allocation was allocated.
+    /// As this is recorded for leak reports, it only exists
+    /// if this allocation is leakable.
+    pub backtrace: Option<Vec<FrameInfo<'tcx>>>,
 }
 
-impl VisitTags for AllocExtra {
+impl VisitTags for AllocExtra<'_> {
     fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
-        let AllocExtra { borrow_tracker, data_race, weak_memory } = self;
+        let AllocExtra { borrow_tracker, data_race, weak_memory, backtrace: _ } = self;
 
         borrow_tracker.visit_tags(visit);
         data_race.visit_tags(visit);
@@ -773,7 +777,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
     type ExtraFnVal = Dlsym;
 
     type FrameExtra = FrameExtra<'tcx>;
-    type AllocExtra = AllocExtra;
+    type AllocExtra = AllocExtra<'tcx>;
 
     type Provenance = Provenance;
     type ProvenanceExtra = ProvenanceExtra;
@@ -967,9 +971,20 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
             )
         });
         let buffer_alloc = ecx.machine.weak_memory.then(weak_memory::AllocState::new_allocation);
+
+        // If an allocation is leaked, we want to report a backtrace to indicate where it was
+        // allocated. We don't need to record a backtrace for allocations which are allowed to
+        // leak.
+        let backtrace = if kind.may_leak() { None } else { Some(ecx.generate_stacktrace()) };
+
         let alloc: Allocation<Provenance, Self::AllocExtra> = alloc.adjust_from_tcx(
             &ecx.tcx,
-            AllocExtra { borrow_tracker, data_race: race_alloc, weak_memory: buffer_alloc },
+            AllocExtra {
+                borrow_tracker,
+                data_race: race_alloc,
+                weak_memory: buffer_alloc,
+                backtrace,
+            },
             |ptr| ecx.global_base_pointer(ptr),
         )?;
         Ok(Cow::Owned(alloc))
@@ -1049,7 +1064,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
     fn before_memory_read(
         _tcx: TyCtxt<'tcx>,
         machine: &Self,
-        alloc_extra: &AllocExtra,
+        alloc_extra: &AllocExtra<'tcx>,
         (alloc_id, prov_extra): (AllocId, Self::ProvenanceExtra),
         range: AllocRange,
     ) -> InterpResult<'tcx> {
@@ -1069,7 +1084,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
     fn before_memory_write(
         _tcx: TyCtxt<'tcx>,
         machine: &mut Self,
-        alloc_extra: &mut AllocExtra,
+        alloc_extra: &mut AllocExtra<'tcx>,
         (alloc_id, prov_extra): (AllocId, Self::ProvenanceExtra),
         range: AllocRange,
     ) -> InterpResult<'tcx> {
@@ -1089,7 +1104,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
     fn before_memory_deallocation(
         _tcx: TyCtxt<'tcx>,
         machine: &mut Self,
-        alloc_extra: &mut AllocExtra,
+        alloc_extra: &mut AllocExtra<'tcx>,
         (alloc_id, prove_extra): (AllocId, Self::ProvenanceExtra),
         range: AllocRange,
     ) -> InterpResult<'tcx> {
diff --git a/src/tools/miri/src/tag_gc.rs b/src/tools/miri/src/tag_gc.rs
@@ -125,7 +125,7 @@ impl VisitTags for Operand<Provenance> {
     }
 }
 
-impl VisitTags for Allocation<Provenance, AllocExtra> {
+impl VisitTags for Allocation<Provenance, AllocExtra<'_>> {
     fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
         for prov in self.provenance().provenances() {
             prov.visit_tags(visit);
diff --git a/src/tools/miri/tests/fail/memleak.rs b/src/tools/miri/tests/fail/memleak.rs
@@ -1,4 +1,4 @@
-//@error-pattern: the evaluated program leaked memory
+//@error-pattern: memory leaked
 //@normalize-stderr-test: ".*│.*" -> "$$stripped$$"
 
 fn main() {
diff --git a/src/tools/miri/tests/fail/memleak.stderr b/src/tools/miri/tests/fail/memleak.stderr
@@ -1,10 +1,21 @@
-The following memory was leaked: ALLOC (Rust heap, size: 4, align: 4) {
-$stripped$
-}
+error: memory leaked: ALLOC (Rust heap, size: 4, align: 4), allocated here:
+  --> RUSTLIB/alloc/src/alloc.rs:LL:CC
+   |
+LL |     unsafe { __rust_alloc(layout.size(), layout.align()) }
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: inside `std::alloc::alloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `std::alloc::Global::alloc_impl` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `<std::alloc::Global as std::alloc::Allocator>::allocate` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `alloc::alloc::exchange_malloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `std::boxed::Box::<i32>::new` at RUSTLIB/alloc/src/boxed.rs:LL:CC
+note: inside `main`
+  --> $DIR/memleak.rs:LL:CC
+   |
+LL |     std::mem::forget(Box::new(42));
+   |                      ^^^^^^^^^^^^
 
-error: the evaluated program leaked memory
-
-note: pass `-Zmiri-ignore-leaks` to disable this check
+note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check
 
 error: aborting due to previous error
 
diff --git a/src/tools/miri/tests/fail/memleak_rc.32bit.stderr b/src/tools/miri/tests/fail/memleak_rc.32bit.stderr
@@ -1,10 +1,22 @@
-The following memory was leaked: ALLOC (Rust heap, size: 16, align: 4) {
-$stripped$
-}
+error: memory leaked: ALLOC (Rust heap, size: 16, align: 4), allocated here:
+  --> RUSTLIB/alloc/src/alloc.rs:LL:CC
+   |
+LL |     unsafe { __rust_alloc(layout.size(), layout.align()) }
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: inside `std::alloc::alloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `std::alloc::Global::alloc_impl` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `<std::alloc::Global as std::alloc::Allocator>::allocate` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `alloc::alloc::exchange_malloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `std::boxed::Box::<std::rc::RcBox<std::cell::RefCell<std::option::Option<Dummy>>>>::new` at RUSTLIB/alloc/src/boxed.rs:LL:CC
+   = note: inside `std::rc::Rc::<std::cell::RefCell<std::option::Option<Dummy>>>::new` at RUSTLIB/alloc/src/rc.rs:LL:CC
+note: inside `main`
+  --> $DIR/memleak_rc.rs:LL:CC
+   |
+LL |     let x = Dummy(Rc::new(RefCell::new(None)));
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: the evaluated program leaked memory
-
-note: pass `-Zmiri-ignore-leaks` to disable this check
+note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check
 
 error: aborting due to previous error
 
diff --git a/src/tools/miri/tests/fail/memleak_rc.64bit.stderr b/src/tools/miri/tests/fail/memleak_rc.64bit.stderr
@@ -1,11 +1,22 @@
-The following memory was leaked: ALLOC (Rust heap, size: 32, align: 8) {
-$stripped$
-$stripped$
-}
+error: memory leaked: ALLOC (Rust heap, size: 32, align: 8), allocated here:
+  --> RUSTLIB/alloc/src/alloc.rs:LL:CC
+   |
+LL |     unsafe { __rust_alloc(layout.size(), layout.align()) }
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: inside `std::alloc::alloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `std::alloc::Global::alloc_impl` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `<std::alloc::Global as std::alloc::Allocator>::allocate` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `alloc::alloc::exchange_malloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `std::boxed::Box::<std::rc::RcBox<std::cell::RefCell<std::option::Option<Dummy>>>>::new` at RUSTLIB/alloc/src/boxed.rs:LL:CC
+   = note: inside `std::rc::Rc::<std::cell::RefCell<std::option::Option<Dummy>>>::new` at RUSTLIB/alloc/src/rc.rs:LL:CC
+note: inside `main`
+  --> $DIR/memleak_rc.rs:LL:CC
+   |
+LL |     let x = Dummy(Rc::new(RefCell::new(None)));
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: the evaluated program leaked memory
-
-note: pass `-Zmiri-ignore-leaks` to disable this check
+note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check
 
 error: aborting due to previous error
 
diff --git a/src/tools/miri/tests/fail/memleak_rc.rs b/src/tools/miri/tests/fail/memleak_rc.rs
@@ -1,4 +1,4 @@
-//@error-pattern: the evaluated program leaked memory
+//@error-pattern: memory leaked
 //@stderr-per-bitwidth
 //@normalize-stderr-test: ".*│.*" -> "$$stripped$$"
 

Original file line number	Diff line number	Diff line change
`@@ -352,7 +352,7 @@ pub enum AllocState {`
`352`	`352`	`TreeBorrows(Box<RefCell<tree_borrows::AllocState>>),`
`353`	`353`	`}`
`354`	`354`
`355`		`-impl machine::AllocExtra {`
	`355`	`+impl machine::AllocExtra<'_> {`
`356`	`356`	`#[track_caller]`
`357`	`357`	`pub fn borrow_tracker_sb(&self) -> &RefCell<stacked_borrows::AllocState> {`
`358`	`358`	`match self.borrow_tracker {`
Original file line number	Diff line number	Diff line change
`@@ -125,7 +125,7 @@ impl VisitTags for Operand<Provenance> {`
`125`	`125`	`}`
`126`	`126`	`}`
`127`	`127`
`128`		`-impl VisitTags for Allocation<Provenance, AllocExtra> {`
	`128`	`+impl VisitTags for Allocation<Provenance, AllocExtra<'_>> {`
`129`	`129`	`fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {`
`130`	`130`	`for prov in self.provenance().provenances() {`
`131`	`131`	`prov.visit_tags(visit);`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-//@error-pattern: the evaluated program leaked memory`
	`1`	`+//@error-pattern: memory leaked`
`2`	`2`	`//@normalize-stderr-test: ".│." -> "$$stripped$$"`
`3`	`3`
`4`	`4`	`fn main() {`