From 479eb3bd2b538c9108880d1b6cafef648a2c8b12 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 28 Nov 2018 09:33:33 +0100 Subject: [PATCH 1/3] 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 9739a7a95b..165b152900 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 31f80fe2f6..3ea434f00f 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 0000000000..0b2a7d53c8 --- /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/3] 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 3ea434f00f..1c8224ef17 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/3] 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 0b2a7d53c8..5b9e5d3ea5 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(); }