generalize the traversal part of validation to a ValueVisitor

6 files changed, 424 insertions, 305 deletions

diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs
index 2658d7f59a0..34684587db2 100644
--- a/src/librustc_mir/const_eval.rs
+++ b/src/librustc_mir/const_eval.rs
@@ -535,14 +535,14 @@ fn validate_const<'a, 'tcx>(
     key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>,
 ) -> ::rustc::mir::interpret::ConstEvalResult<'tcx> {
     let cid = key.value;
-    let ecx = mk_eval_cx(tcx, cid.instance, key.param_env).unwrap();
+    let mut ecx = mk_eval_cx(tcx, cid.instance, key.param_env).unwrap();
     let val = (|| {
         let op = ecx.const_to_op(constant)?;
         let mut ref_tracking = RefTracking::new(op);
-        while let Some((op, mut path)) = ref_tracking.todo.pop() {
+        while let Some((op, path)) = ref_tracking.todo.pop() {
             ecx.validate_operand(
                 op,
-                &mut path,
+                path,
                 Some(&mut ref_tracking),
                 /* const_mode */ true,
             )?;
diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index fc13c5fef2d..e6267012dc2 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -521,7 +521,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
                 // return place is always a local and then this cannot happen.
                 self.validate_operand(
                     self.place_to_op(return_place)?,
-                    &mut vec![],
+                    vec![],
                     None,
                     /*const_mode*/false,
                 )?;
diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs
index 6b31c675cc7..82fe08fa038 100644
--- a/src/librustc_mir/interpret/mod.rs
+++ b/src/librustc_mir/interpret/mod.rs
@@ -23,6 +23,7 @@ mod terminator;
 mod traits;
 mod validity;
 mod intrinsics;
+mod visitor;
 
 pub use rustc::mir::interpret::*; // have all the `interpret` symbols in one place: here
 
@@ -38,4 +39,6 @@ pub use self::machine::{Machine, AllocMap, MayLeak};
 
 pub use self::operand::{ScalarMaybeUndef, Immediate, ImmTy, Operand, OpTy};
 
+pub use self::visitor::ValueVisitor;
+
 pub use self::validity::RefTracking;
diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs
index fa4d31846df..35276fa6265 100644
--- a/src/librustc_mir/interpret/place.rs
+++ b/src/librustc_mir/interpret/place.rs
@@ -489,6 +489,8 @@ where
 
     /// Get the place of a field inside the place, and also the field's type.
     /// Just a convenience function, but used quite a bit.
+    /// This is the only projection that might have a side-effect: We cannot project
+    /// into the field of a local `ScalarPair`, we have to first allocate it.
     pub fn place_field(
         &mut self,
         base: PlaceTy<'tcx, M::PointerTag>,
@@ -501,7 +503,7 @@ where
     }
 
     pub fn place_downcast(
-        &mut self,
+        &self,
         base: PlaceTy<'tcx, M::PointerTag>,
         variant: usize,
     ) -> EvalResult<'tcx, PlaceTy<'tcx, M::PointerTag>> {
@@ -643,7 +645,7 @@ where
 
         if M::enforce_validity(self) {
             // Data got changed, better make sure it matches the type!
-            self.validate_operand(self.place_to_op(dest)?, &mut vec![], None, /*const_mode*/false)?;
+            self.validate_operand(self.place_to_op(dest)?, vec![], None, /*const_mode*/false)?;
         }
 
         Ok(())
@@ -765,7 +767,7 @@ where
 
         if M::enforce_validity(self) {
             // Data got changed, better make sure it matches the type!
-            self.validate_operand(self.place_to_op(dest)?, &mut vec![], None, /*const_mode*/false)?;
+            self.validate_operand(self.place_to_op(dest)?, vec![], None, /*const_mode*/false)?;
         }
 
         Ok(())
@@ -843,7 +845,7 @@ where
 
         if M::enforce_validity(self) {
             // Data got changed, better make sure it matches the type!
-            self.validate_operand(dest.into(), &mut vec![], None, /*const_mode*/false)?;
+            self.validate_operand(dest.into(), vec![], None, /*const_mode*/false)?;
         }
 
         Ok(())
diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs
index 4dbae3c8c3d..8ac3f992cfd 100644
--- a/src/librustc_mir/interpret/validity.rs
+++ b/src/librustc_mir/interpret/validity.rs
@@ -8,24 +8,24 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::fmt::Write;
+use std::fmt::{self, Write};
 use std::hash::Hash;
 
 use syntax_pos::symbol::Symbol;
 use rustc::ty::layout::{self, Size, Align, TyLayout, LayoutOf};
-use rustc::ty;
+use rustc::ty::{self, TyCtxt};
 use rustc_data_structures::fx::FxHashSet;
 use rustc::mir::interpret::{
     Scalar, AllocType, EvalResult, EvalErrorKind
 };
 
 use super::{
-    ImmTy, OpTy, MPlaceTy, Machine, EvalContext, ScalarMaybeUndef
+    OpTy, MPlaceTy, Machine, EvalContext, ScalarMaybeUndef, ValueVisitor
 };
 
 macro_rules! validation_failure {
     ($what:expr, $where:expr, $details:expr) => {{
-        let where_ = path_format($where);
+        let where_ = path_format(&$where);
         let where_ = if where_.is_empty() {
             String::new()
         } else {
@@ -37,7 +37,7 @@ macro_rules! validation_failure {
         )))
     }};
     ($what:expr, $where:expr) => {{
-        let where_ = path_format($where);
+        let where_ = path_format(&$where);
         let where_ = if where_.is_empty() {
             String::new()
         } else {
@@ -129,6 +129,43 @@ fn path_format(path: &Vec<PathElem>) -> String {
     out
 }
 
+fn aggregate_field_path_elem<'a, 'tcx>(
+    layout: TyLayout<'tcx>,
+    field: usize,
+    tcx: TyCtxt<'a, 'tcx, 'tcx>,
+) -> PathElem {
+    match layout.ty.sty {
+        // generators and closures.
+        ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => {
+            if let Some(upvar) = tcx.optimized_mir(def_id).upvar_decls.get(field) {
+                PathElem::ClosureVar(upvar.debug_name)
+            } else {
+                // Sometimes the index is beyond the number of freevars (seen
+                // for a generator).
+                PathElem::ClosureVar(Symbol::intern(&field.to_string()))
+            }
+        }
+
+        // tuples
+        ty::Tuple(_) => PathElem::TupleElem(field),
+
+        // enums
+        ty::Adt(def, ..) if def.is_enum() => {
+            let variant = match layout.variants {
+                layout::Variants::Single { index } => &def.variants[index],
+                _ => bug!("aggregate_field_path_elem: got enum but not in a specific variant"),
+            };
+            PathElem::Field(variant.fields[field].ident.name)
+        }
+
+        // other ADTs
+        ty::Adt(def, _) => PathElem::Field(def.non_enum_variant().fields[field].ident.name),
+
+        // nothing else has an aggregate layout
+        _ => bug!("aggregate_field_path_elem: got non-aggregate type {:?}", layout.ty),
+    }
+}
+
 fn scalar_format<Tag>(value: ScalarMaybeUndef<Tag>) -> String {
     match value {
         ScalarMaybeUndef::Undef =>
@@ -140,37 +177,92 @@ fn scalar_format<Tag>(value: ScalarMaybeUndef<Tag>) -> String {
     }
 }
 
-impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
-    /// Make sure that `value` is valid for `ty`, *assuming* `ty` is a primitive type.
-    fn validate_primitive_type(
-        &self,
-        value: ImmTy<'tcx, M::PointerTag>,
-        path: &Vec<PathElem>,
-        ref_tracking: Option<&mut RefTracking<'tcx, M::PointerTag>>,
-        const_mode: bool,
-    ) -> EvalResult<'tcx> {
+struct ValidityVisitor<'rt, 'tcx, Tag> {
+    op: OpTy<'tcx, Tag>,
+    /// The `path` may be pushed to, but the part that is present when a function
+    /// starts must not be changed!  `visit_fields` and `visit_array` rely on
+    /// this stack discipline.
+    path: Vec<PathElem>,
+    ref_tracking: Option<&'rt mut RefTracking<'tcx, Tag>>,
+    const_mode: bool,
+}
+
+impl<Tag: fmt::Debug> fmt::Debug for ValidityVisitor<'_, '_, Tag> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{:?} ({:?})", *self.op, self.op.layout.ty)
+    }
+}
+
+impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>
+    ValueVisitor<'a, 'mir, 'tcx, M> for ValidityVisitor<'rt, 'tcx, M::PointerTag>
+{
+    #[inline(always)]
+    fn layout(&self) -> TyLayout<'tcx> {
+        self.op.layout
+    }
+
+    fn downcast_enum(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>)
+        -> EvalResult<'tcx>
+    {
+        let variant = match ectx.read_discriminant(self.op) {
+            Ok(res) => res.1,
+            Err(err) => return match err.kind {
+                EvalErrorKind::InvalidDiscriminant(val) =>
+                    validation_failure!(
+                        format!("invalid enum discriminant {}", val), self.path
+                    ),
+                _ =>
+                    validation_failure!(
+                        format!("non-integer enum discriminant"), self.path
+                    ),
+            }
+        };
+        // Put the variant projection onto the path, as a field
+        self.path.push(PathElem::Field(self.op.layout.ty
+                                    .ty_adt_def()
+                                    .unwrap()
+                                    .variants[variant].name));
+        // Proceed with this variant
+        self.op = ectx.operand_downcast(self.op, variant)?;
+        Ok(())
+    }
+
+    fn downcast_dyn_trait(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>)
+        -> EvalResult<'tcx>
+    {
+        // FIXME: Should we reflect this in `self.path`?
+        let dest = self.op.to_mem_place(); // immediate trait objects are not a thing
+        self.op = ectx.unpack_dyn_trait(dest)?.1.into();
+        Ok(())
+    }
+
+    fn visit_primitive(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>)
+        -> EvalResult<'tcx>
+    {
+        let value = try_validation!(ectx.read_immediate(self.op),
+            "uninitialized or unrepresentable data", self.path);
         // Go over all the primitive types
         let ty = value.layout.ty;
         match ty.sty {
             ty::Bool => {
                 let value = value.to_scalar_or_undef();
                 try_validation!(value.to_bool(),
-                    scalar_format(value), path, "a boolean");
+                    scalar_format(value), self.path, "a boolean");
             },
             ty::Char => {
                 let value = value.to_scalar_or_undef();
                 try_validation!(value.to_char(),
-                    scalar_format(value), path, "a valid unicode codepoint");
+                    scalar_format(value), self.path, "a valid unicode codepoint");
             },
             ty::Float(_) | ty::Int(_) | ty::Uint(_) => {
                 // NOTE: Keep this in sync with the array optimization for int/float
                 // types below!
                 let size = value.layout.size;
                 let value = value.to_scalar_or_undef();
-                if const_mode {
+                if self.const_mode {
                     // Integers/floats in CTFE: Must be scalar bits, pointers are dangerous
                     try_validation!(value.to_bits(size),
-                        scalar_format(value), path, "initialized plain bits");
+                        scalar_format(value), self.path, "initialized plain bits");
                 } else {
                     // At run-time, for now, we accept *anything* for these types, including
                     // undef. We should fix that, but let's start low.
@@ -180,33 +272,33 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                 // 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);
+                    "undefined address in pointer", self.path);
                 let _meta = try_validation!(value.to_meta(),
-                    "uninitialized data in fat pointer metadata", path);
+                    "uninitialized data in fat pointer metadata", self.path);
             }
             _ if ty.is_box() || ty.is_region_ptr() => {
                 // Handle fat pointers.
                 // Check metadata early, for better diagnostics
                 let ptr = try_validation!(value.to_scalar_ptr(),
-                    "undefined address in pointer", path);
+                    "undefined address in pointer", self.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)?;
+                    "uninitialized data in fat pointer metadata", self.path);
+                let layout = ectx.layout_of(value.layout.ty.builtin_deref(true).unwrap().ty)?;
                 if layout.is_unsized() {
-                    let tail = self.tcx.struct_tail(layout.ty);
+                    let tail = ectx.tcx.struct_tail(layout.ty);
                     match tail.sty {
                         ty::Dynamic(..) => {
                             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);
-                            try_validation!(self.read_size_and_align_from_vtable(vtable),
-                                "invalid size or align in vtable", path);
+                                "non-pointer vtable in fat pointer", self.path);
+                            try_validation!(ectx.read_drop_type_from_vtable(vtable),
+                                "invalid drop fn in vtable", self.path);
+                            try_validation!(ectx.read_size_and_align_from_vtable(vtable),
+                                "invalid size or align in vtable", self.path);
                             // FIXME: More checks for the vtable.
                         }
                         ty::Slice(..) | ty::Str => {
-                            try_validation!(meta.unwrap().to_usize(self),
-                                "non-integer slice length in fat pointer", path);
+                            try_validation!(meta.unwrap().to_usize(ectx),
+                                "non-integer slice length in fat pointer", self.path);
                         }
                         ty::Foreign(..) => {
                             // Unsized, but not fat.
@@ -216,25 +308,25 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                     }
                 }
                 // Make sure this is non-NULL and aligned
-                let (size, align) = self.size_and_align_of(meta, layout)?
+                let (size, align) = ectx.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) {
+                match ectx.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),
+                                return validation_failure!("NULL reference", self.path),
                             EvalErrorKind::AlignmentCheckFailed { .. } =>
-                                return validation_failure!("unaligned reference", path),
+                                return validation_failure!("unaligned reference", self.path),
                             _ =>
                                 return validation_failure!(
                                     "dangling (out-of-bounds) reference (might be NULL at \
                                         run-time)",
-                                    path
+                                    self.path
                                 ),
                         }
                     }
@@ -242,29 +334,29 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                 // 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)?;
+                let place = ectx.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 let Some(ref mut ref_tracking) = self.ref_tracking {
+                    assert!(self.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);
+                            "integer pointer in non-ZST reference", self.path);
                         // Skip validation entirely for some external statics
-                        let alloc_kind = self.tcx.alloc_map.lock().get(ptr.alloc_id);
+                        let alloc_kind = ectx.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) {
+                            if !did.is_local() || ectx.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);
+                        try_validation!(ectx.memory.check_bounds(ptr, size, false),
+                            "dangling (not entirely in bounds) reference", self.path);
                     }
                     // Check if we have encountered this pointer+layout combination
                     // before.  Proceed recursively even for integer pointers, no
