diff --git a/appveyor.yml b/appveyor.yml index cf578120c9..1e9ccbdf95 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -14,19 +14,20 @@ branches: - master install: - # install Rust + # Install Rust. - set PATH=C:\Program Files\Git\mingw64\bin;C:\msys64\mingw%MSYS2_BITS%\bin;%PATH% - set /p RUST_TOOLCHAIN=>>); diff --git a/src/bin/miri.rs b/src/bin/miri.rs index bacea04c97..1bbf3c8c4a 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -10,6 +10,8 @@ extern crate rustc_codegen_utils; extern crate env_logger; extern crate log_settings; extern crate syntax; + +#[macro_use] extern crate log; use std::path::PathBuf; @@ -212,12 +214,7 @@ fn main() { init_early_loggers(); let mut args: Vec = std::env::args().collect(); - let sysroot_flag = String::from("--sysroot"); - if !args.contains(&sysroot_flag) { - args.push(sysroot_flag); - args.push(find_sysroot()); - } - + // Parse our own -Z flags and remove them before rustc gets their hand on them. let mut validate = true; args.retain(|arg| { match arg.as_str() { @@ -229,7 +226,16 @@ fn main() { } }); + // Determine sysroot and let rustc know about it + let sysroot_flag = String::from("--sysroot"); + if !args.contains(&sysroot_flag) { + args.push(sysroot_flag); + args.push(find_sysroot()); + } + // Finally, add the default flags all the way in the beginning, but after the binary name. + args.splice(1..1, miri::miri_default_args().iter().map(ToString::to_string)); + trace!("rustc arguments: {:?}", args); let result = rustc_driver::run(move || { rustc_driver::run_compiler(&args, Box::new(MiriCompilerCalls { default: Box::new(RustcDefaultCalls), diff --git a/src/lib.rs b/src/lib.rs index cb544e9bb3..a2271a5898 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,7 +26,7 @@ use syntax::attr; pub use rustc_mir::interpret::*; -pub use rustc_mir::interpret::{self, AllocMap}; // resolve ambiguity +pub use rustc_mir::interpret::{self, AllocMap, PlaceTy}; // resolve ambiguity mod fn_call; mod operator; @@ -48,7 +48,16 @@ use crate::mono_hash_map::MonoHashMap; use crate::stacked_borrows::{EvalContextExt as StackedBorEvalContextExt}; // Used by priroda -pub use stacked_borrows::{Borrow, Stacks, Mut as MutBorrow}; +pub use crate::stacked_borrows::{Borrow, Stack, Stacks, Mut as MutBorrow, BorStackItem}; + +/// Insert rustc arguments at the beginning of the argument list that miri wants to be +/// set per default, for maximal validation power. +pub fn miri_default_args() -> &'static [&'static str] { + // The flags here should be kept in sync with what bootstrap adds when `test-miri` is + // set, which happens in `bootstrap/bin/rustc.rs` in the rustc sources; and also + // kept in sync with `xargo/build.sh` in this repo and `appveyor.yml`. + &["-Zalways-encode-mir", "-Zmir-emit-retag", "-Zmir-opt-level=0"] +} // Used by priroda pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>( @@ -438,21 +447,30 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { } #[inline(always)] - fn memory_accessed( + fn memory_read( alloc: &Allocation, ptr: Pointer, size: Size, - access: MemoryAccess, ) -> EvalResult<'tcx> { - alloc.extra.memory_accessed(ptr, size, access) + alloc.extra.memory_read(ptr, size) + } + + #[inline(always)] + fn memory_written( + alloc: &mut Allocation, + ptr: Pointer, + size: Size, + ) -> EvalResult<'tcx> { + alloc.extra.memory_written(ptr, size) } #[inline(always)] fn memory_deallocated( alloc: &mut Allocation, ptr: Pointer, + size: Size, ) -> EvalResult<'tcx> { - alloc.extra.memory_deallocated(ptr) + alloc.extra.memory_deallocated(ptr, size) } #[inline(always)] @@ -507,4 +525,20 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { Ok(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag)) } } + + #[inline(always)] + fn retag( + ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, + fn_entry: bool, + place: PlaceTy<'tcx, Borrow>, + ) -> EvalResult<'tcx> { + if !ecx.tcx.sess.opts.debugging_opts.mir_emit_retag || !Self::enforce_validity(ecx) { + // No tracking, or no retagging. This is possible because a dependency of ours might be + // called with different flags than we are, + // Also, honor the whitelist in `enforce_validity` because otherwise we might retag + // uninitialized data. + return Ok(()) + } + ecx.retag(fn_entry, place) + } } diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index a569ed4e55..afc76fa375 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -1,11 +1,11 @@ -use std::cell::{Cell, RefCell}; +use std::cell::RefCell; -use rustc::ty::{Ty, layout::Size}; +use rustc::ty::{self, Ty, layout::Size}; use rustc::hir; -use super::{ - MemoryAccess, MemoryKind, MiriMemoryKind, RangeMap, EvalResult, AllocId, - Pointer, +use crate::{ + MemoryKind, MiriMemoryKind, RangeMap, EvalResult, AllocId, + Pointer, PlaceTy, }; pub type Timestamp = u64; @@ -64,6 +64,12 @@ impl Borrow { } } +impl Default for Borrow { + fn default() -> Self { + Borrow::Mut(Mut::Raw) + } +} + /// An item in the borrow stack #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] pub enum BorStackItem { @@ -74,26 +80,33 @@ pub enum BorStackItem { FnBarrier(usize) } -impl Default for Borrow { - fn default() -> Self { - Borrow::Mut(Mut::Raw) +impl BorStackItem { + #[inline(always)] + pub fn is_fn_barrier(self) -> bool { + match self { + BorStackItem::FnBarrier(_) => true, + _ => false, + } } } -/// What kind of reference are we talking about? +/// What kind of usage of the pointer are we talking about? #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] -pub enum RefKind { - Mut, - Shr, +pub enum UsageKind { + /// Write, or create &mut + Write, + /// Read, or create & + Read, + /// Create * Raw, } -impl From> for RefKind { +impl From> for UsageKind { fn from(mutbl: Option) -> Self { match mutbl { - None => RefKind::Raw, - Some(hir::MutMutable) => RefKind::Mut, - Some(hir::MutImmutable) => RefKind::Shr, + None => UsageKind::Raw, + Some(hir::MutMutable) => UsageKind::Write, + Some(hir::MutImmutable) => UsageKind::Read, } } } @@ -101,18 +114,18 @@ impl From> for RefKind { /// Extra global machine state #[derive(Clone, Debug)] pub struct State { - clock: Cell + clock: Timestamp } impl State { pub fn new() -> State { - State { clock: Cell::new(0) } + State { clock: 0 } } } /// Extra per-location state #[derive(Clone, Debug)] -struct Stack { +pub struct Stack { borrows: Vec, // used as a stack frozen_since: Option, } @@ -126,87 +139,103 @@ impl Default for Stack { } } +impl Stack { + #[inline(always)] + fn is_frozen(&self) -> bool { + self.frozen_since.is_some() + } +} + /// Extra per-allocation state #[derive(Clone, Debug, Default)] pub struct Stacks { + // Even reading memory can have effects on the stack, so we need a `RefCell` here. stacks: RefCell>, } /// Core operations impl<'tcx> Stack { - /// Check if `bor` is currently active. We accept a `Raw` on a frozen location - /// because this could be a shared (re)borrow. If you want to mutate, this - /// is not the right function to call! - fn check(&self, bor: Borrow) -> bool { - match bor { - Borrow::Frz(acc_t) => - // Must be frozen at least as long as the `acc_t` says. - self.frozen_since.map_or(false, |loc_t| loc_t <= acc_t), - Borrow::Mut(acc_m) => - // Raw pointers are fine with frozen locations. This is important because &Cell is raw! - if self.frozen_since.is_some() { - acc_m.is_raw() - } else { - self.borrows.last().map_or(false, |&loc_itm| loc_itm == BorStackItem::Mut(acc_m)) - } - } - } - /// Check if `bor` could be activated by unfreezing and popping. - /// `force_mut` indicates whether being frozen is potentially acceptable. + /// `usage` indicates whether this is being used to read/write (or, equivalently, to + /// borrow as &/&mut), or to borrow as raw. /// Returns `Err` if the answer is "no"; otherwise the data says /// what needs to happen to activate this: `None` = nothing, /// `Some(n)` = unfreeze and make item `n` the top item of the stack. - fn reactivatable(&self, bor: Borrow, force_mut: bool) -> Result, String> { - // Unless mutation is bound to happen, do NOT change anything if `bor` is already active. - // In particular, if it is a `Mut(Raw)` and we are frozen, this should be a NOP. - if !force_mut && self.check(bor) { - return Ok(None); - } - - let acc_m = match bor { + fn reactivatable(&self, bor: Borrow, usage: UsageKind) -> Result, String> { + let mut_borrow = match bor { Borrow::Frz(since) => - return Err(if force_mut { - format!("Using a shared borrow for mutation") - } else { - format!( - "Location should be frozen since {} but {}", - since, - match self.frozen_since { - None => format!("it is not frozen at all"), - Some(since) => format!("it is only frozen since {}", since), - } - ) - }), - Borrow::Mut(acc_m) => acc_m + // The only way to reactivate a `Frz` is if this is already frozen. + return match self.frozen_since { + _ if usage == UsageKind::Write => + Err(format!("Using a shared borrow for mutation")), + None => + Err(format!("Location should be frozen but it is not")), + Some(loc) if loc <= since => + Ok(None), + Some(loc) => + Err(format!("Location should be frozen since {} but it is only frozen \ + since {}", since, loc)), + }, + Borrow::Mut(Mut::Raw) if self.is_frozen() && usage != UsageKind::Write => + // Non-mutating access with a raw from a frozen location is a special case: The + // shared refs do not mind raw reads, and the raw itself does not assume any + // exclusivity. So we do not even require there to be a raw on the stack, + // the raw is instead "matched" by the fact that this location is frozen. + // This does not break the assumption that an `&mut` we own is + // exclusive for reads, because there we have the invariant that + // the location is *not* frozen. + return Ok(None), + Borrow::Mut(mut_borrow) => mut_borrow }; - // This is where we would unfreeze. + // See if we can get there via popping. for (idx, &itm) in self.borrows.iter().enumerate().rev() { match itm { BorStackItem::FnBarrier(_) => - return Err(format!("Trying to reactivate a mutable borrow ({:?}) that lives behind a barrier", acc_m)), - BorStackItem::Mut(loc_m) => { - if loc_m == acc_m { return Ok(Some(idx)); } + return Err(format!("Trying to reactivate a mutable borrow ({:?}) that lives \ + behind a barrier", mut_borrow)), + BorStackItem::Mut(loc) => { + if loc == mut_borrow { + // We found it! This is good to know. + // Yet, maybe we do not really want to pop? + if usage == UsageKind::Read && self.is_frozen() { + // Whoever had exclusive access to this location allowed it + // to become frozen. That can only happen if they reborrowed + // to a shared ref, at which point they gave up on exclusive access. + // Hence we allow more reads, entirely ignoring everything above + // on the stack (but still making sure it is on the stack). + // This does not break the assumption that an `&mut` we own is + // exclusive for reads, because there we have the invariant that + // the location is *not* frozen. + return Ok(None); + } else { + return Ok(Some(idx)); + } + } } } } // Nothing to be found. - Err(format!("Mutable borrow-to-reactivate ({:?}) does not exist on the stack", acc_m)) + Err(format!("Mutable borrow-to-reactivate ({:?}) does not exist on the stack", mut_borrow)) } - /// Reactive `bor` for this stack. If `force_mut` is set, we want to aggressively - /// unfreeze this location (because we are about to mutate, so a frozen `Raw` is not okay). - fn reactivate(&mut self, bor: Borrow, force_mut: bool) -> EvalResult<'tcx> { - let action = match self.reactivatable(bor, force_mut) { + /// Reactive `bor` for this stack. `usage` indicates whether this is being + /// used to read/write (or, equivalently, to borrow as &/&mut), or to borrow as raw. + fn reactivate(&mut self, bor: Borrow, usage: UsageKind) -> EvalResult<'tcx> { + let action = match self.reactivatable(bor, usage) { Ok(action) => action, Err(err) => return err!(MachineError(err)), }; - + // Execute what `reactivatable` told us to do. match action { None => {}, // nothing to do Some(top) => { + if self.frozen_since.is_some() { + trace!("reactivate: Unfreezing"); + } self.frozen_since = None; - self.borrows.truncate(top+1); + for itm in self.borrows.drain(top+1..).rev() { + trace!("reactivate: Popping {:?}", itm); + } } } @@ -214,7 +243,9 @@ impl<'tcx> Stack { } /// Initiate `bor`; mostly this means freezing or pushing. - fn initiate(&mut self, bor: Borrow) -> EvalResult<'tcx> { + /// This operation cannot fail; it is up to the caller to ensure that the precondition + /// is met: We cannot push onto frozen stacks. + fn initiate(&mut self, bor: Borrow) { match bor { Borrow::Frz(t) => { match self.frozen_since { @@ -240,83 +271,73 @@ impl<'tcx> Stack { // from it is fine with this as well. trace!("initiate: Initiating a raw on a frozen location, not doing a thing"), Some(_) => - return err!(MachineError(format!("Trying to mutate frozen location"))) + bug!("Trying to mutate frozen location") } } } - Ok(()) } } impl State { - fn increment_clock(&self) -> Timestamp { - let val = self.clock.get(); - self.clock.set(val + 1); + fn increment_clock(&mut self) -> Timestamp { + let val = self.clock; + self.clock = val + 1; val } } /// Higher-level operations impl<'tcx> Stacks { - pub fn memory_accessed( + /// The single most operation: Make sure that using `ptr` as `usage` is okay, + /// and if `new_bor` is present then make that the new current borrow. + fn use_and_maybe_re_borrow( &self, ptr: Pointer, size: Size, - access: MemoryAccess, + usage: UsageKind, + new_bor: Option, ) -> EvalResult<'tcx> { - trace!("memory_accessed({:?}) with tag {:?}: {:?}, size {}", access, ptr.tag, ptr, size.bytes()); + trace!("use_and_maybe_re_borrow of tag {:?} as {:?}, new {:?}: {:?}, size {}", + ptr.tag, usage, new_bor, ptr, size.bytes()); let mut stacks = self.stacks.borrow_mut(); for stack in stacks.iter_mut(ptr.offset, size) { - // FIXME: Compare this with what the blog post says. - stack.reactivate(ptr.tag, /*force_mut*/access == MemoryAccess::Write)?; + stack.reactivate(ptr.tag, usage)?; + if let Some(new_bor) = new_bor { + stack.initiate(new_bor); + } } + Ok(()) } - pub fn memory_deallocated( - &mut self, + #[inline(always)] + pub fn memory_read( + &self, ptr: Pointer, + size: Size, ) -> EvalResult<'tcx> { - trace!("memory_deallocated with tag {:?}: {:?}", ptr.tag, ptr); - let stacks = self.stacks.get_mut(); - for stack in stacks.iter_mut_all() { - // This is like mutating. - stack.reactivate(ptr.tag, /*force_mut*/true)?; - } - Ok(()) + // Reads behave exactly like the first half of a reborrow-to-shr + self.use_and_maybe_re_borrow(ptr, size, UsageKind::Read, None) } - fn reborrow( - &self, + #[inline(always)] + pub fn memory_written( + &mut self, ptr: Pointer, size: Size, - new_bor: Borrow, - permit_redundant: bool, ) -> EvalResult<'tcx> { - let mut stacks = self.stacks.borrow_mut(); - for stack in stacks.iter_mut(ptr.offset, size) { - if permit_redundant && stack.check(new_bor) { - // The new borrow is already active! This can happen when creating multiple - // shared references from the same mutable reference. Do nothing. - trace!("reborrow: New borrow {:?} is already active, not doing a thing", new_bor); - } else { - // If we are creating a uniq ref, we certainly want to unfreeze. - // Even if we are doing so from a raw. - // Notice that if this is a local, whenever we access it directly the - // tag here will be the bottommost `Uniq` for that local. That `Uniq` - // never is accessible by the program, so it will not be used by any - // other access. IOW, whenever we directly use a local this will pop - // everything else off the stack, invalidating all previous pointers - // and, in particular, *all* raw pointers. This subsumes the explicit - // `reset` which the blog post [1] says to perform when accessing a local. - // - // [1] https://www.ralfj.de/blog/2018/08/07/stacked-borrows.html - stack.reactivate(ptr.tag, /*force_mut*/new_bor.is_uniq())?; - stack.initiate(new_bor)?; - } - } + // Writes behave exactly like the first half of a reborrow-to-mut + self.use_and_maybe_re_borrow(ptr, size, UsageKind::Write, None) + } - Ok(()) + pub fn memory_deallocated( + &mut self, + ptr: Pointer, + size: Size, + ) -> EvalResult<'tcx> { + // This is like mutating + self.use_and_maybe_re_borrow(ptr, size, UsageKind::Write, None) + // FIXME: Error out of there are any barriers? } /// Pushes the first borrow to the stacks, must be a mutable one. @@ -334,18 +355,12 @@ impl<'tcx> Stacks { } pub trait EvalContextExt<'tcx> { - fn tag_for_pointee( - &self, - pointee_ty: Ty<'tcx>, - ref_kind: RefKind, - ) -> Borrow; - fn tag_reference( - &self, + &mut self, ptr: Pointer, pointee_ty: Ty<'tcx>, size: Size, - ref_kind: RefKind, + usage: UsageKind, ) -> EvalResult<'tcx, Borrow>; @@ -354,7 +369,7 @@ pub trait EvalContextExt<'tcx> { ptr: Pointer, pointee_ty: Ty<'tcx>, size: Size, - ref_kind: RefKind, + usage: UsageKind, ) -> EvalResult<'tcx, Borrow>; fn tag_new_allocation( @@ -362,18 +377,27 @@ pub trait EvalContextExt<'tcx> { id: AllocId, kind: MemoryKind, ) -> Borrow; + + fn retag( + &mut self, + fn_entry: bool, + place: PlaceTy<'tcx, Borrow> + ) -> EvalResult<'tcx>; } impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, 'tcx> { - fn tag_for_pointee( - &self, + /// Called for place-to-value conversion. + fn tag_reference( + &mut self, + ptr: Pointer, pointee_ty: Ty<'tcx>, - ref_kind: RefKind, - ) -> Borrow { + size: Size, + usage: UsageKind, + ) -> EvalResult<'tcx, Borrow> { let time = self.machine.stacked_borrows.increment_clock(); - match ref_kind { - RefKind::Mut => Borrow::Mut(Mut::Uniq(time)), - RefKind::Shr => + let new_bor = match usage { + UsageKind::Write => Borrow::Mut(Mut::Uniq(time)), + UsageKind::Read => // FIXME This does not do enough checking when only part of the data has // interior mutability. When the type is `(i32, Cell)`, we want the // first field to be frozen but not the second. @@ -383,21 +407,10 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' // Shared reference with interior mutability. Borrow::Mut(Mut::Raw) }, - RefKind::Raw => Borrow::Mut(Mut::Raw), - } - } - - /// Called for place-to-value conversion. - fn tag_reference( - &self, - ptr: Pointer, - pointee_ty: Ty<'tcx>, - size: Size, - ref_kind: RefKind, - ) -> EvalResult<'tcx, Borrow> { - let new_bor = self.tag_for_pointee(pointee_ty, ref_kind); + UsageKind::Raw => Borrow::Mut(Mut::Raw), + }; trace!("tag_reference: Creating new reference ({:?}) for {:?} (pointee {}, size {}): {:?}", - ref_kind, ptr, pointee_ty, size.bytes(), new_bor); + usage, ptr, pointee_ty, size.bytes(), new_bor); // Make sure this reference is not dangling or so self.memory().check_bounds(ptr, size, false)?; @@ -405,8 +418,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' // Update the stacks. We cannot use `get_mut` becuse this might be immutable // memory. let alloc = self.memory().get(ptr.alloc_id).expect("We checked that the ptr is fine!"); - let permit_redundant = ref_kind == RefKind::Shr; // redundant shared refs are okay - alloc.extra.reborrow(ptr, size, new_bor, permit_redundant)?; + alloc.extra.use_and_maybe_re_borrow(ptr, size, usage, Some(new_bor))?; Ok(new_bor) } @@ -420,43 +432,40 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' ptr: Pointer, pointee_ty: Ty<'tcx>, size: Size, - ref_kind: RefKind, + usage: UsageKind, ) -> EvalResult<'tcx, Borrow> { + trace!("tag_reference: Accessing reference ({:?}) for {:?} (pointee {}, size {})", + usage, ptr, pointee_ty, size.bytes()); // In principle we should not have to do anything here. However, with transmutes involved, - // it can happen that the tag of `ptr` does not actually match `ref_kind`, and we + // it can happen that the tag of `ptr` does not actually match `usage`, and we // should adjust for that. // Notably, the compiler can introduce such transmutes by optimizing away `&[mut]*`. // That can transmute a raw ptr to a (shared/mut) ref, and a mut ref to a shared one. - match (ref_kind, ptr.tag) { - (RefKind::Raw, _) => { + match (usage, ptr.tag) { + (UsageKind::Raw, _) => { // Don't use the tag, this is a raw access! Even if there is a tag, // that means transmute happened and we ignore the tag. // Also don't do any further validation, this is raw after all. return Ok(Borrow::Mut(Mut::Raw)); } - (RefKind::Mut, Borrow::Mut(Mut::Uniq(_))) | - (RefKind::Shr, Borrow::Frz(_)) | - (RefKind::Shr, Borrow::Mut(Mut::Raw)) => { + (UsageKind::Write, Borrow::Mut(Mut::Uniq(_))) | + (UsageKind::Read, Borrow::Frz(_)) | + (UsageKind::Read, Borrow::Mut(Mut::Raw)) => { // Expected combinations. Nothing to do. // FIXME: We probably shouldn't accept this if we got a raw shr without // interior mutability. } - (RefKind::Mut, Borrow::Mut(Mut::Raw)) => { + (UsageKind::Write, Borrow::Mut(Mut::Raw)) => { // Raw transmuted to mut ref. Keep this as raw access. // We cannot reborrow here; there might be a raw in `&(*var).1` where // `var` is an `&mut`. The other field of the struct might be already frozen, // also using `var`, and that would be okay. } - (RefKind::Shr, Borrow::Mut(Mut::Uniq(_))) => { - // A mut got transmuted to shr. High time we freeze this location! - // Make this a delayed reborrow. Redundant reborows to shr are okay, - // so we do not have to be worried about doing too much. - // FIXME: Reconsider if we really want to mutate things while doing just a deref, - // which, in particular, validation does. - trace!("tag_dereference: Lazy freezing of {:?}", ptr); - return self.tag_reference(ptr, pointee_ty, size, ref_kind); + (UsageKind::Read, Borrow::Mut(Mut::Uniq(_))) => { + // A mut got transmuted to shr. Can happen even from compiler transformations: + // `&*x` gets optimized to `x` even when `x` is a `&mut`. } - (RefKind::Mut, Borrow::Frz(_)) => { + (UsageKind::Write, Borrow::Frz(_)) => { // This is just invalid. // If we ever allow this, we have to consider what we do when a turn a // `Raw`-tagged `&mut` into a raw pointer pointing to a frozen location. @@ -472,12 +481,13 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' let mut stacks = alloc.extra.stacks.borrow_mut(); // We need `iter_mut` because `iter` would skip gaps! for stack in stacks.iter_mut(ptr.offset, size) { - // We accept &mut to a frozen location here, that is just normal. There might - // be shared reborrows that we are about to invalidate with this access. - // We cannot invalidate them aggressively here because the deref might also be - // to just create more shared refs. - if let Err(err) = stack.reactivatable(ptr.tag, /*force_mut*/false) { - return err!(MachineError(format!("Encountered {:?} reference with non-reactivatable tag: {}", ref_kind, err))) + // Conservatively assume that we will only read. + if let Err(err) = stack.reactivatable(ptr.tag, UsageKind::Read) { + return err!(MachineError(format!( + "Encountered {} reference with non-reactivatable tag: {}", + if usage == UsageKind::Write { "mutable" } else { "shared" }, + err + ))) } } // All is good. @@ -491,7 +501,14 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' ) -> Borrow { let mut_borrow = match kind { MemoryKind::Stack => { - // New unique borrow + // New unique borrow. This `Uniq` is not accessible by the program, + // so it will only ever be used when using the local directly (i.e., + // not through a pointer). IOW, whenever we directly use a local this will pop + // everything else off the stack, invalidating all previous pointers + // and, in particular, *all* raw pointers. This subsumes the explicit + // `reset` which the blog post [1] says to perform when accessing a local. + // + // [1] https://www.ralfj.de/blog/2018/08/07/stacked-borrows.html let time = self.machine.stacked_borrows.increment_clock(); Mut::Uniq(time) } @@ -506,4 +523,28 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' alloc.extra.first_borrow(mut_borrow, size); Borrow::Mut(mut_borrow) } + + fn retag( + &mut self, + _fn_entry: bool, + place: PlaceTy<'tcx, Borrow> + ) -> EvalResult<'tcx> { + // For now, we only retag if the toplevel type is a reference. + // TODO: Recurse into structs and enums, sharing code with validation. + let mutbl = match place.layout.ty.sty { + ty::Ref(_, _, mutbl) => mutbl, // go ahead + _ => return Ok(()), // don't do a thing + }; + // We want to reborrow the reference stored there. This will call the hooks + // above. First deref, which will call `tag_dereference`. + // (This is somewhat redundant because validation already did the same thing, + // but what can you do.) + let val = self.read_value(self.place_to_op(place)?)?; + let dest = self.ref_to_mplace(val)?; + // Now put a new ref into the old place, which will call `tag_reference`. + // FIXME: Honor `fn_entry`! + let val = self.create_ref(dest, Some(mutbl))?; + self.write_value(val, place)?; + Ok(()) + } } diff --git a/tests/compile-fail-fullmir/stack_free.rs b/tests/compile-fail-fullmir/stack_free.rs index 6d853e75fb..a33bca1267 100644 --- a/tests/compile-fail-fullmir/stack_free.rs +++ b/tests/compile-fail-fullmir/stack_free.rs @@ -1,5 +1,5 @@ // Validation changes why we fail -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation // error-pattern: tried to deallocate Stack memory but gave Machine(Rust) as the kind diff --git a/tests/compile-fail/cast_box_int_to_fn_ptr.rs b/tests/compile-fail/cast_box_int_to_fn_ptr.rs index c3b1fa5958..7ee3bc6276 100644 --- a/tests/compile-fail/cast_box_int_to_fn_ptr.rs +++ b/tests/compile-fail/cast_box_int_to_fn_ptr.rs @@ -1,5 +1,5 @@ // Validation makes this fail in the wrong place -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation fn main() { let b = Box::new(42); diff --git a/tests/compile-fail/cast_int_to_fn_ptr.rs b/tests/compile-fail/cast_int_to_fn_ptr.rs index 1971ce1557..207af4ae2c 100644 --- a/tests/compile-fail/cast_int_to_fn_ptr.rs +++ b/tests/compile-fail/cast_int_to_fn_ptr.rs @@ -1,5 +1,5 @@ // Validation makes this fail in the wrong place -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation fn main() { let g = unsafe { diff --git a/tests/compile-fail/execute_memory.rs b/tests/compile-fail/execute_memory.rs index d859e9072e..295756ef0f 100644 --- a/tests/compile-fail/execute_memory.rs +++ b/tests/compile-fail/execute_memory.rs @@ -1,5 +1,5 @@ // Validation makes this fail in the wrong place -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation #![feature(box_syntax)] diff --git a/tests/compile-fail/fn_ptr_offset.rs b/tests/compile-fail/fn_ptr_offset.rs index cccb21790d..9d29316fe2 100644 --- a/tests/compile-fail/fn_ptr_offset.rs +++ b/tests/compile-fail/fn_ptr_offset.rs @@ -1,5 +1,5 @@ // Validation makes this fail in the wrong place -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation use std::mem; diff --git a/tests/compile-fail/invalid_bool2.rs b/tests/compile-fail/invalid_bool.rs similarity index 73% rename from tests/compile-fail/invalid_bool2.rs rename to tests/compile-fail/invalid_bool.rs index 2348c62559..e80dc15efa 100644 --- a/tests/compile-fail/invalid_bool2.rs +++ b/tests/compile-fail/invalid_bool.rs @@ -1,5 +1,5 @@ // Validation makes this fail in the wrong place -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation fn main() { let b = unsafe { std::mem::transmute::(2) }; diff --git a/tests/compile-fail/invalid_char2.rs b/tests/compile-fail/invalid_char.rs similarity index 80% rename from tests/compile-fail/invalid_char2.rs rename to tests/compile-fail/invalid_char.rs index 5de2d073f3..b67ed9ba52 100644 --- a/tests/compile-fail/invalid_char2.rs +++ b/tests/compile-fail/invalid_char.rs @@ -1,5 +1,5 @@ // Validation makes this fail in the wrong place -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation fn main() { assert!(std::char::from_u32(-1_i32 as u32).is_none()); diff --git a/tests/compile-fail/invalid_enum_discriminant2.rs b/tests/compile-fail/invalid_enum_discriminant.rs similarity index 79% rename from tests/compile-fail/invalid_enum_discriminant2.rs rename to tests/compile-fail/invalid_enum_discriminant.rs index ea94081693..bd5cb55b6c 100644 --- a/tests/compile-fail/invalid_enum_discriminant2.rs +++ b/tests/compile-fail/invalid_enum_discriminant.rs @@ -1,5 +1,5 @@ // Validation makes this fail in the wrong place -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation // error-pattern: invalid enum discriminant diff --git a/tests/compile-fail/never_say_never.rs b/tests/compile-fail/never_say_never.rs index 634488489b..d7e6a8c09f 100644 --- a/tests/compile-fail/never_say_never.rs +++ b/tests/compile-fail/never_say_never.rs @@ -1,5 +1,5 @@ // This should fail even without validation -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation #![feature(never_type)] #![allow(unreachable_code)] diff --git a/tests/compile-fail/never_transmute_humans.rs b/tests/compile-fail/never_transmute_humans.rs index c5c53d4231..169e861be0 100644 --- a/tests/compile-fail/never_transmute_humans.rs +++ b/tests/compile-fail/never_transmute_humans.rs @@ -1,5 +1,5 @@ // This should fail even without validation -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation #![feature(never_type)] #![allow(unreachable_code)] diff --git a/tests/compile-fail/never_transmute_void.rs b/tests/compile-fail/never_transmute_void.rs index 5620b6559c..9c0165fed2 100644 --- a/tests/compile-fail/never_transmute_void.rs +++ b/tests/compile-fail/never_transmute_void.rs @@ -1,5 +1,5 @@ // This should fail even without validation -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation #![feature(never_type)] #![allow(unreachable_code)] diff --git a/tests/compile-fail/panic.rs b/tests/compile-fail/panic.rs index 80149eeffa..0d594f9bd4 100644 --- a/tests/compile-fail/panic.rs +++ b/tests/compile-fail/panic.rs @@ -1,5 +1,3 @@ -// FIXME: Something in panic handling fails validation with full-MIR -// compile-flags: -Zmir-emit-validate=0 //error-pattern: the evaluated program panicked fn main() { diff --git a/tests/compile-fail/reference_to_packed.rs b/tests/compile-fail/reference_to_packed.rs index 14a2afc33f..befe96f2b3 100644 --- a/tests/compile-fail/reference_to_packed.rs +++ b/tests/compile-fail/reference_to_packed.rs @@ -1,5 +1,5 @@ // This should fail even without validation -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation #![allow(dead_code, unused_variables)] diff --git a/tests/compile-fail/stacked_borrows/alias_through_mutation.rs b/tests/compile-fail/stacked_borrows/alias_through_mutation.rs index 7d56f30b3e..0b2d459366 100644 --- a/tests/compile-fail/stacked_borrows/alias_through_mutation.rs +++ b/tests/compile-fail/stacked_borrows/alias_through_mutation.rs @@ -1,6 +1,3 @@ -// With optimizations, we just store a raw in `x`, and there is no problem. -// compile-flags: -Zmir-opt-level=0 - #![allow(unused_variables)] // This makes a ref that was passed to us via &mut alias with things it should not alias with @@ -14,5 +11,5 @@ fn main() { retarget(&mut target_alias, target); // now `target_alias` points to the same thing as `target` *target = 13; - let _val = *target_alias; //~ ERROR Shr reference with non-reactivatable tag + let _val = *target_alias; //~ ERROR reference with non-reactivatable tag } diff --git a/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs b/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs index dc1e3cc81e..2c48404ddf 100644 --- a/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs +++ b/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs @@ -1,6 +1,3 @@ -// FIXME: Without retagging, optimization kills finding this problem -// compile-flags: -Zmir-opt-level=0 - #![allow(unused_variables)] mod safe { @@ -17,6 +14,6 @@ fn main() { let v = vec![0,1,2]; let v1 = safe::as_mut_slice(&v); let v2 = safe::as_mut_slice(&v); - v1[1] = 5; //~ ERROR Mut reference with non-reactivatable tag + v1[1] = 5; //~ ERROR reference with non-reactivatable tag v1[1] = 6; } diff --git a/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs b/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs index a697ba9167..d8a241cab5 100644 --- a/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs +++ b/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs @@ -1,6 +1,3 @@ -// FIXME: Without retagging, optimization kills finding this problem -// compile-flags: -Zmir-opt-level=0 - #![allow(unused_variables)] mod safe { @@ -14,7 +11,7 @@ mod safe { assert!(mid <= len); (from_raw_parts_mut(ptr, len - mid), // BUG: should be "mid" instead of "len - mid" - //~^ ERROR Mut reference with non-reactivatable tag + //~^ ERROR reference with non-reactivatable tag from_raw_parts_mut(ptr.offset(mid as isize), len - mid)) } } diff --git a/tests/compile-fail/stacked_borrows/illegal_read1.rs b/tests/compile-fail/stacked_borrows/illegal_read1.rs new file mode 100644 index 0000000000..59190a15db --- /dev/null +++ b/tests/compile-fail/stacked_borrows/illegal_read1.rs @@ -0,0 +1,15 @@ +// A callee may not read the destination of our `&mut` without +// us noticing. + +fn main() { + let mut x = 15; + let xraw = &mut x as *mut _; + let xref = unsafe { &mut *xraw }; // derived from raw, so using raw is still okay... + callee(xraw); + let _val = *xref; // ...but any use of raw will invalidate our ref. + //~^ ERROR: mutable reference with non-reactivatable tag +} + +fn callee(xraw: *mut i32) { + let _val = unsafe { *xraw }; +} diff --git a/tests/compile-fail/stacked_borrows/illegal_read2.rs b/tests/compile-fail/stacked_borrows/illegal_read2.rs new file mode 100644 index 0000000000..594117d28a --- /dev/null +++ b/tests/compile-fail/stacked_borrows/illegal_read2.rs @@ -0,0 +1,18 @@ +// A callee may not read the destination of our `&mut` without +// us noticing. + +fn main() { + let mut x = 15; + let xraw = &mut x as *mut _; + let xref = unsafe { &mut *xraw }; // derived from raw, so using raw is still okay... + callee(xraw); + let _val = *xref; // ...but any use of raw will invalidate our ref. + //~^ ERROR: mutable reference with non-reactivatable tag +} + +fn callee(xraw: *mut i32) { + // We are a bit sneaky: We first create a shared ref, exploiting the reborrowing rules, + // and then we read through that. + let shr = unsafe { &*xraw }; + let _val = *shr; +} diff --git a/tests/compile-fail/stacked_borrows/illegal_write1.rs b/tests/compile-fail/stacked_borrows/illegal_write1.rs index 131e89572a..ab951be5ec 100644 --- a/tests/compile-fail/stacked_borrows/illegal_write1.rs +++ b/tests/compile-fail/stacked_borrows/illegal_write1.rs @@ -8,5 +8,5 @@ fn main() { let target = Box::new(42); // has an implicit raw let ref_ = &*target; evil(ref_); // invalidates shared ref, activates raw - let _x = *ref_; //~ ERROR Shr reference with non-reactivatable tag + let _x = *ref_; //~ ERROR reference with non-reactivatable tag } diff --git a/tests/compile-fail/stacked_borrows/illegal_write2.rs b/tests/compile-fail/stacked_borrows/illegal_write2.rs index ac9c3397f5..b53655c821 100644 --- a/tests/compile-fail/stacked_borrows/illegal_write2.rs +++ b/tests/compile-fail/stacked_borrows/illegal_write2.rs @@ -1,4 +1,5 @@ -// The reborow gets optimized away, so we can only detect this issue without optimizations +// We fail to detect this when neither this nor libstd are optimized/have retagging. +// FIXME: Investigate that. // compile-flags: -Zmir-opt-level=0 #![allow(unused_variables)] diff --git a/tests/compile-fail/stacked_borrows/illegal_write4.rs b/tests/compile-fail/stacked_borrows/illegal_write4.rs index 094a389519..12deb518b4 100644 --- a/tests/compile-fail/stacked_borrows/illegal_write4.rs +++ b/tests/compile-fail/stacked_borrows/illegal_write4.rs @@ -1,31 +1,13 @@ -// The compiler inserts some reborrows, enable optimizations to -// get rid of them. -// compile-flags: -Zmir-opt-level=1 - use std::mem; -// This is an example of a piece of code that intuitively seems like we might -// want to reject it, but that doesn't turn out to be possible. - fn main() { - let target = 42; - // Make sure a cannot use a raw-tagged `&mut` pointing to a frozen location, not - // even to create a raw. - let reference = ⌖ // freeze + let mut target = 42; + // Make sure we cannot use a raw-tagged `&mut` pointing to a frozen location. + // Even just creating it unfreezes. + let raw = &mut target as *mut _; // let this leak to raw + let reference = unsafe { &*raw }; // freeze let ptr = reference as *const _ as *mut i32; // raw ptr, with raw tag - let mut_ref: &mut i32 = unsafe { mem::transmute(ptr) }; // &mut, with raw tag - // Now we have an &mut to a frozen location, but that is completely normal: - // We'd just unfreeze the location if we used it. - let bad_ptr = mut_ref as *mut i32; // even just creating this is like a use of `mut_ref`. - // That violates the location being frozen! However, we do not properly detect this: - // We first see a `&mut` with a `Raw` tag being deref'd for a frozen location, - // which can happen legitimately if the compiler optimized away an `&mut*` that - // turns a raw into a `&mut`. Next, we create a raw ref to a frozen location - // from a `Raw` tag, which can happen legitimately when interior mutability - // is involved. - let _val = *reference; // Make sure it is still frozen. - - // We only actually unfreeze once we muteate through the bad pointer. - unsafe { *bad_ptr = 42 }; //~ ERROR does not exist on the stack - let _val = *reference; + let _mut_ref: &mut i32 = unsafe { mem::transmute(ptr) }; // &mut, with raw tag + // Now we retag, making our ref top-of-stack -- and, in particular, unfreezing. + let _val = *reference; //~ ERROR Location should be frozen } diff --git a/tests/compile-fail/stacked_borrows/illegal_write5.rs b/tests/compile-fail/stacked_borrows/illegal_write5.rs new file mode 100644 index 0000000000..f4704ad571 --- /dev/null +++ b/tests/compile-fail/stacked_borrows/illegal_write5.rs @@ -0,0 +1,15 @@ +// A callee may not write to the destination of our `&mut` without +// us noticing. + +fn main() { + let mut x = 15; + let xraw = &mut x as *mut _; + let xref = unsafe { &mut *xraw }; // derived from raw, so using raw is still okay... + callee(xraw); + let _val = *xref; // ...but any use of raw will invalidate our ref. + //~^ ERROR: reference with non-reactivatable tag +} + +fn callee(xraw: *mut i32) { + unsafe { *xraw = 15 }; +} diff --git a/tests/compile-fail/stacked_borrows/load_invalid_mut.rs b/tests/compile-fail/stacked_borrows/load_invalid_mut.rs index 060cec962c..4ea61cd606 100644 --- a/tests/compile-fail/stacked_borrows/load_invalid_mut.rs +++ b/tests/compile-fail/stacked_borrows/load_invalid_mut.rs @@ -5,5 +5,5 @@ fn main() { let xref = unsafe { &mut *xraw }; let xref_in_mem = Box::new(xref); let _val = *x; // invalidate xraw - let _val = *xref_in_mem; //~ ERROR Mut reference with non-reactivatable tag + let _val = *xref_in_mem; //~ ERROR mutable reference with non-reactivatable tag } diff --git a/tests/compile-fail/stacked_borrows/load_invalid_shr.rs b/tests/compile-fail/stacked_borrows/load_invalid_shr.rs index 785a15c470..53179c954d 100644 --- a/tests/compile-fail/stacked_borrows/load_invalid_shr.rs +++ b/tests/compile-fail/stacked_borrows/load_invalid_shr.rs @@ -4,6 +4,6 @@ fn main() { let xraw = x as *mut _; let xref = unsafe { &*xraw }; let xref_in_mem = Box::new(xref); - let _val = *x; // invalidate xraw - let _val = *xref_in_mem; //~ ERROR Shr reference with non-reactivatable tag: Location should be frozen + *x = 42; // invalidate xraw + let _val = *xref_in_mem; //~ ERROR shared reference with non-reactivatable tag: Location should be frozen } diff --git a/tests/compile-fail/stacked_borrows/pass_invalid_mut.rs b/tests/compile-fail/stacked_borrows/pass_invalid_mut.rs index bc950771ad..5e1118160a 100644 --- a/tests/compile-fail/stacked_borrows/pass_invalid_mut.rs +++ b/tests/compile-fail/stacked_borrows/pass_invalid_mut.rs @@ -6,5 +6,5 @@ fn main() { let xraw = x as *mut _; let xref = unsafe { &mut *xraw }; let _val = *x; // invalidate xraw - foo(xref); //~ ERROR Mut reference with non-reactivatable tag + foo(xref); //~ ERROR mutable reference with non-reactivatable tag } diff --git a/tests/compile-fail/stacked_borrows/pass_invalid_shr.rs b/tests/compile-fail/stacked_borrows/pass_invalid_shr.rs index 8b7a846d84..e4b26cfff6 100644 --- a/tests/compile-fail/stacked_borrows/pass_invalid_shr.rs +++ b/tests/compile-fail/stacked_borrows/pass_invalid_shr.rs @@ -5,6 +5,6 @@ fn main() { let x = &mut 42; let xraw = &*x as *const _; let xref = unsafe { &*xraw }; - let _val = *x; // invalidate xraw - foo(xref); //~ ERROR Shr reference with non-reactivatable tag: Location should be frozen + *x = 42; // invalidate xraw + foo(xref); //~ ERROR shared reference with non-reactivatable tag: Location should be frozen } diff --git a/tests/compile-fail/stacked_borrows/return_invalid_mut.rs b/tests/compile-fail/stacked_borrows/return_invalid_mut.rs index c02892671e..949b3829ff 100644 --- a/tests/compile-fail/stacked_borrows/return_invalid_mut.rs +++ b/tests/compile-fail/stacked_borrows/return_invalid_mut.rs @@ -3,7 +3,7 @@ fn foo(x: &mut (i32, i32)) -> &mut i32 { let xraw = x as *mut (i32, i32); let ret = unsafe { &mut (*xraw).1 }; let _val = *x; // invalidate xraw and its children - ret //~ ERROR Mut reference with non-reactivatable tag + ret //~ ERROR mutable reference with non-reactivatable tag } fn main() { diff --git a/tests/compile-fail/stacked_borrows/return_invalid_shr.rs b/tests/compile-fail/stacked_borrows/return_invalid_shr.rs index 89c94127b0..2d34350359 100644 --- a/tests/compile-fail/stacked_borrows/return_invalid_shr.rs +++ b/tests/compile-fail/stacked_borrows/return_invalid_shr.rs @@ -2,8 +2,8 @@ fn foo(x: &mut (i32, i32)) -> &i32 { let xraw = x as *mut (i32, i32); let ret = unsafe { &(*xraw).1 }; - let _val = *x; // invalidate xraw and its children - ret //~ ERROR Shr reference with non-reactivatable tag: Location should be frozen + x.1 = 42; // invalidate xraw on the 2nd field + ret //~ ERROR shared reference with non-reactivatable tag: Location should be frozen } fn main() { diff --git a/tests/compile-fail/stacked_borrows/shared_confusion.rs b/tests/compile-fail/stacked_borrows/shared_confusion.rs index e3e4c4da77..624587932c 100644 --- a/tests/compile-fail/stacked_borrows/shared_confusion.rs +++ b/tests/compile-fail/stacked_borrows/shared_confusion.rs @@ -1,3 +1,11 @@ +// Optimization kills all the reborrows, enough to make this error go away. There are +// no retags either because we don't retag immediately after a `&[mut]`; we rely on +// that creating a fresh reference. +// See `shared_confusion_opt.rs` for a variant that is caught even with optimizations. +// Keep this test to make sure that without optimizations, we do not have to actually +// use the `x_inner_shr`. +// compile-flags: -Zmir-opt-level=0 + #![allow(unused_variables)] use std::cell::RefCell; @@ -9,11 +17,11 @@ fn test(r: &mut RefCell) { { let x_inner_shr = &*x_inner; // frozen let y = &*r; // outer ref, not freezing - let x_inner_shr2 = &*x_inner; // freezing again + let x_inner_shr = &*x_inner; // freezing again } // Our old raw should be dead by now unsafe { *x_evil = 0; } // this falls back to some Raw higher up the stack - *x_inner = 12; //~ ERROR Mut reference with non-reactivatable tag + *x_inner = 12; //~ ERROR reference with non-reactivatable tag } fn main() { diff --git a/tests/compile-fail/stacked_borrows/shared_confusion_opt.rs b/tests/compile-fail/stacked_borrows/shared_confusion_opt.rs new file mode 100644 index 0000000000..3030f5dd40 --- /dev/null +++ b/tests/compile-fail/stacked_borrows/shared_confusion_opt.rs @@ -0,0 +1,25 @@ +// A variant of `shared_confusion.rs` that gets flagged even with optimizations. + +#![allow(unused_variables)] +use std::cell::RefCell; + +fn test(r: &mut RefCell) { + let x = &*r; // not freezing because interior mutability + let mut x_ref = x.borrow_mut(); + let x_inner : &mut i32 = &mut *x_ref; // Uniq reference + let x_evil = x_inner as *mut _; + { + let x_inner_shr = &*x_inner; // frozen + let _val = *x_inner_shr; + let y = &*r; // outer ref, not freezing + let x_inner_shr = &*x_inner; // freezing again + let _val = *x_inner_shr; + } + // Our old raw should be dead by now + unsafe { *x_evil = 0; } // this falls back to some Raw higher up the stack + *x_inner = 12; //~ ERROR reference with non-reactivatable tag +} + +fn main() { + test(&mut RefCell::new(0)); +} diff --git a/tests/compile-fail/stacked_borrows/static_memory_modification.rs b/tests/compile-fail/stacked_borrows/static_memory_modification.rs new file mode 100644 index 0000000000..c092cbfe50 --- /dev/null +++ b/tests/compile-fail/stacked_borrows/static_memory_modification.rs @@ -0,0 +1,8 @@ +static X: usize = 5; + +#[allow(mutable_transmutes)] +fn main() { + let _x = unsafe { + std::mem::transmute::<&usize, &mut usize>(&X) //~ ERROR mutable reference with frozen tag + }; +} diff --git a/tests/compile-fail/static_memory_modification.rs b/tests/compile-fail/static_memory_modification.rs index c758926458..07a277a16f 100644 --- a/tests/compile-fail/static_memory_modification.rs +++ b/tests/compile-fail/static_memory_modification.rs @@ -1,5 +1,5 @@ // Validation detects that we are casting & to &mut and so it changes why we fail -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation static X: usize = 5; diff --git a/tests/compile-fail/static_memory_modification2.rs b/tests/compile-fail/static_memory_modification2.rs index c9857b2059..73928f533c 100644 --- a/tests/compile-fail/static_memory_modification2.rs +++ b/tests/compile-fail/static_memory_modification2.rs @@ -1,5 +1,5 @@ // Validation detects that we are casting & to &mut and so it changes why we fail -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation use std::mem::transmute; diff --git a/tests/compile-fail/static_memory_modification3.rs b/tests/compile-fail/static_memory_modification3.rs index 41a6278729..280f34a5a0 100644 --- a/tests/compile-fail/static_memory_modification3.rs +++ b/tests/compile-fail/static_memory_modification3.rs @@ -1,5 +1,5 @@ // Validation detects that we are casting & to &mut and so it changes why we fail -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation use std::mem::transmute; diff --git a/tests/compile-fail/storage_dead_dangling.rs b/tests/compile-fail/storage_dead_dangling.rs index 69917dce85..85c76f6f41 100644 --- a/tests/compile-fail/storage_dead_dangling.rs +++ b/tests/compile-fail/storage_dead_dangling.rs @@ -1,5 +1,5 @@ // This should fail even without validation -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation static mut LEAK: usize = 0; diff --git a/tests/compile-fail/transmute_fat.rs b/tests/compile-fail/transmute_fat.rs index e1f916910d..ede0486be4 100644 --- a/tests/compile-fail/transmute_fat.rs +++ b/tests/compile-fail/transmute_fat.rs @@ -1,5 +1,5 @@ // This should fail even without validation -// compile-flags: -Zmir-emit-validate=0 +// compile-flags: -Zmiri-disable-validation fn main() { #[cfg(target_pointer_width="64")] diff --git a/tests/compile-fail/unaligned_ptr_cast.rs b/tests/compile-fail/unaligned_ptr_cast.rs index 47317afd36..e64594d3e7 100644 --- a/tests/compile-fail/unaligned_ptr_cast.rs +++ b/tests/compile-fail/unaligned_ptr_cast.rs @@ -1,5 +1,5 @@ // This should fail even without validation -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation fn main() { let x = &2u16; diff --git a/tests/compile-fail/unaligned_ptr_cast2.rs b/tests/compile-fail/unaligned_ptr_cast2.rs index d146f21dfe..1112f2f33c 100644 --- a/tests/compile-fail/unaligned_ptr_cast2.rs +++ b/tests/compile-fail/unaligned_ptr_cast2.rs @@ -1,5 +1,5 @@ // This should fail even without validation -// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation +// compile-flags: -Zmiri-disable-validation fn main() { let x = &2u16; diff --git a/tests/compile-fail/undefined_byte_read.rs b/tests/compile-fail/undefined_byte_read.rs index 1f09293614..fca749ef9c 100644 --- a/tests/compile-fail/undefined_byte_read.rs +++ b/tests/compile-fail/undefined_byte_read.rs @@ -1,6 +1,3 @@ -// This should fail even without validation -// compile-flags: -Zmir-emit-validate=0 - fn main() { let v: Vec = Vec::with_capacity(10); let undef = unsafe { *v.get_unchecked(5) }; diff --git a/tests/compile-fail/validity/execute_memory.rs b/tests/compile-fail/validity/execute_memory.rs new file mode 100644 index 0000000000..426a51faf8 --- /dev/null +++ b/tests/compile-fail/validity/execute_memory.rs @@ -0,0 +1,8 @@ +#![feature(box_syntax)] + +fn main() { + let x = box 42; + unsafe { + let _f = std::mem::transmute::, fn()>(x); //~ ERROR encountered a pointer, but expected a function pointer + } +} diff --git a/tests/compile-fail/validity/fn_ptr_offset.rs b/tests/compile-fail/validity/fn_ptr_offset.rs new file mode 100644 index 0000000000..4989f4d3af --- /dev/null +++ b/tests/compile-fail/validity/fn_ptr_offset.rs @@ -0,0 +1,10 @@ +use std::mem; + +fn f() {} + +fn main() { + let x : fn() = f; + let y : *mut u8 = unsafe { mem::transmute(x) }; + let y = y.wrapping_offset(1); + let _x : fn() = unsafe { mem::transmute(y) }; //~ ERROR encountered a potentially NULL pointer +} diff --git a/tests/compiletest.rs b/tests/compiletest.rs index 0b2058597e..98e3fde54e 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -58,13 +58,11 @@ fn compile_fail(sysroot: &Path, path: &str, target: &str, host: &str, need_fullm let mut flags = Vec::new(); flags.push(format!("--sysroot {}", sysroot.display())); flags.push("-Dwarnings -Dunused".to_owned()); // overwrite the -Aunused in compiletest-rs - flags.push("-Zmir-emit-validate=1".to_owned()); if opt { // Optimizing too aggressivley makes UB detection harder, but test at least // the default value. + // FIXME: Opt level 3 ICEs during stack trace generation. flags.push("-Zmir-opt-level=1".to_owned()); - } else { - flags.push("-Zmir-opt-level=0".to_owned()); } let mut config = compiletest::Config::default().tempdir(); @@ -102,11 +100,8 @@ fn miri_pass(sysroot: &Path, path: &str, target: &str, host: &str, need_fullmir: let mut flags = Vec::new(); flags.push(format!("--sysroot {}", sysroot.display())); flags.push("-Dwarnings -Dunused".to_owned()); // overwrite the -Aunused in compiletest-rs - flags.push("-Zmir-emit-validate=1".to_owned()); if opt { flags.push("-Zmir-opt-level=3".to_owned()); - } else { - flags.push("-Zmir-opt-level=0".to_owned()); } let mut config = compiletest::Config::default().tempdir(); diff --git a/tests/run-pass/deref_partially_dangling_raw.rs b/tests/run-pass/deref_partially_dangling_raw.rs deleted file mode 100644 index 8639a28936..0000000000 --- a/tests/run-pass/deref_partially_dangling_raw.rs +++ /dev/null @@ -1,9 +0,0 @@ -// Deref a raw ptr to access a field of a large struct, where the field -// is allocated but not the entire struct is. -// For now, we want to allow this. - -fn main() { - let x = (1, 1); - let xptr = &x as *const _ as *const (i32, i32, i32); - let _val = unsafe { (*xptr).1 }; -} diff --git a/tests/run-pass/move-undef-primval.rs b/tests/run-pass/move-undef-primval.rs index 2c18c2d368..73c33943a6 100644 --- a/tests/run-pass/move-undef-primval.rs +++ b/tests/run-pass/move-undef-primval.rs @@ -1,6 +1,3 @@ -// Moving around undef is not allowed by validation -// compile-flags: -Zmir-emit-validate=0 - struct Foo { _inner: i32, } diff --git a/tests/run-pass/packed_struct.rs b/tests/run-pass/packed_struct.rs index e10781e656..303e90742f 100644 --- a/tests/run-pass/packed_struct.rs +++ b/tests/run-pass/packed_struct.rs @@ -1,4 +1,3 @@ -// compile-flags: -Zmir-emit-validate=0 #![allow(dead_code)] #![feature(unsize, coerce_unsized)] diff --git a/tests/run-pass/recursive_static.rs b/tests/run-pass/recursive_static.rs index d259ca6361..77f2902917 100644 --- a/tests/run-pass/recursive_static.rs +++ b/tests/run-pass/recursive_static.rs @@ -1,6 +1,3 @@ -// FIXME: Disable validation until we figure out how to handle recursive statics. -// compile-flags: -Zmir-emit-validate=0 - struct S(&'static S); static S1: S = S(&S2); static S2: S = S(&S1); diff --git a/tests/run-pass/regions-mock-trans.rs b/tests/run-pass/regions-mock-trans.rs index 039175e9ac..130eaa288e 100644 --- a/tests/run-pass/regions-mock-trans.rs +++ b/tests/run-pass/regions-mock-trans.rs @@ -8,9 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// FIXME: We handle uninitialized storage here, which currently makes validation fail. -// compile-flags: -Zmir-emit-validate=0 - //ignore-windows: Uses POSIX APIs #![feature(libc)] diff --git a/tests/run-pass/slice-of-zero-size-elements.rs b/tests/run-pass/slice-of-zero-size-elements.rs deleted file mode 100644 index dbe8ec9add..0000000000 --- a/tests/run-pass/slice-of-zero-size-elements.rs +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright 2015 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -// compile-flags: -C debug-assertions - -use std::slice; - -fn foo(v: &[T]) -> Option<&[T]> { - let mut it = v.iter(); - for _ in 0..5 { - let _ = it.next(); - } - Some(it.as_slice()) -} - -fn foo_mut(v: &mut [T]) -> Option<&mut [T]> { - let mut it = v.iter_mut(); - for _ in 0..5 { - let _ = it.next(); - } - Some(it.into_slice()) -} - -pub fn main() { - // In a slice of zero-size elements the pointer is meaningless. - // Ensure iteration still works even if the pointer is at the end of the address space. - let slice: &[()] = unsafe { slice::from_raw_parts(-5isize as *const (), 10) }; - assert_eq!(slice.len(), 10); - assert_eq!(slice.iter().count(), 10); - - // .nth() on the iterator should also behave correctly - let mut it = slice.iter(); - assert!(it.nth(5).is_some()); - assert_eq!(it.count(), 4); - - // Converting Iter to a slice should never have a null pointer - assert!(foo(slice).is_some()); - - // Test mutable iterators as well - let slice: &mut [()] = unsafe { slice::from_raw_parts_mut(-5isize as *mut (), 10) }; - assert_eq!(slice.len(), 10); - assert_eq!(slice.iter_mut().count(), 10); - - { - let mut it = slice.iter_mut(); - assert!(it.nth(5).is_some()); - assert_eq!(it.count(), 4); - } - - assert!(foo_mut(slice).is_some()) -} diff --git a/tests/run-pass/slices.rs b/tests/run-pass/slices.rs new file mode 100644 index 0000000000..119c9b90a0 --- /dev/null +++ b/tests/run-pass/slices.rs @@ -0,0 +1,178 @@ +// FIXME: Still investigating whether there is UB here +// compile-flags: -Zmiri-disable-validation + +use std::slice; + +fn slice_of_zst() { + fn foo(v: &[T]) -> Option<&[T]> { + let mut it = v.iter(); + for _ in 0..5 { + let _ = it.next(); + } + Some(it.as_slice()) + } + + fn foo_mut(v: &mut [T]) -> Option<&mut [T]> { + let mut it = v.iter_mut(); + for _ in 0..5 { + let _ = it.next(); + } + Some(it.into_slice()) + } + + // In a slice of zero-size elements the pointer is meaningless. + // Ensure iteration still works even if the pointer is at the end of the address space. + let slice: &[()] = unsafe { slice::from_raw_parts(-5isize as *const (), 10) }; + assert_eq!(slice.len(), 10); + assert_eq!(slice.iter().count(), 10); + + // .nth() on the iterator should also behave correctly + let mut it = slice.iter(); + assert!(it.nth(5).is_some()); + assert_eq!(it.count(), 4); + + // Converting Iter to a slice should never have a null pointer + assert!(foo(slice).is_some()); + + // Test mutable iterators as well + let slice: &mut [()] = unsafe { slice::from_raw_parts_mut(-5isize as *mut (), 10) }; + assert_eq!(slice.len(), 10); + assert_eq!(slice.iter_mut().count(), 10); + + { + let mut it = slice.iter_mut(); + assert!(it.nth(5).is_some()); + assert_eq!(it.count(), 4); + } + + assert!(foo_mut(slice).is_some()) +} + +fn test_iter_ref_consistency() { + use std::fmt::Debug; + + fn test(x : T) { + let v : &[T] = &[x, x, x]; + let v_ptrs : [*const T; 3] = match v { + [ref v1, ref v2, ref v3] => [v1 as *const _, v2 as *const _, v3 as *const _], + _ => unreachable!() + }; + let len = v.len(); + + // nth(i) + for i in 0..len { + assert_eq!(&v[i] as *const _, v_ptrs[i]); // check the v_ptrs array, just to be sure + let nth = v.iter().nth(i).unwrap(); + assert_eq!(nth as *const _, v_ptrs[i]); + } + assert_eq!(v.iter().nth(len), None, "nth(len) should return None"); + + // stepping through with nth(0) + { + let mut it = v.iter(); + for i in 0..len { + let next = it.nth(0).unwrap(); + assert_eq!(next as *const _, v_ptrs[i]); + } + assert_eq!(it.nth(0), None); + } + + // next() + { + let mut it = v.iter(); + for i in 0..len { + let remaining = len - i; + assert_eq!(it.size_hint(), (remaining, Some(remaining))); + + let next = it.next().unwrap(); + assert_eq!(next as *const _, v_ptrs[i]); + } + assert_eq!(it.size_hint(), (0, Some(0))); + assert_eq!(it.next(), None, "The final call to next() should return None"); + } + + // next_back() + { + let mut it = v.iter(); + for i in 0..len { + let remaining = len - i; + assert_eq!(it.size_hint(), (remaining, Some(remaining))); + + let prev = it.next_back().unwrap(); + assert_eq!(prev as *const _, v_ptrs[remaining-1]); + } + assert_eq!(it.size_hint(), (0, Some(0))); + assert_eq!(it.next_back(), None, "The final call to next_back() should return None"); + } + } + + fn test_mut(x : T) { + let v : &mut [T] = &mut [x, x, x]; + let v_ptrs : [*mut T; 3] = match v { + [ref v1, ref v2, ref v3] => + [v1 as *const _ as *mut _, v2 as *const _ as *mut _, v3 as *const _ as *mut _], + _ => unreachable!() + }; + let len = v.len(); + + // nth(i) + for i in 0..len { + assert_eq!(&mut v[i] as *mut _, v_ptrs[i]); // check the v_ptrs array, just to be sure + let nth = v.iter_mut().nth(i).unwrap(); + assert_eq!(nth as *mut _, v_ptrs[i]); + } + assert_eq!(v.iter().nth(len), None, "nth(len) should return None"); + + // stepping through with nth(0) + { + let mut it = v.iter(); + for i in 0..len { + let next = it.nth(0).unwrap(); + assert_eq!(next as *const _, v_ptrs[i]); + } + assert_eq!(it.nth(0), None); + } + + // next() + { + let mut it = v.iter_mut(); + for i in 0..len { + let remaining = len - i; + assert_eq!(it.size_hint(), (remaining, Some(remaining))); + + let next = it.next().unwrap(); + assert_eq!(next as *mut _, v_ptrs[i]); + } + assert_eq!(it.size_hint(), (0, Some(0))); + assert_eq!(it.next(), None, "The final call to next() should return None"); + } + + // next_back() + { + let mut it = v.iter_mut(); + for i in 0..len { + let remaining = len - i; + assert_eq!(it.size_hint(), (remaining, Some(remaining))); + + let prev = it.next_back().unwrap(); + assert_eq!(prev as *mut _, v_ptrs[remaining-1]); + } + assert_eq!(it.size_hint(), (0, Some(0))); + assert_eq!(it.next_back(), None, "The final call to next_back() should return None"); + } + } + + // Make sure iterators and slice patterns yield consistent addresses for various types, + // including ZSTs. + test(0u32); + test(()); + test([0u32; 0]); // ZST with alignment > 0 + test_mut(0u32); + test_mut(()); + test_mut([0u32; 0]); // ZST with alignment > 0 +} + +fn main() { + slice_of_zst(); + test_iter_ref_consistency(); +} diff --git a/tests/run-pass/stacked-borrows.rs b/tests/run-pass/stacked-borrows.rs new file mode 100644 index 0000000000..cde6968a26 --- /dev/null +++ b/tests/run-pass/stacked-borrows.rs @@ -0,0 +1,27 @@ +// Test various stacked-borrows-related things. +fn main() { + deref_partially_dangling_raw(); + read_does_not_invalidate(); +} + +// Deref a raw ptr to access a field of a large struct, where the field +// is allocated but not the entire struct is. +// For now, we want to allow this. +fn deref_partially_dangling_raw() { + let x = (1, 1); + let xptr = &x as *const _ as *const (i32, i32, i32); + let _val = unsafe { (*xptr).1 }; +} + +// Make sure that reading from an `&mut` does, like reborrowing to `&`, +// NOT invalidate other reborrows. +fn read_does_not_invalidate() { + fn foo(x: &mut (i32, i32)) -> &i32 { + let xraw = x as *mut (i32, i32); + let ret = unsafe { &(*xraw).1 }; + let _val = x.1; // we just read, this does NOT invalidate the reborrows. + ret + } + + foo(&mut (1, 2)); +} diff --git a/tests/run-pass/vecs.rs b/tests/run-pass/vecs.rs index 776791bbc9..bb9e5068f9 100644 --- a/tests/run-pass/vecs.rs +++ b/tests/run-pass/vecs.rs @@ -24,13 +24,45 @@ fn vec_into_iter() -> u8 { .fold(0, |x, y| x + y) } +fn vec_into_iter_rev() -> u8 { + vec![1, 2, 3, 4] + .into_iter() + .map(|x| x * x) + .fold(0, |x, y| x + y) +} + fn vec_into_iter_zst() -> usize { vec![[0u64; 0], [0u64; 0]] .into_iter() + .rev() + .map(|x| x.len()) + .sum() +} + +fn vec_into_iter_rev_zst() -> usize { + vec![[0u64; 0], [0u64; 0]] + .into_iter() + .rev() .map(|x| x.len()) .sum() } +fn vec_iter_and_mut() { + let mut v = vec![1,2,3,4]; + for i in v.iter_mut() { + *i += 1; + } + assert_eq!(v.iter().sum::(), 2+3+4+5); +} + +fn vec_iter_and_mut_rev() { + let mut v = vec![1,2,3,4]; + for i in v.iter_mut().rev() { + *i += 1; + } + assert_eq!(v.iter().sum::(), 2+3+4+5); +} + fn vec_reallocate() -> Vec { let mut v = vec![1, 2]; v.push(3); @@ -41,8 +73,14 @@ fn vec_reallocate() -> Vec { fn main() { assert_eq!(vec_reallocate().len(), 5); + assert_eq!(vec_into_iter(), 30); + assert_eq!(vec_into_iter_rev(), 30); + vec_iter_and_mut(); assert_eq!(vec_into_iter_zst(), 0); + assert_eq!(vec_into_iter_rev_zst(), 0); + vec_iter_and_mut_rev(); + assert_eq!(make_vec().capacity(), 4); assert_eq!(make_vec_macro(), [1, 2]); assert_eq!(make_vec_macro_repeat(), [42; 5]); diff --git a/xargo/build.sh b/xargo/build.sh index 15a7c77091..25c56d31ab 100755 --- a/xargo/build.sh +++ b/xargo/build.sh @@ -1,3 +1,4 @@ #!/bin/sh cd "$(dirname "$0")" -RUSTFLAGS='-Zalways-encode-mir -Zmir-emit-validate=1' xargo build +# The flags here should be kept in sync with `add_miri_default_args` in `src/lib.rs`. +RUSTFLAGS='-Zalways-encode-mir -Zmir-emit-retag -Zmir-opt-level=0' xargo build