Auto merge of #55549 - RalfJung:miri-visitor, r=<try>

Value visitors for miri

Generalize the traversal part of validation to a `ValueVisitor`.

~~This includes #55316, [click here](RalfJung/rust@retagging...RalfJung:miri-visitor) for just the new commits.~~

This PR does not change the interface to miri, so I use the opportunity to update miri.

src/librustc/mir/interpret/error.rs

@@ -15,7 +15,7 @@ use ty::{Ty, layout};
 use ty::layout::{Size, Align, LayoutError};
 use rustc_target::spec::abi::Abi;
 
-use super::Pointer;
+use super::{Pointer, Scalar};
 
 use backtrace::Backtrace;
 
@@ -240,7 +240,7 @@ pub enum EvalErrorKind<'tcx, O> {
     InvalidMemoryAccess,
     InvalidFunctionPointer,
     InvalidBool,
-    InvalidDiscriminant(u128),
+    InvalidDiscriminant(Scalar),
     PointerOutOfBounds {
         ptr: Pointer,
         access: bool,

src/librustc/mir/interpret/value.rs

@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#![allow(unknown_lints)]
+use std::fmt;
 
 use ty::layout::{HasDataLayout, Size};
 use ty::subst::Substs;
@@ -99,6 +99,15 @@ pub enum Scalar<Tag=(), Id=AllocId> {
     Ptr(Pointer<Tag, Id>),
 }
 
+impl<Tag> fmt::Display for Scalar<Tag> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Scalar::Ptr(_) => write!(f, "a pointer"),
+            Scalar::Bits { bits, .. } => write!(f, "{}", bits),
+        }
+    }
+}
+
 impl<'tcx> Scalar<()> {
     #[inline]
     pub fn with_default_tag<Tag>(self) -> Scalar<Tag>

src/librustc_mir/const_eval.rs

@@ -539,10 +539,10 @@ fn validate_const<'a, 'tcx>(
     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,
             )?;

src/librustc_mir/interpret/eval_context.rs

@@ -139,25 +139,34 @@ impl<'tcx, Tag> LocalValue<Tag> {
     }
 }
 
-impl<'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout
-    for &'b EvalContext<'a, 'mir, 'tcx, M>
+impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout
+    for &EvalContext<'a, 'mir, 'tcx, M>
 {
     #[inline]
     fn data_layout(&self) -> &layout::TargetDataLayout {
         &self.tcx.data_layout
     }
 }
 
-impl<'c, 'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout
-    for &'c &'b mut EvalContext<'a, 'mir, 'tcx, M>
+impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout
+    for &&EvalContext<'a, 'mir, 'tcx, M>
 {
     #[inline]
     fn data_layout(&self) -> &layout::TargetDataLayout {
         &self.tcx.data_layout
     }
 }
 
-impl<'b, 'a, 'mir, 'tcx, M> layout::HasTyCtxt<'tcx> for &'b EvalContext<'a, 'mir, 'tcx, M>
+impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout
+    for & &mut EvalContext<'a, 'mir, 'tcx, M>
+{
+    #[inline]
+    fn data_layout(&self) -> &layout::TargetDataLayout {
+        &self.tcx.data_layout
+    }
+}
+
+impl<'a, 'mir, 'tcx, M> layout::HasTyCtxt<'tcx> for &EvalContext<'a, 'mir, 'tcx, M>
     where M: Machine<'a, 'mir, 'tcx>
 {
     #[inline]
@@ -166,17 +175,17 @@ impl<'b, 'a, 'mir, 'tcx, M> layout::HasTyCtxt<'tcx> for &'b EvalContext<'a, 'mir
     }
 }
 
-impl<'c, 'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> layout::HasTyCtxt<'tcx>
-    for &'c &'b mut EvalContext<'a, 'mir, 'tcx, M>
+impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> layout::HasTyCtxt<'tcx>
+    for & &mut EvalContext<'a, 'mir, 'tcx, M>
 {
     #[inline]
     fn tcx<'d>(&'d self) -> TyCtxt<'d, 'tcx, 'tcx> {
         *self.tcx
     }
 }
 
