Auto merge of #55270 - RalfJung:stacked-borrows-ng, r=oli-obk

miri engine: Stacked Borrows NG

For more refined tracking in miri, we do return untagged pointers from the memory abstraction after allocations and let the caller decide how to tag these.

Also refactor the `tag_(de)reference` hooks so they can be more easily called in the ref-to-place and place-to-ref methods, and reorder things in validation: validation calls ref-to-place which (when running in miri) triggers some checks, so we want to run it rather late and catch other problems first. We also do not need to redundantly check the ref to be allocated any more, the checks miri does anyway imply thath.

r? @oli-obk

9 files changed, 174 insertions, 130 deletions

diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs
index 73cb2038094..7db9c3f1102 100644
--- a/src/librustc_mir/const_eval.rs
+++ b/src/librustc_mir/const_eval.rs
@@ -20,8 +20,8 @@ use rustc::hir::{self, def_id::DefId};
 use rustc::hir::def::Def;
 use rustc::mir::interpret::{ConstEvalErr, ErrorHandled};
 use rustc::mir;
-use rustc::ty::{self, Ty, TyCtxt, Instance, query::TyCtxtAt};
-use rustc::ty::layout::{self, Size, LayoutOf, TyLayout};
+use rustc::ty::{self, TyCtxt, Instance, query::TyCtxtAt};
+use rustc::ty::layout::{self, LayoutOf, TyLayout};
 use rustc::ty::subst::Subst;
 use rustc::traits::Reveal;
 use rustc_data_structures::indexed_vec::IndexVec;
@@ -32,7 +32,7 @@ use syntax::ast::Mutability;
 use syntax::source_map::{Span, DUMMY_SP};
 
 use interpret::{self,
-    PlaceTy, MemPlace, OpTy, Operand, Value, Pointer, Scalar, ConstValue,
+    PlaceTy, MemPlace, OpTy, Operand, Value, Scalar, ConstValue, Pointer,
     EvalResult, EvalError, EvalErrorKind, GlobalId, EvalContext, StackPopCleanup,
     Allocation, AllocId, MemoryKind,
     snapshot, RefTracking,
@@ -426,7 +426,7 @@ impl<'a, 'mir, 'tcx> interpret::Machine<'a, 'mir, 'tcx>
     }
 
     #[inline(always)]
-    fn static_with_default_tag(
+    fn adjust_static_allocation(
         alloc: &'_ Allocation
     ) -> Cow<'_, Allocation<Self::PointerTag>> {
         // We do not use a tag so we can just cheaply forward the reference
@@ -467,23 +467,12 @@ impl<'a, 'mir, 'tcx> interpret::Machine<'a, 'mir, 'tcx>
     }
 
     #[inline(always)]
-    fn tag_reference(
+    fn tag_new_allocation(
         _ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,
-        _ptr: Pointer<Self::PointerTag>,
-        _pointee_ty: Ty<'tcx>,
-        _pointee_size: Size,
-        _borrow_kind: Option<mir::BorrowKind>,
-    ) -> EvalResult<'tcx, Self::PointerTag> {
-        Ok(())
-    }
-
-    #[inline(always)]
-    fn tag_dereference(
-        _ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
-        _ptr: Pointer<Self::PointerTag>,
-        _ptr_ty: Ty<'tcx>,
-    ) -> EvalResult<'tcx, Self::PointerTag> {
-        Ok(())
+        ptr: Pointer,
+        _kind: MemoryKind<Self::MemoryKinds>,
+    ) -> EvalResult<'tcx, Pointer> {
+        Ok(ptr)
     }
 }
 
diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs
index 81e7a6e4373..b2c8cba4802 100644
--- a/src/librustc_mir/interpret/cast.rs
+++ b/src/librustc_mir/interpret/cast.rs
@@ -110,7 +110,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                             def_id,
                             substs,
                         ).ok_or_else(|| EvalErrorKind::TooGeneric.into());