@@ -273,16 +365,16 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                     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)));
+                        ref_tracking.todo.push((op, path_clone_and_deref(&self.path)));
                     }
                 }
             }
             ty::FnPtr(_sig) => {
                 let value = value.to_scalar_or_undef();
                 let ptr = try_validation!(value.to_ptr(),
-                    scalar_format(value), path, "a pointer");
-                let _fn = try_validation!(self.memory.get_fn(ptr),
-                    scalar_format(value), path, "a function pointer");
+                    scalar_format(value), self.path, "a pointer");
+                let _fn = try_validation!(ectx.memory.get_fn(ptr),
+                    scalar_format(value), self.path, "a function pointer");
                 // FIXME: Check if the signature matches
             }
             // This should be all the primitive types
@@ -292,16 +384,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         Ok(())
     }
 
-    /// Make sure that `value` matches the
-    fn validate_scalar_layout(
-        &self,
-        value: ScalarMaybeUndef<M::PointerTag>,
-        size: Size,
-        path: &Vec<PathElem>,
-        layout: &layout::Scalar,
-    ) -> EvalResult<'tcx> {
+    fn visit_scalar(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, layout: &layout::Scalar)
+        -> EvalResult<'tcx>
+    {
+        let value = try_validation!(ectx.read_scalar(self.op),
+            "uninitialized or unrepresentable data", self.path);
+        // Determine the allowed range
         let (lo, hi) = layout.valid_range.clone().into_inner();
-        let max_hi = u128::max_value() >> (128 - size.bits()); // as big as the size fits
+        // `max_hi` is as big as the size fits
+        let max_hi = u128::max_value() >> (128 - self.op.layout.size.bits());
         assert!(hi <= max_hi);
         // We could also write `(hi + 1) % (max_hi + 1) == lo` but `max_hi + 1` overflows for `u128`
         if (lo == 0 && hi == max_hi) || (hi + 1 == lo) {
@@ -310,7 +401,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         }
         // At least one value is excluded. Get the bits.
         let value = try_validation!(value.not_undef(),
-            scalar_format(value), path, format!("something in the range {:?}", layout.valid_range));
+            scalar_format(value), self.path,
+            format!("something in the range {:?}", layout.valid_range));
         let bits = match value {
             Scalar::Ptr(ptr) => {
                 if lo == 1 && hi == max_hi {
@@ -318,13 +410,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                     // We can call `check_align` to check non-NULL-ness, but have to also look
                     // for function pointers.
                     let non_null =
-                        self.memory.check_align(
+                        ectx.memory.check_align(
                             Scalar::Ptr(ptr), Align::from_bytes(1, 1).unwrap()
                         ).is_ok() ||
-                        self.memory.get_fn(ptr).is_ok();
+                        ectx.memory.get_fn(ptr).is_ok();
                     if !non_null {
                         // could be NULL
-                        return validation_failure!("a potentially NULL pointer", path);
+                        return validation_failure!("a potentially NULL pointer", self.path);
                     }
                     return Ok(());
                 } else {
@@ -332,7 +424,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                     // value.
                     return validation_failure!(
                         "a pointer",
-                        path,
+                        self.path,
                         format!(
                             "something that cannot possibly be outside the (wrapping) range {:?}",
                             layout.valid_range
@@ -340,8 +432,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                     );
                 }
             }
-            Scalar::Bits { bits, size: value_size } => {
-                assert_eq!(value_size as u64, size.bytes());
+            Scalar::Bits { bits, size } => {
+                assert_eq!(size as u64, self.op.layout.size.bytes());
                 bits
             }
         };
@@ -355,7 +447,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
             } else {
                 validation_failure!(
                     bits,
-                    path,
+                    self.path,
                     format!("something in the range {:?} or {:?}", 0..=hi, lo..=max_hi)
                 )
             }
@@ -365,7 +457,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
             } else {
                 validation_failure!(
                     bits,
-                    path,
+                    self.path,
                     if hi == max_hi {
                         format!("something greater or equal to {}", lo)
                     } else {
@@ -376,250 +468,147 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         }
     }
 
-    /// This function checks the data at `op`.  `op` is assumed to cover valid memory if it
-    /// is an indirect operand.
-    /// It will error if the bits at the destination do not match the ones described by the layout.
-    /// The `path` may be pushed to, but the part that is present when the function
-    /// starts must not be changed!
-    ///
-    /// `ref_tracking` can be None to avoid recursive checking below references.
-    /// This also toggles between "run-time" (no recursion) and "compile-time" (with recursion)
-    /// validation (e.g., pointer values are fine in integers at runtime).
-    pub fn validate_operand(
-        &self,
-        dest: OpTy<'tcx, M::PointerTag>,
-        path: &mut Vec<PathElem>,
-        mut ref_tracking: Option<&mut RefTracking<'tcx, M::PointerTag>>,
-        const_mode: bool,
-    ) -> EvalResult<'tcx> {
-        trace!("validate_operand: {:?}, {:?}", *dest, dest.layout.ty);
-
-        // If this is a multi-variant layout, we have find the right one and proceed with that.
-        // (No good reasoning to make this recursion, but it is equivalent to that.)
-        let dest = match dest.layout.variants {
-            layout::Variants::NicheFilling { .. } |
-            layout::Variants::Tagged { .. } => {
-                let variant = match self.read_discriminant(dest) {
-                    Ok(res) => res.1,
-                    Err(err) => match err.kind {
-                        EvalErrorKind::InvalidDiscriminant(val) =>
-                            return validation_failure!(
-                                format!("invalid enum discriminant {}", val), path
-                            ),
-                        _ =>
-                            return validation_failure!(
-                                String::from("non-integer enum discriminant"), path
-                            ),
-                    }
-                };
-                // Put the variant projection onto the path, as a field
-                path.push(PathElem::Field(dest.layout.ty
-                                          .ty_adt_def()
-                                          .unwrap()
-                                          .variants[variant].name));
-                // Proceed with this variant
-                let dest = self.operand_downcast(dest, variant)?;
-                trace!("variant layout: {:#?}", dest.layout);
-                dest
-            },
-            layout::Variants::Single { .. } => dest,
-        };
-
-        // First thing, find the real type:
-        // If it is a trait object, switch to the actual type that was used to create it.
-        let dest = match dest.layout.ty.sty {
-            ty::Dynamic(..) => {
-                let dest = dest.to_mem_place(); // immediate trait objects are not a thing
-                self.unpack_dyn_trait(dest)?.1.into()
-            },
-            _ => dest
-        };
-
-        // If this is a scalar, validate the scalar layout.
-        // Things can be aggregates and have scalar layout at the same time, and that
-        // is very relevant for `NonNull` and similar structs: We need to validate them
-        // at their scalar layout *before* descending into their fields.
-        // FIXME: We could avoid some redundant checks here. For newtypes wrapping
-        // scalars, we do the same check on every "level" (e.g. first we check
-        // MyNewtype and then the scalar in there).
-        match dest.layout.abi {
-            layout::Abi::Uninhabited =>
-                return validation_failure!("a value of an uninhabited type", path),
-            layout::Abi::Scalar(ref layout) => {
-                let value = try_validation!(self.read_scalar(dest),
-                            "uninitialized or unrepresentable data", path);
-                self.validate_scalar_layout(value, dest.layout.size, &path, layout)?;
-            }
-            // FIXME: Should we do something for ScalarPair? Vector?
-            _ => {}
+    fn visit_fields(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, num_fields: usize)
+        -> EvalResult<'tcx>
+    {
+        // Remember some stuff that will change for the recursive calls
+        let op = self.op;
+        let path_len = self.path.len();
+        // Go look at all the fields
+        for i in 0..num_fields {
+            // Adapt our state
+            self.op = ectx.operand_field(op, i as u64)?;
+            self.path.push(aggregate_field_path_elem(op.layout, i, *ectx.tcx));
+            // Recursive visit
+            ectx.visit_value(self)?;
+            // Restore original state
+            self.op = op;
+            self.path.truncate(path_len);
         }
+        Ok(())
+    }
 
-        // Check primitive types.  We do this after checking the scalar layout,
-        // just to have that done as well.  Primitives can have varying layout,
-        // so we check them separately and before aggregate handling.
-        // It is CRITICAL that we get this check right, or we might be
-        // validating the wrong thing!
-        let primitive = match dest.layout.fields {
-            // Primitives appear as Union with 0 fields -- except for fat pointers.
-            layout::FieldPlacement::Union(0) => true,
-            _ => dest.layout.ty.builtin_deref(true).is_some(),
-        };
-        if primitive {
-            let value = try_validation!(self.read_immediate(dest),
-                "uninitialized or unrepresentable data", path);
-            return self.validate_primitive_type(
-                value,
-                &path,
-                ref_tracking,
-                const_mode,
-            );
-        }
+    fn visit_str(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>)
+        -> EvalResult<'tcx>
+    {
+        let mplace = self.op.to_mem_place(); // strings are never immediate
+        try_validation!(ectx.read_str(mplace),
+            "uninitialized or non-UTF-8 data in str", self.path);
+        Ok(())
+    }
 
-        // Validate all fields of compound data structures
-        let path_len = path.len(); // Remember the length, in case we need to truncate
-        match dest.layout.fields {
-            layout::FieldPlacement::Union(fields) => {
-                // Empty unions are not accepted by rustc. That's great, it means we can
-                // use that as an unambiguous signal for detecting primitives.  Make sure
-                // we did not miss any primitive.
-                debug_assert!(fields > 0);
-                // We can't check unions, their bits are allowed to be anything.
-                // The fields don't need to correspond to any bit pattern of the union's fields.
-                // See https://github.com/rust-lang/rust/issues/32836#issuecomment-406875389
-            },
-            layout::FieldPlacement::Arbitrary { ref offsets, .. } => {
-                // Go look at all the fields
-                for i in 0..offsets.len() {
-                    let field = self.operand_field(dest, i as u64)?;
-                    path.push(self.aggregate_field_path_elem(dest.layout, i));
-                    self.validate_operand(
-                        field,
-                        path,
-                        ref_tracking.as_mut().map(|r| &mut **r),
-                        const_mode,
-                    )?;
-                    path.truncate(path_len);
+    fn visit_array(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx>
+    {
+        let mplace = if self.op.layout.is_zst() {
+            // it's a ZST, the memory content cannot matter
+            MPlaceTy::dangling(self.op.layout, ectx)
+        } else {
+            // non-ZST array/slice/str cannot be immediate
+            self.op.to_mem_place()
+        };
+        match self.op.layout.ty.sty {
+            ty::Str => bug!("Strings should be handled separately"),
+            // Special handling for arrays/slices of builtin integer types
+            ty::Array(tys, ..) | ty::Slice(tys) if {
+                // This optimization applies only for integer and floating point types
+                // (i.e., types that can hold arbitrary bytes).
+                match tys.sty {
+                    ty::Int(..) | ty::Uint(..) | ty::Float(..) => true,
+                    _ => false,
                 }
-            }
-            layout::FieldPlacement::Array { stride, .. } => {
-                let dest = if dest.layout.is_zst() {
-                    // it's a ZST, the memory content cannot matter
-                    MPlaceTy::dangling(dest.layout, self)
-                } else {
-                    // non-ZST array/slice/str cannot be immediate
-                    dest.to_mem_place()
-                };
-                match dest.layout.ty.sty {
-                    // Special handling for strings to verify UTF-8
-                    ty::Str => {
-                        try_validation!(self.read_str(dest),
-                            "uninitialized or non-UTF-8 data in str", path);
-                    }
-                    // Special handling for arrays/slices of builtin integer types
-                    ty::Array(tys, ..) | ty::Slice(tys) if {
-                        // This optimization applies only for integer and floating point types
-                        // (i.e., types that can hold arbitrary bytes).
-                        match tys.sty {
-                            ty::Int(..) | ty::Uint(..) | ty::Float(..) => true,
-                            _ => false,
-                        }
-                    } => {
-                        // This is the length of the array/slice.
-                        let len = dest.len(self)?;
-                        // Since primitive types are naturally aligned and tightly packed in arrays,
-                        // we can use the stride to get the size of the integral type.
-                        let ty_size = stride.bytes();
-                        // This is the size in bytes of the whole array.
-                        let size = Size::from_bytes(ty_size * len);
-
-                        // NOTE: Keep this in sync with the handling of integer and float
-                        // types above, in `validate_primitive_type`.
-                        // In run-time mode, we accept pointers in here.  This is actually more
-                        // permissive than a per-element check would be, e.g. we accept
-                        // an &[u8] that contains a pointer even though bytewise checking would
-                        // reject it.  However, that's good: We don't inherently want
-                        // to reject those pointers, we just do not have the machinery to
-                        // talk about parts of a pointer.
-                        // We also accept undef, for consistency with the type-based checks.
-                        match self.memory.check_bytes(
-                            dest.ptr,
-                            size,
-                            /*allow_ptr_and_undef*/!const_mode,
-                        ) {
-                            // In the happy case, we needn't check anything else.
-                            Ok(()) => {},
-                            // Some error happened, try to provide a more detailed description.
-                            Err(err) => {
-                                // For some errors we might be able to provide extra information
-                                match err.kind {
-                                    EvalErrorKind::ReadUndefBytes(offset) => {
-                                        // Some byte was undefined, determine which
-                                        // element that byte belongs to so we can
-                                        // provide an index.
-                                        let i = (offset.bytes() / ty_size) as usize;
-                                        path.push(PathElem::ArrayElem(i));
-
-                                        return validation_failure!(
-                                            "undefined bytes", path
-                                        )
-                                    },
-                                    // Other errors shouldn't be possible
-                                    _ => return Err(err),
-                                }
-                            }
-                        }
-                    },
-                    _ => {
-                        // This handles the unsized case correctly as well, as well as
-                        // SIMD an all sorts of other array-like types.
-                        for (i, field) in self.mplace_array_fields(dest)?.enumerate() {
-                            let field = field?;
-                            path.push(PathElem::ArrayElem(i));
-                            self.validate_operand(
-                                field.into(),
-                                path,
-                                ref_tracking.as_mut().map(|r| &mut **r),
-                                const_mode,
-                            )?;
-                            path.truncate(path_len);
+            } => {
+                // This is the length of the array/slice.
+                let len = mplace.len(ectx)?;
+                // This is the element type size.
+                let ty_size = ectx.layout_of(tys)?.size;
+                // This is the size in bytes of the whole array.
+                let size = ty_size * len;
+
+                // NOTE: Keep this in sync with the handling of integer and float
+                // types above, in `visit_primitive`.
+                // In run-time mode, we accept pointers in here.  This is actually more
+                // permissive than a per-element check would be, e.g. we accept
+                // an &[u8] that contains a pointer even though bytewise checking would
+                // reject it.  However, that's good: We don't inherently want
+                // to reject those pointers, we just do not have the machinery to
+                // talk about parts of a pointer.
+                // We also accept undef, for consistency with the type-based checks.
+                match ectx.memory.check_bytes(
+                    mplace.ptr,
+                    size,
+                    /*allow_ptr_and_undef*/!self.const_mode,
+                ) {
+                    // In the happy case, we needn't check anything else.
+                    Ok(()) => {},
+                    // Some error happened, try to provide a more detailed description.
+                    Err(err) => {
+                        // For some errors we might be able to provide extra information
+                        match err.kind {
+                            EvalErrorKind::ReadUndefBytes(offset) => {
+                                // Some byte was undefined, determine which
+                                // element that byte belongs to so we can
+                                // provide an index.
+                                let i = (offset.bytes() / ty_size.bytes()) as usize;
+                                self.path.push(PathElem::ArrayElem(i));
+
+                                return validation_failure!(
+                                    "undefined bytes", self.path
+                                )
+                            },
+                            // Other errors shouldn't be possible
+                            _ => return Err(err),
                         }
                     }
                 }
             },
+            _ => {
+                // Remember some stuff that will change for the recursive calls
+                let op = self.op;
+                let path_len = self.path.len();
+                // This handles the unsized case correctly as well, as well as
+                // SIMD and all sorts of other array-like types.
+                for (i, field) in ectx.mplace_array_fields(mplace)?.enumerate() {
+                    // Adapt our state
+                    self.op = field?.into();
+                    self.path.push(PathElem::ArrayElem(i));
+                    // Recursive visit
+                    ectx.visit_value(self)?;
+                    // Restore original state
+                    self.op = op;
+                    self.path.truncate(path_len);
+                }
+            }
         }
         Ok(())
     }
+}
 
-    fn aggregate_field_path_elem(&self, layout: TyLayout<'tcx>, field: usize) -> PathElem {
-        match layout.ty.sty {
-            // generators and closures.
-            ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => {
-                if let Some(upvar) = self.tcx.optimized_mir(def_id).upvar_decls.get(field) {
-                    PathElem::ClosureVar(upvar.debug_name)
-                } else {
-                    // Sometimes the index is beyond the number of freevars (seen
-                    // for a generator).
-                    PathElem::ClosureVar(Symbol::intern(&field.to_string()))
-                }
-            }
-
-            // tuples
-            ty::Tuple(_) => PathElem::TupleElem(field),
-
-            // enums
-            ty::Adt(def, ..) if def.is_enum() => {
-                let variant = match layout.variants {
-                    layout::Variants::Single { index } => &def.variants[index],
-                    _ => bug!("aggregate_field_path_elem: got enum but not in a specific variant"),
-                };
-                PathElem::Field(variant.fields[field].ident.name)
-            }
+impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
+    /// This function checks the data at `op`.  `op` is assumed to cover valid memory if it
+    /// is an indirect operand.
+    /// It will error if the bits at the destination do not match the ones described by the layout.
+    ///
+    /// `ref_tracking` can be None to avoid recursive checking below references.
+    /// This also toggles between "run-time" (no recursion) and "compile-time" (with recursion)
+    /// validation (e.g., pointer values are fine in integers at runtime).
+    pub fn validate_operand(
+        &mut self,
+        op: OpTy<'tcx, M::PointerTag>,
+        path: Vec<PathElem>,
+        ref_tracking: Option<&mut RefTracking<'tcx, M::PointerTag>>,
+        const_mode: bool,
+    ) -> EvalResult<'tcx> {
+        trace!("validate_operand: {:?}, {:?}", *op, op.layout.ty);
 
-            // other ADTs
-            ty::Adt(def, _) => PathElem::Field(def.non_enum_variant().fields[field].ident.name),
+        // Construct a visitor
+        let mut visitor = ValidityVisitor {
+            op,
+            path,
+            ref_tracking,
+            const_mode
+        };
 
-            // nothing else has an aggregate layout
-            _ => bug!("aggregate_field_path_elem: got non-aggregate type {:?}", layout.ty),
-        }
+        // Run it
+        self.visit_value(&mut visitor)
     }
 }
diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs
new file mode 100644
index 00000000000..892eaceff1b
--- /dev/null
+++ b/src/librustc_mir/interpret/visitor.rs
@@ -0,0 +1,125 @@
+//! Visitor for a run-time value with a given layout: Traverse enums, structs and other compound
+//! types until we arrive at the leaves, with custom handling for primitive types.
+
+use std::fmt;
+
+use rustc::ty::layout::{self, TyLayout};
+use rustc::ty;
+use rustc::mir::interpret::{
+    EvalResult,
+};
+
+use super::{
+    Machine, EvalContext,
+};
+
+// How to traverse a value and what to do when we are at the leaves.
+// In the future, we might want to turn this into two traits, but so far the
+// only implementations we have couldn't share any code anyway.
+pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug {
+    // Get this value's layout.
+    fn layout(&self) -> TyLayout<'tcx>;
+
+    // Downcast functions.  These change the value as a side-effect.
+    fn downcast_enum(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>)
+        -> EvalResult<'tcx>;
+    fn downcast_dyn_trait(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>)
+        -> EvalResult<'tcx>;
+
+    // Visit all fields of a compound.
+    // Just call `visit_value` if you want to go on recursively.
+    fn visit_fields(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, num_fields: usize)
+        -> EvalResult<'tcx>;
+    // Optimized handling for arrays -- avoid computing the layout for every field.
+    // Also it is the value's responsibility to figure out the length.
+    fn visit_array(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx>;
+    // Special handling for strings.
+    fn visit_str(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>)
+        -> EvalResult<'tcx>;
+
+    // Actions on the leaves.
+    fn visit_scalar(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, layout: &layout::Scalar)
+        -> EvalResult<'tcx>;
+    fn visit_primitive(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>)
+        -> EvalResult<'tcx>;
+}
+
+impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
+    pub fn visit_value<V: ValueVisitor<'a, 'mir, 'tcx, M>>(&mut self, v: &mut V) -> EvalResult<'tcx> {
+        trace!("visit_value: {:?}", v);
+
+        // If this is a multi-variant layout, we have find the right one and proceed with that.
+        // (No benefit from making this recursion, but it is equivalent to that.)
+        match v.layout().variants {
+            layout::Variants::NicheFilling { .. } |
+            layout::Variants::Tagged { .. } => {
+                v.downcast_enum(self)?;
+                trace!("variant layout: {:#?}", v.layout());
+            }
+            layout::Variants::Single { .. } => {}
+        }
+
+        // Even for single variants, we might be able to get a more refined type:
+        // If it is a trait object, switch to the actual type that was used to create it.
+        match v.layout().ty.sty {
+            ty::Dynamic(..) => {
+                v.downcast_dyn_trait(self)?;
+            },
+            _ => {},
+        };
+
+        // If this is a scalar, visit it as such.
+        // Things can be aggregates and have scalar layout at the same time, and that
+        // is very relevant for `NonNull` and similar structs: We need to visit them
+        // at their scalar layout *before* descending into their fields.
+        // FIXME: We could avoid some redundant checks here. For newtypes wrapping
+        // scalars, we do the same check on every "level" (e.g. first we check
+        // MyNewtype and then the scalar in there).
+        match v.layout().abi {
+            layout::Abi::Scalar(ref layout) => {
+                v.visit_scalar(self, layout)?;
+            }
+            // FIXME: Should we do something for ScalarPair? Vector?
+            _ => {}
+        }
+
+        // Check primitive types.  We do this after checking the scalar layout,
+        // just to have that done as well.  Primitives can have varying layout,
+        // so we check them separately and before aggregate handling.
+        // It is CRITICAL that we get this check right, or we might be
+        // validating the wrong thing!
+        let primitive = match v.layout().fields {
+            // Primitives appear as Union with 0 fields -- except for Boxes and fat pointers.
+            layout::FieldPlacement::Union(0) => true,
+            _ => v.layout().ty.builtin_deref(true).is_some(),
+        };
+        if primitive {
+            return v.visit_primitive(self);
+        }
+
+        // Proceed into the fields.
+        match v.layout().fields {
+            layout::FieldPlacement::Union(fields) => {
+                // Empty unions are not accepted by rustc. That's great, it means we can
+                // use that as an unambiguous signal for detecting primitives.  Make sure
+                // we did not miss any primitive.
+                debug_assert!(fields > 0);
+                // We can't traverse unions, their bits are allowed to be anything.
+                // The fields don't need to correspond to any bit pattern of the union's fields.
+                // See https://github.com/rust-lang/rust/issues/32836#issuecomment-406875389
+                Ok(())
+            },
+            layout::FieldPlacement::Arbitrary { ref offsets, .. } => {
+                v.visit_fields(self, offsets.len())
+            },
+            layout::FieldPlacement::Array { .. } => {
+                match v.layout().ty.sty {
+                    // Strings have properties that cannot be expressed pointwise.
+                    ty::Str => v.visit_str(self),
+                    // General case -- might also be SIMD vector or so
+                    _ => v.visit_array(self),
+                }
+            }
+        }
+    }
+}