diff --git a/cargo-miri-test/run-test.py b/cargo-miri-test/run-test.py index 506de85ffcb..f686cc47449 100755 --- a/cargo-miri-test/run-test.py +++ b/cargo-miri-test/run-test.py @@ -10,9 +10,8 @@ import sys, subprocess def test_cargo_miri(): print("==> Testing `cargo miri` <==") ## Call `cargo miri`, capture all output - # FIXME: Disabling validation, still investigating whether there is UB here p = subprocess.Popen( - ["cargo", "miri", "-q", "--", "-Zmiri-disable-validation"], + ["cargo", "miri", "-q"], stdout=subprocess.PIPE, stderr=subprocess.PIPE ) diff --git a/rust-version b/rust-version index 48887947cdf..0ad81587241 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -nightly-2018-11-07 +nightly-2018-11-08 diff --git a/src/helpers.rs b/src/helpers.rs index 6dc5b768ea6..880b18c7d54 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -1,6 +1,6 @@ use std::mem; -use rustc::ty; +use rustc::ty::{self, layout}; use rustc::hir::def_id::{DefId, CRATE_DEF_INDEX}; use crate::*; @@ -32,6 +32,15 @@ impl ScalarExt for ScalarMaybeUndef { pub trait EvalContextExt<'tcx> { fn resolve_path(&self, path: &[&str]) -> EvalResult<'tcx, ty::Instance<'tcx>>; + + /// Visit the memory covered by `place` that is frozen -- i.e., NOT + /// what is inside an `UnsafeCell`. + fn visit_frozen( + &self, + place: MPlaceTy<'tcx, Borrow>, + size: Size, + action: impl FnMut(Pointer, Size) -> EvalResult<'tcx>, + ) -> EvalResult<'tcx>; } @@ -69,4 +78,153 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: EvalErrorKind::PathNotFound(path).into() }) } + + /// Visit the memory covered by `place` that is frozen -- i.e., NOT + /// what is inside an `UnsafeCell`. + fn visit_frozen( + &self, + place: MPlaceTy<'tcx, Borrow>, + size: Size, + mut frozen_action: impl FnMut(Pointer, Size) -> EvalResult<'tcx>, + ) -> EvalResult<'tcx> { + trace!("visit_frozen(place={:?}, size={:?})", *place, size); + debug_assert_eq!(size, + self.size_and_align_of_mplace(place)? + .map(|(size, _)| size) + .unwrap_or_else(|| place.layout.size) + ); + // Store how far we proceeded into the place so far. Everything to the left of + // this offset has already been handled, in the sense that the frozen parts + // have had `action` called on them. + let mut end_ptr = place.ptr; + // Called when we detected an `UnsafeCell` at the given offset and size. + // Calls `action` and advances `end_ptr`. + let mut unsafe_cell_action = |unsafe_cell_offset, unsafe_cell_size| { + // We assume that we are given the fields in increasing offset order, + // and nothing else changes. + let end_offset = end_ptr.get_ptr_offset(self); + assert!(unsafe_cell_offset >= end_offset); + let frozen_size = unsafe_cell_offset - end_offset; + // Everything between the end_ptr and this `UnsafeCell` is frozen. + if frozen_size != Size::ZERO { + frozen_action(end_ptr.to_ptr()?, frozen_size)?; + } + // Update end end_ptr. + end_ptr = end_ptr.ptr_wrapping_offset(frozen_size+unsafe_cell_size, self); + // Done + Ok(()) + }; + // Run a visitor + { + let mut visitor = UnsafeCellVisitor { + ecx: self, + unsafe_cell_action: |place| { + trace!("unsafe_cell_action on {:?}", place.ptr); + // We need a size to go on. + let (unsafe_cell_size, _) = self.size_and_align_of_mplace(place)? + // for extern types, just cover what we can + .unwrap_or_else(|| place.layout.size_and_align()); + // Now handle this `UnsafeCell`, unless it is empty. + if unsafe_cell_size != Size::ZERO { + unsafe_cell_action(place.ptr.get_ptr_offset(self), unsafe_cell_size) + } else { + Ok(()) + } + }, + }; + visitor.visit_value(place)?; + } + // The part between the end_ptr and the end of the place is also frozen. + // So pretend there is a 0-sized `UnsafeCell` at the end. + unsafe_cell_action(place.ptr.get_ptr_offset(self) + size, Size::ZERO)?; + // Done! + return Ok(()); + + /// Visiting the memory covered by a `MemPlace`, being aware of + /// whether we are inside an `UnsafeCell` or not. + struct UnsafeCellVisitor<'ecx, 'a, 'mir, 'tcx, F> + where F: FnMut(MPlaceTy<'tcx, Borrow>) -> EvalResult<'tcx> + { + ecx: &'ecx MiriEvalContext<'a, 'mir, 'tcx>, + unsafe_cell_action: F, + } + + impl<'ecx, 'a, 'mir, 'tcx, F> ValueVisitor<'a, 'mir, 'tcx, Evaluator<'tcx>> + for UnsafeCellVisitor<'ecx, 'a, 'mir, 'tcx, F> + where + F: FnMut(MPlaceTy<'tcx, Borrow>) -> EvalResult<'tcx> + { + type V = MPlaceTy<'tcx, Borrow>; + + #[inline(always)] + fn ecx(&self) -> &MiriEvalContext<'a, 'mir, 'tcx> { + &self.ecx + } + + // Hook to detect `UnsafeCell` + fn visit_value(&mut self, v: MPlaceTy<'tcx, Borrow>) -> EvalResult<'tcx> + { + trace!("UnsafeCellVisitor: {:?} {:?}", *v, v.layout.ty); + let is_unsafe_cell = match v.layout.ty.sty { + ty::Adt(adt, _) => Some(adt.did) == self.ecx.tcx.lang_items().unsafe_cell_type(), + _ => false, + }; + if is_unsafe_cell { + // We do not have to recurse further, this is an `UnsafeCell`. + (self.unsafe_cell_action)(v) + } else if self.ecx.type_is_freeze(v.layout.ty) { + // This is `Freeze`, there cannot be an `UnsafeCell` + Ok(()) + } else { + // Proceed further + self.walk_value(v) + } + } + + // Make sure we visit aggregrates in increasing offset order + fn visit_aggregate( + &mut self, + place: MPlaceTy<'tcx, Borrow>, + fields: impl Iterator>>, + ) -> EvalResult<'tcx> { + match place.layout.fields { + layout::FieldPlacement::Array { .. } => { + // For the array layout, we know the iterator will yield sorted elements so + // we can avoid the allocation. + self.walk_aggregate(place, fields) + } + layout::FieldPlacement::Arbitrary { .. } => { + // Gather the subplaces and sort them before visiting. + let mut places = fields.collect::>>>()?; + places[..].sort_by_key(|place| place.ptr.get_ptr_offset(self.ecx())); + self.walk_aggregate(place, places.into_iter().map(Ok)) + } + layout::FieldPlacement::Union { .. } => { + // Uh, what? + bug!("A union is not an aggregate we should ever visit") + } + } + } + + // We have to do *something* for unions + fn visit_union(&mut self, v: MPlaceTy<'tcx, Borrow>) -> EvalResult<'tcx> + { + // With unions, we fall back to whatever the type says, to hopefully be consistent + // with LLVM IR. + // FIXME Are we consistent? And is this really the behavior we want? + let frozen = self.ecx.type_is_freeze(v.layout.ty); + if frozen { + Ok(()) + } else { + (self.unsafe_cell_action)(v) + } + } + + // We should never get to a primitive, but always short-circuit somewhere above + fn visit_primitive(&mut self, _val: ImmTy<'tcx, Borrow>) -> EvalResult<'tcx> + { + bug!("We should always short-circit before coming to a primitive") + } + } + } } diff --git a/src/lib.rs b/src/lib.rs index b4b7a310ca9..134986c814d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,7 +17,7 @@ use std::collections::HashMap; use std::borrow::Cow; use std::env; -use rustc::ty::{self, Ty, TyCtxt, query::TyCtxtAt}; +use rustc::ty::{self, TyCtxt, query::TyCtxtAt}; use rustc::ty::layout::{TyLayout, LayoutOf, Size}; use rustc::hir::{self, def_id::DefId}; use rustc::mir; @@ -48,7 +48,7 @@ use crate::mono_hash_map::MonoHashMap; use crate::stacked_borrows::{EvalContextExt as StackedBorEvalContextExt}; // Used by priroda -pub use crate::stacked_borrows::{Borrow, Stack, Stacks, Mut as MutBorrow, BorStackItem}; +pub use crate::stacked_borrows::{Borrow, Stack, Stacks, BorStackItem}; /// Insert rustc arguments at the beginning of the argument list that miri wants to be /// set per default, for maximal validation power. @@ -476,38 +476,38 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { #[inline(always)] fn tag_reference( ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, - place: MemPlace, - ty: Ty<'tcx>, - size: Size, + place: MPlaceTy<'tcx, Borrow>, mutability: Option, - ) -> EvalResult<'tcx, MemPlace> { + ) -> EvalResult<'tcx, Scalar> { + let (size, _) = ecx.size_and_align_of_mplace(place)? + // for extern types, just cover what we can + .unwrap_or_else(|| place.layout.size_and_align()); if !ecx.machine.validate || size == Size::ZERO { // No tracking - Ok(place) + Ok(place.ptr) } else { let ptr = place.ptr.to_ptr()?; - let tag = ecx.tag_reference(ptr, ty, size, mutability.into())?; - let ptr = Scalar::Ptr(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag)); - Ok(MemPlace { ptr, ..place }) + let tag = ecx.tag_reference(place, size, mutability.into())?; + Ok(Scalar::Ptr(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag))) } } #[inline(always)] fn tag_dereference( ecx: &EvalContext<'a, 'mir, 'tcx, Self>, - place: MemPlace, - ty: Ty<'tcx>, - size: Size, + place: MPlaceTy<'tcx, Borrow>, mutability: Option, - ) -> EvalResult<'tcx, MemPlace> { + ) -> EvalResult<'tcx, Scalar> { + let (size, _) = ecx.size_and_align_of_mplace(place)? + // for extern types, just cover what we can + .unwrap_or_else(|| place.layout.size_and_align()); if !ecx.machine.validate || size == Size::ZERO { // No tracking - Ok(place) + Ok(place.ptr) } else { let ptr = place.ptr.to_ptr()?; - let tag = ecx.tag_dereference(ptr, ty, size, mutability.into())?; - let ptr = Scalar::Ptr(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag)); - Ok(MemPlace { ptr, ..place }) + let tag = ecx.tag_dereference(place, size, mutability.into())?; + Ok(Scalar::Ptr(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag))) } } diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 6d3abbcc315..e1abcb20af7 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -1,64 +1,43 @@ use std::cell::RefCell; -use rustc::ty::{self, Ty, layout::Size}; +use rustc::ty::{self, layout::Size}; use rustc::hir; use crate::{ - MemoryKind, MiriMemoryKind, RangeMap, EvalResult, AllocId, - Pointer, PlaceTy, + EvalResult, MiriEvalContext, HelpersEvalContextExt, + MemoryKind, MiriMemoryKind, RangeMap, AllocId, + Pointer, PlaceTy, MPlaceTy, }; pub type Timestamp = u64; -/// Information about a potentially mutable borrow -#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] -pub enum Mut { - /// A unique, mutable reference - Uniq(Timestamp), - /// Any raw pointer, or a shared borrow with interior mutability - Raw, -} - -impl Mut { - #[inline(always)] - pub fn is_raw(self) -> bool { - match self { - Mut::Raw => true, - _ => false, - } - } - - #[inline(always)] - pub fn is_uniq(self) -> bool { - match self { - Mut::Uniq(_) => true, - _ => false, - } - } -} - -/// Information about any kind of borrow +/// Information about which kind of borrow was used to create the reference this is tagged +/// with. #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] pub enum Borrow { - /// A mutable borrow, a raw pointer, or a shared borrow with interior mutability - Mut(Mut), - /// A shared borrow without interior mutability - Frz(Timestamp) + /// A unique (mutable) reference. + Uniq(Timestamp), + /// A shared reference. This is also used by raw pointers, which do not track details + /// of how or when they were created, hence the timestamp is optional. + /// Shr(Some(_)) does NOT mean that the destination of this reference is frozen; + /// that depends on the type! Only those parts outside of an `UnsafeCell` are actually + /// frozen. + Shr(Option), } impl Borrow { #[inline(always)] - pub fn is_uniq(self) -> bool { + pub fn is_shr(self) -> bool { match self { - Borrow::Mut(m) => m.is_uniq(), + Borrow::Shr(_) => true, _ => false, } } #[inline(always)] - pub fn is_frz(self) -> bool { + pub fn is_uniq(self) -> bool { match self { - Borrow::Frz(_) => true, + Borrow::Uniq(_) => true, _ => false, } } @@ -66,15 +45,19 @@ impl Borrow { impl Default for Borrow { fn default() -> Self { - Borrow::Mut(Mut::Raw) + Borrow::Shr(None) } } -/// An item in the borrow stack +/// An item in the per-location borrow stack #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] pub enum BorStackItem { - /// Defines which references are permitted to mutate *if* the location is not frozen - Mut(Mut), + /// Indicates the unique reference that may mutate. + Uniq(Timestamp), + /// Indicates that the location has been shared. Used for raw pointers, but + /// also for shared references. The latter *additionally* get frozen + /// when there is no `UnsafeCell`. + Shr, /// A barrier, tracking the function it belongs to by its index on the call stack #[allow(dead_code)] // for future use FnBarrier(usize) @@ -90,6 +73,29 @@ impl BorStackItem { } } +/// Extra per-location state +#[derive(Clone, Debug)] +pub struct Stack { + borrows: Vec, // used as a stack; never empty + frozen_since: Option, // virtual frozen "item" on top of the stack +} + +impl Default for Stack { + fn default() -> Self { + Stack { + borrows: vec![BorStackItem::Shr], + frozen_since: None, + } + } +} + +impl Stack { + #[inline(always)] + pub fn is_frozen(&self) -> bool { + self.frozen_since.is_some() + } +} + /// What kind of usage of the pointer are we talking about? #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] pub enum UsageKind { @@ -97,7 +103,7 @@ pub enum UsageKind { Write, /// Read, or create & Read, - /// Create * + /// Create * (raw ptr) Raw, } @@ -123,29 +129,6 @@ impl State { } } -/// Extra per-location state -#[derive(Clone, Debug)] -pub struct Stack { - borrows: Vec, // used as a stack - frozen_since: Option, -} - -impl Default for Stack { - fn default() -> Self { - Stack { - borrows: vec![BorStackItem::Mut(Mut::Raw)], - frozen_since: None, - } - } -} - -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 { @@ -156,124 +139,125 @@ pub struct Stacks { /// Core operations impl<'tcx> Stack { /// Check if `bor` could be activated by unfreezing and popping. - /// `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, usage: UsageKind) -> Result, String> { - let mut_borrow = match bor { - Borrow::Frz(since) => - // 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 - }; - // 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", 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)); + /// `is_write` indicates whether this is being used to write (or, equivalently, to + /// borrow as &mut). + /// Returns `Err` if the answer is "no"; otherwise the return value indicates what to + /// do: With `Some(n)` you need to unfreeze, and then additionally pop `n` items. + fn reactivatable(&self, bor: Borrow, is_write: bool) -> Result, String> { + // Check if we can match the frozen "item". Not possible on writes! + if !is_write { + // For now, we do NOT check the timestamp. That might be surprising, but + // we cannot even notice when a location should be frozen but is not! + // Those checks are both done in `tag_dereference`, where we have type information. + // Either way, it is crucial that the frozen "item" matches raw pointers: + // Reading through a raw should not unfreeze. + match (self.frozen_since, bor) { + (Some(_), Borrow::Shr(_)) => { + return Ok(None) + } + _ => {}, + } + } + // See if we can find this borrow. + for (idx, &itm) in self.borrows.iter().rev().enumerate() { + // Check borrow and stack item for compatibility. + match (itm, bor) { + (BorStackItem::FnBarrier(_), _) => { + return Err(format!("Trying to reactivate a borrow ({:?}) that lives \ + behind a barrier", bor)) + } + (BorStackItem::Uniq(itm_t), Borrow::Uniq(bor_t)) if itm_t == bor_t => { + // Found matching unique item. + if !is_write { + // As a special case, if we are reading and since we *did* find the `Uniq`, + // we try to pop less: We are happy with making a `Shr` or `Frz` active; + // that one will not mind concurrent reads. + match self.reactivatable(Borrow::default(), is_write) { + // If we got something better that `idx`, use that + Ok(None) => return Ok(None), + Ok(Some(shr_idx)) if shr_idx <= idx => return Ok(Some(shr_idx)), + // Otherwise just go on. + _ => {}, } } + return Ok(Some(idx)) } + (BorStackItem::Shr, Borrow::Shr(_)) => { + // Found matching shared/raw item. + return Ok(Some(idx)) + } + // Go on looking. + _ => {} } } // Nothing to be found. - Err(format!("Mutable borrow-to-reactivate ({:?}) does not exist on the stack", mut_borrow)) + Err(format!("Borrow-to-reactivate {:?} does not exist on the stack", bor)) } - /// 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, + /// Reactive `bor` for this stack. `is_write` indicates whether this is being + /// used to write (or, equivalently, to borrow as &mut). + fn reactivate(&mut self, bor: Borrow, is_write: bool) -> EvalResult<'tcx> { + let mut pop = match self.reactivatable(bor, is_write) { + Ok(None) => return Ok(()), + Ok(Some(pop)) => pop, 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; - for itm in self.borrows.drain(top+1..).rev() { - trace!("reactivate: Popping {:?}", itm); - } - } + // Pop what `reactivatable` told us to pop. Always unfreeze. + if self.is_frozen() { + trace!("reactivate: Unfreezing"); + } + self.frozen_since = None; + while pop > 0 { + let itm = self.borrows.pop().unwrap(); + trace!("reactivate: Popping {:?}", itm); + pop -= 1; } - Ok(()) } - /// Initiate `bor`; mostly this means freezing or pushing. + /// Initiate `bor`; mostly this means pushing. /// 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 { - None => { - trace!("initiate: Freezing"); - self.frozen_since = Some(t); - } - Some(since) => { - trace!("initiate: Already frozen"); - assert!(since <= t); - } - } - } - Borrow::Mut(m) => { - match self.frozen_since { - None => { - trace!("initiate: Pushing {:?}", bor); - self.borrows.push(BorStackItem::Mut(m)) - } - Some(_) if m.is_raw() => - // We only ever initiate right after activating the ref we come from. - // If the source ref is fine being frozen, then a raw ref we create - // from it is fine with this as well. - trace!("initiate: Initiating a raw on a frozen location, not doing a thing"), - Some(_) => - bug!("Trying to mutate frozen location") - } + if let Some(_) = self.frozen_since { + // "Pushing" a Shr or Frz on top is redundant. + match bor { + Borrow::Uniq(_) => bug!("Trying to create unique ref to frozen location"), + Borrow::Shr(_) => trace!("initiate: New shared ref to frozen location is a NOP"), } + } else { + // Just push. + let itm = match bor { + Borrow::Uniq(t) => BorStackItem::Uniq(t), + Borrow::Shr(_) if *self.borrows.last().unwrap() == BorStackItem::Shr => { + // Optimization: Don't push a Shr onto a Shr. + trace!("initiate: New shared ref to already shared location is a NOP"); + return + }, + Borrow::Shr(_) => BorStackItem::Shr, + }; + trace!("initiate: Pushing {:?}", itm); + self.borrows.push(itm) + } + } + + /// Check if this location is "frozen enough". + fn check_frozen(&self, bor_t: Timestamp) -> EvalResult<'tcx> { + let frozen = self.frozen_since.map_or(false, |itm_t| itm_t <= bor_t); + if !frozen { + err!(MachineError(format!("Location is not frozen long enough"))) + } else { + Ok(()) + } + } + + /// Freeze this location, since `bor_t`. + fn freeze(&mut self, bor_t: Timestamp) { + if let Some(itm_t) = self.frozen_since { + assert!(itm_t <= bor_t, "Trying to freeze shorter than it was frozen?"); + } else { + trace!("Freezing"); + self.frozen_since = Some(bor_t); } } } @@ -288,7 +272,7 @@ impl State { /// Higher-level operations impl<'tcx> Stacks { - /// The single most operation: Make sure that using `ptr` as `usage` is okay, + /// The single most important operation: Make sure that using `ptr` is okay, /// and if `new_bor` is present then make that the new current borrow. fn use_and_maybe_re_borrow( &self, @@ -301,15 +285,65 @@ impl<'tcx> Stacks { ptr.tag, usage, new_bor, ptr, size.bytes()); let mut stacks = self.stacks.borrow_mut(); for stack in stacks.iter_mut(ptr.offset, size) { - stack.reactivate(ptr.tag, usage)?; + stack.reactivate(ptr.tag, usage == UsageKind::Write)?; if let Some(new_bor) = new_bor { stack.initiate(new_bor); } } - Ok(()) } + /// Freeze the given memory range. + fn freeze( + &self, + ptr: Pointer, + size: Size, + bor_t: Timestamp + ) -> EvalResult<'tcx> { + let mut stacks = self.stacks.borrow_mut(); + for stack in stacks.iter_mut(ptr.offset, size) { + stack.freeze(bor_t); + } + Ok(()) + } + + /// Check that this stack is fine with being dereferenced + fn check_deref( + &self, + ptr: Pointer, + size: Size, + ) -> EvalResult<'tcx> { + let mut stacks = self.stacks.borrow_mut(); + // We need `iter_mut` because `iter` would skip gaps! + for stack in stacks.iter_mut(ptr.offset, size) { + // Conservatively assume we will just read + if let Err(err) = stack.reactivatable(ptr.tag, /*is_write*/false) { + return err!(MachineError(format!( + "Encountered reference with non-reactivatable tag: {}", + err + ))) + } + } + Ok(()) + } + + /// Check that this stack is appropriately frozen + fn check_frozen( + &self, + ptr: Pointer, + size: Size, + bor_t: Timestamp + ) -> EvalResult<'tcx> { + let mut stacks = self.stacks.borrow_mut(); + for stack in stacks.iter_mut(ptr.offset, size) { + stack.check_frozen(bor_t)?; + } + Ok(()) + } +} + +/// Hooks and glue +impl<'tcx> Stacks { #[inline(always)] pub fn memory_read( &self, @@ -340,34 +374,34 @@ impl<'tcx> Stacks { // FIXME: Error out of there are any barriers? } - /// Pushes the first borrow to the stacks, must be a mutable one. - pub fn first_borrow( + /// Pushes the first item to the stacks. + pub fn first_item( &mut self, - mut_borrow: Mut, + itm: BorStackItem, size: Size ) { + assert!(!itm.is_fn_barrier()); for stack in self.stacks.get_mut().iter_mut(Size::ZERO, size) { - assert!(stack.borrows.len() == 1 && stack.frozen_since.is_none()); - assert_eq!(stack.borrows.pop().unwrap(), BorStackItem::Mut(Mut::Raw)); - stack.borrows.push(BorStackItem::Mut(mut_borrow)); + assert!(stack.borrows.len() == 1); + assert_eq!(stack.borrows.pop().unwrap(), BorStackItem::Shr); + stack.borrows.push(itm); } } } + + pub trait EvalContextExt<'tcx> { fn tag_reference( &mut self, - ptr: Pointer, - pointee_ty: Ty<'tcx>, + place: MPlaceTy<'tcx, Borrow>, size: Size, usage: UsageKind, ) -> EvalResult<'tcx, Borrow>; - fn tag_dereference( &self, - ptr: Pointer, - pointee_ty: Ty<'tcx>, + place: MPlaceTy<'tcx, Borrow>, size: Size, usage: UsageKind, ) -> EvalResult<'tcx, Borrow>; @@ -385,40 +419,36 @@ pub trait EvalContextExt<'tcx> { ) -> EvalResult<'tcx>; } -impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, 'tcx> { +impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> { /// Called for place-to-value conversion. fn tag_reference( &mut self, - ptr: Pointer, - pointee_ty: Ty<'tcx>, + place: MPlaceTy<'tcx, Borrow>, size: Size, usage: UsageKind, ) -> EvalResult<'tcx, Borrow> { + let ptr = place.ptr.to_ptr()?; let time = self.machine.stacked_borrows.increment_clock(); 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. - if self.type_is_freeze(pointee_ty) { - Borrow::Frz(time) - } else { - // Shared reference with interior mutability. - Borrow::Mut(Mut::Raw) - }, - UsageKind::Raw => Borrow::Mut(Mut::Raw), + UsageKind::Write => Borrow::Uniq(time), + UsageKind::Read => Borrow::Shr(Some(time)), + UsageKind::Raw => Borrow::Shr(None), }; - trace!("tag_reference: Creating new reference ({:?}) for {:?} (pointee {}, size {}): {:?}", - usage, ptr, pointee_ty, size.bytes(), new_bor); + trace!("tag_reference: Creating new reference ({:?}) for {:?} (pointee {}): {:?}", + usage, ptr, place.layout.ty, new_bor); - // Make sure this reference is not dangling or so + // Update the stacks. First create the new ref as usual, then maybe freeze stuff. self.memory().check_bounds(ptr, size, false)?; - - // 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!"); alloc.extra.use_and_maybe_re_borrow(ptr, size, usage, Some(new_bor))?; + // Maybe freeze stuff + if let Borrow::Shr(Some(bor_t)) = new_bor { + self.visit_frozen(place, size, |frz_ptr, size| { + debug_assert_eq!(frz_ptr.alloc_id, ptr.alloc_id); + // Be frozen! + alloc.extra.freeze(frz_ptr, size, bor_t) + })?; + } Ok(new_bor) } @@ -429,13 +459,13 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' /// We could be in the middle of `&(*var).1`. fn tag_dereference( &self, - ptr: Pointer, - pointee_ty: Ty<'tcx>, + place: MPlaceTy<'tcx, Borrow>, size: Size, usage: UsageKind, ) -> EvalResult<'tcx, Borrow> { - trace!("tag_reference: Accessing reference ({:?}) for {:?} (pointee {}, size {})", - usage, ptr, pointee_ty, size.bytes()); + let ptr = place.ptr.to_ptr()?; + trace!("tag_dereference: Accessing reference ({:?}) for {:?} (pointee {})", + usage, ptr, place.layout.ty); // 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 `usage`, and we // should adjust for that. @@ -446,27 +476,24 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' // 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)); + return Ok(Borrow::default()); } - (UsageKind::Write, Borrow::Mut(Mut::Uniq(_))) | - (UsageKind::Read, Borrow::Frz(_)) | - (UsageKind::Read, Borrow::Mut(Mut::Raw)) => { + (UsageKind::Write, Borrow::Uniq(_)) | + (UsageKind::Read, Borrow::Shr(_)) => { // Expected combinations. Nothing to do. - // FIXME: We probably shouldn't accept this if we got a raw shr without - // interior mutability. } - (UsageKind::Write, Borrow::Mut(Mut::Raw)) => { + (UsageKind::Write, Borrow::Shr(None)) => { // 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. } - (UsageKind::Read, Borrow::Mut(Mut::Uniq(_))) => { + (UsageKind::Read, Borrow::Uniq(_)) => { // A mut got transmuted to shr. Can happen even from compiler transformations: // `&*x` gets optimized to `x` even when `x` is a `&mut`. } - (UsageKind::Write, Borrow::Frz(_)) => { - // This is just invalid. + (UsageKind::Write, Borrow::Shr(Some(_))) => { + // This is just invalid: A shr got transmuted to a mut. // 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. // We probably do not want to allow that, but we have to allow @@ -474,23 +501,21 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' return err!(MachineError(format!("Encountered mutable reference with frozen tag {:?}", ptr.tag))) } } - // Even if we don't touch the tag, this operation is only okay if we *could* - // activate it. Also it must not be dangling. + + // If we got here, we do some checking, *but* we leave the tag unchanged. self.memory().check_bounds(ptr, size, false)?; let alloc = self.memory().get(ptr.alloc_id).expect("We checked that the ptr is fine!"); - 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) { - // 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 - ))) - } + alloc.extra.check_deref(ptr, size)?; + // Maybe check frozen stuff + if let Borrow::Shr(Some(bor_t)) = ptr.tag { + self.visit_frozen(place, size, |frz_ptr, size| { + debug_assert_eq!(frz_ptr.alloc_id, ptr.alloc_id); + // Are you frozen? + alloc.extra.check_frozen(frz_ptr, size, bor_t) + })?; } - // All is good. + + // All is good, and do not change the tag Ok(ptr.tag) } @@ -499,7 +524,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' id: AllocId, kind: MemoryKind, ) -> Borrow { - let mut_borrow = match kind { + let time = match kind { MemoryKind::Stack => { // 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., @@ -509,19 +534,18 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' // `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) + self.machine.stacked_borrows.increment_clock() } _ => { - // Raw for everything else - Mut::Raw + // Nothing to do for everything else + return Borrow::default() } }; // Make this the active borrow for this allocation let alloc = self.memory_mut().get_mut(id).expect("This is a new allocation, it must still exist"); let size = Size::from_bytes(alloc.bytes.len() as u64); - alloc.extra.first_borrow(mut_borrow, size); - Borrow::Mut(mut_borrow) + alloc.extra.first_item(BorStackItem::Uniq(time), size); + Borrow::Uniq(time) } fn retag( diff --git a/src/tls.rs b/src/tls.rs index 1aacc67f5cc..af1d7b138b8 100644 --- a/src/tls.rs +++ b/src/tls.rs @@ -133,7 +133,7 @@ impl<'tcx> TlsData<'tcx> { } } -impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, 'tcx> { +impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, 'tcx> { fn run_tls_dtors(&mut self) -> EvalResult<'tcx> { let mut dtor = self.machine.tls.fetch_tls_dtor(None, &*self.tcx); // FIXME: replace loop by some structure that works with stepping 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 2c48404ddf3..4857ada7fb2 100644 --- a/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs +++ b/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs @@ -14,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 reference with non-reactivatable tag + v1[1] = 5; //~ ERROR does not exist on the stack 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 d8a241cab5d..a6daa5d93d7 100644 --- a/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs +++ b/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs @@ -11,7 +11,7 @@ mod safe { assert!(mid <= len); (from_raw_parts_mut(ptr, len - mid), // BUG: should be "mid" instead of "len - mid" - //~^ ERROR reference with non-reactivatable tag + //~^ ERROR does not exist on the stack 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 index 59190a15db4..dbaccae8827 100644 --- a/tests/compile-fail/stacked_borrows/illegal_read1.rs +++ b/tests/compile-fail/stacked_borrows/illegal_read1.rs @@ -7,7 +7,7 @@ fn main() { 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 + //~^ ERROR: does not exist on the stack } fn callee(xraw: *mut i32) { diff --git a/tests/compile-fail/stacked_borrows/illegal_read2.rs b/tests/compile-fail/stacked_borrows/illegal_read2.rs index 594117d28ab..2da755d9aab 100644 --- a/tests/compile-fail/stacked_borrows/illegal_read2.rs +++ b/tests/compile-fail/stacked_borrows/illegal_read2.rs @@ -7,7 +7,7 @@ fn main() { 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 + //~^ ERROR: does not exist on the stack } fn callee(xraw: *mut i32) { diff --git a/tests/compile-fail/stacked_borrows/illegal_write1.rs b/tests/compile-fail/stacked_borrows/illegal_write1.rs index ab951be5ec9..7378907fa74 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 reference with non-reactivatable tag + let _x = *ref_; //~ ERROR is not frozen long enough } diff --git a/tests/compile-fail/stacked_borrows/illegal_write3.rs b/tests/compile-fail/stacked_borrows/illegal_write3.rs index a653aa5003f..c82da1e9c46 100644 --- a/tests/compile-fail/stacked_borrows/illegal_write3.rs +++ b/tests/compile-fail/stacked_borrows/illegal_write3.rs @@ -3,6 +3,6 @@ fn main() { // Make sure raw ptr with raw tag cannot mutate frozen location without breaking the shared ref. let r#ref = ⌖ // freeze let ptr = r#ref as *const _ as *mut _; // raw ptr, with raw tag - unsafe { *ptr = 42; } //~ ERROR does not exist on the stack - let _val = *r#ref; + unsafe { *ptr = 42; } + let _val = *r#ref; //~ ERROR is not frozen long enough } diff --git a/tests/compile-fail/stacked_borrows/illegal_write4.rs b/tests/compile-fail/stacked_borrows/illegal_write4.rs index 12deb518b4e..49bf9279faa 100644 --- a/tests/compile-fail/stacked_borrows/illegal_write4.rs +++ b/tests/compile-fail/stacked_borrows/illegal_write4.rs @@ -9,5 +9,5 @@ fn main() { 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 retag, making our ref top-of-stack -- and, in particular, unfreezing. - let _val = *reference; //~ ERROR Location should be frozen + let _val = *reference; //~ ERROR is not frozen long enough } diff --git a/tests/compile-fail/stacked_borrows/load_invalid_mut.rs b/tests/compile-fail/stacked_borrows/load_invalid_mut.rs index 4ea61cd606f..d3db462343e 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 mutable reference with non-reactivatable tag + let _val = *xref_in_mem; //~ ERROR does not exist on the stack } diff --git a/tests/compile-fail/stacked_borrows/load_invalid_shr.rs b/tests/compile-fail/stacked_borrows/load_invalid_shr.rs index 53179c954de..71b578817a7 100644 --- a/tests/compile-fail/stacked_borrows/load_invalid_shr.rs +++ b/tests/compile-fail/stacked_borrows/load_invalid_shr.rs @@ -5,5 +5,5 @@ fn main() { let xref = unsafe { &*xraw }; let xref_in_mem = Box::new(xref); *x = 42; // invalidate xraw - let _val = *xref_in_mem; //~ ERROR shared reference with non-reactivatable tag: Location should be frozen + let _val = *xref_in_mem; //~ ERROR does not exist on the stack } diff --git a/tests/compile-fail/stacked_borrows/pass_invalid_mut.rs b/tests/compile-fail/stacked_borrows/pass_invalid_mut.rs index 5e1118160a3..41cf89d874d 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 mutable reference with non-reactivatable tag + foo(xref); //~ ERROR does not exist on the stack } diff --git a/tests/compile-fail/stacked_borrows/pass_invalid_shr.rs b/tests/compile-fail/stacked_borrows/pass_invalid_shr.rs index e4b26cfff6d..0bdb1b4a41e 100644 --- a/tests/compile-fail/stacked_borrows/pass_invalid_shr.rs +++ b/tests/compile-fail/stacked_borrows/pass_invalid_shr.rs @@ -6,5 +6,5 @@ fn main() { let xraw = &*x as *const _; let xref = unsafe { &*xraw }; *x = 42; // invalidate xraw - foo(xref); //~ ERROR shared reference with non-reactivatable tag: Location should be frozen + foo(xref); //~ ERROR does not exist on the stack } diff --git a/tests/compile-fail/stacked_borrows/return_invalid_mut.rs b/tests/compile-fail/stacked_borrows/return_invalid_mut.rs index 949b3829ff8..aef8fafdf5d 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 mutable reference with non-reactivatable tag + ret //~ ERROR does not exist on the stack } 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 2d34350359d..074942eb95b 100644 --- a/tests/compile-fail/stacked_borrows/return_invalid_shr.rs +++ b/tests/compile-fail/stacked_borrows/return_invalid_shr.rs @@ -3,7 +3,7 @@ fn foo(x: &mut (i32, i32)) -> &i32 { let xraw = x as *mut (i32, i32); let ret = unsafe { &(*xraw).1 }; x.1 = 42; // invalidate xraw on the 2nd field - ret //~ ERROR shared reference with non-reactivatable tag: Location should be frozen + ret //~ ERROR does not exist on the stack } fn main() { diff --git a/tests/compile-fail/stacked_borrows/shared_confusion.rs b/tests/compile-fail/stacked_borrows/shared_confusion.rs deleted file mode 100644 index 624587932cb..00000000000 --- a/tests/compile-fail/stacked_borrows/shared_confusion.rs +++ /dev/null @@ -1,29 +0,0 @@ -// 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; - -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 y = &*r; // outer ref, not freezing - 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 reference with non-reactivatable tag -} - -fn main() { - test(&mut RefCell::new(0)); -} diff --git a/tests/compile-fail/stacked_borrows/shared_confusion_opt.rs b/tests/compile-fail/stacked_borrows/shared_confusion_opt.rs deleted file mode 100644 index 3030f5dd400..00000000000 --- a/tests/compile-fail/stacked_borrows/shared_confusion_opt.rs +++ /dev/null @@ -1,25 +0,0 @@ -// 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/validity/invalid_bool.rs b/tests/compile-fail/validity/invalid_bool.rs index af4ad67a4f0..ce464616195 100644 --- a/tests/compile-fail/validity/invalid_bool.rs +++ b/tests/compile-fail/validity/invalid_bool.rs @@ -1,3 +1,3 @@ fn main() { - let _b = unsafe { std::mem::transmute::(2) }; //~ ERROR encountered 2, but expected something in the range 0..=1 + let _b = unsafe { std::mem::transmute::(2) }; //~ ERROR encountered 2, but expected something less or equal to 1 } diff --git a/tests/compile-fail/validity/invalid_char.rs b/tests/compile-fail/validity/invalid_char.rs index 3ff0ed60f66..0d75ad9d289 100644 --- a/tests/compile-fail/validity/invalid_char.rs +++ b/tests/compile-fail/validity/invalid_char.rs @@ -1,6 +1,6 @@ fn main() { assert!(std::char::from_u32(-1_i32 as u32).is_none()); - let _ = match unsafe { std::mem::transmute::(-1) } { //~ ERROR encountered 4294967295, but expected something in the range 0..=1114111 + let _ = match unsafe { std::mem::transmute::(-1) } { //~ ERROR encountered 4294967295, but expected something less or equal to 1114111 'a' => {true}, 'b' => {false}, _ => {true}, diff --git a/tests/compile-fail/validity/invalid_enum_discriminant.rs b/tests/compile-fail/validity/invalid_enum_discriminant.rs index 543a797d44f..13be4e7dcea 100644 --- a/tests/compile-fail/validity/invalid_enum_discriminant.rs +++ b/tests/compile-fail/validity/invalid_enum_discriminant.rs @@ -4,5 +4,5 @@ pub enum Foo { } fn main() { - let _f = unsafe { std::mem::transmute::(42) }; //~ ERROR encountered invalid enum discriminant 42 + let _f = unsafe { std::mem::transmute::(42) }; //~ ERROR encountered 42, but expected a valid enum discriminant } diff --git a/tests/compile-fail/validity/transmute_through_ptr.rs b/tests/compile-fail/validity/transmute_through_ptr.rs index 0a4c64bb9bb..d6bc0305e69 100644 --- a/tests/compile-fail/validity/transmute_through_ptr.rs +++ b/tests/compile-fail/validity/transmute_through_ptr.rs @@ -12,5 +12,5 @@ fn main() { let mut x = Bool::True; evil(&mut x); let _y = x; // reading this ought to be enough to trigger validation - //~^ ERROR invalid enum discriminant 44 + //~^ ERROR encountered 44, but expected a valid enum discriminant } diff --git a/tests/run-pass-fullmir/send-is-not-static-par-for.rs b/tests/run-pass-fullmir/send-is-not-static-par-for.rs index 282f7a35950..1b913aed4c8 100644 --- a/tests/run-pass-fullmir/send-is-not-static-par-for.rs +++ b/tests/run-pass-fullmir/send-is-not-static-par-for.rs @@ -8,9 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// FIXME: Still investigating whether there is UB here -// compile-flags: -Zmiri-disable-validation - use std::sync::Mutex; fn par_for(iter: I, f: F) diff --git a/tests/run-pass-fullmir/u128.rs b/tests/run-pass-fullmir/u128.rs index 0f1b6e8b587..ca33bd5f9e3 100644 --- a/tests/run-pass-fullmir/u128.rs +++ b/tests/run-pass-fullmir/u128.rs @@ -8,9 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// FIXME: Still investigating whether there is UB here -// compile-flags: -Zmiri-disable-validation - fn b(t: T) -> T { t } fn main() { diff --git a/tests/run-pass-fullmir/vecdeque.rs b/tests/run-pass-fullmir/vecdeque.rs index ffbb1166849..381169505ec 100644 --- a/tests/run-pass-fullmir/vecdeque.rs +++ b/tests/run-pass-fullmir/vecdeque.rs @@ -1,6 +1,3 @@ -// FIXME: Still investigating whether there is UB here -// compile-flags: -Zmiri-disable-validation - use std::collections::VecDeque; fn main() { diff --git a/tests/run-pass/slices.rs b/tests/run-pass/slices.rs index 119c9b90a05..45a2a74db08 100644 --- a/tests/run-pass/slices.rs +++ b/tests/run-pass/slices.rs @@ -1,6 +1,3 @@ -// FIXME: Still investigating whether there is UB here -// compile-flags: -Zmiri-disable-validation - use std::slice; fn slice_of_zst() { diff --git a/tests/run-pass/stacked-borrows.rs b/tests/run-pass/stacked-borrows.rs index cde6968a264..8faa4d65911 100644 --- a/tests/run-pass/stacked-borrows.rs +++ b/tests/run-pass/stacked-borrows.rs @@ -1,7 +1,8 @@ // Test various stacked-borrows-related things. fn main() { deref_partially_dangling_raw(); - read_does_not_invalidate(); + read_does_not_invalidate1(); + read_does_not_invalidate2(); } // Deref a raw ptr to access a field of a large struct, where the field @@ -15,13 +16,23 @@ fn deref_partially_dangling_raw() { // Make sure that reading from an `&mut` does, like reborrowing to `&`, // NOT invalidate other reborrows. -fn read_does_not_invalidate() { +fn read_does_not_invalidate1() { 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)); +} +// Same as above, but this time we first create a raw, then read from `&mut` +// and then freeze from the raw. +fn read_does_not_invalidate2() { + fn foo(x: &mut (i32, i32)) -> &i32 { + let xraw = x as *mut (i32, i32); + let _val = x.1; // we just read, this does NOT invalidate the raw reborrow. + let ret = unsafe { &(*xraw).1 }; + ret + } foo(&mut (1, 2)); }