-                        let fn_ptr = self.memory.create_fn_alloc(instance?);
+                        let fn_ptr = self.memory.create_fn_alloc(instance?).with_default_tag();
                         self.write_scalar(Scalar::Ptr(fn_ptr.into()), dest)?;
                     }
                     ref other => bug!("reify fn pointer on {:?}", other),
@@ -143,7 +143,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                             substs,
                             ty::ClosureKind::FnOnce,
                         );
-                        let fn_ptr = self.memory.create_fn_alloc(instance);
+                        let fn_ptr = self.memory.create_fn_alloc(instance).with_default_tag();
                         let val = Value::Scalar(Scalar::Ptr(fn_ptr.into()).into());
                         self.write_value(val, dest)?;
                     }
diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index b578ec65900..bc7ad16dc97 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -47,7 +47,7 @@ pub struct EvalContext<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'a, 'mir, 'tcx>> {
     pub(crate) param_env: ty::ParamEnv<'tcx>,
 
     /// The virtual memory system.
-    pub memory: Memory<'a, 'mir, 'tcx, M>,
+    pub(crate) memory: Memory<'a, 'mir, 'tcx, M>,
 
     /// The virtual call stack.
     pub(crate) stack: Vec<Frame<'mir, 'tcx, M::PointerTag>>,
@@ -334,7 +334,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
     }
 
     pub fn str_to_value(&mut self, s: &str) -> EvalResult<'tcx, Value<M::PointerTag>> {
-        let ptr = self.memory.allocate_static_bytes(s.as_bytes());
+        let ptr = self.memory.allocate_static_bytes(s.as_bytes()).with_default_tag();
         Ok(Value::new_slice(Scalar::Ptr(ptr), s.len() as u64, self.tcx.tcx))
     }
 
diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs
index 1318bbe1c2b..7811dcb0663 100644
--- a/src/librustc_mir/interpret/machine.rs
+++ b/src/librustc_mir/interpret/machine.rs
@@ -15,13 +15,13 @@
 use std::borrow::{Borrow, Cow};
 use std::hash::Hash;
 
-use rustc::hir::def_id::DefId;
+use rustc::hir::{self, def_id::DefId};
 use rustc::mir;
 use rustc::ty::{self, Ty, layout::{Size, TyLayout}, query::TyCtxtAt};
 
 use super::{
     Allocation, AllocId, EvalResult, Scalar,
-    EvalContext, PlaceTy, OpTy, Pointer, MemoryKind,
+    EvalContext, PlaceTy, OpTy, Pointer, MemPlace, MemoryKind,
 };
 
 /// Classifying memory accesses
