diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 31631635186..f634f17109c 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -137,75 +137,64 @@ impl<'tcx> Stack { } /// Check if `bor` could be activated by unfreezing and popping. - /// This should be in sync with `reactivate`! - fn reactivatable(&self, bor: Borrow) -> bool { - if self.check(bor) { - return true; - } - - let acc_m = match bor { - Borrow::Frz(_) => return false, - Borrow::Mut(acc_m) => acc_m - }; - // This is where we would unfreeze. - for &itm in self.borrows.iter().rev() { - match itm { - BorStackItem::FnBarrier(_) => return false, - BorStackItem::Mut(loc_m) => { - if loc_m == acc_m { return true; } - // Go on looking. - } - } - } - // Nothing to be found. - false - } - - /// 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> { + /// `force_mut` indicates whether being frozen is potentially acceptable. + /// 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(()); + return Ok(None); } let acc_m = match bor { Borrow::Frz(since) => - if force_mut { - return err!(MachineError(format!("Using a shared borrow for mutation"))) + return Err(if force_mut { + format!("Using a shared borrow for mutation") } else { - return err!(MachineError(format!( + 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, + ) + }), + Borrow::Mut(acc_m) => acc_m }; - // We definitely have to unfreeze this, even if we use the topmost item. - if self.frozen_since.is_some() { - trace!("reactivate: Unfreezing"); - } - self.frozen_since = None; - // Pop until we see the one we are looking for. - while let Some(&itm) = self.borrows.last() { + // This is where we would unfreeze. + for (idx, &itm) in self.borrows.iter().enumerate().rev() { match itm { - BorStackItem::FnBarrier(_) => { - return err!(MachineError(format!("Trying to reactivate a borrow that lives behind a barrier"))); - } + 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(()); } - trace!("reactivate: Popping {:?}", itm); - self.borrows.pop(); + if loc_m == acc_m { return Ok(Some(idx)); } } } } // Nothing to be found. - err!(MachineError(format!("Borrow-to-reactivate does not exist on the stack"))) + Err(format!("Mutable borrow-to-reactivate ({:?}) does not exist on the stack", acc_m)) + } + + /// 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) { + Ok(action) => action, + Err(err) => return err!(MachineError(err)), + }; + + match action { + None => {}, // nothing to do + Some(top) => { + self.frozen_since = None; + self.borrows.truncate(top+1); + } + } + + Ok(()) } /// Initiate `bor`; mostly this means freezing or pushing. @@ -471,8 +460,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' // 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 !stack.reactivatable(ptr.tag) { - return err!(MachineError(format!("Encountered {:?} reference with non-reactivatable tag {:?}", ref_kind, ptr.tag))) + if let Err(err) = stack.reactivatable(ptr.tag, /*force_mut*/false) { + return err!(MachineError(format!("Encountered {:?} reference with non-reactivatable tag: {}", ref_kind, 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 83132195fe4..7d56f30b3e6 100644 --- a/tests/compile-fail/stacked_borrows/alias_through_mutation.rs +++ b/tests/compile-fail/stacked_borrows/alias_through_mutation.rs @@ -14,5 +14,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 Frz + let _val = *target_alias; //~ ERROR Shr 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 9e94aa8885d..dc1e3cc81e9 100644 --- a/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs +++ b/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs @@ -17,6 +17,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 Mut(Uniq + v1[1] = 5; //~ ERROR Mut 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 9fbcec4a8ef..a697ba9167c 100644 --- a/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs +++ b/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs @@ -14,7 +14,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 Mut(Uniq + //~^ ERROR Mut 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_write1.rs b/tests/compile-fail/stacked_borrows/illegal_write1.rs index ff3a3cd8224..131e89572a5 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 Frz + let _x = *ref_; //~ ERROR Shr reference with non-reactivatable tag } diff --git a/tests/compile-fail/stacked_borrows/load_invalid_mut.rs b/tests/compile-fail/stacked_borrows/load_invalid_mut.rs index e52b84a907e..060cec962c4 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 Mut(Uniq + let _val = *xref_in_mem; //~ ERROR Mut reference with non-reactivatable tag } diff --git a/tests/compile-fail/stacked_borrows/pass_invalid_mut.rs b/tests/compile-fail/stacked_borrows/pass_invalid_mut.rs index f3de3f0c4ac..bc950771add 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 Mut(Uniq + foo(xref); //~ ERROR Mut reference with non-reactivatable tag } diff --git a/tests/compile-fail/stacked_borrows/return_invalid_mut.rs b/tests/compile-fail/stacked_borrows/return_invalid_mut.rs index 6a4123d325b..c02892671e9 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 Mut(Uniq + ret //~ ERROR Mut reference with non-reactivatable tag } fn main() { diff --git a/tests/compile-fail/stacked_borrows/shared_confusion.rs b/tests/compile-fail/stacked_borrows/shared_confusion.rs index 5098f493ccd..e3e4c4da776 100644 --- a/tests/compile-fail/stacked_borrows/shared_confusion.rs +++ b/tests/compile-fail/stacked_borrows/shared_confusion.rs @@ -13,7 +13,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 Mut(Uniq + *x_inner = 12; //~ ERROR Mut reference with non-reactivatable tag } fn main() {