From 7e9098ff692eccd07c509f1a6bee1328e8882d34 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 12 Dec 2018 11:11:20 +0100 Subject: [PATCH 1/2] treat ref-to-raw cast like a reborrow: do a special kind of retag --- src/lib.rs | 30 +---------- src/stacked_borrows.rs | 52 +++++++++---------- .../stacked_borrows/transmute-is-no-escape.rs | 9 ++-- 3 files changed, 31 insertions(+), 60 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 28639976aa..e821b4f76a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -474,36 +474,10 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { } } - #[inline] - fn escape_to_raw( - ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, - ptr: OpTy<'tcx, Self::PointerTag>, - ) -> EvalResult<'tcx> { - // It is tempting to check the type here, but drop glue does EscapeToRaw - // on a raw pointer. - // This is deliberately NOT `deref_operand` as we do not want `tag_dereference` - // to be called! That would kill the original tag if we got a raw ptr. - let place = ecx.ref_to_mplace(ecx.read_immediate(ptr)?)?; - let size = ecx.size_and_align_of_mplace(place)?.map(|(size, _)| size) - // for extern types, just cover what we can - .unwrap_or_else(|| place.layout.size); - if !ecx.tcx.sess.opts.debugging_opts.mir_emit_retag || - !ecx.machine.validate || size == Size::ZERO - { - // No tracking, or no retagging. The latter is possible because a dependency of ours - // might be called with different flags than we are, so there are `Retag` - // statements but we do not want to execute them. - Ok(()) - } else { - ecx.escape_to_raw(place, size) - } - } - #[inline(always)] fn retag( ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, - fn_entry: bool, - two_phase: bool, + kind: mir::RetagKind, place: PlaceTy<'tcx, Borrow>, ) -> EvalResult<'tcx> { if !ecx.tcx.sess.opts.debugging_opts.mir_emit_retag || !Self::enforce_validity(ecx) { @@ -514,7 +488,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { // uninitialized data. Ok(()) } else { - ecx.retag(fn_entry, two_phase, place) + ecx.retag(kind, place) } } diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 748ac020d6..27d4a4f7f7 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -4,6 +4,7 @@ use std::rc::Rc; use rustc::ty::{self, layout::Size}; use rustc::hir::{Mutability, MutMutable, MutImmutable}; +use rustc::mir::RetagKind; use crate::{ EvalResult, EvalErrorKind, MiriEvalContext, HelpersEvalContextExt, Evaluator, MutValueVisitor, @@ -550,10 +551,11 @@ trait EvalContextPrivExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a, } /// Retag an indidual pointer, returning the retagged version. + /// `mutbl` can be `None` to make this a raw pointer. fn retag_reference( &mut self, val: ImmTy<'tcx, Borrow>, - mutbl: Mutability, + mutbl: Option, fn_barrier: bool, two_phase: bool, ) -> EvalResult<'tcx, Immediate> { @@ -571,8 +573,9 @@ trait EvalContextPrivExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a, // Compute new borrow. let time = this.machine.stacked_borrows.increment_clock(); let new_bor = match mutbl { - MutMutable => Borrow::Uniq(time), - MutImmutable => Borrow::Shr(Some(time)), + Some(MutMutable) => Borrow::Uniq(time), + Some(MutImmutable) => Borrow::Shr(Some(time)), + None => Borrow::default(), }; // Reborrow. @@ -580,7 +583,7 @@ trait EvalContextPrivExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a, 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"); + assert!(mutbl == Some(MutMutable), "two-phase shared borrows make no sense"); // We immediately share it, to allow read accesses let two_phase_time = this.machine.stacked_borrows.increment_clock(); let two_phase_bor = Borrow::Shr(Some(two_phase_time)); @@ -665,35 +668,24 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a, Ok(()) } - /// The given place may henceforth be accessed through raw pointers. - #[inline(always)] - fn escape_to_raw( - &mut self, - place: MPlaceTy<'tcx, Borrow>, - size: Size, - ) -> EvalResult<'tcx> { - let this = self.eval_context_mut(); - this.reborrow(place, size, /*fn_barrier*/ false, Borrow::default())?; - Ok(()) - } - fn retag( &mut self, - fn_entry: bool, - two_phase: bool, + kind: RetagKind, place: PlaceTy<'tcx, Borrow> ) -> EvalResult<'tcx> { let this = self.eval_context_mut(); // Determine mutability and whether to add a barrier. // Cannot use `builtin_deref` because that reports *immutable* for `Box`, // making it useless. - fn qualify(ty: ty::Ty<'_>, fn_entry: bool) -> Option<(Mutability, bool)> { + fn qualify(ty: ty::Ty<'_>, kind: RetagKind) -> Option<(Option, bool)> { match ty.sty { // References are simple - ty::Ref(_, _, mutbl) => Some((mutbl, fn_entry)), + ty::Ref(_, _, mutbl) => Some((Some(mutbl), kind == RetagKind::FnEntry)), + // Raw pointers need to be enabled + ty::RawPtr(..) if kind == RetagKind::Raw => Some((None, false)), // Boxes do not get a barrier: Barriers reflect that references outlive the call // they were passed in to; that's just not the case for boxes. - ty::Adt(..) if ty.is_box() => Some((MutMutable, false)), + ty::Adt(..) if ty.is_box() => Some((Some(MutMutable), false)), _ => None, } } @@ -701,23 +693,22 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a, // 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. - if let Some((mutbl, barrier)) = qualify(place.layout.ty, fn_entry) { + if let Some((mutbl, barrier)) = qualify(place.layout.ty, kind) { // fast path let val = this.read_immediate(this.place_to_op(place)?)?; - let val = this.retag_reference(val, mutbl, barrier, two_phase)?; + let val = this.retag_reference(val, mutbl, barrier, kind == RetagKind::TwoPhase)?; this.write_immediate(val, place)?; return Ok(()); } let place = this.force_allocation(place)?; - let mut visitor = RetagVisitor { ecx: this, fn_entry, two_phase }; + let mut visitor = RetagVisitor { ecx: this, kind }; 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, + kind: RetagKind, } impl<'ecx, 'a, 'mir, 'tcx> MutValueVisitor<'a, 'mir, 'tcx, Evaluator<'tcx>> @@ -736,9 +727,14 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a, { // Cannot use `builtin_deref` because that reports *immutable* for `Box`, // making it useless. - if let Some((mutbl, barrier)) = qualify(place.layout.ty, self.fn_entry) { + if let Some((mutbl, barrier)) = qualify(place.layout.ty, self.kind) { let val = self.ecx.read_immediate(place.into())?; - let val = self.ecx.retag_reference(val, mutbl, barrier, self.two_phase)?; + let val = self.ecx.retag_reference( + val, + mutbl, + barrier, + self.kind == RetagKind::TwoPhase + )?; self.ecx.write_immediate(val, place.into())?; } Ok(()) diff --git a/tests/compile-fail/stacked_borrows/transmute-is-no-escape.rs b/tests/compile-fail/stacked_borrows/transmute-is-no-escape.rs index 75abce3111..197c11197e 100644 --- a/tests/compile-fail/stacked_borrows/transmute-is-no-escape.rs +++ b/tests/compile-fail/stacked_borrows/transmute-is-no-escape.rs @@ -1,13 +1,14 @@ // Make sure we cannot use raw ptrs that got transmuted from mutable references // (i.e, no EscapeToRaw happened). -// We could, in principle, to EscapeToRaw lazily to allow this code, but that +// We could, in principle, do EscapeToRaw lazily to allow this code, but that // would no alleviate the need for EscapeToRaw (see `ref_raw_int_raw` in // `run-pass/stacked-borrows.rs`), and thus increase overall complexity. use std::mem; fn main() { - let mut x: i32 = 42; - let raw: *mut i32 = unsafe { mem::transmute(&mut x) }; - let raw = raw as usize as *mut i32; // make sure we killed the tag + let mut x: [i32; 2] = [42, 43]; + let _raw: *mut i32 = unsafe { mem::transmute(&mut x[0]) }; + // `raw` still carries a tag, so we get another pointer to the same location that does not carry a tag + let raw = (&mut x[1] as *mut i32).wrapping_offset(-1); unsafe { *raw = 13; } //~ ERROR does not exist on the stack } From 2c2587bc8d18f9fea886b9a852218fe8c89d6f73 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Dec 2018 09:29:42 +0100 Subject: [PATCH 2/2] bump Rust version --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index d1ff80dec0..7a16560006 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -nightly-2018-12-18 +nightly-2018-12-21