@@ -81,6 +81,7 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized {
 
     /// Tag tracked alongside every pointer.  This is used to implement "Stacked Borrows"
     /// <https://www.ralfj.de/blog/2018/08/07/stacked-borrows.html>.
+    /// The `default()` is used for pointers to consts, statics, vtables and functions.
     type PointerTag: ::std::fmt::Debug + Default + Copy + Eq + Hash + 'static;
 
     /// Extra data stored in every allocation.
@@ -151,13 +152,13 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized {
     ) -> EvalResult<'tcx, Cow<'tcx, Allocation<Self::PointerTag, Self::AllocExtra>>>;
 
     /// Called to turn an allocation obtained from the `tcx` into one that has
-    /// the appropriate tags on each pointer.
+    /// the right type for this machine.
     ///
     /// This should avoid copying if no work has to be done! If this returns an owned
-    /// allocation (because a copy had to be done to add the tags), machine memory will
+    /// allocation (because a copy had to be done to add tags or metadata), machine memory will
     /// cache the result. (This relies on `AllocMap::get_or` being able to add the
     /// owned allocation to the map even when the map is shared.)
-    fn static_with_default_tag(
+    fn adjust_static_allocation(
         alloc: &'_ Allocation
     ) -> Cow<'_, Allocation<Self::PointerTag, Self::AllocExtra>>;
 
@@ -204,24 +205,40 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized {
         Ok(())
     }
 
+    /// Add the tag for a newly allocated pointer.
+    fn tag_new_allocation(
+        ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,
+        ptr: Pointer,
+        kind: MemoryKind<Self::MemoryKinds>,
+    ) -> EvalResult<'tcx, Pointer<Self::PointerTag>>;
+
     /// Executed when evaluating the `&` operator: Creating a new reference.
-    /// This has the chance to adjust the tag.
-    /// `borrow_kind` can be `None` in case a raw ptr is being created.
+    /// This has the chance to adjust the tag.  It should not change anything else!
+    /// `mutability` can be `None` in case a raw ptr is being created.
+    #[inline]
     fn tag_reference(
-        ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,
-        ptr: Pointer<Self::PointerTag>,
-        pointee_ty: Ty<'tcx>,
-        pointee_size: Size,
-        borrow_kind: Option<mir::BorrowKind>,
-    ) -> EvalResult<'tcx, Self::PointerTag>;
+        _ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,
+        place: MemPlace<Self::PointerTag>,
+        _ty: Ty<'tcx>,
+        _size: Size,
+        _mutability: Option<hir::Mutability>,
+    ) -> EvalResult<'tcx, MemPlace<Self::PointerTag>> {
+        Ok(place)
+    }
 
     /// Executed when evaluating the `*` operator: Following a reference.
-    /// This has the change to adjust the tag.
+    /// This has the change to adjust the tag.  It should not change anything else!
+    /// `mutability` can be `None` in case a raw ptr is being dereferenced.
+    #[inline]
     fn tag_dereference(
-        ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
-        ptr: Pointer<Self::PointerTag>,
-        ptr_ty: Ty<'tcx>,
-    ) -> EvalResult<'tcx, Self::PointerTag>;
+        _ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
+        place: MemPlace<Self::PointerTag>,
+        _ty: Ty<'tcx>,
+        _size: Size,
+        _mutability: Option<hir::Mutability>,
+    ) -> EvalResult<'tcx, MemPlace<Self::PointerTag>> {
+        Ok(place)
+    }
 
     /// Execute a validation operation
     #[inline]
diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index ff3fdffcd76..689a29cff6e 100644
--- a/src/librustc_mir/interpret/memory.rs
+++ b/src/librustc_mir/interpret/memory.rs
@@ -117,12 +117,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
         }
     }
 
-    pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer<M::PointerTag> {
-        Pointer::from(self.tcx.alloc_map.lock().create_fn_alloc(instance)).with_default_tag()
+    pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer {
+        Pointer::from(self.tcx.alloc_map.lock().create_fn_alloc(instance))
     }
 
-    pub fn allocate_static_bytes(&mut self, bytes: &[u8]) -> Pointer<M::PointerTag> {
-        Pointer::from(self.tcx.allocate_bytes(bytes)).with_default_tag()
+    pub fn allocate_static_bytes(&mut self, bytes: &[u8]) -> Pointer {
+        Pointer::from(self.tcx.allocate_bytes(bytes))
     }
 
     pub fn allocate_with(
@@ -140,9 +140,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
         size: Size,
         align: Align,
         kind: MemoryKind<M::MemoryKinds>,
-    ) -> EvalResult<'tcx, Pointer<M::PointerTag>> {
-        let ptr = Pointer::from(self.allocate_with(Allocation::undef(size, align), kind)?);
-        Ok(ptr.with_default_tag())
+    ) -> EvalResult<'tcx, Pointer> {
+        Ok(Pointer::from(self.allocate_with(Allocation::undef(size, align), kind)?))
     }
 
     pub fn reallocate(
@@ -153,17 +152,18 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
         new_size: Size,
         new_align: Align,
         kind: MemoryKind<M::MemoryKinds>,
-    ) -> EvalResult<'tcx, Pointer<M::PointerTag>> {
+    ) -> EvalResult<'tcx, Pointer> {
         if ptr.offset.bytes() != 0 {
             return err!(ReallocateNonBasePtr);
         }
 
