no more action on ref or cast, but add new MIR statement for escaping a ptr to raw

6 files changed, 69 insertions, 32 deletions

diff --git a/src/librustc_mir/transform/add_retag.rs b/src/librustc_mir/transform/add_retag.rs
index a50011cf5a1..0be91c3c339 100644
--- a/src/librustc_mir/transform/add_retag.rs
+++ b/src/librustc_mir/transform/add_retag.rs
@@ -20,20 +20,22 @@ 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>(
+/// Determines whether this place is "stable": Whether, if we evaluate it again
+/// after the assignment, we can be sure to obtain the same place value.
+/// (Concurrent accesses by other threads are no problem as these are anyway non-atomic
+/// copies.  Data races are UB.)
+fn is_stable<'tcx>(
     place: &Place<'tcx>,
 ) -> bool {
     use rustc::mir::Place::*;
 
     match *place {
-        Local { .. } => true,
-        Promoted(_) |
-        Static(_) => false,
+        // Locals and statics have stable addresses, for sure
+        Local { .. } |
+        Promoted { .. } |
+        Static { .. } =>
+            true,
+        // Recurse for projections
         Projection(ref proj) => {
             match proj.elem {
                 ProjectionElem::Deref |
@@ -47,15 +49,15 @@ fn is_local<'tcx>(
                 ProjectionElem::Subslice { .. } |
                 ProjectionElem::Downcast { .. } =>
                     // These just offset by a constant, entirely independent of everything else.
-                    is_local(&proj.base),
+                    is_stable(&proj.base),
             }
         }
     }
 }
 
-/// Determine whether this type has a reference in it, recursing below compound types but
+/// Determine whether this type may have 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 {
+fn may_have_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 |
@@ -68,12 +70,12 @@ fn has_reference<'a, 'gcx, 'tcx>(ty: Ty<'tcx>, tcx: TyCtxt<'a, 'gcx, 'tcx>) -> b
         ty::Adt(..) if ty.is_box() => true,
         // Compound types
         ty::Array(ty, ..) | ty::Slice(ty) =>
-            has_reference(ty, tcx),
+            may_have_reference(ty, tcx),
         ty::Tuple(tys) =>
-            tys.iter().any(|ty| has_reference(ty, tcx)),
+            tys.iter().any(|ty| may_have_reference(ty, tcx)),
         ty::Adt(adt, substs) =>
             adt.variants.iter().any(|v| v.fields.iter().any(|f|
-                has_reference(f.ty(tcx, substs), tcx)
+                may_have_reference(f.ty(tcx, substs), tcx)
             )),
         // Conservative fallback
         _ => true,
@@ -92,7 +94,9 @@ impl MirPass for AddRetag {
         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)
+            // FIXME: Instead of giving up for unstable places, we should introduce
+            // a temporary and retag on that.
+            is_stable(place) && may_have_reference(place.ty(&*local_decls, tcx).to_ty(tcx), tcx)
         };
 
         // PART 1
@@ -118,23 +122,29 @@ impl MirPass for AddRetag {
         }
 
         // PART 2
-        // Retag return values of functions.
+        // Retag return values of functions.  Also escape-to-raw the argument of `drop`.
         // 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 }) => {
+            match block_data.terminator().kind {
+                TerminatorKind::Call { ref destination, .. } => {
                     // 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));
+                            returns.push((
+                                block_data.terminator().source_info,
+                                destination.0.clone(),
+                                destination.1,
+                            ));
                         }
                     }
                 }
+                TerminatorKind::Drop { .. } |
+                TerminatorKind::DropAndReplace { .. } => {
+                    // `Drop` is also a call, but it doesn't return anything so we are good.
+                }
                 _ => {
                     // Not a block ending in a Call -> ignore.
-                    // `Drop` is also a call, but it doesn't return anything so we are good.
                 }
             }
         }
