Auto merge of #67192 - oli-obk:const_zst_addr, r=RalfJung,varkor

Various const eval and pattern matching ICE fixes

r? @RalfJung
cc @spastorino

This PR does not change existing behaviour anymore and just fixes a bunch of ICEs reachable from user code (sometimes even on stable via obscure union transmutes).

Author: bors

src/librustc/mir/interpret/error.rs

@@ -432,6 +432,7 @@ pub enum UnsupportedOpInfo<'tcx> {
     HeapAllocNonPowerOfTwoAlignment(u64),
     ReadFromReturnPointer,
     PathNotFound(Vec<String>),
+    TransmuteSizeDiff(Ty<'tcx>, Ty<'tcx>),
 }
 
 impl fmt::Debug for UnsupportedOpInfo<'tcx> {
@@ -460,6 +461,11 @@ impl fmt::Debug for UnsupportedOpInfo<'tcx> {
                            passing data of type {:?}",
                 callee_ty, caller_ty
             ),
+            TransmuteSizeDiff(from_ty, to_ty) => write!(
+                f,
+                "tried to transmute from {:?} to {:?}, but their sizes differed",
+                from_ty, to_ty
+            ),
             FunctionRetMismatch(caller_ty, callee_ty) => write!(
                 f,
                 "tried to call a function with return type {:?} \

src/librustc/mir/interpret/value.rs

@@ -367,8 +367,9 @@ impl<'tcx, Tag> Scalar<Tag> {
     }
 
     /// Do not call this method!  Use either `assert_ptr` or `force_ptr`.
+    /// This method is intentionally private, do not make it public.
     #[inline]
-    pub fn to_ptr(self) -> InterpResult<'tcx, Pointer<Tag>> {
+    fn to_ptr(self) -> InterpResult<'tcx, Pointer<Tag>> {
         match self {
             Scalar::Raw { data: 0, .. } => throw_unsup!(InvalidNullPointerUsage),
             Scalar::Raw { .. } => throw_unsup!(ReadBytesAsPointer),
@@ -544,12 +545,6 @@ impl<'tcx, Tag> ScalarMaybeUndef<Tag> {
         }
     }
 
-    /// Do not call this method!  Use either `assert_ptr` or `force_ptr`.
-    #[inline(always)]
-    pub fn to_ptr(self) -> InterpResult<'tcx, Pointer<Tag>> {
-        self.not_undef()?.to_ptr()
-    }
-
     /// Do not call this method!  Use either `assert_bits` or `force_bits`.
     #[inline(always)]
     pub fn to_bits(self, target_size: Size) -> InterpResult<'tcx, u128> {

src/librustc/ty/relate.rs

@@ -537,8 +537,8 @@ pub fn super_relate_consts<R: TypeRelation<'tcx>>(
                         Ok(ConstValue::Scalar(a_val))
                     } else if let ty::FnPtr(_) = a.ty.kind {
                         let alloc_map = tcx.alloc_map.lock();
-                        let a_instance = alloc_map.unwrap_fn(a_val.to_ptr().unwrap().alloc_id);
-                        let b_instance = alloc_map.unwrap_fn(b_val.to_ptr().unwrap().alloc_id);
+                        let a_instance = alloc_map.unwrap_fn(a_val.assert_ptr().alloc_id);
+                        let b_instance = alloc_map.unwrap_fn(b_val.assert_ptr().alloc_id);
                         if a_instance == b_instance {
                             Ok(ConstValue::Scalar(a_val))
                         } else {

src/librustc_mir/const_eval/eval_queries.rs

@@ -119,7 +119,7 @@ pub(super) fn op_to_const<'tcx>(
     };
     let val = match immediate {
         Ok(mplace) => {
-            let ptr = mplace.ptr.to_ptr().unwrap();
+            let ptr = mplace.ptr.assert_ptr();
             let alloc = ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id);
             ConstValue::ByRef { alloc, offset: ptr.offset }
         }
@@ -133,7 +133,7 @@ pub(super) fn op_to_const<'tcx>(
                 // comes from a constant so it can happen have `Undef`, because the indirect
                 // memory that was read had undefined bytes.
                 let mplace = op.assert_mem_place();
-                let ptr = mplace.ptr.to_ptr().unwrap();
+                let ptr = mplace.ptr.assert_ptr();
                 let alloc = ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id);
                 ConstValue::ByRef { alloc, offset: ptr.offset }
             }