-impl<'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf
-    for &'b EvalContext<'a, 'mir, 'tcx, M>
+impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf
+    for &EvalContext<'a, 'mir, 'tcx, M>
 {
     type Ty = Ty<'tcx>;
     type TyLayout = EvalResult<'tcx, TyLayout<'tcx>>;
@@ -188,8 +197,8 @@ impl<'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf
     }
 }
 
-impl<'c, 'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf
-    for &'c &'b mut EvalContext<'a, 'mir, 'tcx, M>
+impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf
+    for & &mut EvalContext<'a, 'mir, 'tcx, M>
 {
     type Ty = Ty<'tcx>;
     type TyLayout = EvalResult<'tcx, TyLayout<'tcx>>;
@@ -551,7 +560,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,
                 )?;

src/librustc_mir/interpret/machine.rs

@@ -17,11 +17,11 @@ use std::hash::Hash;
 
 use rustc::hir::{self, def_id::DefId};
 use rustc::mir;
-use rustc::ty::{self, Ty, layout::{Size, TyLayout}, query::TyCtxtAt};
+use rustc::ty::{self, layout::{Size, TyLayout}, query::TyCtxtAt};
 
 use super::{
     Allocation, AllocId, EvalResult, Scalar,
-    EvalContext, PlaceTy, OpTy, Pointer, MemPlace, MemoryKind,
+    EvalContext, PlaceTy, MPlaceTy, OpTy, Pointer, MemoryKind,
 };
 
 /// Whether this kind of memory is allowed to leak
@@ -217,26 +217,22 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized {
     #[inline]
     fn tag_reference(
         _ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,
-        place: MemPlace<Self::PointerTag>,
-        _ty: Ty<'tcx>,
-        _size: Size,
+        place: MPlaceTy<'tcx, Self::PointerTag>,
         _mutability: Option<hir::Mutability>,
-    ) -> EvalResult<'tcx, MemPlace<Self::PointerTag>> {
-        Ok(place)
+    ) -> EvalResult<'tcx, Scalar<Self::PointerTag>> {
+        Ok(place.ptr)
     }
 
     /// Executed when evaluating the `*` operator: Following a reference.
-    /// This has the change to adjust the tag.  It should not change anything else!
+    /// 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 dereferenced.
     #[inline]
     fn tag_dereference(
         _ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
-        place: MemPlace<Self::PointerTag>,
-        _ty: Ty<'tcx>,
-        _size: Size,
+        place: MPlaceTy<'tcx, Self::PointerTag>,
         _mutability: Option<hir::Mutability>,
-    ) -> EvalResult<'tcx, MemPlace<Self::PointerTag>> {
-        Ok(place)
+    ) -> EvalResult<'tcx, Scalar<Self::PointerTag>> {
+        Ok(place.ptr)
     }
 
     /// Execute a validation operation

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, Value, ValTy, Operand, OpTy};
 
+pub use self::visitor::ValueVisitor;
+
 pub use self::validity::RefTracking;

src/librustc_mir/interpret/operand.rs

@@ -12,6 +12,7 @@
 //! All high-level functions to read from memory work on operands as sources.
 
 use std::convert::TryInto;
+use std::fmt;
 
 use rustc::{mir, ty};
 use rustc::ty::layout::{self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt};
@@ -36,6 +37,15 @@ impl<Tag> From<Scalar<Tag>> for ScalarMaybeUndef<Tag> {
     }
 }
 
+impl<Tag> fmt::Display for ScalarMaybeUndef<Tag> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            ScalarMaybeUndef::Undef => write!(f, "uninitialized bytes"),
+            ScalarMaybeUndef::Scalar(s) => write!(f, "{}", s),
+        }
+    }
+}
+
 impl<'tcx> ScalarMaybeUndef<()> {
     #[inline]
     pub fn with_default_tag<Tag>(self) -> ScalarMaybeUndef<Tag>
