add an option to disable Stacked Borrows

This commit is contained in:
Ralf Jung 2020-02-24 16:22:02 +01:00
parent 36305e3a50
commit 274ae0438f
15 changed files with 66 additions and 41 deletions

View file

@ -14,7 +14,8 @@ for example:
* Not sufficiently aligned memory accesses and references
* Violation of *some* basic type invariants (a `bool` that is not 0 or 1, for example,
or an invalid enum discriminant)
* **Experimental**: Violations of the rules governing aliasing for reference types
* **Experimental**: Violations of the [Stacked Borrows] rules governing aliasing
for reference types
Miri has already discovered some [real-world bugs](#bugs-found-by-miri). If you
found a bug with Miri, we'd appreciate if you tell us and we'll add it to the
@ -47,6 +48,7 @@ program, and cannot run all programs:
[mir]: https://github.com/rust-lang/rfcs/blob/master/text/1211-mir.md
[`unreachable_unchecked`]: https://doc.rust-lang.org/stable/std/hint/fn.unreachable_unchecked.html
[`copy_nonoverlapping`]: https://doc.rust-lang.org/stable/std/ptr/fn.copy_nonoverlapping.html
[Stacked Borrows]: https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md
## Using Miri
@ -152,10 +154,13 @@ Several `-Z` flags are relevant for Miri:
**NOTE**: This entropy is not good enough for cryptographic use! Do not
generate secret keys in Miri or perform other kinds of cryptographic
operations that rely on proper random numbers.
* `-Zmiri-disable-validation` disables enforcing validity invariants and
reference aliasing rules, which are enforced by default. This is mostly
useful for debugging. It means Miri will miss bugs in your program. However,
this can also help to make Miri run faster.
* `-Zmiri-disable-validation` disables enforcing validity invariants, which are
enforced by default. This is mostly useful for debugging. It means Miri will
miss bugs in your program. However, this can also help to make Miri run
faster.
* `-Zmiri-disable-stacked-borrows` disables checking the experimental
[Stacked Borrows] aliasing rules. This can make Miri run faster, but it also
means no aliasing violations will be detected.
* `-Zmiri-disable-isolation` disables host host isolation. As a consequence,
the program has access to host resources such as environment variables, file
systems, and randomness.
@ -234,7 +239,7 @@ Definite bugs found:
* [The Unix allocator calling `posix_memalign` in an invalid way](https://github.com/rust-lang/rust/issues/62251)
* [`getrandom` calling the `getrandom` syscall in an invalid way](https://github.com/rust-random/getrandom/pull/73)
Violations of Stacked Borrows found that are likely bugs (but Stacked Borrows is currently just an experiment):
Violations of [Stacked Borrows] found that are likely bugs (but Stacked Borrows is currently just an experiment):
* [`VecDeque` creating overlapping mutable references](https://github.com/rust-lang/rust/pull/56161)
* [`BTreeMap` creating mutable references that overlap with shared references](https://github.com/rust-lang/rust/pull/58431)

View file

@ -130,6 +130,7 @@ fn main() {
// Parse our arguments and split them across `rustc` and `miri`.
let mut validate = true;
let mut stacked_borrows = true;
let mut communicate = false;
let mut ignore_leaks = false;
let mut seed: Option<u64> = None;
@ -150,6 +151,9 @@ fn main() {
"-Zmiri-disable-validation" => {
validate = false;
}
"-Zmiri-disable-stacked-borrows" => {
stacked_borrows = false;
}
"-Zmiri-disable-isolation" => {
communicate = true;
}
@ -229,6 +233,7 @@ fn main() {
debug!("miri arguments: {:?}", miri_args);
let miri_config = miri::MiriConfig {
validate,
stacked_borrows,
communicate,
ignore_leaks,
excluded_env_vars,

View file

@ -14,8 +14,10 @@ use crate::*;
/// Configuration needed to spawn a Miri instance.
#[derive(Clone)]
pub struct MiriConfig {
/// Determine if validity checking and Stacked Borrows are enabled.
/// Determine if validity checking is enabled.
pub validate: bool,
/// Determines if Stacked Borrows is enabled.
pub stacked_borrows: bool,
/// Determines if communication with the host environment is enabled.
pub communicate: bool,
/// Determines if memory leaks should be ignored.
@ -52,6 +54,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
MemoryExtra::new(
StdRng::seed_from_u64(config.seed.unwrap_or(0)),
config.validate,
config.stacked_borrows,
config.tracked_pointer_tag,
),
);

View file

@ -56,7 +56,7 @@ pub use crate::mono_hash_map::MonoHashMap;
pub use crate::operator::EvalContextExt as OperatorEvalContextExt;
pub use crate::range_map::RangeMap;
pub use crate::stacked_borrows::{
EvalContextExt as StackedBorEvalContextExt, GlobalState, Item, Permission, PtrId, Stack,
EvalContextExt as StackedBorEvalContextExt, Item, Permission, PtrId, Stack,
Stacks, Tag,
};

View file

@ -4,6 +4,7 @@
use std::borrow::Cow;
use std::cell::RefCell;
use std::rc::Rc;
use std::num::NonZeroU64;
use rand::rngs::StdRng;
@ -63,14 +64,14 @@ impl Into<MemoryKind<MiriMemoryKind>> for MiriMemoryKind {
/// Extra per-allocation data
#[derive(Debug, Clone)]
pub struct AllocExtra {
/// Stacked Borrows state is only added if validation is enabled.
/// Stacked Borrows state is only added if it is enabled.
pub stacked_borrows: Option<stacked_borrows::AllocExtra>,
}
/// Extra global memory data
#[derive(Clone, Debug)]
pub struct MemoryExtra {
pub stacked_borrows: stacked_borrows::MemoryExtra,
pub stacked_borrows: Option<stacked_borrows::MemoryExtra>,
pub intptrcast: intptrcast::MemoryExtra,
/// The random number generator used for resolving non-determinism.
@ -81,9 +82,14 @@ pub struct MemoryExtra {
}
impl MemoryExtra {
pub fn new(rng: StdRng, validate: bool, tracked_pointer_tag: Option<PtrId>) -> Self {
pub fn new(rng: StdRng, validate: bool, stacked_borrows: bool, tracked_pointer_tag: Option<PtrId>) -> Self {
let stacked_borrows = if stacked_borrows {
Some(Rc::new(RefCell::new(stacked_borrows::GlobalState::new(tracked_pointer_tag))))
} else {
None
};
MemoryExtra {
stacked_borrows: Rc::new(RefCell::new(GlobalState::new(tracked_pointer_tag))),
stacked_borrows,
intptrcast: Default::default(),
rng: RefCell::new(rng),
validate,
@ -299,11 +305,11 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
) -> (Cow<'b, Allocation<Self::PointerTag, Self::AllocExtra>>, Self::PointerTag) {
let kind = kind.expect("we set our STATIC_KIND so this cannot be None");
let alloc = alloc.into_owned();
let (stacks, base_tag) = if memory_extra.validate {
let (stacks, base_tag) = if let Some(stacked_borrows) = memory_extra.stacked_borrows.as_ref() {
let (stacks, base_tag) = Stacks::new_allocation(
id,
alloc.size,
Rc::clone(&memory_extra.stacked_borrows),
Rc::clone(stacked_borrows),
kind,
);
(Some(stacks), base_tag)
@ -311,15 +317,15 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
// No stacks, no tag.
(None, Tag::Untagged)
};
let mut stacked_borrows = memory_extra.stacked_borrows.borrow_mut();
let mut stacked_borrows = memory_extra.stacked_borrows.as_ref().map(|sb| sb.borrow_mut());
let alloc: Allocation<Tag, Self::AllocExtra> = alloc.with_tags_and_extra(
|alloc| {
if !memory_extra.validate {
Tag::Untagged
} else {
if let Some(stacked_borrows) = stacked_borrows.as_mut() {
// Only statics may already contain pointers at this point
assert_eq!(kind, MiriMemoryKind::Static.into());
stacked_borrows.static_base_ptr(alloc)
} else {
Tag::Untagged
}
},
AllocExtra { stacked_borrows: stacks },
@ -329,10 +335,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
#[inline(always)]
fn tag_static_base_pointer(memory_extra: &MemoryExtra, id: AllocId) -> Self::PointerTag {
if !memory_extra.validate {
Tag::Untagged
if let Some(stacked_borrows) = memory_extra.stacked_borrows.as_ref() {
stacked_borrows.borrow_mut().static_base_ptr(id)
} else {
memory_extra.stacked_borrows.borrow_mut().static_base_ptr(id)
Tag::Untagged
}
}
@ -342,7 +348,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
kind: mir::RetagKind,
place: PlaceTy<'tcx, Tag>,
) -> InterpResult<'tcx> {
if !Self::enforce_validity(ecx) {
if ecx.memory.extra.stacked_borrows.is_none() {
// No tracking.
Ok(())
} else {
@ -352,8 +358,12 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
#[inline(always)]
fn stack_push(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx, FrameData<'tcx>> {
let call_id = ecx.memory.extra.stacked_borrows.as_ref().map_or(
NonZeroU64::new(1).unwrap(),
|stacked_borrows| stacked_borrows.borrow_mut().new_call(),
);
Ok(FrameData {
call_id: ecx.memory.extra.stacked_borrows.borrow_mut().new_call(),
call_id,
catch_panic: None,
})
}

View file

@ -146,7 +146,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
} else {
StackPopInfo::Normal
};
this.memory.extra.stacked_borrows.borrow_mut().end_call(extra.call_id);
if let Some(stacked_borrows) = this.memory.extra.stacked_borrows.as_ref() {
stacked_borrows.borrow_mut().end_call(extra.call_id);
}
Ok(res)
}

View file

@ -575,7 +575,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// breaking `Rc::from_raw`.
RefKind::Raw { .. } => Tag::Untagged,
// All other pointesr are properly tracked.
_ => Tag::Tagged(this.memory.extra.stacked_borrows.borrow_mut().new_ptr()),
_ => Tag::Tagged(this.memory.extra.stacked_borrows.as_ref().unwrap().borrow_mut().new_ptr()),
};
// Reborrow.

View file

@ -1,5 +1,5 @@
// This should fail even without validation
// compile-flags: -Zmiri-disable-validation
// This should fail even without validation/SB
// compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows
fn main() {
let x = &1; // the `&1` is promoted to a constant, but it used to be that only the pointer is marked static, not the pointee

View file

@ -1,5 +1,5 @@
// This should fail even without validation
// compile-flags: -Zmiri-disable-validation
// This should fail even without validation/SB
// compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows
#![allow(dead_code, unused_variables)]

View file

@ -1,5 +1,5 @@
// Validation changes why we fail
// compile-flags: -Zmiri-disable-validation
// Validation/SB changes why we fail
// compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows
// error-pattern: tried to deallocate `Stack` memory but gave `Machine(Rust)` as the kind

View file

@ -1,5 +1,5 @@
// Validation detects that we are casting & to &mut and so it changes why we fail
// compile-flags: -Zmiri-disable-validation
// Stacked Borrows detects that we are casting & to &mut and so it changes why we fail
// compile-flags: -Zmiri-disable-stacked-borrows
static X: usize = 5;

View file

@ -1,5 +1,5 @@
// Validation detects that we are casting & to &mut and so it changes why we fail
// compile-flags: -Zmiri-disable-validation
// Stacked Borrows detects that we are casting & to &mut and so it changes why we fail
// compile-flags: -Zmiri-disable-stacked-borrows
use std::mem::transmute;

View file

@ -1,5 +1,5 @@
// Validation detects that we are casting & to &mut and so it changes why we fail
// compile-flags: -Zmiri-disable-validation
// Stacked Borrows detects that we are casting & to &mut and so it changes why we fail
// compile-flags: -Zmiri-disable-stacked-borrows
use std::mem::transmute;

View file

@ -1,5 +1,5 @@
// Validation catches this (correctly) as UB.
// compile-flags: -Zmiri-disable-validation
// Stacked Borrows catches this (correctly) as UB.
// compile-flags: -Zmiri-disable-stacked-borrows
// This test is intended to guard against the problem described in commit
// 39bb1254d1eaf74f45a4e741097e33fc942168d5.

View file

@ -1,5 +1,5 @@
// Validation disallows this becuase the reference is never cast to a raw pointer.
// compile-flags: -Zmiri-disable-validation
// Stacked Borrows disallows this becuase the reference is never cast to a raw pointer.
// compile-flags: -Zmiri-disable-stacked-borrows
fn main() {
// If we are careful, we can exploit data layout...