@@ -176,7 +176,7 @@ fn validate_and_turn_into_const<'tcx>(
         // Statics/promoteds are always `ByRef`, for the rest `op_to_const` decides
         // whether they become immediates.
         if is_static || cid.promoted.is_some() {
-            let ptr = mplace.ptr.to_ptr()?;
+            let ptr = mplace.ptr.assert_ptr();
             Ok(tcx.mk_const(ty::Const {
                 val: ty::ConstKind::Value(ConstValue::ByRef {
                     alloc: ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id),

src/librustc_mir/hair/pattern/_match.rs

@@ -252,6 +252,7 @@ use syntax_pos::{Span, DUMMY_SP};
 use arena::TypedArena;
 
 use smallvec::{smallvec, SmallVec};
+use std::borrow::Cow;
 use std::cmp::{self, max, min, Ordering};
 use std::convert::TryInto;
 use std::fmt;
@@ -260,11 +261,12 @@ use std::ops::RangeInclusive;
 use std::u128;
 
 pub fn expand_pattern<'a, 'tcx>(cx: &MatchCheckCtxt<'a, 'tcx>, pat: Pat<'tcx>) -> Pat<'tcx> {
-    LiteralExpander { tcx: cx.tcx }.fold_pattern(&pat)
+    LiteralExpander { tcx: cx.tcx, param_env: cx.param_env }.fold_pattern(&pat)
 }
 
 struct LiteralExpander<'tcx> {
     tcx: TyCtxt<'tcx>,
+    param_env: ty::ParamEnv<'tcx>,
 }
 
 impl LiteralExpander<'tcx> {
@@ -284,9 +286,23 @@ impl LiteralExpander<'tcx> {
         debug!("fold_const_value_deref {:?} {:?} {:?}", val, rty, crty);
         match (val, &crty.kind, &rty.kind) {
             // the easy case, deref a reference
-            (ConstValue::Scalar(Scalar::Ptr(p)), x, y) if x == y => {
-                let alloc = self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id);
-                ConstValue::ByRef { alloc, offset: p.offset }
+            (ConstValue::Scalar(p), x, y) if x == y => {
+                match p {
+                    Scalar::Ptr(p) => {
+                        let alloc = self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id);
+                        ConstValue::ByRef { alloc, offset: p.offset }
+                    }
+                    Scalar::Raw { .. } => {
+                        let layout = self.tcx.layout_of(self.param_env.and(rty)).unwrap();
+                        if layout.is_zst() {
+                            // Deref of a reference to a ZST is a nop.
+                            ConstValue::Scalar(Scalar::zst())
+                        } else {
+                            // FIXME(oli-obk): this is reachable for `const FOO: &&&u32 = &&&42;`
+                            bug!("cannot deref {:#?}, {} -> {}", val, crty, rty);
+                        }
+                    }
+                }
             }
             // unsize array to slice if pattern is array but match value or other patterns are slice
             (ConstValue::Scalar(Scalar::Ptr(p)), ty::Array(t, n), ty::Slice(u)) => {
@@ -2348,16 +2364,30 @@ fn specialize_one_pattern<'p, 'tcx>(
             // just integers. The only time they should be pointing to memory
             // is when they are subslices of nonzero slices.
             let (alloc, offset, n, ty) = match value.ty.kind {
-                ty::Array(t, n) => match value.val {
-                    ty::ConstKind::Value(ConstValue::ByRef { offset, alloc, .. }) => {
-                        (alloc, offset, n.eval_usize(cx.tcx, cx.param_env), t)
+                ty::Array(t, n) => {
+                    let n = n.eval_usize(cx.tcx, cx.param_env);
+                    // Shortcut for `n == 0` where no matter what `alloc` and `offset` we produce,
+                    // the result would be exactly what we early return here.
+                    if n == 0 {
+                        if ctor_wild_subpatterns.len() as u64 == 0 {
+                            return Some(PatStack::from_slice(&[]));
+                        } else {
+                            return None;
+                        }
                     }
-                    _ => span_bug!(pat.span, "array pattern is {:?}", value,),
-                },
+                    match value.val {
+                        ty::ConstKind::Value(ConstValue::ByRef { offset, alloc, .. }) => {
+                            (Cow::Borrowed(alloc), offset, n, t)
+                        }
+                        _ => span_bug!(pat.span, "array pattern is {:?}", value,),
+                    }
+                }
                 ty::Slice(t) => {
                     match value.val {
                         ty::ConstKind::Value(ConstValue::Slice { data, start, end }) => {
-                            (data, Size::from_bytes(start as u64), (end - start) as u64, t)
+                            let offset = Size::from_bytes(start as u64);
+                            let n = (end - start) as u64;
+                            (Cow::Borrowed(data), offset, n, t)
                         }
                         ty::ConstKind::Value(ConstValue::ByRef { .. }) => {
                             // FIXME(oli-obk): implement `deref` for `ConstValue`

src/librustc_mir/hair/pattern/mod.rs

@@ -992,6 +992,12 @@ pub fn compare_const_vals<'tcx>(
         return fallback();
     }
 
+    // Early return for equal constants (so e.g. references to ZSTs can be compared, even if they
+    // are just integer addresses).
+    if a.val == b.val {
+        return from_bool(true);
+    }
+
     let a_bits = a.try_eval_bits(tcx, param_env, ty);
     let b_bits = b.try_eval_bits(tcx, param_env, ty);
 

src/librustc_mir/interpret/eval_context.rs

@@ -20,8 +20,8 @@ use rustc_macros::HashStable;
 use syntax::source_map::{self, Span, DUMMY_SP};
 
 use super::{
-    Immediate, MPlaceTy, Machine, MemPlace, Memory, Operand, Place, PlaceTy, ScalarMaybeUndef,
-    StackPopInfo,
+    Immediate, MPlaceTy, Machine, MemPlace, Memory, OpTy, Operand, Place, PlaceTy,
+    ScalarMaybeUndef, StackPopInfo,
 };
 
 pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> {
@@ -118,7 +118,7 @@ pub struct LocalState<'tcx, Tag = (), Id = AllocId> {
 }
 
 /// Current value of a local variable
-#[derive(Clone, PartialEq, Eq, Debug, HashStable)] // Miri debug-prints these
+#[derive(Copy, Clone, PartialEq, Eq, Debug, HashStable)] // Miri debug-prints these
 pub enum LocalValue<Tag = (), Id = AllocId> {
     /// This local is not currently alive, and cannot be used at all.
     Dead,
@@ -743,7 +743,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // FIXME: should we tell the user that there was a local which was never written to?
         if let LocalValue::Live(Operand::Indirect(MemPlace { ptr, .. })) = local {
             trace!("deallocating local");
-            let ptr = ptr.to_ptr()?;
+            // All locals have a backing allocation, even if the allocation is empty
+            // due to the local having ZST type.
+            let ptr = ptr.assert_ptr();
             if log_enabled!(::log::Level::Trace) {
                 self.memory.dump_alloc(ptr.alloc_id);
             }
@@ -752,13 +754,33 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         Ok(())
     }
 
+    pub(super) fn const_eval(
+        &self,
+        gid: GlobalId<'tcx>,
+    ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
+        let val = if self.tcx.is_static(gid.instance.def_id()) {
+            self.tcx.const_eval_poly(gid.instance.def_id())?
+        } else if let Some(promoted) = gid.promoted {
+            self.tcx.const_eval_promoted(gid.instance, promoted)?
+        } else {
+            self.tcx.const_eval_instance(self.param_env, gid.instance, Some(self.tcx.span))?
+        };
+        // Even though `ecx.const_eval` is called from `eval_const_to_op` we can never have a
+        // recursion deeper than one level, because the `tcx.const_eval` above is guaranteed to not
+        // return `ConstValue::Unevaluated`, which is the only way that `eval_const_to_op` will call
+        // `ecx.const_eval`.
+        self.eval_const_to_op(val, None)
+    }
+
     pub fn const_eval_raw(
         &self,
         gid: GlobalId<'tcx>,
     ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
-        // FIXME(oli-obk): make this check an assertion that it's not a static here
-        // FIXME(RalfJ, oli-obk): document that `Place::Static` can never be anything but a static
-        // and `ConstValue::Unevaluated` can never be a static
+        // For statics we pick `ParamEnv::reveal_all`, because statics don't have generics
+        // and thus don't care about the parameter environment. While we could just use
+        // `self.param_env`, that would mean we invoke the query to evaluate the static
+        // with different parameter environments, thus causing the static to be evaluated
+        // multiple times.
         let param_env = if self.tcx.is_static(gid.instance.def_id()) {
             ty::ParamEnv::reveal_all()
         } else {

src/librustc_mir/interpret/intern.rs

@@ -187,14 +187,21 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx
         if let ty::Ref(_, referenced_ty, mutability) = ty.kind {
             let value = self.ecx.read_immediate(mplace.into())?;
             let mplace = self.ecx.ref_to_mplace(value)?;
-            // Handle trait object vtables
+            // Handle trait object vtables.
             if let ty::Dynamic(..) =
                 self.ecx.tcx.struct_tail_erasing_lifetimes(referenced_ty, self.ecx.param_env).kind
             {
-                if let Ok(vtable) = mplace.meta.unwrap().to_ptr() {
-                    // explitly choose `Immutable` here, since vtables are immutable, even
-                    // if the reference of the fat pointer is mutable
+                // Validation has already errored on an invalid vtable pointer so we can safely not
+                // do anything if this is not a real pointer.
+                if let Scalar::Ptr(vtable) = mplace.meta.unwrap() {
+                    // Explicitly choose `Immutable` here, since vtables are immutable, even
+                    // if the reference of the fat pointer is mutable.
                     self.intern_shallow(vtable.alloc_id, Mutability::Not, None)?;
+                } else {
+                    self.ecx().tcx.sess.delay_span_bug(
+                        syntax_pos::DUMMY_SP,
+                        "vtables pointers cannot be integer pointers",
+                    );
                 }
             }
             // Check if we have encountered this pointer+layout combination before.
@@ -280,7 +287,9 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
         ecx,
         leftover_allocations,
         base_intern_mode,
-        ret.ptr.to_ptr()?.alloc_id,
+        // The outermost allocation must exist, because we allocated it with
+        // `Memory::allocate`.
+        ret.ptr.assert_ptr().alloc_id,
         base_mutability,
         Some(ret.layout.ty),
     )?;

src/librustc_mir/interpret/intrinsics.rs

@@ -5,7 +5,7 @@
 use rustc::hir::def_id::DefId;
 use rustc::mir::{
     self,
-    interpret::{ConstValue, InterpResult, Scalar},
+    interpret::{ConstValue, GlobalId, InterpResult, Scalar},
     BinOp,
 };
 use rustc::ty;
@@ -118,9 +118,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             | sym::size_of
             | sym::type_id
             | sym::type_name => {
-                let val =
-                    self.tcx.const_eval_instance(self.param_env, instance, Some(self.tcx.span))?;
-                let val = self.eval_const_to_op(val, None)?;
+                let gid = GlobalId { instance, promoted: None };
+                let val = self.const_eval(gid)?;
                 self.copy_op(val, dest)?;
             }
 

src/librustc_mir/interpret/operand.rs

@@ -578,7 +578,15 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             ty::ConstKind::Param(_) => throw_inval!(TooGeneric),
             ty::ConstKind::Unevaluated(def_id, substs) => {
                 let instance = self.resolve(def_id, substs)?;
-                return Ok(OpTy::from(self.const_eval_raw(GlobalId { instance, promoted: None })?));
+                // We use `const_eval` here and `const_eval_raw` elsewhere in mir interpretation.
+                // The reason we use `const_eval_raw` everywhere else is to prevent cycles during
+                // validation, because validation automatically reads through any references, thus
+                // potentially requiring the current static to be evaluated again. This is not a
+                // problem here, because we are building an operand which means an actual read is
+                // happening.
+                // FIXME(oli-obk): eliminate all the `const_eval_raw` usages when we get rid of
+                // `StaticKind` once and for all.
+                return self.const_eval(GlobalId { instance, promoted: None });
             }
             ty::ConstKind::Infer(..)
             | ty::ConstKind::Bound(..)

src/librustc_mir/interpret/place.rs

@@ -923,12 +923,14 @@ where
             return self.copy_op(src, dest);
         }
         // We still require the sizes to match.
-        assert!(
-            src.layout.size == dest.layout.size,
-            "Size mismatch when transmuting!\nsrc: {:#?}\ndest: {:#?}",
-            src,
-            dest
-        );
+        if src.layout.size != dest.layout.size {
+            // FIXME: This should be an assert instead of an error, but if we transmute within an
+            // array length computation, `typeck` may not have yet been run and errored out. In fact
+            // most likey we *are* running `typeck` right now. Investigate whether we can bail out
+            // on `typeck_tables().has_errors` at all const eval entry points.
+            debug!("Size mismatch when transmuting!\nsrc: {:#?}\ndest: {:#?}", src, dest);
+            throw_unsup!(TransmuteSizeDiff(src.layout.ty, dest.layout.ty));
+        }
         // Unsized copies rely on interpreting `src.meta` with `dest.layout`, we want
         // to avoid that here.
         assert!(
@@ -974,31 +976,20 @@ where
         let (mplace, size) = match place.place {
             Place::Local { frame, local } => {
                 match self.stack[frame].locals[local].access_mut()? {
-                    Ok(local_val) => {
+                    Ok(&mut local_val) => {
                         // We need to make an allocation.
-                        // FIXME: Consider not doing anything for a ZST, and just returning
-                        // a fake pointer?  Are we even called for ZST?
-
-                        // We cannot hold on to the reference `local_val` while allocating,
-                        // but we can hold on to the value in there.
-                        let old_val =
-                            if let LocalValue::Live(Operand::Immediate(value)) = *local_val {
-                                Some(value)
-                            } else {
-                                None
-                            };
 
                         // We need the layout of the local.  We can NOT use the layout we got,
                         // that might e.g., be an inner field of a struct with `Scalar` layout,
                         // that has different alignment than the outer field.
-                        // We also need to support unsized types, and hence cannot use `allocate`.
                         let local_layout = self.layout_of_local(&self.stack[frame], local, None)?;
+                        // We also need to support unsized types, and hence cannot use `allocate`.
                         let (size, align) = self
                             .size_and_align_of(meta, local_layout)?
                             .expect("Cannot allocate for non-dyn-sized type");
                         let ptr = self.memory.allocate(size, align, MemoryKind::Stack);
                         let mplace = MemPlace { ptr: ptr.into(), align, meta };
-                        if let Some(value) = old_val {
+                        if let LocalValue::Live(Operand::Immediate(value)) = local_val {
                             // Preserve old value.
                             // We don't have to validate as we can assume the local
                             // was already valid for its type.