-        // For simplicities' sake, we implement reallocate as "alloc, copy, dealloc"
+        // For simplicities' sake, we implement reallocate as "alloc, copy, dealloc".
+        // This happens so rarely, the perf advantage is outweighed by the maintenance cost.
         let new_ptr = self.allocate(new_size, new_align, kind)?;
         self.copy(
             ptr.into(),
             old_align,
-            new_ptr.into(),
+            new_ptr.with_default_tag().into(),
             new_align,
             old_size.min(new_size),
             /*nonoverlapping*/ true,
@@ -347,7 +347,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
             Some(AllocType::Memory(mem)) => {
                 // We got tcx memory. Let the machine figure out whether and how to
                 // turn that into memory with the right pointer tag.
-                return Ok(M::static_with_default_tag(mem))
+                return Ok(M::adjust_static_allocation(mem))
             }
             Some(AllocType::Function(..)) => {
                 return err!(DerefFunctionPointer)
@@ -381,7 +381,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
             if let ConstValue::ByRef(_, allocation, _) = const_val.val {
                 // We got tcx memory. Let the machine figure out whether and how to
                 // turn that into memory with the right pointer tag.
-                M::static_with_default_tag(allocation)
+                M::adjust_static_allocation(allocation)
             } else {
                 bug!("Matching on non-ByRef static")
             }
diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
index 021e2d58f84..d0a32161485 100644
--- a/src/librustc_mir/interpret/operand.rs
+++ b/src/librustc_mir/interpret/operand.rs
@@ -217,6 +217,16 @@ impl<'tcx, Tag> Value<Tag> {
             Value::ScalarPair(ptr, _) => ptr.not_undef(),
         }
     }
+
+    /// Convert the value into its metadata.
+    /// Throws away the first half of a ScalarPair!
+    #[inline]
+    pub fn to_meta(self) -> EvalResult<'tcx, Option<Scalar<Tag>>> {
+        Ok(match self {
+            Value::Scalar(_) => None,
+            Value::ScalarPair(_, meta) => Some(meta.not_undef()?),
+        })
+    }
 }
 
 // ScalarPair needs a type to interpret, so we often have a value and a type together
diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs
index 4fa08e8c311..0eae2bfb226 100644
--- a/src/librustc_mir/interpret/place.rs
+++ b/src/librustc_mir/interpret/place.rs
@@ -15,6 +15,7 @@
 use std::convert::TryFrom;
 use std::hash::Hash;
 
+use rustc::hir;
 use rustc::mir;
 use rustc::ty::{self, Ty};
 use rustc::ty::layout::{self, Size, Align, LayoutOf, TyLayout, HasDataLayout};
@@ -270,24 +271,28 @@ where
         &self,
         val: ValTy<'tcx, M::PointerTag>,
     ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
-        let ptr = match val.to_scalar_ptr()? {
-            Scalar::Ptr(ptr) if M::ENABLE_PTR_TRACKING_HOOKS => {
-                // Machine might want to track the `*` operator
-                let tag = M::tag_dereference(self, ptr, val.layout.ty)?;
-                Scalar::Ptr(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag))
-            }
-            other => other,
-        };
-
         let pointee_type = val.layout.ty.builtin_deref(true).unwrap().ty;
         let layout = self.layout_of(pointee_type)?;
-        let align = layout.align;
 
