diff --git a/src/lib.rs b/src/lib.rs index b397e9a6d22..5a3e1c7cab0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -48,7 +48,7 @@ use mono_hash_map::MonoHashMap; use stacked_borrows::{EvalContextExt as StackedBorEvalContextExt}; // Used by priroda -pub use stacked_borrows::{Borrow, Stacks, Mut as MutBorrow}; +pub use stacked_borrows::{Borrow, Stack, Stacks, Mut as MutBorrow, BorStackItem}; /// Insert rustc arguments at the beginning of the argument listthat miri wants to be /// set per default, for maximal validation power. diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index dcb284b1bc3..d520c6ff5d4 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -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, } } } @@ -112,7 +125,7 @@ impl State { /// Extra per-location state #[derive(Clone, Debug)] -struct Stack { +pub struct Stack { borrows: Vec, // used as a stack frozen_since: Option, } @@ -143,17 +156,17 @@ pub struct Stacks { /// Core operations impl<'tcx> Stack { /// Check if `bor` could be activated by unfreezing and popping. - /// `ref_kind` indicates whether this is being used to read/write (or, equivalently, to + /// `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, ref_kind: RefKind) -> Result, String> { + 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 ref_kind == RefKind::Mut => + _ if usage == UsageKind::Write => Err(format!("Using a shared borrow for mutation")), None => Err(format!("Location should be frozen but it is not")), @@ -163,10 +176,11 @@ impl<'tcx> Stack { Err(format!("Location should be frozen since {} but it is only frozen \ since {}", since, loc)), }, - Borrow::Mut(Mut::Raw) if self.is_frozen() && ref_kind != RefKind::Mut => + 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. + // 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. @@ -183,7 +197,7 @@ impl<'tcx> Stack { if loc == mut_borrow { // We found it! This is good to know. // Yet, maybe we do not really want to pop? - if ref_kind == RefKind::Shr && self.is_frozen() { + 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. @@ -204,10 +218,10 @@ impl<'tcx> Stack { Err(format!("Mutable borrow-to-reactivate ({:?}) does not exist on the stack", mut_borrow)) } - /// Reactive `bor` for this stack. `ref_kind` indicates whether this is being + /// 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, ref_kind: RefKind) -> EvalResult<'tcx> { - let action = match self.reactivatable(bor, ref_kind) { + 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)), }; @@ -274,20 +288,20 @@ impl State { /// Higher-level operations impl<'tcx> Stacks { - /// The single most operation: Make sure that using `ptr` as `ref_kind` is okay, + /// 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, - ref_kind: RefKind, + usage: UsageKind, new_bor: Option, ) -> EvalResult<'tcx> { trace!("use_and_maybe_re_borrow of tag {:?} as {:?}, new {:?}: {:?}, size {}", - ptr.tag, ref_kind, new_bor, ptr, size.bytes()); + 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, ref_kind)?; + stack.reactivate(ptr.tag, usage)?; if let Some(new_bor) = new_bor { stack.initiate(new_bor); } @@ -303,7 +317,7 @@ impl<'tcx> Stacks { size: Size, ) -> EvalResult<'tcx> { // Reads behave exactly like the first half of a reborrow-to-shr - self.use_and_maybe_re_borrow(ptr, size, RefKind::Shr, None) + self.use_and_maybe_re_borrow(ptr, size, UsageKind::Read, None) } #[inline(always)] @@ -313,7 +327,7 @@ impl<'tcx> Stacks { size: Size, ) -> EvalResult<'tcx> { // Writes behave exactly like the first half of a reborrow-to-mut - self.use_and_maybe_re_borrow(ptr, size, RefKind::Mut, None) + self.use_and_maybe_re_borrow(ptr, size, UsageKind::Write, None) } pub fn memory_deallocated( @@ -322,7 +336,7 @@ impl<'tcx> Stacks { size: Size, ) -> EvalResult<'tcx> { // This is like mutating - self.use_and_maybe_re_borrow(ptr, size, RefKind::Mut, None) + self.use_and_maybe_re_borrow(ptr, size, UsageKind::Write, None) // FIXME: Error out of there are any barriers? } @@ -346,7 +360,7 @@ pub trait EvalContextExt<'tcx> { ptr: Pointer, pointee_ty: Ty<'tcx>, size: Size, - ref_kind: RefKind, + usage: UsageKind, ) -> EvalResult<'tcx, Borrow>; @@ -355,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( @@ -378,12 +392,12 @@ 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> { let time = self.machine.stacked_borrows.increment_clock(); - let new_bor = 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. @@ -393,10 +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), + 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)?; @@ -404,7 +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!"); - alloc.extra.use_and_maybe_re_borrow(ptr, size, ref_kind, Some(new_bor))?; + alloc.extra.use_and_maybe_re_borrow(ptr, size, usage, Some(new_bor))?; Ok(new_bor) } @@ -418,39 +432,39 @@ 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 {})", - ref_kind, ptr, pointee_ty, size.bytes()); + 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(_))) => { + (UsageKind::Read, Borrow::Mut(Mut::Uniq(_))) => { // A mut got transmuted to shr. The mut borrow must be reactivatable. } - (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. @@ -467,8 +481,12 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' // 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, RefKind::Shr) { - return err!(MachineError(format!("Encountered {:?} reference with non-reactivatable tag: {}", ref_kind, err))) + 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. diff --git a/tests/compile-fail/stacked_borrows/alias_through_mutation.rs b/tests/compile-fail/stacked_borrows/alias_through_mutation.rs index eb8966f3a5c..0b2d459366c 100644 --- a/tests/compile-fail/stacked_borrows/alias_through_mutation.rs +++ b/tests/compile-fail/stacked_borrows/alias_through_mutation.rs @@ -11,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 e86eb9ba6de..2c48404ddf3 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 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 a4f5f536b77..d8a241cab5d 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 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 index f0e696c457c..59190a15db4 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: Mut reference with non-reactivatable tag + //~^ ERROR: mutable reference with non-reactivatable tag } 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 ec59c57d31b..594117d28ab 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: Mut reference with non-reactivatable tag + //~^ ERROR: mutable reference with non-reactivatable tag } 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 131e89572a5..ab951be5ec9 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_write5.rs b/tests/compile-fail/stacked_borrows/illegal_write5.rs index 4b5d08c03a4..f4704ad5716 100644 --- a/tests/compile-fail/stacked_borrows/illegal_write5.rs +++ b/tests/compile-fail/stacked_borrows/illegal_write5.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: Mut reference with non-reactivatable tag + //~^ ERROR: reference with non-reactivatable tag } fn callee(xraw: *mut i32) { diff --git a/tests/compile-fail/stacked_borrows/load_invalid_mut.rs b/tests/compile-fail/stacked_borrows/load_invalid_mut.rs index 060cec962c4..4ea61cd606f 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 b4b180e350f..53179c954de 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 Shr reference with non-reactivatable tag: Location should be frozen + 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 bc950771add..5e1118160a3 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 477c86f6060..e4b26cfff6d 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 Shr reference with non-reactivatable tag: Location should be frozen + 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 c02892671e9..949b3829ff8 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 954b5ec8e59..2d34350359d 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 Shr reference with non-reactivatable tag: Location should be frozen + 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 c02822ed4d3..624587932cb 100644 --- a/tests/compile-fail/stacked_borrows/shared_confusion.rs +++ b/tests/compile-fail/stacked_borrows/shared_confusion.rs @@ -21,7 +21,7 @@ fn test(r: &mut RefCell) { } // 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 index 4a88d4d6413..3030f5dd400 100644 --- a/tests/compile-fail/stacked_borrows/shared_confusion_opt.rs +++ b/tests/compile-fail/stacked_borrows/shared_confusion_opt.rs @@ -17,7 +17,7 @@ fn test(r: &mut RefCell) { } // 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() {