1
Fork 0

Auto merge of #109061 - saethlin:leak-backtraces, r=oli-obk

Add a backtrace to Allocation, display it in leak reports

This addresses https://github.com/rust-lang/miri/issues/2813

Information like this from diagnostics is indispensable for diagnosing problems that are difficult to reproduce such as https://github.com/rust-lang/miri-test-libstd/actions/runs/4395316008/jobs/7697019211#step:4:770 (which has not been reproduced or diagnosed).
This commit is contained in:
bors 2023-04-17 00:22:28 +00:00
commit 23eb90ffa7
17 changed files with 191 additions and 62 deletions

View file

@ -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

View file

@ -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;

View file

@ -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));
self.memory.alloc_map.filter_map_collect(|id, (kind, alloc)| {
if kind.may_leak() || reachable.contains(id) {
None
} else {
Some((*id, *kind, alloc.clone()))
}
n
})
}
}

View file

@ -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=<var>` 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=<action>` 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=<var>` 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.

View file

@ -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" {

View file

@ -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 {

View file

@ -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>,

View file

@ -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;
@ -145,6 +146,8 @@ pub struct MiriConfig {
pub num_cpus: u32,
/// Requires Miri to emulate pages of a certain size
pub page_size: Option<u64>,
/// 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 @@ impl Default for 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!("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);
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;

View file

@ -253,20 +253,25 @@ 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. The backtrace is not
/// pruned yet; that should be done before printing it.
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);
@ -467,12 +472,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> {
@ -581,6 +591,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
page_size,
stack_addr,
stack_size,
collect_leak_backtraces: config.collect_leak_backtraces,
}
}
@ -728,6 +739,7 @@ impl VisitTags for MiriMachine<'_, '_> {
page_size: _,
stack_addr: _,
stack_size: _,
collect_leak_backtraces: _,
} = self;
threads.visit_tags(visit);
@ -773,7 +785,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 +979,24 @@ 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() || !ecx.machine.collect_leak_backtraces {
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 +1076,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 +1096,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 +1116,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> {

View file

@ -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);

View file

@ -1,4 +1,4 @@
//@error-pattern: the evaluated program leaked memory
//@error-pattern: memory leaked
//@normalize-stderr-test: ".*│.*" -> "$$stripped$$"
fn main() {

View file

@ -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 `<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: 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

View file

@ -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));
}

View file

@ -0,0 +1,4 @@
error: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check
error: aborting due to previous error

View file

@ -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 `<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: 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

View file

@ -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 `<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: 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

View file

@ -1,4 +1,4 @@
//@error-pattern: the evaluated program leaked memory
//@error-pattern: memory leaked
//@stderr-per-bitwidth
//@normalize-stderr-test: ".*│.*" -> "$$stripped$$"