diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 129fec8cc8f..640a953dac9 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -301,6 +301,15 @@ environment variable. We first document the most relevant and most commonly used * `-Zmiri-disable-isolation` disables host isolation. As a consequence, the program has access to host resources such as environment variables, file systems, and randomness. +* `-Zmiri-disable-leak-backtraces` disables backtraces reports for memory leaks. By default, a + backtrace is captured for every allocation when it is created, just in case it leaks. This incurs + some memory overhead to store data that is almost never used. This flag is implied by + `-Zmiri-ignore-leaks`. +* `-Zmiri-env-forward=` forwards the `var` environment variable to the interpreted program. Can + be used multiple times to forward several variables. Execution will still be deterministic if the + value of forwarded variables stays the same. Has no effect if `-Zmiri-disable-isolation` is set. +* `-Zmiri-ignore-leaks` disables the memory leak checker, and also allows some + remaining threads to exist when the main thread exits. * `-Zmiri-isolation-error=` configures Miri's response to operations requiring host access while isolation is enabled. `abort`, `hide`, `warn`, and `warn-nobacktrace` are the supported actions. The default is to `abort`, @@ -308,11 +317,6 @@ environment variable. We first document the most relevant and most commonly used execution with a "permission denied" error being returned to the program. `warn` prints a full backtrace when that happens; `warn-nobacktrace` is less verbose. `hide` hides the warning entirely. -* `-Zmiri-env-forward=` forwards the `var` environment variable to the interpreted program. Can - be used multiple times to forward several variables. Execution will still be deterministic if the - value of forwarded variables stays the same. Has no effect if `-Zmiri-disable-isolation` is set. -* `-Zmiri-ignore-leaks` disables the memory leak checker, and also allows some - remaining threads to exist when the main thread exits. * `-Zmiri-num-cpus` states the number of available CPUs to be reported by miri. By default, the number of available CPUs is `1`. Note that this flag does not affect how miri handles threads in any way. diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index d394ab5a7ae..6e0092308fa 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -71ef9ecbdedb67c32f074884f503f8e582855c2f +53ac4f8e2fc15e49ef3a04f98622a9b9db755fd4 diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index cdf06b6a107..e4ca40570b7 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -359,6 +359,8 @@ fn main() { isolation_enabled = Some(false); } miri_config.isolated_op = miri::IsolatedOp::Allow; + } else if arg == "-Zmiri-disable-leak-backtraces" { + miri_config.collect_leak_backtraces = false; } else if arg == "-Zmiri-disable-weak-memory-emulation" { miri_config.weak_memory_emulation = false; } else if arg == "-Zmiri-track-weak-memory-loads" { @@ -385,6 +387,7 @@ fn main() { }; } else if arg == "-Zmiri-ignore-leaks" { miri_config.ignore_leaks = true; + miri_config.collect_leak_backtraces = false; } else if arg == "-Zmiri-panic-on-unsupported" { miri_config.panic_on_unsupported = true; } else if arg == "-Zmiri-tag-raw-pointers" { diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index 827dd2b6212..ffc49eedb5a 100644 --- 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>), } -impl machine::AllocExtra { +impl machine::AllocExtra<'_> { #[track_caller] pub fn borrow_tracker_sb(&self) -> &RefCell { match self.borrow_tracker { diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 3c13118122c..7a726be00da 100644 --- 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>, machine: &MiriMachine<'_, 'tcx>, ) -> (Vec>, 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, Allocation>)>, +) { + 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` 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, diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index 430229c132d..43d8f221ce3 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -10,6 +10,7 @@ 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; @@ -145,6 +146,8 @@ pub struct MiriConfig { pub num_cpus: u32, /// Requires Miri to emulate pages of a certain size pub page_size: Option, + /// Whether to collect a backtrace when each allocation is created, just in case it leaks. + pub collect_leak_backtraces: bool, } impl Default for MiriConfig { @@ -179,6 +182,7 @@ fn default() -> MiriConfig { gc_interval: 10_000, num_cpus: 1, page_size: None, + collect_leak_backtraces: true, } } } @@ -457,10 +461,17 @@ pub fn eval_entry<'tcx>( } // Check for memory leaks. info!("Additional 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); + let leak_message = "the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check"; + if ecx.machine.collect_leak_backtraces { + // If we are collecting leak backtraces, each leak is a distinct error diagnostic. + tcx.sess.note_without_error(leak_message); + } else { + // If we do not have backtraces, we just report an error without any span. + tcx.sess.err(leak_message); + }; // 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 index 176e3758a6b..21c5a9c1b70 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -259,20 +259,25 @@ pub fn and_then(self, f: impl FnOnce(BorTag) -> Option) -> Option { /// 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, - /// 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, - /// 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, + /// A backtrace to where this allocation was allocated. + /// As this is recorded for leak reports, it only exists + /// if this allocation is leakable. The backtrace is not + /// pruned yet; that should be done before printing it. + pub backtrace: Option>>, } -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); @@ -473,12 +478,17 @@ pub struct MiriMachine<'mir, 'tcx> { pub(crate) gc_interval: u32, /// The number of blocks that passed since the last BorTag GC pass. pub(crate) since_gc: u32, + /// The number of CPUs to be reported by miri. pub(crate) num_cpus: u32, + /// Determines Miri's page size and associated values pub(crate) page_size: u64, pub(crate) stack_addr: u64, pub(crate) stack_size: u64, + + /// Whether to collect a backtrace when each allocation is created, just in case it leaks. + pub(crate) collect_leak_backtraces: bool, } impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { @@ -587,6 +597,7 @@ pub(crate) fn new(config: &MiriConfig, layout_cx: LayoutCx<'tcx, TyCtxt<'tcx>>) page_size, stack_addr, stack_size, + collect_leak_backtraces: config.collect_leak_backtraces, } } @@ -734,6 +745,7 @@ fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { page_size: _, stack_addr: _, stack_size: _, + collect_leak_backtraces: _, } = self; threads.visit_tags(visit); @@ -779,7 +791,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; @@ -973,9 +985,24 @@ fn adjust_allocation<'b>( ) }); 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() || !ecx.machine.collect_leak_backtraces { + None + } else { + Some(ecx.generate_stacktrace()) + }; + let alloc: Allocation = 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)) @@ -1055,7 +1082,7 @@ fn ptr_get_alloc( 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> { @@ -1075,7 +1102,7 @@ fn before_memory_read( 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> { @@ -1095,7 +1122,7 @@ fn before_memory_write( 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 index c1194fe2216..cefdcc2b5b8 100644 --- a/src/tools/miri/src/tag_gc.rs +++ b/src/tools/miri/src/tag_gc.rs @@ -125,7 +125,7 @@ fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) { } } -impl VisitTags for Allocation { +impl VisitTags for Allocation> { 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 index d384caf81a5..cbeb163b56c 100644 --- 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 index f8b62af3eb8..6d9b664c8f4 100644 --- a/src/tools/miri/tests/fail/memleak.stderr +++ b/src/tools/miri/tests/fail/memleak.stderr @@ -1,10 +1,23 @@ -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 `::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::::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: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -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_no_backtrace.rs b/src/tools/miri/tests/fail/memleak_no_backtrace.rs new file mode 100644 index 00000000000..24d4a02df71 --- /dev/null +++ b/src/tools/miri/tests/fail/memleak_no_backtrace.rs @@ -0,0 +1,7 @@ +//@compile-flags: -Zmiri-disable-leak-backtraces +//@error-pattern: the evaluated program leaked memory +//@normalize-stderr-test: ".*│.*" -> "$$stripped$$" + +fn main() { + std::mem::forget(Box::new(42)); +} diff --git a/src/tools/miri/tests/fail/memleak_no_backtrace.stderr b/src/tools/miri/tests/fail/memleak_no_backtrace.stderr new file mode 100644 index 00000000000..f44e6ce0797 --- /dev/null +++ b/src/tools/miri/tests/fail/memleak_no_backtrace.stderr @@ -0,0 +1,4 @@ +error: 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 index da222609091..0e1146cf4ad 100644 --- a/src/tools/miri/tests/fail/memleak_rc.32bit.stderr +++ b/src/tools/miri/tests/fail/memleak_rc.32bit.stderr @@ -1,10 +1,24 @@ -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 `::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::>>>::new` at RUSTLIB/alloc/src/boxed.rs:LL:CC + = note: inside `std::rc::Rc::>>::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: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -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 index 8c24bbc779b..4979588f370 100644 --- a/src/tools/miri/tests/fail/memleak_rc.64bit.stderr +++ b/src/tools/miri/tests/fail/memleak_rc.64bit.stderr @@ -1,11 +1,24 @@ -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 `::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::>>>::new` at RUSTLIB/alloc/src/boxed.rs:LL:CC + = note: inside `std::rc::Rc::>>::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: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -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 index 76ecd71b011..cf4671912ad 100644 --- 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$$"