@@ -153,21 +163,43 @@ impl MirPass for AddRetag {
             // 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) => {
+                    // If we are casting *from* a reference, we may have to escape-to-raw.
+                    StatementKind::Assign(_, box Rvalue::Cast(
+                        CastKind::Misc,
+                        ref src,
+                        dest_ty,
+                    )) => {
+                        let src_ty = src.ty(&*local_decls, tcx);
+                        if src_ty.is_region_ptr() {
+                            // The only `Misc` casts on references are those creating raw pointers.
+                            assert!(dest_ty.is_unsafe_ptr());
+                            // Insert escape-to-raw before the cast.  We are not concerned
+                            // with stability here: Our EscapeToRaw will not change the value
+                            // that the cast will then use.
+                            // `src` might be a "move", but we rely on this not actually moving
+                            // but just doing a memcpy.  It is crucial that we do EscapeToRaw
+                            // on the src because we need it with its original type.
+                            let source_info = block_data.statements[i].source_info;
+                            block_data.statements.insert(i, Statement {
+                                source_info,
+                                kind: StatementKind::EscapeToRaw(src.clone()),
+                            });
+                        }
+                    }
+                    // Assignments of reference or ptr type are the ones where we may have
+                    // to update tags.  This includes `x = &[mut] ...` and hence
+                    // we also retag after taking a reference!
+                    StatementKind::Assign(ref place, _) 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 {
+                        block_data.statements.insert(i+1, Statement {
                             source_info,
                             kind: StatementKind::Retag { fn_entry: false, place: place.clone() },
                         });
                     }
+                    // Do nothing for the rest
                     _ => {},
-                }
+                };
             }
         }
     }
diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs
index c28bb0ca357..4ebeebca227 100644
--- a/src/librustc_mir/transform/check_unsafety.rs
+++ b/src/librustc_mir/transform/check_unsafety.rs
@@ -114,6 +114,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
             StatementKind::StorageDead(..) |
             StatementKind::EndRegion(..) |
             StatementKind::Retag { .. } |
+            StatementKind::EscapeToRaw { .. } |
             StatementKind::AscribeUserType(..) |
             StatementKind::Nop => {
                 // safe (at least as emitted during MIR construction)
diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs
index ca9c4eb9b8b..5fd83321296 100644
--- a/src/librustc_mir/transform/qualify_consts.rs
+++ b/src/librustc_mir/transform/qualify_consts.rs
@@ -1168,6 +1168,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
                 StatementKind::InlineAsm {..} |
                 StatementKind::EndRegion(_) |
                 StatementKind::Retag { .. } |
+                StatementKind::EscapeToRaw { .. } |
                 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 1e193485950..ed13063cfdf 100644
--- a/src/librustc_mir/transform/qualify_min_const_fn.rs
+++ b/src/librustc_mir/transform/qualify_min_const_fn.rs
@@ -242,6 +242,7 @@ fn check_statement(
         | StatementKind::StorageLive(_)
         | StatementKind::StorageDead(_)
         | StatementKind::Retag { .. }
+        | StatementKind::EscapeToRaw { .. }
         | 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 c1c127fa8d6..445ffbbcf34 100644
--- a/src/librustc_mir/transform/remove_noop_landing_pads.rs
+++ b/src/librustc_mir/transform/remove_noop_landing_pads.rs
@@ -65,10 +65,11 @@ impl RemoveNoopLandingPads {
                     // turn a landing pad to a non-nop
                 }
 
-                StatementKind::Assign(_, _) |
+                StatementKind::Assign { .. } |
                 StatementKind::SetDiscriminant { .. } |
                 StatementKind::InlineAsm { .. } |
-                StatementKind::Retag { .. } => {
+                StatementKind::Retag { .. } |
+                StatementKind::EscapeToRaw { .. } => {
                     return false;
                 }
             }
diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs
index a5a19f04b7e..8f026c706fd 100644
--- a/src/librustc_mir/transform/rustc_peek.rs
+++ b/src/librustc_mir/transform/rustc_peek.rs
@@ -163,6 +163,7 @@ fn each_block<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
             mir::StatementKind::InlineAsm { .. } |
             mir::StatementKind::EndRegion(_) |
             mir::StatementKind::Retag { .. } |
+            mir::StatementKind::EscapeToRaw { .. } |
             mir::StatementKind::AscribeUserType(..) |
             mir::StatementKind::Nop => continue,
             mir::StatementKind::SetDiscriminant{ .. } =>