diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 0fe42f7974..c276f83de6 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -12,7 +12,7 @@ use rustc::hir::{MutMutable, MutImmutable}; use rustc::mir::RetagKind; use crate::{ - InterpResult, InterpError, MiriEvalContext, HelpersEvalContextExt, Evaluator, MutValueVisitor, + InterpResult, InterpError, HelpersEvalContextExt, MemoryKind, MiriMemoryKind, RangeMap, AllocId, Pointer, Immediate, ImmTy, PlaceTy, MPlaceTy, }; @@ -632,54 +632,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } } - // We need a visitor to visit all references. However, that requires - // a `MemPlace`, so we have a fast path for reference types that - // avoids allocating. + // We only reborrow "bare" references/boxes. + // Not traversing into fields helps with , + // but might also cost us optimization and analyses. We will have to experiment more with this. if let Some((mutbl, protector)) = qualify(place.layout.ty, kind) { // Fast path. let val = this.read_immediate(this.place_to_op(place)?)?; let val = this.retag_reference(val, mutbl, protector)?; this.write_immediate(val, place)?; - return Ok(()); - } - let place = this.force_allocation(place)?; - - let mut visitor = RetagVisitor { ecx: this, kind }; - visitor.visit_value(place)?; - - // The actual visitor. - struct RetagVisitor<'ecx, 'mir, 'tcx> { - ecx: &'ecx mut MiriEvalContext<'mir, 'tcx>, - kind: RetagKind, - } - impl<'ecx, 'mir, 'tcx> - MutValueVisitor<'mir, 'tcx, Evaluator<'tcx>> - for - RetagVisitor<'ecx, 'mir, 'tcx> - { - type V = MPlaceTy<'tcx, Tag>; - - #[inline(always)] - fn ecx(&mut self) -> &mut MiriEvalContext<'mir, 'tcx> { - &mut self.ecx - } - - // Primitives of reference type, that is the one thing we are interested in. - fn visit_primitive(&mut self, place: MPlaceTy<'tcx, Tag>) -> InterpResult<'tcx> - { - // Cannot use `builtin_deref` because that reports *immutable* for `Box`, - // making it useless. - if let Some((mutbl, protector)) = qualify(place.layout.ty, self.kind) { - let val = self.ecx.read_immediate(place.into())?; - let val = self.ecx.retag_reference( - val, - mutbl, - protector - )?; - self.ecx.write_immediate(val, place.into())?; - } - Ok(()) - } } Ok(()) 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 812dd47ef1..798f68fa13 100644 --- a/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs +++ b/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs @@ -9,7 +9,6 @@ mod safe { assert!(mid <= len); (from_raw_parts_mut(ptr, len - mid), // BUG: should be "mid" instead of "len - mid" - //~^ ERROR borrow stack from_raw_parts_mut(ptr.offset(mid as isize), len - mid)) } } @@ -18,6 +17,7 @@ mod safe { fn main() { let mut array = [1,2,3,4]; let (a, b) = safe::split_at_mut(&mut array, 0); + //~^ ERROR borrow stack a[1] = 5; b[1] = 6; } diff --git a/tests/compile-fail/stacked_borrows/return_invalid_mut_option.rs b/tests/compile-fail/stacked_borrows/return_invalid_mut_option.rs index 2eb2df81f5..1cb3f3f920 100644 --- a/tests/compile-fail/stacked_borrows/return_invalid_mut_option.rs +++ b/tests/compile-fail/stacked_borrows/return_invalid_mut_option.rs @@ -1,11 +1,16 @@ // Make sure that we cannot return a `&mut` that got already invalidated, not even in an `Option`. +// Due to shallow reborrowing, the error only surfaces when we look into the `Option`. fn foo(x: &mut (i32, i32)) -> Option<&mut i32> { let xraw = x as *mut (i32, i32); - let ret = Some(unsafe { &mut (*xraw).1 }); + let ret = unsafe { &mut (*xraw).1 }; // let-bind to avoid 2phase + let ret = Some(ret); let _val = unsafe { *xraw }; // invalidate xref - ret //~ ERROR borrow stack + ret } fn main() { - foo(&mut (1, 2)); + match foo(&mut (1, 2)) { + Some(_x) => {}, //~ ERROR borrow stack + None => {}, + } } diff --git a/tests/compile-fail/stacked_borrows/return_invalid_mut_tuple.rs b/tests/compile-fail/stacked_borrows/return_invalid_mut_tuple.rs index 8b73df4bd1..fbd9a6e5d2 100644 --- a/tests/compile-fail/stacked_borrows/return_invalid_mut_tuple.rs +++ b/tests/compile-fail/stacked_borrows/return_invalid_mut_tuple.rs @@ -1,11 +1,12 @@ // Make sure that we cannot return a `&mut` that got already invalidated, not even in a tuple. +// Due to shallow reborrowing, the error only surfaces when we look into the tuple. fn foo(x: &mut (i32, i32)) -> (&mut i32,) { let xraw = x as *mut (i32, i32); let ret = (unsafe { &mut (*xraw).1 },); let _val = unsafe { *xraw }; // invalidate xref - ret //~ ERROR borrow stack + ret } fn main() { - foo(&mut (1, 2)); + foo(&mut (1, 2)).0; //~ ERROR: borrow stack } diff --git a/tests/compile-fail/stacked_borrows/return_invalid_shr_option.rs b/tests/compile-fail/stacked_borrows/return_invalid_shr_option.rs index f3a35ca266..2d8527fa3f 100644 --- a/tests/compile-fail/stacked_borrows/return_invalid_shr_option.rs +++ b/tests/compile-fail/stacked_borrows/return_invalid_shr_option.rs @@ -1,11 +1,15 @@ // Make sure that we cannot return a `&` that got already invalidated, not even in an `Option`. +// Due to shallow reborrowing, the error only surfaces when we look into the `Option`. fn foo(x: &mut (i32, i32)) -> Option<&i32> { let xraw = x as *mut (i32, i32); let ret = Some(unsafe { &(*xraw).1 }); unsafe { *xraw = (42, 23) }; // unfreeze - ret //~ ERROR borrow stack + ret } fn main() { - foo(&mut (1, 2)); + match foo(&mut (1, 2)) { + Some(_x) => {}, //~ ERROR borrow stack + None => {}, + } } diff --git a/tests/compile-fail/stacked_borrows/return_invalid_shr_tuple.rs b/tests/compile-fail/stacked_borrows/return_invalid_shr_tuple.rs index 82723bade2..d7494d6ee6 100644 --- a/tests/compile-fail/stacked_borrows/return_invalid_shr_tuple.rs +++ b/tests/compile-fail/stacked_borrows/return_invalid_shr_tuple.rs @@ -1,11 +1,12 @@ // Make sure that we cannot return a `&` that got already invalidated, not even in a tuple. +// Due to shallow reborrowing, the error only surfaces when we look into the tuple. fn foo(x: &mut (i32, i32)) -> (&i32,) { let xraw = x as *mut (i32, i32); let ret = (unsafe { &(*xraw).1 },); unsafe { *xraw = (42, 23) }; // unfreeze - ret //~ ERROR borrow stack + ret } fn main() { - foo(&mut (1, 2)); + foo(&mut (1, 2)).0; //~ ERROR borrow stack } diff --git a/tests/run-pass/refcell.rs b/tests/run-pass/refcell.rs deleted file mode 100644 index 93cef1572a..0000000000 --- a/tests/run-pass/refcell.rs +++ /dev/null @@ -1,33 +0,0 @@ -use std::cell::RefCell; - -fn main() { - let c = RefCell::new(42); - { - let s1 = c.borrow(); - let _x: i32 = *s1; - let s2 = c.borrow(); - let _x: i32 = *s1; - let _y: i32 = *s2; - let _x: i32 = *s1; - let _y: i32 = *s2; - } - { - let mut m = c.borrow_mut(); - let _z: i32 = *m; - { - let s: &i32 = &*m; - let _x = *s; - } - *m = 23; - let _z: i32 = *m; - } - { - let s1 = c.borrow(); - let _x: i32 = *s1; - let s2 = c.borrow(); - let _x: i32 = *s1; - let _y: i32 = *s2; - let _x: i32 = *s1; - let _y: i32 = *s2; - } -} diff --git a/tests/run-pass/stacked-borrows/refcell.rs b/tests/run-pass/stacked-borrows/refcell.rs new file mode 100644 index 0000000000..0939a66619 --- /dev/null +++ b/tests/run-pass/stacked-borrows/refcell.rs @@ -0,0 +1,68 @@ +use std::cell::{RefCell, Ref, RefMut}; + +fn main() { + basic(); + ref_protector(); + ref_mut_protector(); +} + +fn basic() { + let c = RefCell::new(42); + { + let s1 = c.borrow(); + let _x: i32 = *s1; + let s2 = c.borrow(); + let _x: i32 = *s1; + let _y: i32 = *s2; + let _x: i32 = *s1; + let _y: i32 = *s2; + } + { + let mut m = c.borrow_mut(); + let _z: i32 = *m; + { + let s: &i32 = &*m; + let _x = *s; + } + *m = 23; + let _z: i32 = *m; + } + { + let s1 = c.borrow(); + let _x: i32 = *s1; + let s2 = c.borrow(); + let _x: i32 = *s1; + let _y: i32 = *s2; + let _x: i32 = *s1; + let _y: i32 = *s2; + } +} + +// Adding a Stacked Borrows protector for `Ref` would break this +fn ref_protector() { + fn break_it(rc: &RefCell, r: Ref<'_, i32>) { + // `r` has a shared reference, it is passed in as argument and hence + // a protector is added that marks this memory as read-only for the entire + // duration of this function. + drop(r); + // *oops* here we can mutate that memory. + *rc.borrow_mut() = 2; + } + + let rc = RefCell::new(0); + break_it(&rc, rc.borrow()) +} + +fn ref_mut_protector() { + fn break_it(rc: &RefCell, r: RefMut<'_, i32>) { + // `r` has a shared reference, it is passed in as argument and hence + // a protector is added that marks this memory as inaccessible for the entire + // duration of this function + drop(r); + // *oops* here we can mutate that memory. + *rc.borrow_mut() = 2; + } + + let rc = RefCell::new(0); + break_it(&rc, rc.borrow_mut()) +}