From 479eb3bd2b538c9108880d1b6cafef648a2c8b12 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 28 Nov 2018 09:33:33 +0100 Subject: [PATCH 1/6] support for basic (non-overlapping) 2-phase borrows --- src/lib.rs | 3 +- src/stacked_borrows.rs | 33 ++++++++++++------- tests/run-pass/2phase.rs | 69 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 12 deletions(-) create mode 100644 tests/run-pass/2phase.rs diff --git a/src/lib.rs b/src/lib.rs index 9739a7a95b6d..165b15290031 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -525,6 +525,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { fn retag( ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, fn_entry: bool, + two_phase: bool, place: PlaceTy<'tcx, Borrow>, ) -> EvalResult<'tcx> { if !ecx.tcx.sess.opts.debugging_opts.mir_emit_retag || !Self::enforce_validity(ecx) { @@ -535,7 +536,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { // uninitialized data. Ok(()) } else { - ecx.retag(fn_entry, place) + ecx.retag(fn_entry, two_phase, place) } } diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 31f80fe2f6c6..3ea434f00f79 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -8,7 +8,7 @@ use rustc::hir::{Mutability, MutMutable, MutImmutable}; use crate::{ EvalResult, EvalErrorKind, MiriEvalContext, HelpersEvalContextExt, Evaluator, MutValueVisitor, MemoryKind, MiriMemoryKind, RangeMap, AllocId, Allocation, AllocationExtra, - Pointer, MemPlace, Scalar, Immediate, ImmTy, PlaceTy, MPlaceTy, + Pointer, Immediate, ImmTy, PlaceTy, MPlaceTy, }; pub type Timestamp = u64; @@ -534,7 +534,7 @@ pub trait EvalContextExt<'tcx> { size: Size, fn_barrier: bool, new_bor: Borrow - ) -> EvalResult<'tcx, Pointer>; + ) -> EvalResult<'tcx>; /// Retag an indidual pointer, returning the retagged version. fn retag_reference( @@ -542,11 +542,13 @@ pub trait EvalContextExt<'tcx> { ptr: ImmTy<'tcx, Borrow>, mutbl: Mutability, fn_barrier: bool, + two_phase: bool, ) -> EvalResult<'tcx, Immediate>; fn retag( &mut self, fn_entry: bool, + two_phase: bool, place: PlaceTy<'tcx, Borrow> ) -> EvalResult<'tcx>; @@ -644,9 +646,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> { size: Size, fn_barrier: bool, new_bor: Borrow - ) -> EvalResult<'tcx, Pointer> { + ) -> EvalResult<'tcx> { let ptr = place.ptr.to_ptr()?; - let new_ptr = Pointer::new_with_tag(ptr.alloc_id, ptr.offset, new_bor); let barrier = if fn_barrier { Some(self.frame().extra) } else { None }; trace!("reborrow: Creating new reference for {:?} (pointee {}): {:?}", ptr, place.layout.ty, new_bor); @@ -666,7 +667,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> { let kind = if new_bor.is_unique() { RefKind::Unique } else { RefKind::Raw }; alloc.extra.reborrow(ptr, size, barrier, new_bor, kind)?; } - Ok(new_ptr) + Ok(()) } fn retag_reference( @@ -674,6 +675,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> { val: ImmTy<'tcx, Borrow>, mutbl: Mutability, fn_barrier: bool, + two_phase: bool, ) -> EvalResult<'tcx, Immediate> { // We want a place for where the ptr *points to*, so we get one. let place = self.ref_to_mplace(val)?; @@ -693,16 +695,24 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> { }; // Reborrow. - let new_ptr = self.reborrow(place, size, fn_barrier, new_bor)?; + self.reborrow(place, size, fn_barrier, new_bor)?; + let new_place = place.with_tag(new_bor); + // Handle two-phase borrows. + if two_phase { + // We immediately share it, to allow read accesses + let two_phase_time = self.machine.stacked_borrows.increment_clock(); + let two_phase_bor = Borrow::Shr(Some(two_phase_time)); + self.reborrow(new_place, size, /*fn_barrier*/false, two_phase_bor)?; + } - // Return new ptr - let new_place = MemPlace { ptr: Scalar::Ptr(new_ptr), ..*place }; + // Return new ptr. Ok(new_place.to_ref()) } fn retag( &mut self, fn_entry: bool, + two_phase: bool, place: PlaceTy<'tcx, Borrow> ) -> EvalResult<'tcx> { // Determine mutability and whether to add a barrier. @@ -725,19 +735,20 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> { if let Some((mutbl, barrier)) = qualify(place.layout.ty, fn_entry) { // fast path let val = self.read_immediate(self.place_to_op(place)?)?; - let val = self.retag_reference(val, mutbl, barrier)?; + let val = self.retag_reference(val, mutbl, barrier, two_phase)?; self.write_immediate(val, place)?; return Ok(()); } let place = self.force_allocation(place)?; - let mut visitor = RetagVisitor { ecx: self, fn_entry }; + let mut visitor = RetagVisitor { ecx: self, fn_entry, two_phase }; visitor.visit_value(place)?; // The actual visitor struct RetagVisitor<'ecx, 'a, 'mir, 'tcx> { ecx: &'ecx mut MiriEvalContext<'a, 'mir, 'tcx>, fn_entry: bool, + two_phase: bool, } impl<'ecx, 'a, 'mir, 'tcx> MutValueVisitor<'a, 'mir, 'tcx, Evaluator<'tcx>> @@ -758,7 +769,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> { // making it useless. if let Some((mutbl, barrier)) = qualify(place.layout.ty, self.fn_entry) { let val = self.ecx.read_immediate(place.into())?; - let val = self.ecx.retag_reference(val, mutbl, barrier)?; + let val = self.ecx.retag_reference(val, mutbl, barrier, self.two_phase)?; self.ecx.write_immediate(val, place.into())?; } Ok(()) diff --git a/tests/run-pass/2phase.rs b/tests/run-pass/2phase.rs new file mode 100644 index 000000000000..0b2a7d53c855 --- /dev/null +++ b/tests/run-pass/2phase.rs @@ -0,0 +1,69 @@ +#![feature(nll)] + +trait S: Sized { + fn tpb(&mut self, _s: Self) {} +} + +impl S for i32 {} + +fn two_phase1() { + let mut x = 3; + x.tpb(x); +} + +fn two_phase2() { + let mut v = vec![]; + v.push(v.len()); +} + +/* +fn two_phase_overlapping1() { + let mut x = vec![]; + let p = &x; + x.push(p.len()); +} + +fn two_phase_overlapping2() { + use std::ops::AddAssign; + let mut x = 1; + let l = &x; + x.add_assign(x + *l); +} +*/ + +fn match_two_phase() { + let mut x = 3; + match x { + ref mut y if { let _val = x; let _val = *y; true } => {}, + _ => (), + } +} + +fn with_interior_mutability() { + use std::cell::Cell; + + trait Thing: Sized { + fn do_the_thing(&mut self, _s: i32) {} + } + + impl Thing for Cell {} + + let mut x = Cell::new(1); + let l = &x; + x + .do_the_thing({ + x.set(3); + l.set(4); + x.get() + l.get() + }) + ; +} + +fn main() { + two_phase1(); + two_phase2(); + //two_phase_overlapping1(); + //two_phase_overlapping2(); + match_two_phase(); + with_interior_mutability(); +} From b2305da8d0f4932cad3cea444d598a6e4054f804 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 28 Nov 2018 10:35:10 +0100 Subject: [PATCH 2/6] assert some sense --- src/stacked_borrows.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 3ea434f00f79..1c8224ef1784 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -699,6 +699,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> { let new_place = place.with_tag(new_bor); // Handle two-phase borrows. if two_phase { + assert!(mutbl == MutMutable, "two-phase shared borrows make no sense"); // We immediately share it, to allow read accesses let two_phase_time = self.machine.stacked_borrows.increment_clock(); let two_phase_bor = Borrow::Shr(Some(two_phase_time)); From b6e5822601beb443faed97d7d7333ba543ea119d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 3 Dec 2018 10:28:32 +0100 Subject: [PATCH 3/6] add FIXME --- tests/run-pass/2phase.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/run-pass/2phase.rs b/tests/run-pass/2phase.rs index 0b2a7d53c855..5b9e5d3ea5ff 100644 --- a/tests/run-pass/2phase.rs +++ b/tests/run-pass/2phase.rs @@ -62,8 +62,9 @@ fn with_interior_mutability() { fn main() { two_phase1(); two_phase2(); - //two_phase_overlapping1(); - //two_phase_overlapping2(); match_two_phase(); with_interior_mutability(); + //FIXME: enable these, or remove them, depending on how https://github.com/rust-lang/rust/issues/56254 gets resolved + //two_phase_overlapping1(); + //two_phase_overlapping2(); } From bbdc3380d57fafbe66d2f6422df35d542331555f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 7 Dec 2018 10:15:25 +0100 Subject: [PATCH 4/6] fix tests --- tests/compile-fail/validity/nonzero.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/compile-fail/validity/nonzero.rs b/tests/compile-fail/validity/nonzero.rs index d1de0f8095e3..f820b0e810b8 100644 --- a/tests/compile-fail/validity/nonzero.rs +++ b/tests/compile-fail/validity/nonzero.rs @@ -7,5 +7,5 @@ pub(crate) struct NonZero(pub(crate) T); fn main() { // Make sure that we detect this even when no function call is happening along the way - let _x = Some(NonZero(0)); //~ ERROR encountered 0, but expected something greater or equal to 1 + let _x = Some(unsafe { NonZero(0) }); //~ ERROR encountered 0, but expected something greater or equal to 1 } From f06e25f9b201dafe5f402e116b0ab7ec16727093 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 8 Dec 2018 10:33:29 +0100 Subject: [PATCH 5/6] bump Rust version, fix build --- rust-version | 2 +- src/bin/miri-rustc-tests.rs | 4 ++-- src/bin/miri.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/rust-version b/rust-version index 087b7c664272..69d072f1bfa0 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -nightly-2018-12-03 +nightly-2018-12-08 diff --git a/src/bin/miri-rustc-tests.rs b/src/bin/miri-rustc-tests.rs index 6fa9b817ffee..92d4237146dc 100644 --- a/src/bin/miri-rustc-tests.rs +++ b/src/bin/miri-rustc-tests.rs @@ -93,7 +93,7 @@ fn after_analysis<'a, 'tcx>(state: &mut CompileState<'a, 'tcx>) { fn visit_item(&mut self, i: &'hir hir::Item) { if let hir::ItemKind::Fn(.., body_id) = i.node { if i.attrs.iter().any(|attr| attr.name() == "test") { - let did = self.0.hir.body_owner_def_id(body_id); + let did = self.0.hir().body_owner_def_id(body_id); println!("running test: {}", self.0.def_path_debug_str(did)); miri::eval_main(self.0, did, /*validate*/true); self.1.session.abort_if_errors(); @@ -105,7 +105,7 @@ fn after_analysis<'a, 'tcx>(state: &mut CompileState<'a, 'tcx>) { } state.hir_crate.unwrap().visit_all_item_likes(&mut Visitor(tcx, state)); } else if let Some((entry_node_id, _, _)) = *state.session.entry_fn.borrow() { - let entry_def_id = tcx.hir.local_def_id(entry_node_id); + let entry_def_id = tcx.hir().local_def_id(entry_node_id); miri::eval_main(tcx, entry_def_id, /*validate*/true); state.session.abort_if_errors(); diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 1bbf3c8c4a4a..e88c13305d15 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -128,7 +128,7 @@ fn after_analysis<'a, 'tcx>( attr.name() == "test" }) { - let did = self.tcx.hir.body_owner_def_id(body_id); + let did = self.tcx.hir().body_owner_def_id(body_id); println!( "running test: {}", self.tcx.def_path_debug_str(did), @@ -145,7 +145,7 @@ fn after_analysis<'a, 'tcx>( &mut Visitor { tcx, state, validate } ); } else if let Some((entry_node_id, _, _)) = *state.session.entry_fn.borrow() { - let entry_def_id = tcx.hir.local_def_id(entry_node_id); + let entry_def_id = tcx.hir().local_def_id(entry_node_id); miri::eval_main(tcx, entry_def_id, validate); state.session.abort_if_errors(); From 8d1e1179a17bc2aba017a5a16f0b42772032e00d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 8 Dec 2018 10:47:50 +0100 Subject: [PATCH 6/6] fix benches --- benches/helpers/miri_helper.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benches/helpers/miri_helper.rs b/benches/helpers/miri_helper.rs index 015e36c4f5ad..c7df34eaf706 100644 --- a/benches/helpers/miri_helper.rs +++ b/benches/helpers/miri_helper.rs @@ -47,7 +47,7 @@ pub fn run(filename: &str, bencher: &mut Bencher) { let (entry_node_id, _, _) = state.session.entry_fn.borrow().expect( "no main or start function found", ); - let entry_def_id = tcx.hir.local_def_id(entry_node_id); + let entry_def_id = tcx.hir().local_def_id(entry_node_id); bencher.borrow_mut().iter(|| { eval_main(tcx, entry_def_id, false);