-        let mplace = match *val {
-            Value::Scalar(_) =>
-                MemPlace { ptr, align, meta: None },
-            Value::ScalarPair(_, meta) =>
-                MemPlace { ptr, align, meta: Some(meta.not_undef()?) },
+        let align = layout.align;
+        let meta = val.to_meta()?;
+        let ptr = val.to_scalar_ptr()?;
+        let mplace = MemPlace { ptr, align, meta };
+        // Pointer tag tracking might want to adjust the tag.
+        let mplace = if M::ENABLE_PTR_TRACKING_HOOKS {
+            let (size, _) = self.size_and_align_of(meta, layout)?
+                // for extern types, just cover what we can
+                .unwrap_or_else(|| layout.size_and_align());
+            let mutbl = match val.layout.ty.sty {
+                // `builtin_deref` considers boxes immutable, that's useless for our purposes
+                ty::Ref(_, _, mutbl) => Some(mutbl),
+                ty::Adt(def, _) if def.is_box() => Some(hir::MutMutable),
+                ty::RawPtr(_) => None,
+                _ => bug!("Unexpected pointer type {}", val.layout.ty.sty),
+            };
+            M::tag_dereference(self, mplace, pointee_type, size, mutbl)?
+        } else {
+            mplace
         };
         Ok(MPlaceTy { mplace, layout })
     }
@@ -299,19 +304,25 @@ where
         place: MPlaceTy<'tcx, M::PointerTag>,
         borrow_kind: Option<mir::BorrowKind>,
     ) -> EvalResult<'tcx, Value<M::PointerTag>> {
-        let ptr = match place.ptr {
-            Scalar::Ptr(ptr) if M::ENABLE_PTR_TRACKING_HOOKS => {
-                // Machine might want to track the `&` operator
-                let (size, _) = self.size_and_align_of_mplace(place)?
-                    .expect("create_ref cannot determine size");
-                let tag = M::tag_reference(self, ptr, place.layout.ty, size, borrow_kind)?;
-                Scalar::Ptr(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag))
-            },
-            other => other,
+        // 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
         };
         Ok(match place.meta {
-            None => Value::Scalar(ptr.into()),
-            Some(meta) => Value::ScalarPair(ptr.into(), meta.into()),
+            None => Value::Scalar(place.ptr.into()),
+            Some(meta) => Value::ScalarPair(place.ptr.into(), meta.into()),
         })
     }
 
@@ -845,6 +856,8 @@ where
     }
 
     /// Make sure that a place is in memory, and return where it is.
+    /// If the place currently refers to a local that doesn't yet have a matching allocation,
+    /// create such an allocation.
     /// This is essentially `force_to_memplace`.
     pub fn force_allocation(
         &mut self,
@@ -888,10 +901,11 @@ where
     ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
         if layout.is_unsized() {
             assert!(self.tcx.features().unsized_locals, "cannot alloc memory for unsized type");
-            // FIXME: What should we do here?
+            // FIXME: What should we do here? We should definitely also tag!
             Ok(MPlaceTy::dangling(layout, &self))
         } else {
             let ptr = self.memory.allocate(layout.size, layout.align, kind)?;
+            let ptr = M::tag_new_allocation(self, ptr, kind)?;
             Ok(MPlaceTy::from_aligned_ptr(ptr, layout))
         }
     }
diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs
index a2d4eee2842..c189ec0ca5c 100644
--- a/src/librustc_mir/interpret/traits.rs
+++ b/src/librustc_mir/interpret/traits.rs
@@ -54,10 +54,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
             ptr_size * (3 + methods.len() as u64),
             ptr_align,
             MemoryKind::Vtable,
-        )?;
+        )?.with_default_tag();
 
         let drop = ::monomorphize::resolve_drop_in_place(*self.tcx, ty);
-        let drop = self.memory.create_fn_alloc(drop);
+        let drop = self.memory.create_fn_alloc(drop).with_default_tag();
         self.memory.write_ptr_sized(vtable, ptr_align, Scalar::Ptr(drop).into())?;
 
         let size_ptr = vtable.offset(ptr_size, &self)?;
