Poison any MemPlace created from a zst Operand (or otherwise via MPlaceTy::dangling) so you can't get the address back out.

oli-obk · oli-obk · commit 4fbe434c5c10 · 2020-01-07T15:41:48.000+01:00
diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
@@ -20,7 +20,7 @@ use rustc_macros::HashStable;
 use rustc_span::source_map::{self, Span, DUMMY_SP};
 
 use super::{
-    Immediate, MPlaceTy, Machine, MemPlace, Memory, OpTy, Operand, Place, PlaceTy,
+    Immediate, MPlaceTy, Machine, MemPlace, MemPlaceMeta, Memory, OpTy, Operand, Place, PlaceTy,
     ScalarMaybeUndef, StackPopInfo,
 };
 
@@ -393,7 +393,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     /// This can fail to provide an answer for extern types.
     pub(super) fn size_and_align_of(
         &self,
-        metadata: Option<Scalar<M::PointerTag>>,
+        metadata: MemPlaceMeta<M::PointerTag>,
         layout: TyLayout<'tcx>,
     ) -> InterpResult<'tcx, Option<(Size, Align)>> {
         if !layout.is_unsized() {
@@ -465,14 +465,13 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 Ok(Some((size, align)))
             }
             ty::Dynamic(..) => {
-                let vtable = metadata.expect("dyn trait fat ptr must have vtable");
+                let vtable = metadata.unwrap_unsized();
                 // Read size and align from vtable (already checks size).
                 Ok(Some(self.read_size_and_align_from_vtable(vtable)?))
             }
 
             ty::Slice(_) | ty::Str => {
-                let len =
-                    metadata.expect("slice fat ptr must have length").to_machine_usize(self)?;
+                let len = metadata.unwrap_unsized().to_machine_usize(self)?;
                 let elem = layout.field(self, 0)?;
 
                 // Make sure the slice is not too big.
@@ -818,8 +817,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                                 " by align({}){} ref:",
                                 mplace.align.bytes(),
                                 match mplace.meta {
-                                    Some(meta) => format!(" meta({:?})", meta),
-                                    None => String::new(),
+                                    MemPlaceMeta::Unsized(meta) => format!(" meta({:?})", meta),
+                                    MemPlaceMeta::Poison | MemPlaceMeta::None => String::new(),
                                 }
                             )
                             .unwrap();
diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs
@@ -193,7 +193,7 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx
             {
                 // 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() {
+                if let Scalar::Ptr(vtable) = mplace.meta.unwrap_unsized() {
                     // 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)?;
@@ -226,7 +226,8 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx
                     | (InternMode::Const, hir::Mutability::Mut) => match referenced_ty.kind {
                         ty::Array(_, n)
                             if n.eval_usize(self.ecx.tcx.tcx, self.ecx.param_env) == 0 => {}
-                        ty::Slice(_) if mplace.meta.unwrap().to_machine_usize(self.ecx)? == 0 => {}
+                        ty::Slice(_)
+                            if mplace.meta.unwrap_unsized().to_machine_usize(self.ecx)? == 0 => {}
                         _ => bug!("const qualif failed to prevent mutable references"),
                     },
                 }
diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs
@@ -20,7 +20,7 @@ pub use rustc::mir::interpret::*; // have all the `interpret` symbols in one pla
 
 pub use self::eval_context::{Frame, InterpCx, LocalState, LocalValue, StackPopCleanup};
 
-pub use self::place::{MPlaceTy, MemPlace, Place, PlaceTy};
+pub use self::place::{MPlaceTy, MemPlace, MemPlaceMeta, Place, PlaceTy};
 
 pub use self::memory::{AllocCheck, FnVal, Memory, MemoryKind};
 
diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs
@@ -20,6 +20,45 @@ use super::{
     RawConst, Scalar, ScalarMaybeUndef,
 };
 
+#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, HashStable)]
+pub enum MemPlaceMeta<Tag = (), Id = AllocId> {
+    Unsized(Scalar<Tag, Id>),
+    /// `Sized` types or unsized `extern type`
+    None,
+    /// The address of this place may not be taken. This protects the `MemPlace` from coming from
+    /// a ZST Operand with a backing allocation and being converted to an integer address. This
+    /// should be impossible, because you can't take the address of an operand, but this is a second
+    /// protection layer ensuring that we don't mess up.
+    Poison,
+}
+
+impl<Tag, Id> MemPlaceMeta<Tag, Id> {
+    pub fn unwrap_unsized(self) -> Scalar<Tag, Id> {
+        match self {
+            Self::Unsized(s) => s,
+            Self::None | Self::Poison => {
+                bug!("expected wide pointer extra data (e.g. slice length or trait object vtable)")
+            }
+        }
+    }
+    fn is_unsized(self) -> bool {
+        match self {
+            Self::Unsized(_) => true,
+            Self::None | Self::Poison => false,
+        }
+    }
+}
+
+impl<Tag> MemPlaceMeta<Tag> {
+    pub fn erase_tag(self) -> MemPlaceMeta<()> {
+        match self {
+            Self::Unsized(s) => MemPlaceMeta::Unsized(s.erase_tag()),
+            Self::None => MemPlaceMeta::None,
+            Self::Poison => MemPlaceMeta::Poison,
+        }
+    }
+}
+
 #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, HashStable)]
 pub struct MemPlace<Tag = (), Id = AllocId> {
     /// A place may have an integral pointer for ZSTs, and since it might
@@ -30,7 +69,7 @@ pub struct MemPlace<Tag = (), Id = AllocId> {
     /// Metadata for unsized places. Interpretation is up to the type.
     /// Must not be present for sized types, but can be missing for unsized types
     /// (e.g., `extern type`).
-    pub meta: Option<Scalar<Tag, Id>>,
+    pub meta: MemPlaceMeta<Tag, Id>,
 }
 
 #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, HashStable)]
@@ -88,16 +127,12 @@ impl<Tag> MemPlace<Tag> {
 
     #[inline]
     pub fn erase_tag(self) -> MemPlace {
-        MemPlace {
-            ptr: self.ptr.erase_tag(),
-            align: self.align,
-            meta: self.meta.map(Scalar::erase_tag),
-        }
+        MemPlace { ptr: self.ptr.erase_tag(), align: self.align, meta: self.meta.erase_tag() }
     }
 
     #[inline(always)]
     pub fn from_scalar_ptr(ptr: Scalar<Tag>, align: Align) -> Self {
-        MemPlace { ptr, align, meta: None }
+        MemPlace { ptr, align, meta: MemPlaceMeta::None }
     }
 
     /// Produces a Place that will error if attempted to be read from or written to
@@ -116,15 +151,19 @@ impl<Tag> MemPlace<Tag> {
     #[inline(always)]
     pub fn to_ref(self) -> Immediate<Tag> {
         match self.meta {
-            None => Immediate::Scalar(self.ptr.into()),
-            Some(meta) => Immediate::ScalarPair(self.ptr.into(), meta.into()),
+            MemPlaceMeta::None => Immediate::Scalar(self.ptr.into()),
+            MemPlaceMeta::Unsized(meta) => Immediate::ScalarPair(self.ptr.into(), meta.into()),
+            MemPlaceMeta::Poison => bug!(
+                "MPlaceTy::dangling may never be used to produce a \
+                place that will have the address of its pointee taken"
+            ),
         }
     }
 
     pub fn offset(
         self,
         offset: Size,
-        meta: Option<Scalar<Tag>>,
+        meta: MemPlaceMeta<Tag>,
         cx: &impl HasDataLayout,
     ) -> InterpResult<'tcx, Self> {
         Ok(MemPlace {
@@ -158,7 +197,7 @@ impl<'tcx, Tag> MPlaceTy<'tcx, Tag> {
     pub fn offset(
         self,
         offset: Size,
-        meta: Option<Scalar<Tag>>,
+        meta: MemPlaceMeta<Tag>,
         layout: TyLayout<'tcx>,
         cx: &impl HasDataLayout,
     ) -> InterpResult<'tcx, Self> {
@@ -175,7 +214,9 @@ impl<'tcx, Tag> MPlaceTy<'tcx, Tag> {
         if self.layout.is_unsized() {
             // We need to consult `meta` metadata
             match self.layout.ty.kind {
-                ty::Slice(..) | ty::Str => return self.mplace.meta.unwrap().to_machine_usize(cx),
+                ty::Slice(..) | ty::Str => {
+                    return self.mplace.meta.unwrap_unsized().to_machine_usize(cx);
+                }
                 _ => bug!("len not supported on unsized type {:?}", self.layout.ty),
             }
         } else {
@@ -191,7 +232,7 @@ impl<'tcx, Tag> MPlaceTy<'tcx, Tag> {
     #[inline]
     pub(super) fn vtable(self) -> Scalar<Tag> {
         match self.layout.ty.kind {
-            ty::Dynamic(..) => self.mplace.meta.unwrap(),
+            ty::Dynamic(..) => self.mplace.meta.unwrap_unsized(),
             _ => bug!("vtable not supported on type {:?}", self.layout.ty),
         }
     }
@@ -276,8 +317,10 @@ where
             val.layout.ty.builtin_deref(true).expect("`ref_to_mplace` called on non-ptr type").ty;
         let layout = self.layout_of(pointee_type)?;
         let (ptr, meta) = match *val {
-            Immediate::Scalar(ptr) => (ptr.not_undef()?, None),
-            Immediate::ScalarPair(ptr, meta) => (ptr.not_undef()?, Some(meta.not_undef()?)),
+            Immediate::Scalar(ptr) => (ptr.not_undef()?, MemPlaceMeta::None),
+            Immediate::ScalarPair(ptr, meta) => {
+                (ptr.not_undef()?, MemPlaceMeta::Unsized(meta.not_undef()?))
+            }
         };
 
         let mplace = MemPlace {
@@ -318,7 +361,7 @@ where
     ) -> InterpResult<'tcx, Option<Pointer<M::PointerTag>>> {
         let size = size.unwrap_or_else(|| {
             assert!(!place.layout.is_unsized());
-            assert!(place.meta.is_none());
+            assert!(!place.meta.is_unsized());
             place.layout.size
         });
         self.memory.check_ptr_access(place.ptr, size, place.align)
@@ -411,7 +454,7 @@ where
         } else {
             // base.meta could be present; we might be accessing a sized field of an unsized
             // struct.
-            (None, offset)
+            (MemPlaceMeta::None, offset)
         };
 
         // We do not look at `base.layout.align` nor `field_layout.align`, unlike
@@ -433,7 +476,7 @@ where
         };
         let layout = base.layout.field(self, 0)?;
         let dl = &self.tcx.data_layout;
-        Ok((0..len).map(move |i| base.offset(i * stride, None, layout, dl)))
+        Ok((0..len).map(move |i| base.offset(i * stride, MemPlaceMeta::None, layout, dl)))
     }
 
     fn mplace_subslice(
@@ -466,10 +509,10 @@ where
         let (meta, ty) = match base.layout.ty.kind {
             // It is not nice to match on the type, but that seems to be the only way to
             // implement this.
-            ty::Array(inner, _) => (None, self.tcx.mk_array(inner, inner_len)),
+            ty::Array(inner, _) => (MemPlaceMeta::None, self.tcx.mk_array(inner, inner_len)),
             ty::Slice(..) => {
                 let len = Scalar::from_uint(inner_len, self.pointer_size());
-                (Some(len), base.layout.ty)
+                (MemPlaceMeta::Unsized(len), base.layout.ty)
             }
             _ => bug!("cannot subslice non-array type: `{:?}`", base.layout.ty),
         };
@@ -483,7 +526,7 @@ where
         variant: VariantIdx,
     ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
         // Downcasts only change the layout
-        assert!(base.meta.is_none());
+        assert!(!base.meta.is_unsized());
         Ok(MPlaceTy { layout: base.layout.for_variant(self, variant), ..base })
     }
 
@@ -977,7 +1020,7 @@ where
     pub fn force_allocation_maybe_sized(
         &mut self,
         place: PlaceTy<'tcx, M::PointerTag>,
-        meta: Option<Scalar<M::PointerTag>>,
+        meta: MemPlaceMeta<M::PointerTag>,
     ) -> InterpResult<'tcx, (MPlaceTy<'tcx, M::PointerTag>, Option<Size>)> {
         let (mplace, size) = match place.place {
             Place::Local { frame, local } => {
@@ -1022,7 +1065,7 @@ where
         &mut self,
         place: PlaceTy<'tcx, M::PointerTag>,
     ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
-        Ok(self.force_allocation_maybe_sized(place, None)?.0)
+        Ok(self.force_allocation_maybe_sized(place, MemPlaceMeta::None)?.0)
     }
 
     pub fn allocate(
@@ -1042,8 +1085,11 @@ where
     ) -> MPlaceTy<'tcx, M::PointerTag> {
         let ptr = self.memory.allocate_static_bytes(str.as_bytes(), kind);
         let meta = Scalar::from_uint(str.len() as u128, self.pointer_size());
-        let mplace =
-            MemPlace { ptr: ptr.into(), align: Align::from_bytes(1).unwrap(), meta: Some(meta) };
+        let mplace = MemPlace {
+            ptr: ptr.into(),
+            align: Align::from_bytes(1).unwrap(),
+            meta: MemPlaceMeta::Unsized(meta),
+        };
 
         let layout = self.layout_of(self.tcx.mk_static_str()).unwrap();
         MPlaceTy { mplace, layout }
@@ -1151,7 +1197,7 @@ where
             assert_eq!(align, layout.align.abi);
         }
 
-        let mplace = MPlaceTy { mplace: MemPlace { meta: None, ..*mplace }, layout };
+        let mplace = MPlaceTy { mplace: MemPlace { meta: MemPlaceMeta::None, ..*mplace }, layout };
         Ok((instance, mplace))
     }
 }
diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs
@@ -23,7 +23,9 @@ use rustc_span::source_map::Span;
 use syntax::ast::Mutability;
 
 use super::eval_context::{LocalState, StackPopCleanup};
-use super::{Frame, Immediate, LocalValue, MemPlace, Memory, Operand, Place, ScalarMaybeUndef};
+use super::{
+    Frame, Immediate, LocalValue, MemPlace, MemPlaceMeta, Memory, Operand, Place, ScalarMaybeUndef,
+};
 use crate::const_eval::CompileTimeInterpreter;
 
 #[derive(Default)]
@@ -205,6 +207,14 @@ impl_snapshot_for!(
     }
 );
 
+impl_snapshot_for!(
+    enum MemPlaceMeta {
+        Unsized(s),
+        None,
+        Poison,
+    }
+);
+
 impl_snapshot_for!(struct MemPlace {
     ptr,
     meta,
diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs
@@ -16,7 +16,7 @@ use rustc_span::symbol::{sym, Symbol};
 use std::hash::Hash;
 
 use super::{
-    CheckInAllocMsg, GlobalAlloc, InterpCx, InterpResult, MPlaceTy, Machine, OpTy, Scalar,
+    CheckInAllocMsg, GlobalAlloc, InterpCx, InterpResult, MPlaceTy, Machine, MemPlaceMeta, OpTy,
     ValueVisitor,
 };
 
@@ -246,13 +246,13 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M
 
     fn check_wide_ptr_meta(
         &mut self,
-        meta: Option<Scalar<M::PointerTag>>,
+        meta: MemPlaceMeta<M::PointerTag>,
         pointee: TyLayout<'tcx>,
     ) -> InterpResult<'tcx> {
         let tail = self.ecx.tcx.struct_tail_erasing_lifetimes(pointee.ty, self.ecx.param_env);
         match tail.kind {
             ty::Dynamic(..) => {
-                let vtable = meta.unwrap();
+                let vtable = meta.unwrap_unsized();
                 try_validation!(
                     self.ecx.memory.check_ptr_access(
                         vtable,
@@ -276,7 +276,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M
             }
             ty::Slice(..) | ty::Str => {
                 let _len = try_validation!(
-                    meta.unwrap().to_machine_usize(self.ecx),
+                    meta.unwrap_unsized().to_machine_usize(self.ecx),
                     "non-integer slice length in wide pointer",
                     self.path
                 );
@@ -572,8 +572,11 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
         match op.layout.ty.kind {
             ty::Str => {
                 let mplace = op.assert_mem_place(self.ecx); // strings are never immediate
-                try_validation!(self.ecx.read_str(mplace),
-                    "uninitialized or non-UTF-8 data in str", self.path);
+                try_validation!(
+                    self.ecx.read_str(mplace),
+                    "uninitialized or non-UTF-8 data in str",
+                    self.path
+                );
             }
             ty::Array(tys, ..) | ty::Slice(tys)
                 if {
