Auto merge of #99013 - RalfJung:dont-poison-my-places, r=oli-obk

interpret: get rid of MemPlaceMeta::Poison

This is achieved by refactoring the projection code (`{mplace,place,operand}_{downcast,field,index,...}`) so that we no longer need to call `assert_mem_place` in the operand handling.

Author: bors

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -165,6 +165,7 @@ pub(super) fn op_to_const<'tcx>(
         Ok(ref mplace) => to_const_value(mplace),
         // see comment on `let try_as_immediate` above
         Err(imm) => match *imm {
+            _ if imm.layout.is_zst() => ConstValue::ZeroSized,
             Immediate::Scalar(x) => match x {
                 ScalarMaybeUninit::Scalar(s) => ConstValue::Scalar(s),
                 ScalarMaybeUninit::Uninit => to_const_value(&op.assert_mem_place()),

compiler/rustc_const_eval/src/const_eval/mod.rs

@@ -147,7 +147,6 @@ pub(crate) fn deref_mir_constant<'tcx>(
 
     let ty = match mplace.meta {
         MemPlaceMeta::None => mplace.layout.ty,
-        MemPlaceMeta::Poison => bug!("poison metadata in `deref_mir_constant`: {:#?}", mplace),
         // In case of unsized types, figure out the real type behind.
         MemPlaceMeta::Meta(scalar) => match mplace.layout.ty.kind() {
             ty::Str => bug!("there's no sized equivalent of a `str`"),

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -987,7 +987,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> std::fmt::Debug
                             " by {} ref {:?}:",
                             match mplace.meta {
                                 MemPlaceMeta::Meta(meta) => format!(" meta({:?})", meta),
-                                MemPlaceMeta::Poison | MemPlaceMeta::None => String::new(),
+                                MemPlaceMeta::None => String::new(),
                             },
                             mplace.ptr,
                         )?;

compiler/rustc_const_eval/src/interpret/place.rs

@@ -24,11 +24,6 @@ pub enum MemPlaceMeta<Tag: Provenance = AllocId> {
     Meta(Scalar<Tag>),
     /// `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 without 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,
 }
 
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
@@ -38,15 +33,16 @@ impl<Tag: Provenance> MemPlaceMeta<Tag> {
     pub fn unwrap_meta(self) -> Scalar<Tag> {
         match self {
             Self::Meta(s) => s,
-            Self::None | Self::Poison => {
+            Self::None => {
                 bug!("expected wide pointer extra data (e.g. slice length or trait object vtable)")
             }
         }
     }
+
     pub fn has_meta(self) -> bool {
         match self {
             Self::Meta(_) => true,
-            Self::None | Self::Poison => false,
+            Self::None => false,
         }
     }
 }
@@ -163,10 +159,6 @@ impl<Tag: Provenance> MemPlace<Tag> {
             MemPlaceMeta::Meta(meta) => {
                 Immediate::ScalarPair(Scalar::from_maybe_pointer(self.ptr, cx).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"
-            ),
         }
     }
 
@@ -195,13 +187,15 @@ impl<Tag: Provenance> Place<Tag> {
 }
 
 impl<'tcx, Tag: Provenance> MPlaceTy<'tcx, Tag> {
-    /// Produces a MemPlace that works for ZST but nothing else
+    /// Produces a MemPlace that works for ZST but nothing else.
+    /// Conceptually this is a new allocation, but it doesn't actually create an allocation so you
+    /// don't need to worry about memory leaks.
     #[inline]
-    pub fn dangling(layout: TyAndLayout<'tcx>) -> Self {
+    pub fn fake_alloc_zst(layout: TyAndLayout<'tcx>) -> Self {
+        assert!(layout.is_zst());
         let align = layout.align.abi;
         let ptr = Pointer::from_addr(align.bytes()); // no provenance, absolute address
-        // `Poison` this to make sure that the pointer value `ptr` is never observable by the program.
-        MPlaceTy { mplace: MemPlace { ptr, meta: MemPlaceMeta::Poison }, layout, align }
+        MPlaceTy { mplace: MemPlace { ptr, meta: MemPlaceMeta::None }, layout, align }
     }
 
     #[inline]
@@ -273,7 +267,6 @@ impl<'tcx, Tag: Provenance> OpTy<'tcx, Tag> {
             Operand::Indirect(mplace) => {
                 Ok(MPlaceTy { mplace, layout: self.layout, align: self.align.unwrap() })
             }
-            Operand::Immediate(_) if self.layout.is_zst() => Ok(MPlaceTy::dangling(self.layout)),
             Operand::Immediate(imm) => Err(ImmTy::from_immediate(imm, self.layout)),
         }
     }

compiler/rustc_const_eval/src/interpret/terminator.rs

@@ -617,16 +617,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             place.to_ref(self),
             self.layout_of(self.tcx.mk_mut_ptr(place.layout.ty))?,
         );
-
-        let ty = self.tcx.mk_unit(); // return type is ()
-        let dest = MPlaceTy::dangling(self.layout_of(ty)?);
+        let ret = MPlaceTy::fake_alloc_zst(self.layout_of(self.tcx.types.unit)?);
 
         self.eval_fn_call(
             FnVal::Instance(instance),
             (Abi::Rust, fn_abi),
             &[arg.into()],
             false,
-            &dest.into(),
+            &ret.into(),
             Some(target),
             match unwind {
                 Some(cleanup) => StackPopUnwind::Cleanup(cleanup),

compiler/rustc_middle/src/mir/interpret/allocation.rs

@@ -225,6 +225,8 @@ impl<Tag> Allocation<Tag> {
 
     /// Try to create an Allocation of `size` bytes, failing if there is not enough memory
     /// available to the compiler to do so.
+    ///
+    /// If `panic_on_fail` is true, this will never return `Err`.
     pub fn uninit<'tcx>(size: Size, align: Align, panic_on_fail: bool) -> InterpResult<'tcx, Self> {
         let bytes = Box::<[u8]>::try_new_zeroed_slice(size.bytes_usize()).map_err(|_| {
             // This results in an error that can happen non-deterministically, since the memory