@@ -69,7 +69,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         for (i, method) in methods.iter().enumerate() {
             if let Some((def_id, substs)) = *method {
                 let instance = self.resolve(def_id, substs)?;
-                let fn_ptr = self.memory.create_fn_alloc(instance);
+                let fn_ptr = self.memory.create_fn_alloc(instance).with_default_tag();
                 let method_ptr = vtable.offset(ptr_size * (3 + i as u64), &self)?;
                 self.memory.write_ptr_sized(method_ptr, ptr_align, Scalar::Ptr(fn_ptr).into())?;
             }
diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs
index 25e2ff6edb7..226717538a2 100644
--- a/src/librustc_mir/interpret/validity.rs
+++ b/src/librustc_mir/interpret/validity.rs
@@ -12,7 +12,7 @@ use std::fmt::Write;
 use std::hash::Hash;
 
 use syntax_pos::symbol::Symbol;
-use rustc::ty::layout::{self, Size, Align, TyLayout};
+use rustc::ty::layout::{self, Size, Align, TyLayout, LayoutOf};
 use rustc::ty;
 use rustc_data_structures::fx::FxHashSet;
 use rustc::mir::interpret::{
@@ -176,19 +176,27 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                     // undef. We should fix that, but let's start low.
                 }
             }
-            _ if ty.is_box() || ty.is_region_ptr() || ty.is_unsafe_ptr() => {
-                // Handle fat pointers. We also check fat raw pointers,
-                // their metadata must be valid!
-                // This also checks that the ptr itself is initialized, which
-                // seems reasonable even for raw pointers.
-                let place = try_validation!(self.ref_to_mplace(value),
-                    "undefined data in pointer", path);
+            ty::RawPtr(..) => {
+                // No undef allowed here.  Eventually this should be consistent with
+                // the integer types.
+                let _ptr = try_validation!(value.to_scalar_ptr(),
+                    "undefined address in pointer", path);
+                let _meta = try_validation!(value.to_meta(),
+                    "uninitialized data in fat pointer metadata", path);
+            }
+            _ if ty.is_box() || ty.is_region_ptr() => {
+                // Handle fat pointers.
                 // Check metadata early, for better diagnostics
-                if place.layout.is_unsized() {
-                    let tail = self.tcx.struct_tail(place.layout.ty);
+                let ptr = try_validation!(value.to_scalar_ptr(),
+                    "undefined address in pointer", path);
+                let meta = try_validation!(value.to_meta(),
+                    "uninitialized data in fat pointer metadata", path);
+                let layout = self.layout_of(value.layout.ty.builtin_deref(true).unwrap().ty)?;
+                if layout.is_unsized() {
+                    let tail = self.tcx.struct_tail(layout.ty);
                     match tail.sty {
                         ty::Dynamic(..) => {
-                            let vtable = try_validation!(place.meta.unwrap().to_ptr(),
+                            let vtable = try_validation!(meta.unwrap().to_ptr(),
                                 "non-pointer vtable in fat pointer", path);
                             try_validation!(self.read_drop_type_from_vtable(vtable),
                                 "invalid drop fn in vtable", path);
@@ -197,7 +205,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                             // FIXME: More checks for the vtable.
                         }
                         ty::Slice(..) | ty::Str => {
-                            try_validation!(place.meta.unwrap().to_usize(self),
+                            try_validation!(meta.unwrap().to_usize(self),
                                 "non-integer slice length in fat pointer", path);
                         }
                         ty::Foreign(..) => {
@@ -207,17 +215,17 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                             bug!("Unexpected unsized type tail: {:?}", tail),
                     }
                 }
-                // for safe ptrs, also check the ptr values itself
-                if !ty.is_unsafe_ptr() {
-                    // Make sure this is non-NULL and aligned
-                    let (size, align) = self.size_and_align_of(place.meta, place.layout)?
-                        // for the purpose of validity, consider foreign types to have
-                        // alignment and size determined by the layout (size will be 0,
-                        // alignment should take attributes into account).
-                        .unwrap_or_else(|| place.layout.size_and_align());
-                    match self.memory.check_align(place.ptr, align) {
-                        Ok(_) => {},
-                        Err(err) => match err.kind {
+                // Make sure this is non-NULL and aligned
+                let (size, align) = self.size_and_align_of(meta, layout)?
+                    // for the purpose of validity, consider foreign types to have
+                    // alignment and size determined by the layout (size will be 0,
+                    // alignment should take attributes into account).
+                    .unwrap_or_else(|| layout.size_and_align());
+                match self.memory.check_align(ptr, align) {
+                    Ok(_) => {},
+                    Err(err) => {
+                        error!("{:?} is not aligned to {:?}", ptr, align);
+                        match err.kind {
                             EvalErrorKind::InvalidNullPointerUsage =>
                                 return validation_failure!("NULL reference", path),
                             EvalErrorKind::AlignmentCheckFailed { .. } =>
@@ -225,41 +233,47 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                             _ =>
                                 return validation_failure!(
                                     "dangling (out-of-bounds) reference (might be NULL at \
-                                     run-time)",
+                                        run-time)",
                                     path
                                 ),
                         }
                     }
-                    // non-ZST also have to be dereferenceable
+                }
+                // Turn ptr into place.
+                // `ref_to_mplace` also calls the machine hook for (re)activating the tag,
+                // which in turn will (in full miri) check if the pointer is dereferencable.
+                let place = self.ref_to_mplace(value)?;
+                // Recursive checking
+                if let Some(ref_tracking) = ref_tracking {
+                    assert!(const_mode, "We should only do recursie checking in const mode");
                     if size != Size::ZERO {
+                        // Non-ZST also have to be dereferencable
                         let ptr = try_validation!(place.ptr.to_ptr(),
                             "integer pointer in non-ZST reference", path);
-                        if const_mode {
-                            // Skip validation entirely for some external statics
-                            let alloc_kind = self.tcx.alloc_map.lock().get(ptr.alloc_id);
-                            if let Some(AllocType::Static(did)) = alloc_kind {
-                                // `extern static` cannot be validated as they have no body.
-                                // FIXME: Statics from other crates are also skipped.
-                                // They might be checked at a different type, but for now we
-                                // want to avoid recursing too deeply.  This is not sound!
-                                if !did.is_local() || self.tcx.is_foreign_item(did) {
-                                    return Ok(());
-                                }
+                        // Skip validation entirely for some external statics
+                        let alloc_kind = self.tcx.alloc_map.lock().get(ptr.alloc_id);
+                        if let Some(AllocType::Static(did)) = alloc_kind {
+                            // `extern static` cannot be validated as they have no body.
+                            // FIXME: Statics from other crates are also skipped.
+                            // They might be checked at a different type, but for now we
+                            // want to avoid recursing too deeply.  This is not sound!
+                            if !did.is_local() || self.tcx.is_foreign_item(did) {
+                                return Ok(());
                             }
                         }
+                        // Maintain the invariant that the place we are checking is
+                        // already verified to be in-bounds.
                         try_validation!(self.memory.check_bounds(ptr, size, false),
                             "dangling (not entirely in bounds) reference", path);
                     }
-                    if let Some(ref_tracking) = ref_tracking {
-                        // Check if we have encountered this pointer+layout combination
-                        // before.  Proceed recursively even for integer pointers, no
-                        // reason to skip them! They are (recursively) valid for some ZST,
-                        // but not for others (e.g. `!` is a ZST).
-                        let op = place.into();
-                        if ref_tracking.seen.insert(op) {
-                            trace!("Recursing below ptr {:#?}", *op);
-                            ref_tracking.todo.push((op, path_clone_and_deref(path)));
-                        }
+                    // Check if we have encountered this pointer+layout combination
+                    // before.  Proceed recursively even for integer pointers, no
+                    // reason to skip them! They are (recursively) valid for some ZST,
+                    // but not for others (e.g. `!` is a ZST).
+                    let op = place.into();
+                    if ref_tracking.seen.insert(op) {
+                        trace!("Recursing below ptr {:#?}", *op);
+                        ref_tracking.todo.push((op, path_clone_and_deref(path)));
                     }
                 }
             }