From aafcf2c9422e7e1de2ebefd51d78cf1d07d02cd6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 24 Oct 2018 11:47:17 +0200 Subject: [PATCH 1/9] Emit Retag statements, kill Validate statements Also "rename" -Zmir-emit-validate to -Zmir-emit-retag, which is just a boolean (yes or no). --- src/bootstrap/bin/rustc.rs | 7 +- src/librustc/ich/impls_mir.rs | 23 +- src/librustc/mir/mod.rs | 74 +--- src/librustc/mir/visit.rs | 20 +- src/librustc/session/config.rs | 5 +- src/librustc/ty/context.rs | 4 +- src/librustc_codegen_llvm/mir/analyze.rs | 3 +- src/librustc_codegen_llvm/mir/statement.rs | 2 +- src/librustc_mir/borrow_check/mod.rs | 4 +- .../borrow_check/nll/invalidation.rs | 4 +- .../borrow_check/nll/type_check/mod.rs | 2 +- src/librustc_mir/dataflow/impls/borrows.rs | 2 +- .../dataflow/move_paths/builder.rs | 2 +- src/librustc_mir/interpret/machine.rs | 6 +- src/librustc_mir/interpret/step.rs | 9 +- src/librustc_mir/transform/add_retag.rs | 169 ++++++++ src/librustc_mir/transform/add_validation.rs | 395 ------------------ src/librustc_mir/transform/check_unsafety.rs | 2 +- src/librustc_mir/transform/erase_regions.rs | 19 +- src/librustc_mir/transform/mod.rs | 12 +- src/librustc_mir/transform/qualify_consts.rs | 2 +- .../transform/qualify_min_const_fn.rs | 2 +- .../transform/remove_noop_landing_pads.rs | 2 +- src/librustc_mir/transform/rustc_peek.rs | 2 +- src/librustc_mir/util/liveness.rs | 2 +- src/librustc_passes/mir_stats.rs | 2 +- 26 files changed, 233 insertions(+), 543 deletions(-) create mode 100644 src/librustc_mir/transform/add_retag.rs delete mode 100644 src/librustc_mir/transform/add_validation.rs diff --git a/src/bootstrap/bin/rustc.rs b/src/bootstrap/bin/rustc.rs index b6764c1aaeab6..07a05aeae5d64 100644 --- a/src/bootstrap/bin/rustc.rs +++ b/src/bootstrap/bin/rustc.rs @@ -254,7 +254,12 @@ fn main() { // When running miri tests, we need to generate MIR for all libraries if env::var("TEST_MIRI").ok().map_or(false, |val| val == "true") { cmd.arg("-Zalways-encode-mir"); - cmd.arg("-Zmir-emit-validate=1"); + // These options are preferred by miri, to be able to perform better validation, + // but the bootstrap compiler might not understand them. + if stage != "0" { + cmd.arg("-Zmir-emit-retag"); + cmd.arg("-Zmir-opt-level=0"); + } } // Force all crates compiled by this compiler to (a) be unstable and (b) diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs index 274a2df283cbd..a73fe2b8a1ab3 100644 --- a/src/librustc/ich/impls_mir.rs +++ b/src/librustc/ich/impls_mir.rs @@ -257,9 +257,9 @@ for mir::StatementKind<'gcx> { mir::StatementKind::EndRegion(ref region_scope) => { region_scope.hash_stable(hcx, hasher); } - mir::StatementKind::Validate(ref op, ref places) => { - op.hash_stable(hcx, hasher); - places.hash_stable(hcx, hasher); + mir::StatementKind::Retag { fn_entry, ref place } => { + fn_entry.hash_stable(hcx, hasher); + place.hash_stable(hcx, hasher); } mir::StatementKind::AscribeUserType(ref place, ref variance, ref c_ty) => { place.hash_stable(hcx, hasher); @@ -278,23 +278,6 @@ for mir::StatementKind<'gcx> { impl_stable_hash_for!(enum mir::FakeReadCause { ForMatchGuard, ForMatchedPlace, ForLet }); -impl<'a, 'gcx, T> HashStable> - for mir::ValidationOperand<'gcx, T> - where T: HashStable> -{ - fn hash_stable(&self, - hcx: &mut StableHashingContext<'a>, - hasher: &mut StableHasher) - { - self.place.hash_stable(hcx, hasher); - self.ty.hash_stable(hcx, hasher); - self.re.hash_stable(hcx, hasher); - self.mutbl.hash_stable(hcx, hasher); - } -} - -impl_stable_hash_for!(enum mir::ValidationOp { Acquire, Release, Suspend(region_scope) }); - impl<'a, 'gcx> HashStable> for mir::Place<'gcx> { fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 4fea07011ccfb..4f6a8158933af 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1754,10 +1754,13 @@ pub enum StatementKind<'tcx> { inputs: Box<[Operand<'tcx>]>, }, - /// Assert the given places to be valid inhabitants of their type. These statements are - /// currently only interpreted by miri and only generated when "-Z mir-emit-validate" is passed. - /// See for more details. - Validate(ValidationOp, Vec>>), + /// Retag references in the given place, ensuring they got fresh tags. This is + /// part of the Stacked Borrows model. `fn_entry` indicates whether this + /// is the initial retag that happens in the function prolog. These statements are + /// currently only interpreted by miri and only generated when "-Z mir-emit-retag" is passed. + /// See + /// for more details. + Retag { fn_entry: bool, place: Place<'tcx> }, /// Mark one terminating point of a region scope (i.e. static region). /// (The starting point(s) arise implicitly from borrows.) @@ -1810,57 +1813,6 @@ pub enum FakeReadCause { ForLet, } -/// The `ValidationOp` describes what happens with each of the operands of a -/// `Validate` statement. -#[derive(Copy, Clone, RustcEncodable, RustcDecodable, PartialEq, Eq)] -pub enum ValidationOp { - /// Recursively traverse the place following the type and validate that all type - /// invariants are maintained. Furthermore, acquire exclusive/read-only access to the - /// memory reachable from the place. - Acquire, - /// Recursive traverse the *mutable* part of the type and relinquish all exclusive - /// access. - Release, - /// Recursive traverse the *mutable* part of the type and relinquish all exclusive - /// access *until* the given region ends. Then, access will be recovered. - Suspend(region::Scope), -} - -impl Debug for ValidationOp { - fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { - use self::ValidationOp::*; - match *self { - Acquire => write!(fmt, "Acquire"), - Release => write!(fmt, "Release"), - // (reuse lifetime rendering policy from ppaux.) - Suspend(ref ce) => write!(fmt, "Suspend({})", ty::ReScope(*ce)), - } - } -} - -// This is generic so that it can be reused by miri -#[derive(Clone, Hash, PartialEq, Eq, RustcEncodable, RustcDecodable)] -pub struct ValidationOperand<'tcx, T> { - pub place: T, - pub ty: Ty<'tcx>, - pub re: Option, - pub mutbl: hir::Mutability, -} - -impl<'tcx, T: Debug> Debug for ValidationOperand<'tcx, T> { - fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { - write!(fmt, "{:?}: {:?}", self.place, self.ty)?; - if let Some(ce) = self.re { - // (reuse lifetime rendering policy from ppaux.) - write!(fmt, "/{}", ty::ReScope(ce))?; - } - if let hir::MutImmutable = self.mutbl { - write!(fmt, " (imm)")?; - } - Ok(()) - } -} - impl<'tcx> Debug for Statement<'tcx> { fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { use self::StatementKind::*; @@ -1869,7 +1821,8 @@ impl<'tcx> Debug for Statement<'tcx> { FakeRead(ref cause, ref place) => write!(fmt, "FakeRead({:?}, {:?})", cause, place), // (reuse lifetime rendering policy from ppaux.) EndRegion(ref ce) => write!(fmt, "EndRegion({})", ty::ReScope(*ce)), - Validate(ref op, ref places) => write!(fmt, "Validate({:?}, {:?})", op, places), + Retag { fn_entry, ref place } => + write!(fmt, "Retag({}{:?})", if fn_entry { "[fn entry]: " } else { "" }, place), StorageLive(ref place) => write!(fmt, "StorageLive({:?})", place), StorageDead(ref place) => write!(fmt, "StorageDead({:?})", place), SetDiscriminant { @@ -2944,7 +2897,6 @@ CloneTypeFoldableAndLiftImpls! { SourceInfo, UpvarDecl, FakeReadCause, - ValidationOp, SourceScope, SourceScopeData, SourceScopeLocalData, @@ -2997,12 +2949,6 @@ BraceStructTypeFoldableImpl! { } } -BraceStructTypeFoldableImpl! { - impl<'tcx> TypeFoldable<'tcx> for ValidationOperand<'tcx, Place<'tcx>> { - place, ty, re, mutbl - } -} - BraceStructTypeFoldableImpl! { impl<'tcx> TypeFoldable<'tcx> for Statement<'tcx> { source_info, kind @@ -3017,7 +2963,7 @@ EnumTypeFoldableImpl! { (StatementKind::StorageLive)(a), (StatementKind::StorageDead)(a), (StatementKind::InlineAsm) { asm, outputs, inputs }, - (StatementKind::Validate)(a, b), + (StatementKind::Retag) { fn_entry, place }, (StatementKind::EndRegion)(a), (StatementKind::AscribeUserType)(a, v, b), (StatementKind::Nop), diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index caa627441ceb6..d60fff18b9482 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -371,16 +371,12 @@ macro_rules! make_mir_visitor { ); } StatementKind::EndRegion(_) => {} - StatementKind::Validate(_, ref $($mutability)* places) => { - for operand in places { - self.visit_place( - & $($mutability)* operand.place, - PlaceContext::NonUse(NonUseContext::Validate), - location - ); - self.visit_ty(& $($mutability)* operand.ty, - TyContext::Location(location)); - } + StatementKind::Retag { fn_entry: _, ref $($mutability)* place } => { + self.visit_place( + place, + PlaceContext::MutatingUse(MutatingUseContext::Retag), + location, + ); } StatementKind::SetDiscriminant{ ref $($mutability)* place, .. } => { self.visit_place( @@ -1010,6 +1006,8 @@ pub enum MutatingUseContext<'tcx> { /// f(&mut x.y); /// Projection, + /// Retagging (updating the "Stacked Borrows" tag) + Retag, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -1020,8 +1018,6 @@ pub enum NonUseContext { StorageDead, /// User type annotation assertions for NLL. AscribeUserTy, - /// Validation command. - Validate, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 569e7a24d2353..7620077758401 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1282,9 +1282,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "in addition to `.mir` files, create graphviz `.dot` files"), dump_mir_exclude_pass_number: bool = (false, parse_bool, [UNTRACKED], "if set, exclude the pass number when dumping MIR (used in tests)"), - mir_emit_validate: usize = (0, parse_uint, [TRACKED], - "emit Validate MIR statements, interpreted e.g. by miri (0: do not emit; 1: if function \ - contains unsafe block, only validate arguments; 2: always emit full validation)"), + mir_emit_retag: bool = (false, parse_bool, [TRACKED], + "emit Retagging MIR statements, interpreted e.g. by miri; implies -Zmir-opt-level=0"), perf_stats: bool = (false, parse_bool, [UNTRACKED], "print some performance-related statistics"), hir_stats: bool = (false, parse_bool, [UNTRACKED], diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index d4b47db608163..409665e477745 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1547,11 +1547,9 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { } /// Should we emit EndRegion MIR statements? These are consumed by - /// MIR borrowck, but not when NLL is used. They are also consumed - /// by the validation stuff. + /// MIR borrowck, but not when NLL is used. pub fn emit_end_regions(self) -> bool { self.sess.opts.debugging_opts.emit_end_regions || - self.sess.opts.debugging_opts.mir_emit_validate > 0 || self.use_mir_borrowck() } diff --git a/src/librustc_codegen_llvm/mir/analyze.rs b/src/librustc_codegen_llvm/mir/analyze.rs index a63cbe70df611..a93c6faaf7ba9 100644 --- a/src/librustc_codegen_llvm/mir/analyze.rs +++ b/src/librustc_codegen_llvm/mir/analyze.rs @@ -219,7 +219,8 @@ impl Visitor<'tcx> for LocalAnalyzer<'mir, 'a, 'll, 'tcx> { self.assign(local, location); } - PlaceContext::NonUse(_) => {} + PlaceContext::NonUse(_) | + PlaceContext::MutatingUse(MutatingUseContext::Retag) => {} PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) | PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) => { diff --git a/src/librustc_codegen_llvm/mir/statement.rs b/src/librustc_codegen_llvm/mir/statement.rs index 93be0074f6e9b..cc4e64e07881d 100644 --- a/src/librustc_codegen_llvm/mir/statement.rs +++ b/src/librustc_codegen_llvm/mir/statement.rs @@ -109,7 +109,7 @@ impl FunctionCx<'a, 'll, 'tcx> { } mir::StatementKind::FakeRead(..) | mir::StatementKind::EndRegion(_) | - mir::StatementKind::Validate(..) | + mir::StatementKind::Retag { .. } | mir::StatementKind::AscribeUserType(..) | mir::StatementKind::Nop => bx, } diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 6ef8b15545872..25e8b4ba7220f 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -574,9 +574,9 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx } StatementKind::Nop | StatementKind::AscribeUserType(..) - | StatementKind::Validate(..) + | StatementKind::Retag { .. } | StatementKind::StorageLive(..) => { - // `Nop`, `AscribeUserType`, `Validate`, and `StorageLive` are irrelevant + // `Nop`, `AscribeUserType`, `Retag`, and `StorageLive` are irrelevant // to borrow check. } StatementKind::StorageDead(local) => { diff --git a/src/librustc_mir/borrow_check/nll/invalidation.rs b/src/librustc_mir/borrow_check/nll/invalidation.rs index 002f35880ae6b..0d0f5a60ed414 100644 --- a/src/librustc_mir/borrow_check/nll/invalidation.rs +++ b/src/librustc_mir/borrow_check/nll/invalidation.rs @@ -136,9 +136,9 @@ impl<'cx, 'tcx, 'gcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx, 'gcx> { StatementKind::EndRegion(..) | StatementKind::Nop | StatementKind::AscribeUserType(..) | - StatementKind::Validate(..) | + StatementKind::Retag { .. } | StatementKind::StorageLive(..) => { - // `Nop`, `AscribeUserType`, `Validate`, and `StorageLive` are irrelevant + // `Nop`, `AscribeUserType`, `Retag`, and `StorageLive` are irrelevant // to borrow check. } StatementKind::StorageDead(local) => { diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index 3098acffa23dc..c4eac2c9341dc 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -1264,7 +1264,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { | StatementKind::StorageDead(_) | StatementKind::InlineAsm { .. } | StatementKind::EndRegion(_) - | StatementKind::Validate(..) + | StatementKind::Retag { .. } | StatementKind::Nop => {} } } diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index cfccb950e8276..69d2a89b5f237 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -338,7 +338,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> { mir::StatementKind::FakeRead(..) | mir::StatementKind::SetDiscriminant { .. } | mir::StatementKind::StorageLive(..) | - mir::StatementKind::Validate(..) | + mir::StatementKind::Retag { .. } | mir::StatementKind::AscribeUserType(..) | mir::StatementKind::Nop => {} diff --git a/src/librustc_mir/dataflow/move_paths/builder.rs b/src/librustc_mir/dataflow/move_paths/builder.rs index 08696dc098e00..968e39cc3d156 100644 --- a/src/librustc_mir/dataflow/move_paths/builder.rs +++ b/src/librustc_mir/dataflow/move_paths/builder.rs @@ -302,7 +302,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> { "SetDiscriminant should not exist during borrowck"); } StatementKind::EndRegion(_) | - StatementKind::Validate(..) | + StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Nop => {} } diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 7811dcb0663d5..5a5002dece5ab 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -242,10 +242,10 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized { /// Execute a validation operation #[inline] - fn validation_op( + fn retag( _ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, - _op: ::rustc::mir::ValidationOp, - _operand: &::rustc::mir::ValidationOperand<'tcx, ::rustc::mir::Place<'tcx>>, + _fn_entry: bool, + _place: PlaceTy<'tcx, Self::PointerTag>, ) -> EvalResult<'tcx> { Ok(()) } diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 1bab536e3e0f0..80b9948f612e4 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -118,11 +118,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // interpreter is solely intended for borrowck'ed code. FakeRead(..) => {} - // Validity checks. - Validate(op, ref places) => { - for operand in places { - M::validation_op(self, op, operand)?; - } + // Retagging. + Retag { fn_entry, ref place } => { + let dest = self.eval_place(place)?; + M::retag(self, fn_entry, dest)?; } EndRegion(..) => {} diff --git a/src/librustc_mir/transform/add_retag.rs b/src/librustc_mir/transform/add_retag.rs new file mode 100644 index 0000000000000..0f16e29aae904 --- /dev/null +++ b/src/librustc_mir/transform/add_retag.rs @@ -0,0 +1,169 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! This pass adds validation calls (AcquireValid, ReleaseValid) where appropriate. +//! It has to be run really early, before transformations like inlining, because +//! introducing these calls *adds* UB -- so, conceptually, this pass is actually part +//! of MIR building, and only after this pass we think of the program has having the +//! normal MIR semantics. + +use rustc::ty::{self, Ty, TyCtxt}; +use rustc::mir::*; +use transform::{MirPass, MirSource}; + +pub struct AddRetag; + +/// Determines whether this place is local: If it is part of a local variable. +/// We do not consider writes to pointers local, only writes that immediately assign +/// to a local variable. +/// One important property here is that evaluating the place immediately after +/// the assignment must produce the same place as what was used during the assignment. +fn is_local<'tcx>( + place: &Place<'tcx>, +) -> bool { + use rustc::mir::Place::*; + + match *place { + Local { .. } => true, + Promoted(_) | + Static(_) => false, + Projection(ref proj) => { + match proj.elem { + ProjectionElem::Deref | + ProjectionElem::Index(_) => + // Which place these point to depends on external circumstances + // (a local storing the array index, the current value of + // the projection base), so we stop tracking here. + false, + _ => is_local(&proj.base), + } + } + } +} + +/// Determine whether this type has a reference in it, recursing below compound types but +/// not below references. +fn has_reference<'a, 'gcx, 'tcx>(ty: Ty<'tcx>, tcx: TyCtxt<'a, 'gcx, 'tcx>) -> bool { + match ty.sty { + // Primitive types that are not references + ty::Bool | ty::Char | + ty::Float(_) | ty::Int(_) | ty::Uint(_) | + ty::RawPtr(..) | ty::FnPtr(..) | + ty::Str | ty::FnDef(..) | ty::Never => + false, + // References + ty::Ref(..) => true, + ty::Adt(..) if ty.is_box() => true, + // Compound types + ty::Array(ty, ..) | ty::Slice(ty) => + has_reference(ty, tcx), + ty::Tuple(tys) => + tys.iter().any(|ty| has_reference(ty, tcx)), + ty::Adt(adt, substs) => + adt.variants.iter().any(|v| v.fields.iter().any(|f| + has_reference(f.ty(tcx, substs), tcx) + )), + // Conservative fallback + _ => true, + } +} + +impl MirPass for AddRetag { + fn run_pass<'a, 'tcx>(&self, + tcx: TyCtxt<'a, 'tcx, 'tcx>, + _src: MirSource, + mir: &mut Mir<'tcx>) + { + if !tcx.sess.opts.debugging_opts.mir_emit_retag { + return; + } + let (span, arg_count) = (mir.span, mir.arg_count); + let (basic_blocks, local_decls) = mir.basic_blocks_and_local_decls_mut(); + let needs_retag = |place: &Place<'tcx>| { + is_local(place) && has_reference(place.ty(&*local_decls, tcx).to_ty(tcx), tcx) + }; + + // PART 1 + // Retag arguments at the beginning of the start block. + { + let source_info = SourceInfo { + scope: OUTERMOST_SOURCE_SCOPE, + span: span, // FIXME: Consider using just the span covering the function + // argument declaration. + }; + // Gather all arguments, skip return value. + let places = local_decls.iter_enumerated().skip(1).take(arg_count) + .map(|(local, _)| Place::Local(local)) + .filter(needs_retag) + .collect::>(); + // Emit their retags. + basic_blocks[START_BLOCK].statements.splice(0..0, + places.into_iter().map(|place| Statement { + source_info, + kind: StatementKind::Retag { fn_entry: true, place }, + }) + ); + } + + // PART 2 + // Retag return values of functions. + // We collect the return destinations because we cannot mutate while iterating. + let mut returns: Vec<(SourceInfo, Place<'tcx>, BasicBlock)> = Vec::new(); + for block_data in basic_blocks.iter_mut() { + match block_data.terminator { + Some(Terminator { kind: TerminatorKind::Call { ref destination, .. }, + source_info }) => { + // Remember the return destination for later + if let &Some(ref destination) = destination { + if needs_retag(&destination.0) { + returns.push((source_info, destination.0.clone(), destination.1)); + } + } + } + _ => { + // Not a block ending in a Call -> ignore. + // `Drop` is also a call, but it doesn't return anything so we are good. + } + } + } + // Now we go over the returns we collected to retag the return values. + for (source_info, dest_place, dest_block) in returns { + basic_blocks[dest_block].statements.insert(0, Statement { + source_info, + kind: StatementKind::Retag { fn_entry: false, place: dest_place }, + }); + } + + // PART 3 + // Add retag after assignment. + for block_data in basic_blocks { + // We want to insert statements as we iterate. To this end, we + // iterate backwards using indices. + for i in (0..block_data.statements.len()).rev() { + match block_data.statements[i].kind { + // Assignments can make values obtained elsewhere "local". + // We could try to be smart here and e.g. only retag if the assignment + // loaded from memory, but that seems risky: We might miss a subtle corner + // case. + StatementKind::Assign(ref place, box Rvalue::Use(..)) + if needs_retag(place) => { + // Insert a retag after the assignment. + let source_info = block_data.statements[i].source_info; + block_data.statements.insert(i+1,Statement { + source_info, + kind: StatementKind::Retag { fn_entry: false, place: place.clone() }, + }); + } + _ => {}, + } + } + } + } +} diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs deleted file mode 100644 index 5b489b5db942b..0000000000000 --- a/src/librustc_mir/transform/add_validation.rs +++ /dev/null @@ -1,395 +0,0 @@ -// Copyright 2015 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -//! This pass adds validation calls (AcquireValid, ReleaseValid) where appropriate. -//! It has to be run really early, before transformations like inlining, because -//! introducing these calls *adds* UB -- so, conceptually, this pass is actually part -//! of MIR building, and only after this pass we think of the program has having the -//! normal MIR semantics. - -use rustc::ty::{self, TyCtxt, RegionKind}; -use rustc::hir; -use rustc::mir::*; -use rustc::middle::region; -use transform::{MirPass, MirSource}; - -pub struct AddValidation; - -/// Determine the "context" of the place: Mutability and region. -fn place_context<'a, 'tcx, D>( - place: &Place<'tcx>, - local_decls: &D, - tcx: TyCtxt<'a, 'tcx, 'tcx> -) -> (Option, hir::Mutability) - where D: HasLocalDecls<'tcx> -{ - use rustc::mir::Place::*; - - match *place { - Local { .. } => (None, hir::MutMutable), - Promoted(_) | - Static(_) => (None, hir::MutImmutable), - Projection(ref proj) => { - match proj.elem { - ProjectionElem::Deref => { - // Computing the inside the recursion makes this quadratic. - // We don't expect deep paths though. - let ty = proj.base.ty(local_decls, tcx).to_ty(tcx); - // A Deref projection may restrict the context, this depends on the type - // being deref'd. - let context = match ty.sty { - ty::Ref(re, _, mutbl) => { - let re = match re { - &RegionKind::ReScope(ce) => Some(ce), - &RegionKind::ReErased => - bug!("AddValidation pass must be run before erasing lifetimes"), - _ => None - }; - (re, mutbl) - } - ty::RawPtr(_) => - // There is no guarantee behind even a mutable raw pointer, - // no write locks are acquired there, so we also don't want to - // release any. - (None, hir::MutImmutable), - ty::Adt(adt, _) if adt.is_box() => (None, hir::MutMutable), - _ => bug!("Deref on a non-pointer type {:?}", ty), - }; - // "Intersect" this restriction with proj.base. - if let (Some(_), hir::MutImmutable) = context { - // This is already as restricted as it gets, no need to even recurse - context - } else { - let base_context = place_context(&proj.base, local_decls, tcx); - // The region of the outermost Deref is always most restrictive. - let re = context.0.or(base_context.0); - let mutbl = context.1.and(base_context.1); - (re, mutbl) - } - - } - _ => place_context(&proj.base, local_decls, tcx), - } - } - } -} - -/// Check if this function contains an unsafe block or is an unsafe function. -fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) -> bool { - use rustc::hir::intravisit::{self, Visitor, FnKind}; - use rustc::hir::map::blocks::FnLikeNode; - use rustc::hir::Node; - - /// Decide if this is an unsafe block - fn block_is_unsafe(block: &hir::Block) -> bool { - use rustc::hir::BlockCheckMode::*; - - match block.rules { - UnsafeBlock(_) | PushUnsafeBlock(_) => true, - // For PopUnsafeBlock, we don't actually know -- but we will always also check all - // parent blocks, so we can safely declare the PopUnsafeBlock to not be unsafe. - DefaultBlock | PopUnsafeBlock(_) => false, - } - } - - /// Decide if this FnLike is a closure - fn fn_is_closure<'a>(fn_like: FnLikeNode<'a>) -> bool { - match fn_like.kind() { - FnKind::Closure(_) => true, - FnKind::Method(..) | FnKind::ItemFn(..) => false, - } - } - - let node_id = tcx.hir.as_local_node_id(src.def_id).unwrap(); - let fn_like = match tcx.hir.body_owner_kind(node_id) { - hir::BodyOwnerKind::Fn => { - match FnLikeNode::from_node(tcx.hir.get(node_id)) { - Some(fn_like) => fn_like, - None => return false, // e.g. struct ctor shims -- such auto-generated code cannot - // contain unsafe. - } - }, - _ => return false, // only functions can have unsafe - }; - - // Test if the function is marked unsafe. - if fn_like.unsafety() == hir::Unsafety::Unsafe { - return true; - } - - // For closures, we need to walk up the parents and see if we are inside an unsafe fn or - // unsafe block. - if fn_is_closure(fn_like) { - let mut cur = fn_like.id(); - loop { - // Go further upwards. - cur = tcx.hir.get_parent_node(cur); - let node = tcx.hir.get(cur); - // Check if this is an unsafe function - if let Some(fn_like) = FnLikeNode::from_node(node) { - if !fn_is_closure(fn_like) { - if fn_like.unsafety() == hir::Unsafety::Unsafe { - return true; - } - } - } - // Check if this is an unsafe block, or an item - match node { - Node::Expr(&hir::Expr { node: hir::ExprKind::Block(ref block, _), ..}) => { - if block_is_unsafe(&*block) { - // Found an unsafe block, we can bail out here. - return true; - } - } - Node::Item(..) => { - // No walking up beyond items. This makes sure the loop always terminates. - break; - } - _ => {}, - } - } - } - - // Visit the entire body of the function and check for unsafe blocks in there - struct FindUnsafe { - found_unsafe: bool, - } - let mut finder = FindUnsafe { found_unsafe: false }; - // Run the visitor on the NodeId we got. Seems like there is no uniform way to do that. - finder.visit_body(tcx.hir.body(fn_like.body())); - - impl<'tcx> Visitor<'tcx> for FindUnsafe { - fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'tcx> { - intravisit::NestedVisitorMap::None - } - - fn visit_block(&mut self, b: &'tcx hir::Block) { - if self.found_unsafe { return; } // short-circuit - - if block_is_unsafe(b) { - // We found an unsafe block. We can stop searching. - self.found_unsafe = true; - } else { - // No unsafe block here, go on searching. - intravisit::walk_block(self, b); - } - } - } - - finder.found_unsafe -} - -impl MirPass for AddValidation { - fn run_pass<'a, 'tcx>(&self, - tcx: TyCtxt<'a, 'tcx, 'tcx>, - src: MirSource, - mir: &mut Mir<'tcx>) - { - let emit_validate = tcx.sess.opts.debugging_opts.mir_emit_validate; - if emit_validate == 0 { - return; - } - let restricted_validation = emit_validate == 1 && fn_contains_unsafe(tcx, src); - let (span, arg_count) = (mir.span, mir.arg_count); - let (basic_blocks, local_decls) = mir.basic_blocks_and_local_decls_mut(); - - // Convert a place to a validation operand. - let place_to_operand = |place: Place<'tcx>| -> ValidationOperand<'tcx, Place<'tcx>> { - let (re, mutbl) = place_context(&place, local_decls, tcx); - let ty = place.ty(local_decls, tcx).to_ty(tcx); - ValidationOperand { place, ty, re, mutbl } - }; - - // Emit an Acquire at the beginning of the given block. If we are in restricted emission - // mode (mir_emit_validate=1), also emit a Release immediately after the Acquire. - let emit_acquire = |block: &mut BasicBlockData<'tcx>, source_info, operands: Vec<_>| { - if operands.len() == 0 { - return; // Nothing to do - } - // Emit the release first, to avoid cloning if we do not emit it - if restricted_validation { - let release_stmt = Statement { - source_info, - kind: StatementKind::Validate(ValidationOp::Release, operands.clone()), - }; - block.statements.insert(0, release_stmt); - } - // Now, the acquire - let acquire_stmt = Statement { - source_info, - kind: StatementKind::Validate(ValidationOp::Acquire, operands), - }; - block.statements.insert(0, acquire_stmt); - }; - - // PART 1 - // Add an AcquireValid at the beginning of the start block. - { - let source_info = SourceInfo { - scope: OUTERMOST_SOURCE_SCOPE, - span: span, // FIXME: Consider using just the span covering the function - // argument declaration. - }; - // Gather all arguments, skip return value. - let operands = local_decls.iter_enumerated().skip(1).take(arg_count) - .map(|(local, _)| place_to_operand(Place::Local(local))).collect(); - emit_acquire(&mut basic_blocks[START_BLOCK], source_info, operands); - } - - // PART 2 - // Add ReleaseValid/AcquireValid around function call terminators. We don't use a visitor - // because we need to access the block that a Call jumps to. - let mut returns : Vec<(SourceInfo, Place<'tcx>, BasicBlock)> = Vec::new(); - for block_data in basic_blocks.iter_mut() { - match block_data.terminator { - Some(Terminator { kind: TerminatorKind::Call { ref args, ref destination, .. }, - source_info }) => { - // Before the call: Release all arguments *and* the return value. - // The callee may write into the return value! Note that this relies - // on "release of uninitialized" to be a NOP. - if !restricted_validation { - let release_stmt = Statement { - source_info, - kind: StatementKind::Validate(ValidationOp::Release, - destination.iter().map(|dest| place_to_operand(dest.0.clone())) - .chain( - args.iter().filter_map(|op| { - match op { - &Operand::Copy(ref place) | - &Operand::Move(ref place) => - Some(place_to_operand(place.clone())), - &Operand::Constant(..) => { None }, - } - }) - ).collect()) - }; - block_data.statements.push(release_stmt); - } - // Remember the return destination for later - if let &Some(ref destination) = destination { - returns.push((source_info, destination.0.clone(), destination.1)); - } - } - Some(Terminator { kind: TerminatorKind::Drop { location: ref place, .. }, - source_info }) | - Some(Terminator { kind: TerminatorKind::DropAndReplace { location: ref place, .. }, - source_info }) => { - // Before the call: Release all arguments - if !restricted_validation { - let release_stmt = Statement { - source_info, - kind: StatementKind::Validate(ValidationOp::Release, - vec![place_to_operand(place.clone())]), - }; - block_data.statements.push(release_stmt); - } - // drop doesn't return anything, so we need no acquire. - } - _ => { - // Not a block ending in a Call -> ignore. - } - } - } - // Now we go over the returns we collected to acquire the return values. - for (source_info, dest_place, dest_block) in returns { - emit_acquire( - &mut basic_blocks[dest_block], - source_info, - vec![place_to_operand(dest_place)] - ); - } - - if restricted_validation { - // No part 3 for us. - return; - } - - // PART 3 - // Add ReleaseValid/AcquireValid around Ref and Cast. Again an iterator does not seem very - // suited as we need to add new statements before and after each Ref. - for block_data in basic_blocks { - // We want to insert statements around Ref commands as we iterate. To this end, we - // iterate backwards using indices. - for i in (0..block_data.statements.len()).rev() { - match block_data.statements[i].kind { - // When the borrow of this ref expires, we need to recover validation. - StatementKind::Assign(_, box Rvalue::Ref(_, _, _)) => { - // Due to a lack of NLL; we can't capture anything directly here. - // Instead, we have to re-match and clone there. - let (dest_place, re, src_place) = match block_data.statements[i].kind { - StatementKind::Assign(ref dest_place, - box Rvalue::Ref(re, _, ref src_place)) => { - (dest_place.clone(), re, src_place.clone()) - }, - _ => bug!("We already matched this."), - }; - // So this is a ref, and we got all the data we wanted. - // Do an acquire of the result -- but only what it points to, so add a Deref - // projection. - let acquire_stmt = Statement { - source_info: block_data.statements[i].source_info, - kind: StatementKind::Validate(ValidationOp::Acquire, - vec![place_to_operand(dest_place.deref())]), - }; - block_data.statements.insert(i+1, acquire_stmt); - - // The source is released until the region of the borrow ends. - let op = match re { - &RegionKind::ReScope(ce) => ValidationOp::Suspend(ce), - &RegionKind::ReErased => - bug!("AddValidation pass must be run before erasing lifetimes"), - _ => ValidationOp::Release, - }; - let release_stmt = Statement { - source_info: block_data.statements[i].source_info, - kind: StatementKind::Validate(op, vec![place_to_operand(src_place)]), - }; - block_data.statements.insert(i, release_stmt); - } - // Casts can change what validation does (e.g. unsizing) - StatementKind::Assign(_, box Rvalue::Cast(kind, Operand::Copy(_), _)) | - StatementKind::Assign(_, box Rvalue::Cast(kind, Operand::Move(_), _)) - if kind != CastKind::Misc => - { - // Due to a lack of NLL; we can't capture anything directly here. - // Instead, we have to re-match and clone there. - let (dest_place, src_place) = match block_data.statements[i].kind { - StatementKind::Assign(ref dest_place, - box Rvalue::Cast(_, Operand::Copy(ref src_place), _)) | - StatementKind::Assign(ref dest_place, - box Rvalue::Cast(_, Operand::Move(ref src_place), _)) => - { - (dest_place.clone(), src_place.clone()) - }, - _ => bug!("We already matched this."), - }; - - // Acquire of the result - let acquire_stmt = Statement { - source_info: block_data.statements[i].source_info, - kind: StatementKind::Validate(ValidationOp::Acquire, - vec![place_to_operand(dest_place)]), - }; - block_data.statements.insert(i+1, acquire_stmt); - - // Release of the input - let release_stmt = Statement { - source_info: block_data.statements[i].source_info, - kind: StatementKind::Validate(ValidationOp::Release, - vec![place_to_operand(src_place)]), - }; - block_data.statements.insert(i, release_stmt); - } - _ => {}, - } - } - } - } -} diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index edd15c39fed3e..c28bb0ca35704 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -113,7 +113,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { StatementKind::StorageLive(..) | StatementKind::StorageDead(..) | StatementKind::EndRegion(..) | - StatementKind::Validate(..) | + StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Nop => { // safe (at least as emitted during MIR construction) diff --git a/src/librustc_mir/transform/erase_regions.rs b/src/librustc_mir/transform/erase_regions.rs index c697391d86776..6351a6b40cb03 100644 --- a/src/librustc_mir/transform/erase_regions.rs +++ b/src/librustc_mir/transform/erase_regions.rs @@ -22,23 +22,19 @@ use transform::{MirPass, MirSource}; struct EraseRegionsVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, - in_validation_statement: bool, } impl<'a, 'tcx> EraseRegionsVisitor<'a, 'tcx> { pub fn new(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Self { EraseRegionsVisitor { tcx, - in_validation_statement: false, } } } impl<'a, 'tcx> MutVisitor<'tcx> for EraseRegionsVisitor<'a, 'tcx> { fn visit_ty(&mut self, ty: &mut Ty<'tcx>, _: TyContext) { - if !self.in_validation_statement { - *ty = self.tcx.erase_regions(ty); - } + *ty = self.tcx.erase_regions(ty); self.super_ty(ty); } @@ -58,20 +54,11 @@ impl<'a, 'tcx> MutVisitor<'tcx> for EraseRegionsVisitor<'a, 'tcx> { block: BasicBlock, statement: &mut Statement<'tcx>, location: Location) { - // Do NOT delete EndRegion if validation statements are emitted. - // Validation needs EndRegion. - if self.tcx.sess.opts.debugging_opts.mir_emit_validate == 0 { - if let StatementKind::EndRegion(_) = statement.kind { - statement.kind = StatementKind::Nop; - } + if let StatementKind::EndRegion(_) = statement.kind { + statement.kind = StatementKind::Nop; } - self.in_validation_statement = match statement.kind { - StatementKind::Validate(..) => true, - _ => false, - }; self.super_statement(block, statement, location); - self.in_validation_statement = false; } } diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 46c73c27fe10d..92cfcb3fd56cb 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -23,7 +23,7 @@ use std::borrow::Cow; use syntax::ast; use syntax_pos::Span; -pub mod add_validation; +pub mod add_retag; pub mod add_moves_for_packed_drops; pub mod cleanup_post_borrowck; pub mod check_unsafety; @@ -258,19 +258,21 @@ fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx // Remove all `FakeRead` statements and the borrows that are only // used for checking matches &cleanup_post_borrowck::CleanFakeReadsAndBorrows, + &simplify::SimplifyCfg::new("early-opt"), // These next passes must be executed together &add_call_guards::CriticalCallEdges, &elaborate_drops::ElaborateDrops, &no_landing_pads::NoLandingPads, - // AddValidation needs to run after ElaborateDrops and before EraseRegions, and it needs - // an AllCallEdges pass right before it. - &add_call_guards::AllCallEdges, - &add_validation::AddValidation, // AddMovesForPackedDrops needs to run after drop // elaboration. &add_moves_for_packed_drops::AddMovesForPackedDrops, + // AddRetag needs to run after ElaborateDrops, and it needs + // an AllCallEdges pass right before it. Otherwise it should + // run fairly late, but before optimizations begin. + &add_call_guards::AllCallEdges, + &add_retag::AddRetag, &simplify::SimplifyCfg::new("elaborate-drops"), diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index a232176eacc8f..ca9c4eb9b8bb9 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -1167,7 +1167,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> { StatementKind::StorageDead(_) | StatementKind::InlineAsm {..} | StatementKind::EndRegion(_) | - StatementKind::Validate(..) | + StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Nop => {} } diff --git a/src/librustc_mir/transform/qualify_min_const_fn.rs b/src/librustc_mir/transform/qualify_min_const_fn.rs index 6ab68789c027b..1e19348595057 100644 --- a/src/librustc_mir/transform/qualify_min_const_fn.rs +++ b/src/librustc_mir/transform/qualify_min_const_fn.rs @@ -241,7 +241,7 @@ fn check_statement( // These are all NOPs | StatementKind::StorageLive(_) | StatementKind::StorageDead(_) - | StatementKind::Validate(..) + | StatementKind::Retag { .. } | StatementKind::EndRegion(_) | StatementKind::AscribeUserType(..) | StatementKind::Nop => Ok(()), diff --git a/src/librustc_mir/transform/remove_noop_landing_pads.rs b/src/librustc_mir/transform/remove_noop_landing_pads.rs index 4b4b284b02cd5..c1c127fa8d648 100644 --- a/src/librustc_mir/transform/remove_noop_landing_pads.rs +++ b/src/librustc_mir/transform/remove_noop_landing_pads.rs @@ -68,7 +68,7 @@ impl RemoveNoopLandingPads { StatementKind::Assign(_, _) | StatementKind::SetDiscriminant { .. } | StatementKind::InlineAsm { .. } | - StatementKind::Validate { .. } => { + StatementKind::Retag { .. } => { return false; } } diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index 05044574e5ca3..a5a19f04b7e8e 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -162,7 +162,7 @@ fn each_block<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>, mir::StatementKind::StorageDead(_) | mir::StatementKind::InlineAsm { .. } | mir::StatementKind::EndRegion(_) | - mir::StatementKind::Validate(..) | + mir::StatementKind::Retag { .. } | mir::StatementKind::AscribeUserType(..) | mir::StatementKind::Nop => continue, mir::StatementKind::SetDiscriminant{ .. } => diff --git a/src/librustc_mir/util/liveness.rs b/src/librustc_mir/util/liveness.rs index d16094e8238de..12c13b8f81531 100644 --- a/src/librustc_mir/util/liveness.rs +++ b/src/librustc_mir/util/liveness.rs @@ -204,7 +204,7 @@ pub fn categorize<'tcx>(context: PlaceContext<'tcx>) -> Option { PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) | PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) | PlaceContext::NonUse(NonUseContext::AscribeUserTy) | - PlaceContext::NonUse(NonUseContext::Validate) => + PlaceContext::MutatingUse(MutatingUseContext::Retag) => Some(DefUse::Use), /////////////////////////////////////////////////////////////////////////// diff --git a/src/librustc_passes/mir_stats.rs b/src/librustc_passes/mir_stats.rs index 06c8545aacfd8..ecfe7d13782de 100644 --- a/src/librustc_passes/mir_stats.rs +++ b/src/librustc_passes/mir_stats.rs @@ -84,7 +84,7 @@ impl<'a, 'tcx> mir_visit::Visitor<'tcx> for StatCollector<'a, 'tcx> { StatementKind::Assign(..) => "StatementKind::Assign", StatementKind::FakeRead(..) => "StatementKind::FakeRead", StatementKind::EndRegion(..) => "StatementKind::EndRegion", - StatementKind::Validate(..) => "StatementKind::Validate", + StatementKind::Retag { .. } => "StatementKind::Retag", StatementKind::SetDiscriminant { .. } => "StatementKind::SetDiscriminant", StatementKind::StorageLive(..) => "StatementKind::StorageLive", StatementKind::StorageDead(..) => "StatementKind::StorageDead", From 96ba4af258aa60a64a9e08b12a48ddab070e3efc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 24 Oct 2018 13:38:09 +0200 Subject: [PATCH 2/9] Remove validation test cases; add retagging test case --- src/librustc/mir/mod.rs | 2 +- src/test/mir-opt/retag.rs | 106 +++++++++++++++++++++++++++++++++ src/test/mir-opt/validate_1.rs | 76 ----------------------- src/test/mir-opt/validate_2.rs | 37 ------------ src/test/mir-opt/validate_3.rs | 77 ------------------------ src/test/mir-opt/validate_4.rs | 90 ---------------------------- src/test/mir-opt/validate_5.rs | 69 --------------------- 7 files changed, 107 insertions(+), 350 deletions(-) create mode 100644 src/test/mir-opt/retag.rs delete mode 100644 src/test/mir-opt/validate_1.rs delete mode 100644 src/test/mir-opt/validate_2.rs delete mode 100644 src/test/mir-opt/validate_3.rs delete mode 100644 src/test/mir-opt/validate_4.rs delete mode 100644 src/test/mir-opt/validate_5.rs diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 4f6a8158933af..d299f0e3b1229 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1822,7 +1822,7 @@ impl<'tcx> Debug for Statement<'tcx> { // (reuse lifetime rendering policy from ppaux.) EndRegion(ref ce) => write!(fmt, "EndRegion({})", ty::ReScope(*ce)), Retag { fn_entry, ref place } => - write!(fmt, "Retag({}{:?})", if fn_entry { "[fn entry]: " } else { "" }, place), + write!(fmt, "Retag({}{:?})", if fn_entry { "[fn entry] " } else { "" }, place), StorageLive(ref place) => write!(fmt, "StorageLive({:?})", place), StorageDead(ref place) => write!(fmt, "StorageDead({:?})", place), SetDiscriminant { diff --git a/src/test/mir-opt/retag.rs b/src/test/mir-opt/retag.rs new file mode 100644 index 0000000000000..9c013008ab272 --- /dev/null +++ b/src/test/mir-opt/retag.rs @@ -0,0 +1,106 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// ignore-tidy-linelength +// compile-flags: -Z mir-emit-retag -Z mir-opt-level=0 -Z span_free_formats + +#![allow(unused)] + +struct Test(i32); + +impl Test { + // Make sure we run the pass on a method, not just on bare functions. + fn foo<'x>(&self, x: &'x mut i32) -> &'x mut i32 { x } + fn foo_shr<'x>(&self, x: &'x i32) -> &'x i32 { x } +} + +fn main() { + let mut x = 0; + { + let v = Test(0).foo(&mut x); // just making sure we do not panic when there is a tuple struct ctor + let w = { v }; // assignment + let _w = w; // reborrow + } + + // Also test closures + let c: fn(&i32) -> &i32 = |x: &i32| -> &i32 { let _y = x; x }; + let _w = c(&x); + + // need to call `foo_shr` or it doesn't even get generated + Test(0).foo_shr(&0); +} + +// END RUST SOURCE +// START rustc.{{impl}}-foo.EraseRegions.after.mir +// bb0: { +// Retag([fn entry] _1); +// Retag([fn entry] _2); +// ... +// _0 = &mut (*_3); +// ... +// return; +// } +// END rustc.{{impl}}-foo.EraseRegions.after.mir +// START rustc.{{impl}}-foo_shr.EraseRegions.after.mir +// bb0: { +// Retag([fn entry] _1); +// Retag([fn entry] _2); +// ... +// _0 = _2; +// Retag(_0); +// ... +// return; +// } +// END rustc.{{impl}}-foo_shr.EraseRegions.after.mir +// START rustc.main.EraseRegions.after.mir +// fn main() -> () { +// ... +// bb0: { +// ... +// _3 = const Test::foo(move _4, move _6) -> bb1; +// } +// +// bb1: { +// Retag(_3); +// ... +// _9 = move _3; +// Retag(_9); +// _8 = &mut (*_9); +// StorageDead(_9); +// StorageLive(_10); +// _10 = move _8; +// Retag(_10); +// ... +// _13 = move _14(move _15) -> bb2; +// } +// +// bb2: { +// Retag(_13); +// ... +// } +// ... +// } +// END rustc.main.EraseRegions.after.mir +// START rustc.main-{{closure}}.EraseRegions.after.mir +// fn main::{{closure}}(_1: &[closure@NodeId(117)], _2: &i32) -> &i32 { +// ... +// bb0: { +// Retag([fn entry] _1); +// Retag([fn entry] _2); +// StorageLive(_3); +// _3 = _2; +// Retag(_3); +// _0 = _2; +// Retag(_0); +// StorageDead(_3); +// return; +// } +// } +// END rustc.main-{{closure}}.EraseRegions.after.mir diff --git a/src/test/mir-opt/validate_1.rs b/src/test/mir-opt/validate_1.rs deleted file mode 100644 index f1544968adb6a..0000000000000 --- a/src/test/mir-opt/validate_1.rs +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -// ignore-tidy-linelength -// compile-flags: -Z verbose -Z mir-emit-validate=1 -Z span_free_formats - -struct Test(i32); - -impl Test { - // Make sure we run the pass on a method, not just on bare functions. - fn foo(&self, _x: &mut i32) {} -} - -fn main() { - let mut x = 0; - Test(0).foo(&mut x); // just making sure we do not panic when there is a tuple struct ctor - - // Also test closures - let c = |x: &mut i32| { let y = &*x; *y }; - c(&mut x); -} - -// END RUST SOURCE -// START rustc.{{impl}}-foo.EraseRegions.after.mir -// bb0: { -// Validate(Acquire, [_1: &ReFree(DefId(0/0:5 ~ validate_1[317d]::{{impl}}[0]::foo[0]), BrAnon(0)) Test, _2: &ReFree(DefId(0/0:5 ~ validate_1[317d]::{{impl}}[0]::foo[0]), BrAnon(1)) mut i32]); -// ... -// return; -// } -// END rustc.{{impl}}-foo.EraseRegions.after.mir -// START rustc.main.EraseRegions.after.mir -// fn main() -> () { -// ... -// bb0: { -// ... -// Validate(Suspend(ReScope(Node(ItemLocalId(13)))), [_1: i32]); -// _6 = &ReErased mut _1; -// Validate(Acquire, [(*_6): i32/ReScope(Node(ItemLocalId(13)))]); -// Validate(Suspend(ReScope(Node(ItemLocalId(13)))), [(*_6): i32/ReScope(Node(ItemLocalId(13)))]); -// _5 = &ReErased mut (*_6); -// Validate(Acquire, [(*_5): i32/ReScope(Node(ItemLocalId(13)))]); -// Validate(Release, [_2: (), _3: &ReScope(Node(ItemLocalId(13))) Test, _5: &ReScope(Node(ItemLocalId(13))) mut i32]); -// _2 = const Test::foo(move _3, move _5) -> bb1; -// } -// -// bb1: { -// Validate(Acquire, [_2: ()]); -// EndRegion(ReScope(Node(ItemLocalId(13)))); -// ... -// return; -// } -// } -// END rustc.main.EraseRegions.after.mir -// START rustc.main-{{closure}}.EraseRegions.after.mir -// fn main::{{closure}}(_1: &ReErased [closure@NodeId(65)], _2: &ReErased mut i32) -> i32 { -// ... -// bb0: { -// Validate(Acquire, [_1: &ReFree(DefId(0/1:11 ~ validate_1[317d]::main[0]::{{closure}}[0]), BrEnv) [closure@NodeId(65)], _2: &ReFree(DefId(0/1:11 ~ validate_1[317d]::main[0]::{{closure}}[0]), BrAnon(0)) mut i32]); -// StorageLive(_3); -// Validate(Suspend(ReScope(Remainder { block: ItemLocalId(31), first_statement_index: 0 })), [(*_2): i32]); -// _3 = &ReErased (*_2); -// Validate(Acquire, [(*_3): i32/ReScope(Remainder { block: ItemLocalId(31), first_statement_index: 0 }) (imm)]); -// _0 = (*_3); -// EndRegion(ReScope(Remainder { block: ItemLocalId(31), first_statement_index: 0 })); -// StorageDead(_3); -// return; -// } -// } -// END rustc.main-{{closure}}.EraseRegions.after.mir diff --git a/src/test/mir-opt/validate_2.rs b/src/test/mir-opt/validate_2.rs deleted file mode 100644 index 3776a11b3ab82..0000000000000 --- a/src/test/mir-opt/validate_2.rs +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -// ignore-tidy-linelength -// ignore-wasm32-bare unwinding being disabled causes differences in output -// ignore-wasm64-bare unwinding being disabled causes differences in output -// compile-flags: -Z verbose -Z mir-emit-validate=1 - -fn main() { - let _x : Box<[i32]> = Box::new([1, 2, 3]); -} - -// END RUST SOURCE -// START rustc.main.EraseRegions.after.mir -// fn main() -> () { -// ... -// bb1: { -// Validate(Acquire, [_2: std::boxed::Box<[i32; 3]>]); -// Validate(Release, [_2: std::boxed::Box<[i32; 3]>]); -// _1 = move _2 as std::boxed::Box<[i32]> (Unsize); -// Validate(Acquire, [_1: std::boxed::Box<[i32]>]); -// StorageDead(_2); -// StorageDead(_3); -// _0 = (); -// Validate(Release, [_1: std::boxed::Box<[i32]>]); -// drop(_1) -> [return: bb2, unwind: bb3]; -// } -// ... -// } -// END rustc.main.EraseRegions.after.mir diff --git a/src/test/mir-opt/validate_3.rs b/src/test/mir-opt/validate_3.rs deleted file mode 100644 index ce840397713ad..0000000000000 --- a/src/test/mir-opt/validate_3.rs +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -// ignore-tidy-linelength -// compile-flags: -Z verbose -Z mir-emit-validate=1 - -struct Test { - x: i32 -} - -fn foo(_x: &i32) {} - -fn main() { - // These internal unsafe functions should have no effect on the code generation. - unsafe fn _unused1() {} - fn _unused2(x: *const i32) -> i32 { unsafe { *x }} - - let t = Test { x: 0 }; - let t = &t; - foo(&t.x); -} - -// END RUST SOURCE -// START rustc.main.EraseRegions.after.mir -// fn main() -> (){ -// let mut _0: (); -// scope 1 { -// scope 3 { -// } -// scope 4 { -// let _2: &ReErased Test; -// } -// } -// scope 2 { -// let _1: Test; -// } -// let mut _3: (); -// let mut _4: &ReErased i32; -// let mut _5: &ReErased i32; -// bb0: { -// StorageLive(_1); -// _1 = Test { x: const 0i32 }; -// StorageLive(_2); -// Validate(Suspend(ReScope(Remainder { block: ItemLocalId(24), first_statement_index: 3 })), [_1: Test]); -// _2 = &ReErased _1; -// Validate(Acquire, [(*_2): Test/ReScope(Remainder { block: ItemLocalId(24), first_statement_index: 3 }) (imm)]); -// StorageLive(_4); -// StorageLive(_5); -// Validate(Suspend(ReScope(Node(ItemLocalId(22)))), [((*_2).0: i32): i32/ReScope(Remainder { block: ItemLocalId(24), first_statement_index: 3 }) (imm)]); -// _5 = &ReErased ((*_2).0: i32); -// Validate(Acquire, [(*_5): i32/ReScope(Node(ItemLocalId(22))) (imm)]); -// Validate(Suspend(ReScope(Node(ItemLocalId(22)))), [(*_5): i32/ReScope(Node(ItemLocalId(22))) (imm)]); -// _4 = &ReErased (*_5); -// Validate(Acquire, [(*_4): i32/ReScope(Node(ItemLocalId(22))) (imm)]); -// Validate(Release, [_3: (), _4: &ReScope(Node(ItemLocalId(22))) i32]); -// _3 = const foo(move _4) -> bb1; -// } -// bb1: { -// Validate(Acquire, [_3: ()]); -// EndRegion(ReScope(Node(ItemLocalId(22)))); -// StorageDead(_4); -// StorageDead(_5); -// _0 = (); -// EndRegion(ReScope(Remainder { block: ItemLocalId(24), first_statement_index: 3 })); -// StorageDead(_2); -// StorageDead(_1); -// return; -// } -// } -// END rustc.main.EraseRegions.after.mir diff --git a/src/test/mir-opt/validate_4.rs b/src/test/mir-opt/validate_4.rs deleted file mode 100644 index 542ac8a42411f..0000000000000 --- a/src/test/mir-opt/validate_4.rs +++ /dev/null @@ -1,90 +0,0 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -// ignore-tidy-linelength -// compile-flags: -Z verbose -Z mir-emit-validate=1 -Z span_free_formats - -// Make sure unsafe fns and fns with an unsafe block only get restricted validation. - -unsafe fn write_42(x: *mut i32) -> bool { - let test_closure = |x: *mut i32| *x = 23; - test_closure(x); - *x = 42; - true -} - -fn test(x: &mut i32) { - unsafe { write_42(x) }; -} - -fn main() { - test(&mut 0); - - let test_closure = unsafe { |x: &mut i32| write_42(x) }; - test_closure(&mut 0); -} - -// FIXME: Also test code generated inside the closure, make sure it only does restricted validation -// because it is entirely inside an unsafe block. Unfortunately, the interesting lines of code also -// contain name of the source file, so we cannot test for it. - -// END RUST SOURCE -// START rustc.write_42.EraseRegions.after.mir -// fn write_42(_1: *mut i32) -> bool { -// ... -// bb0: { -// Validate(Acquire, [_1: *mut i32]); -// Validate(Release, [_1: *mut i32]); -// ... -// return; -// } -// } -// END rustc.write_42.EraseRegions.after.mir -// START rustc.write_42-{{closure}}.EraseRegions.after.mir -// fn write_42::{{closure}}(_1: &ReErased [closure@NodeId(32)], _2: *mut i32) -> () { -// ... -// bb0: { -// Validate(Acquire, [_1: &ReFree(DefId(0/1:9 ~ validate_4[317d]::write_42[0]::{{closure}}[0]), BrEnv) [closure@NodeId(32)], _2: *mut i32]); -// Validate(Release, [_1: &ReFree(DefId(0/1:9 ~ validate_4[317d]::write_42[0]::{{closure}}[0]), BrEnv) [closure@NodeId(32)], _2: *mut i32]); -// (*_2) = const 23i32; -// _0 = (); -// return; -// } -// } -// END rustc.write_42-{{closure}}.EraseRegions.after.mir -// START rustc.test.EraseRegions.after.mir -// fn test(_1: &ReErased mut i32) -> () { -// ... -// bb0: { -// Validate(Acquire, [_1: &ReFree(DefId(0/0:4 ~ validate_4[317d]::test[0]), BrAnon(0)) mut i32]); -// Validate(Release, [_1: &ReFree(DefId(0/0:4 ~ validate_4[317d]::test[0]), BrAnon(0)) mut i32]); -// ... -// _2 = const write_42(move _3) -> bb1; -// } -// bb1: { -// Validate(Acquire, [_2: bool]); -// Validate(Release, [_2: bool]); -// ... -// } -// } -// END rustc.test.EraseRegions.after.mir -// START rustc.main-{{closure}}.EraseRegions.after.mir -// fn main::{{closure}}(_1: &ReErased [closure@NodeId(80)], _2: &ReErased mut i32) -> bool { -// ... -// bb0: { -// Validate(Acquire, [_1: &ReFree(DefId(0/1:10 ~ validate_4[317d]::main[0]::{{closure}}[0]), BrEnv) [closure@NodeId(80)], _2: &ReFree(DefId(0/1:10 ~ validate_4[317d]::main[0]::{{closure}}[0]), BrAnon(0)) mut i32]); -// Validate(Release, [_1: &ReFree(DefId(0/1:10 ~ validate_4[317d]::main[0]::{{closure}}[0]), BrEnv) [closure@NodeId(80)], _2: &ReFree(DefId(0/1:10 ~ validate_4[317d]::main[0]::{{closure}}[0]), BrAnon(0)) mut i32]); -// StorageLive(_3); -// ... -// _0 = const write_42(move _3) -> bb1; -// } -// ... -// } -// END rustc.main-{{closure}}.EraseRegions.after.mir diff --git a/src/test/mir-opt/validate_5.rs b/src/test/mir-opt/validate_5.rs deleted file mode 100644 index 955de0c3bad04..0000000000000 --- a/src/test/mir-opt/validate_5.rs +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -// ignore-tidy-linelength -// compile-flags: -Z verbose -Z mir-emit-validate=2 -Z span_free_formats - -// Make sure unsafe fns and fns with an unsafe block still get full validation. - -unsafe fn write_42(x: *mut i32) -> bool { - *x = 42; - true -} - -fn test(x: &mut i32) { - unsafe { write_42(x) }; -} - -fn main() { - test(&mut 0); - - let test_closure = unsafe { |x: &mut i32| write_42(x) }; - // Note that validation will fail if this is executed: The closure keeps the lock on - // x, so the write in write_42 fails. This test just checks code generation, - // so the UB doesn't matter. - test_closure(&mut 0); -} - -// END RUST SOURCE -// START rustc.test.EraseRegions.after.mir -// fn test(_1: &ReErased mut i32) -> () { -// ... -// bb0: { -// Validate(Acquire, [_1: &ReFree(DefId(0/0:4 ~ validate_5[317d]::test[0]), BrAnon(0)) mut i32]); -// ... -// Validate(Release, [_2: bool, _3: *mut i32]); -// _2 = const write_42(move _3) -> bb1; -// } -// ... -// } -// END rustc.test.EraseRegions.after.mir -// START rustc.main-{{closure}}.EraseRegions.after.mir -// fn main::{{closure}}(_1: &ReErased [closure@NodeId(62)], _2: &ReErased mut i32) -> bool { -// ... -// bb0: { -// Validate(Acquire, [_1: &ReFree(DefId(0/1:9 ~ validate_5[317d]::main[0]::{{closure}}[0]), BrEnv) [closure@NodeId(62)], _2: &ReFree(DefId(0/1:9 ~ validate_5[317d]::main[0]::{{closure}}[0]), BrAnon(0)) mut i32]); -// StorageLive(_3); -// StorageLive(_4); -// StorageLive(_5); -// Validate(Suspend(ReScope(Node(ItemLocalId(16)))), [(*_2): i32]); -// _5 = &ReErased mut (*_2); -// Validate(Acquire, [(*_5): i32/ReScope(Node(ItemLocalId(16)))]); -// _4 = move _5 as *mut i32 (Misc); -// _3 = move _4; -// EndRegion(ReScope(Node(ItemLocalId(16)))); -// StorageDead(_4); -// StorageDead(_5); -// Validate(Release, [_0: bool, _3: *mut i32]); -// _0 = const write_42(move _3) -> bb1; -// } -// ... -// } -// END rustc.main-{{closure}}.EraseRegions.after.mir From 8a61d492a97d829638f60b19ac2196e7b9076720 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 24 Oct 2018 13:47:48 +0200 Subject: [PATCH 3/9] make inliner remove the fn_entry flag on Retag statements --- src/librustc/mir/visit.rs | 28 ++++++++++++----- src/librustc_mir/transform/inline.rs | 8 +++++ src/test/mir-opt/inline-retag.rs | 45 ++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 src/test/mir-opt/inline-retag.rs diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index d60fff18b9482..aa4004cf07bfc 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -152,6 +152,13 @@ macro_rules! make_mir_visitor { self.super_ascribe_user_ty(place, variance, user_ty, location); } + fn visit_retag(&mut self, + fn_entry: & $($mutability)* bool, + place: & $($mutability)* Place<'tcx>, + location: Location) { + self.super_retag(fn_entry, place, location); + } + fn visit_place(&mut self, place: & $($mutability)* Place<'tcx>, context: PlaceContext<'tcx>, @@ -371,13 +378,6 @@ macro_rules! make_mir_visitor { ); } StatementKind::EndRegion(_) => {} - StatementKind::Retag { fn_entry: _, ref $($mutability)* place } => { - self.visit_place( - place, - PlaceContext::MutatingUse(MutatingUseContext::Retag), - location, - ); - } StatementKind::SetDiscriminant{ ref $($mutability)* place, .. } => { self.visit_place( place, @@ -413,6 +413,9 @@ macro_rules! make_mir_visitor { self.visit_operand(input, location); } } + StatementKind::Retag { ref $($mutability)* fn_entry, ref $($mutability)* place } => { + self.visit_retag(fn_entry, place, location); + } StatementKind::AscribeUserType( ref $($mutability)* place, ref $($mutability)* variance, @@ -715,6 +718,17 @@ macro_rules! make_mir_visitor { self.visit_user_type_projection(user_ty); } + fn super_retag(&mut self, + _fn_entry: & $($mutability)* bool, + place: & $($mutability)* Place<'tcx>, + location: Location) { + self.visit_place( + place, + PlaceContext::MutatingUse(MutatingUseContext::Retag), + location, + ); + } + fn super_place(&mut self, place: & $($mutability)* Place<'tcx>, context: PlaceContext<'tcx>, diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index 5963f1a481c65..199cf5650fda8 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -691,6 +691,14 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Integrator<'a, 'tcx> { self.in_cleanup_block = false; } + fn visit_retag(&mut self, fn_entry: &mut bool, place: &mut Place<'tcx>, loc: Location) { + self.super_retag(fn_entry, place, loc); + + // We have to patch all inlined retags to be aware that they are no longer + // happening on function entry. + *fn_entry = false; + } + fn visit_terminator_kind(&mut self, block: BasicBlock, kind: &mut TerminatorKind<'tcx>, loc: Location) { self.super_terminator_kind(block, kind, loc); diff --git a/src/test/mir-opt/inline-retag.rs b/src/test/mir-opt/inline-retag.rs new file mode 100644 index 0000000000000..4b3280ee56151 --- /dev/null +++ b/src/test/mir-opt/inline-retag.rs @@ -0,0 +1,45 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z span_free_formats -Z mir-emit-retag + +// Tests that MIR inliner fixes up `Retag`'s `fn_entry` flag + +fn main() { + println!("{}", bar()); +} + +#[inline(always)] +fn foo(x: &i32, y: &i32) -> bool { + *x == *y +} + +fn bar() -> bool { + let f = foo; + f(&1, &-1) +} + +// END RUST SOURCE +// START rustc.bar.Inline.after.mir +// ... +// bb0: { +// ... +// Retag(_3); +// Retag(_6); +// StorageLive(_9); +// _9 = (*_3); +// StorageLive(_10); +// _10 = (*_6); +// _0 = Eq(move _9, move _10); +// ... +// return; +// } +// ... +// END rustc.bar.Inline.after.mir From 07829bc0f09a80d64487318f7e7ad44b701dac8b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 24 Oct 2018 14:08:47 +0200 Subject: [PATCH 4/9] don't forget to sync these flags with miri --- src/bootstrap/bin/rustc.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/bootstrap/bin/rustc.rs b/src/bootstrap/bin/rustc.rs index 07a05aeae5d64..344eb789ff60a 100644 --- a/src/bootstrap/bin/rustc.rs +++ b/src/bootstrap/bin/rustc.rs @@ -253,6 +253,8 @@ fn main() { // When running miri tests, we need to generate MIR for all libraries if env::var("TEST_MIRI").ok().map_or(false, |val| val == "true") { + // The flags here should be kept in sync with `add_miri_default_args` + // in miri's `src/lib.rs`. cmd.arg("-Zalways-encode-mir"); // These options are preferred by miri, to be able to perform better validation, // but the bootstrap compiler might not understand them. From c5abbd4be360cb433a84d86aa6d7cc6e63258a3b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 24 Oct 2018 17:27:16 +0200 Subject: [PATCH 5/9] all hail tidy --- src/librustc/mir/visit.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index aa4004cf07bfc..4d3135f9d41cd 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -413,7 +413,8 @@ macro_rules! make_mir_visitor { self.visit_operand(input, location); } } - StatementKind::Retag { ref $($mutability)* fn_entry, ref $($mutability)* place } => { + StatementKind::Retag { ref $($mutability)* fn_entry, + ref $($mutability)* place } => { self.visit_retag(fn_entry, place, location); } StatementKind::AscribeUserType( From f2f0f1a0a86ce658e1e037b4aece6d54ab1f7aa7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 24 Oct 2018 21:59:42 +0200 Subject: [PATCH 6/9] fix nits --- src/librustc/mir/mod.rs | 12 ++++++++---- src/librustc_mir/transform/add_retag.rs | 9 +++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index d299f0e3b1229..636fe115746fb 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1755,12 +1755,16 @@ pub enum StatementKind<'tcx> { }, /// Retag references in the given place, ensuring they got fresh tags. This is - /// part of the Stacked Borrows model. `fn_entry` indicates whether this - /// is the initial retag that happens in the function prolog. These statements are - /// currently only interpreted by miri and only generated when "-Z mir-emit-retag" is passed. + /// part of the Stacked Borrows model. These statements are currently only interpreted + /// by miri and only generated when "-Z mir-emit-retag" is passed. /// See /// for more details. - Retag { fn_entry: bool, place: Place<'tcx> }, + Retag { + /// `fn_entry` indicates whether this is the initial retag that happens in the + /// function prolog. + fn_entry: bool, + place: Place<'tcx>, + }, /// Mark one terminating point of a region scope (i.e. static region). /// (The starting point(s) arise implicitly from borrows.) diff --git a/src/librustc_mir/transform/add_retag.rs b/src/librustc_mir/transform/add_retag.rs index 0f16e29aae904..a50011cf5a15e 100644 --- a/src/librustc_mir/transform/add_retag.rs +++ b/src/librustc_mir/transform/add_retag.rs @@ -42,7 +42,12 @@ fn is_local<'tcx>( // (a local storing the array index, the current value of // the projection base), so we stop tracking here. false, - _ => is_local(&proj.base), + ProjectionElem::Field { .. } | + ProjectionElem::ConstantIndex { .. } | + ProjectionElem::Subslice { .. } | + ProjectionElem::Downcast { .. } => + // These just offset by a constant, entirely independent of everything else. + is_local(&proj.base), } } } @@ -121,7 +126,7 @@ impl MirPass for AddRetag { Some(Terminator { kind: TerminatorKind::Call { ref destination, .. }, source_info }) => { // Remember the return destination for later - if let &Some(ref destination) = destination { + if let Some(ref destination) = destination { if needs_retag(&destination.0) { returns.push((source_info, destination.0.clone(), destination.1)); } From 3545dae6a35eb675c3cf6e312ef9a5724d721020 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 26 Oct 2018 11:12:22 +0200 Subject: [PATCH 7/9] let create_ref take a mutability, and leave it to step.rs to interpret mir::BorrowKind --- src/librustc_mir/interpret/place.rs | 10 ++-------- src/librustc_mir/interpret/step.rs | 12 ++++++++++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 0eae2bfb226c6..3b104e2284fe2 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -299,23 +299,17 @@ where /// Turn a mplace into a (thin or fat) pointer, as a reference, pointing to the same space. /// This is the inverse of `ref_to_mplace`. + /// `mutbl` indicates whether we are create a shared or mutable ref, or a raw pointer (`None`). pub fn create_ref( &mut self, place: MPlaceTy<'tcx, M::PointerTag>, - borrow_kind: Option, + mutbl: Option, ) -> EvalResult<'tcx, Value> { // Pointer tag tracking might want to adjust the tag let place = if M::ENABLE_PTR_TRACKING_HOOKS { let (size, _) = self.size_and_align_of_mplace(place)? // for extern types, just cover what we can .unwrap_or_else(|| place.layout.size_and_align()); - let mutbl = match borrow_kind { - Some(mir::BorrowKind::Mut { .. }) | - Some(mir::BorrowKind::Unique) => - Some(hir::MutMutable), - Some(_) => Some(hir::MutImmutable), - None => None, - }; M::tag_reference(self, *place, place.layout.ty, size, mutbl)? } else { *place diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 80b9948f612e4..97431cfe6808e 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -12,7 +12,7 @@ //! //! The main entry point is the `step` method. -use rustc::mir; +use rustc::{hir, mir}; use rustc::ty::layout::LayoutOf; use rustc::mir::interpret::{EvalResult, Scalar, PointerArithmetic}; @@ -250,7 +250,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Ref(_, borrow_kind, ref place) => { let src = self.eval_place(place)?; let val = self.force_allocation(src)?; - let val = self.create_ref(val, Some(borrow_kind))?; + let mutbl = match borrow_kind { + mir::BorrowKind::Mut { .. } | + mir::BorrowKind::Unique => + hir::MutMutable, + mir::BorrowKind::Shared | + mir::BorrowKind::Shallow => + hir::MutImmutable, + }; + let val = self.create_ref(val, Some(mutbl))?; self.write_value(val, dest)?; } From 2a5eae3ac78a63196c119c8ca5f8f0bd99227971 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 29 Oct 2018 16:34:54 +0100 Subject: [PATCH 8/9] provide mutable borrows when hooking memory write access --- src/librustc_mir/interpret/machine.rs | 26 ++++++++++++-------------- src/librustc_mir/interpret/memory.rs | 6 +++--- src/librustc_mir/interpret/mod.rs | 2 +- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 5a5002dece5ab..7aeadfa408a90 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -24,13 +24,6 @@ use super::{ EvalContext, PlaceTy, OpTy, Pointer, MemPlace, MemoryKind, }; -/// Classifying memory accesses -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum MemoryAccess { - Read, - Write, -} - /// Whether this kind of memory is allowed to leak pub trait MayLeak: Copy { fn may_leak(self) -> bool; @@ -181,17 +174,22 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized { dest: PlaceTy<'tcx, Self::PointerTag>, ) -> EvalResult<'tcx>; - /// Hook for performing extra checks on a memory access. - /// - /// Takes read-only access to the allocation so we can keep all the memory read - /// operations take `&self`. Use a `RefCell` in `AllocExtra` if you - /// need to mutate. + /// Hook for performing extra checks on a memory read access. #[inline] - fn memory_accessed( + fn memory_read( _alloc: &Allocation, _ptr: Pointer, _size: Size, - _access: MemoryAccess, + ) -> EvalResult<'tcx> { + Ok(()) + } + + /// Hook for performing extra checks on a memory write access. + #[inline] + fn memory_written( + _alloc: &mut Allocation, + _ptr: Pointer, + _size: Size, ) -> EvalResult<'tcx> { Ok(()) } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 689a29cff6e9e..91f3813e2b1fd 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -30,7 +30,7 @@ use syntax::ast::Mutability; use super::{ Pointer, AllocId, Allocation, ConstValue, GlobalId, EvalResult, Scalar, EvalErrorKind, AllocType, PointerArithmetic, - Machine, MemoryAccess, AllocMap, MayLeak, ScalarMaybeUndef, ErrorHandled, + Machine, AllocMap, MayLeak, ScalarMaybeUndef, ErrorHandled, }; #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] @@ -644,7 +644,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } let alloc = self.get(ptr.alloc_id)?; - M::memory_accessed(alloc, ptr, size, MemoryAccess::Read)?; + M::memory_read(alloc, ptr, size)?; assert_eq!(ptr.offset.bytes() as usize as u64, ptr.offset.bytes()); assert_eq!(size.bytes() as usize as u64, size.bytes()); @@ -690,7 +690,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { self.clear_relocations(ptr, size)?; let alloc = self.get_mut(ptr.alloc_id)?; - M::memory_accessed(alloc, ptr, size, MemoryAccess::Write)?; + M::memory_written(alloc, ptr, size)?; assert_eq!(ptr.offset.bytes() as usize as u64, ptr.offset.bytes()); assert_eq!(size.bytes() as usize as u64, size.bytes()); diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index 55037a99e0124..5620ea4cee254 100644 --- a/src/librustc_mir/interpret/mod.rs +++ b/src/librustc_mir/interpret/mod.rs @@ -34,7 +34,7 @@ pub use self::place::{Place, PlaceTy, MemPlace, MPlaceTy}; pub use self::memory::{Memory, MemoryKind}; -pub use self::machine::{Machine, AllocMap, MemoryAccess, MayLeak}; +pub use self::machine::{Machine, AllocMap, MayLeak}; pub use self::operand::{ScalarMaybeUndef, Value, ValTy, Operand, OpTy}; From d10304eeb5c9e63597b9ee1094aca0f63c4f31ca Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 29 Oct 2018 20:40:19 +0100 Subject: [PATCH 9/9] the memory_deallocated hook can make good use of knowing the size --- src/librustc_mir/interpret/machine.rs | 1 + src/librustc_mir/interpret/memory.rs | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 7aeadfa408a90..e9d181479e52e 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -199,6 +199,7 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized { fn memory_deallocated( _alloc: &mut Allocation, _ptr: Pointer, + _size: Size, ) -> EvalResult<'tcx> { Ok(()) } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 91f3813e2b1fd..a9ef35845fc82 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -232,7 +232,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } // Let the machine take some extra action - M::memory_deallocated(&mut alloc, ptr)?; + let size = Size::from_bytes(alloc.bytes.len() as u64); + M::memory_deallocated(&mut alloc, ptr, size)?; // Don't forget to remember size and align of this now-dead allocation let old = self.dead_alloc_map.insert(