@@ -729,8 +739,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         Ok(match rval.layout.variants {
             layout::Variants::Single { .. } => bug!(),
             layout::Variants::Tagged { .. } => {
+                let bits_discr = match raw_discr.to_bits(discr_val.layout.size) {
+                    Ok(raw_discr) => raw_discr,
+                    Err(_) => return err!(InvalidDiscriminant(raw_discr.erase_tag())),
+                };
                 let real_discr = if discr_val.layout.ty.is_signed() {
-                    let i = raw_discr.to_bits(discr_val.layout.size)? as i128;
+                    let i = bits_discr as i128;
                     // going from layout tag type to typeck discriminant type
                     // requires first sign extending with the layout discriminant
                     let shift = 128 - discr_val.layout.size.bits();
@@ -745,15 +759,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                     let truncatee = sexted as u128;
                     (truncatee << shift) >> shift
                 } else {
-                    raw_discr.to_bits(discr_val.layout.size)?
+                    bits_discr
                 };
                 // Make sure we catch invalid discriminants
                 let index = rval.layout.ty
                     .ty_adt_def()
                     .expect("tagged layout for non adt")
                     .discriminants(self.tcx.tcx)
                     .position(|var| var.val == real_discr)
-                    .ok_or_else(|| EvalErrorKind::InvalidDiscriminant(real_discr))?;
+                    .ok_or_else(|| EvalErrorKind::InvalidDiscriminant(raw_discr.erase_tag()))?;
                 (real_discr, index)
             },
             layout::Variants::NicheFilling {

src/librustc_mir/interpret/place.rs

@@ -278,42 +278,34 @@ where
         let meta = val.to_meta()?;
         let ptr = val.to_scalar_ptr()?;
         let mplace = MemPlace { ptr, align, meta };
+        let mut mplace = MPlaceTy { mplace, layout };
         // 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());
+        if M::ENABLE_PTR_TRACKING_HOOKS {
             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 })
+            mplace.mplace.ptr = M::tag_dereference(self, mplace, mutbl)?;
+        }
+        // Done
+        Ok(mplace)
     }
 
     /// Turn a mplace into a (thin or fat) pointer, as a reference, pointing to the same space.
     /// This is the inverse of `ref_to_mplace`.
     /// `mutbl` indicates whether we are create a shared or mutable ref, or a raw pointer (`None`).
     pub fn create_ref(
         &mut self,
-        place: MPlaceTy<'tcx, M::PointerTag>,
+        mut place: MPlaceTy<'tcx, M::PointerTag>,
         mutbl: Option<hir::Mutability>,
     ) -> EvalResult<'tcx, Value<M::PointerTag>> {
         // 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());
-            M::tag_reference(self, *place, place.layout.ty, size, mutbl)?
-        } else {
-            *place
-        };
+        if M::ENABLE_PTR_TRACKING_HOOKS {
+            place.mplace.ptr = M::tag_reference(self, place, mutbl)?
+        }
         Ok(match place.meta {
             None => Value::Scalar(place.ptr.into()),
             Some(meta) => Value::ScalarPair(place.ptr.into(), meta.into()),
@@ -489,6 +481,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 +495,7 @@ where
     }
 
     pub fn place_downcast(
-        &mut self,
+        &self,
         base: PlaceTy<'tcx, M::PointerTag>,
         variant: usize,
     ) -> EvalResult<'tcx, PlaceTy<'tcx, M::PointerTag>> {
@@ -510,7 +504,7 @@ where
             Place::Ptr(mplace) =>
                 self.mplace_downcast(MPlaceTy { mplace, layout: base.layout }, variant)?.into(),
             Place::Local { .. } => {
-                let layout = base.layout.for_variant(&self, variant);
+                let layout = base.layout.for_variant(self, variant);
                 PlaceTy { layout, ..base }
             }
         })
@@ -643,7 +637,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 +759,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 +837,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(())