Auto merge of rust-lang#107728 - RalfJung:miri-dyn-star, r=RalfJung,o…

…li-obk

Miri: basic dyn* support

As usual I am very unsure about the dynamic dispatch stuff, but it passes even the `Pin<&mut dyn* Trait>` test so that is something.

TBH I think it was a mistake to make `dyn Trait` and `dyn* Trait` part of the same `TyKind` variant. Almost everywhere in Miri this lead to the wrong default behavior, resulting in strange ICEs instead of nice "unimplemented" messages. The two types describe pretty different runtime data layout after all.

Strangely I did not need to do the equivalent of [this diff](rust-lang#106532 (comment)) in Miri. Maybe that is because the unsizing logic matches on `ty::Dynamic(.., ty::Dyn)` already? In `unsized_info` I don't think the `target_dyn_kind` can be `DynStar`, since then it wouldn't be unsized!

r? `@oli-obk` Cc `@eholk` (dyn-star) rust-lang#102425

compiler/rustc_const_eval/src/interpret/cast.rs

@@ -312,6 +312,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         }
     }
 
+    /// `src` is a *pointer to* a `source_ty`, and in `dest` we should store a pointer to th same
+    /// data at type `cast_ty`.
     fn unsize_into_ptr(
         &mut self,
         src: &OpTy<'tcx, M::Provenance>,
@@ -335,7 +337,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 );
                 self.write_immediate(val, dest)
             }
-            (ty::Dynamic(data_a, ..), ty::Dynamic(data_b, ..)) => {
+            (ty::Dynamic(data_a, _, ty::Dyn), ty::Dynamic(data_b, _, ty::Dyn)) => {
                 let val = self.read_immediate(src)?;
                 if data_a.principal() == data_b.principal() {
                     // A NOP cast that doesn't actually change anything, should be allowed even with mismatching vtables.
@@ -359,7 +361,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }
 
             _ => {
-                span_bug!(self.cur_span(), "invalid unsizing {:?} -> {:?}", src.layout.ty, cast_ty)
+                span_bug!(
+                    self.cur_span(),
+                    "invalid pointer unsizing {:?} -> {:?}",
+                    src.layout.ty,
+                    cast_ty
+                )
             }
         }
     }

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -632,7 +632,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 }
                 Ok(Some((size, align)))
             }
-            ty::Dynamic(..) => {
+            ty::Dynamic(_, _, ty::Dyn) => {
                 let vtable = metadata.unwrap_meta().to_pointer(self)?;
                 // Read size and align from vtable (already checks size).
                 Ok(Some(self.get_vtable_size_and_align(vtable)?))

compiler/rustc_const_eval/src/interpret/intern.rs

@@ -242,7 +242,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
             let mplace = self.ecx.ref_to_mplace(&value)?;
             assert_eq!(mplace.layout.ty, referenced_ty);
             // Handle trait object vtables.
-            if let ty::Dynamic(..) =
+            if let ty::Dynamic(_, _, ty::Dyn) =
                 tcx.struct_tail_erasing_lifetimes(referenced_ty, self.ecx.param_env).kind()
             {
                 let ptr = mplace.meta.unwrap_meta().to_pointer(&tcx)?;

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -255,7 +255,22 @@ impl<'tcx, Prov: Provenance> OpTy<'tcx, Prov> {
         }
     }
 
-    pub fn offset_with_meta(
+    /// Replace the layout of this operand. There's basically no sanity check that this makes sense,
+    /// you better know what you are doing! If this is an immediate, applying the wrong layout can
+    /// not just lead to invalid data, it can actually *shift the data around* since the offsets of
+    /// a ScalarPair are entirely determined by the layout, not the data.
+    pub fn transmute(&self, layout: TyAndLayout<'tcx>) -> Self {
+        assert_eq!(
+            self.layout.size, layout.size,
+            "transmuting with a size change, that doesn't seem right"
+        );
+        OpTy { layout, ..*self }
+    }
+
+    /// Offset the operand in memory (if possible) and change its metadata.
+    ///
+    /// This can go wrong very easily if you give the wrong layout for the new place!
+    pub(super) fn offset_with_meta(
         &self,
         offset: Size,
         meta: MemPlaceMeta<Prov>,
@@ -276,6 +291,9 @@ impl<'tcx, Prov: Provenance> OpTy<'tcx, Prov> {
         }
     }
 
+    /// Offset the operand in memory (if possible).
+    ///
+    /// This can go wrong very easily if you give the wrong layout for the new place!
     pub fn offset(
         &self,
         offset: Size,

compiler/rustc_const_eval/src/interpret/place.rs

@@ -26,6 +26,7 @@ pub enum MemPlaceMeta<Prov: Provenance = AllocId> {
 }
 
 impl<Prov: Provenance> MemPlaceMeta<Prov> {
+    #[cfg_attr(debug_assertions, track_caller)] // only in debug builds due to perf (see #98980)
     pub fn unwrap_meta(self) -> Scalar<Prov> {
         match self {
             Self::Meta(s) => s,
@@ -147,12 +148,16 @@ impl<Prov: Provenance> MemPlace<Prov> {
     }
 
     #[inline]
-    pub fn offset_with_meta<'tcx>(
+    pub(super) fn offset_with_meta<'tcx>(
         self,
         offset: Size,
         meta: MemPlaceMeta<Prov>,
         cx: &impl HasDataLayout,
     ) -> InterpResult<'tcx, Self> {
+        debug_assert!(
+            !meta.has_meta() || self.meta.has_meta(),
+            "cannot use `offset_with_meta` to add metadata to a place"
+        );
         Ok(MemPlace { ptr: self.ptr.offset(offset, cx)?, meta })
     }
 }
@@ -182,8 +187,11 @@ impl<'tcx, Prov: Provenance> MPlaceTy<'tcx, Prov> {
         MPlaceTy { mplace: MemPlace { ptr, meta: MemPlaceMeta::None }, layout, align }
     }
 
+    /// Offset the place in memory and change its metadata.
+    ///
+    /// This can go wrong very easily if you give the wrong layout for the new place!
     #[inline]
-    pub fn offset_with_meta(
+    pub(crate) fn offset_with_meta(
         &self,
         offset: Size,
         meta: MemPlaceMeta<Prov>,
@@ -197,6 +205,9 @@ impl<'tcx, Prov: Provenance> MPlaceTy<'tcx, Prov> {
         })
     }
 
+    /// Offset the place in memory.
+    ///
+    /// This can go wrong very easily if you give the wrong layout for the new place!
     pub fn offset(
         &self,
         offset: Size,
@@ -241,14 +252,6 @@ impl<'tcx, Prov: Provenance> MPlaceTy<'tcx, Prov> {
             }
         }
     }
-
-    #[inline]
-    pub(super) fn vtable(&self) -> Scalar<Prov> {
-        match self.layout.ty.kind() {
-            ty::Dynamic(..) => self.mplace.meta.unwrap_meta(),
-            _ => bug!("vtable not supported on type {:?}", self.layout.ty),
-        }
-    }
 }
 
 // These are defined here because they produce a place.
@@ -266,7 +269,12 @@ impl<'tcx, Prov: Provenance> OpTy<'tcx, Prov> {
     #[inline(always)]
     #[cfg_attr(debug_assertions, track_caller)] // only in debug builds due to perf (see #98980)
     pub fn assert_mem_place(&self) -> MPlaceTy<'tcx, Prov> {
-        self.as_mplace_or_imm().left().unwrap()
+        self.as_mplace_or_imm().left().unwrap_or_else(|| {
+            bug!(
+                "OpTy of type {} was immediate when it was expected to be an MPlace",
+                self.layout.ty
+            )
+        })
     }
 }
 
@@ -283,7 +291,12 @@ impl<'tcx, Prov: Provenance> PlaceTy<'tcx, Prov> {
     #[inline(always)]
     #[cfg_attr(debug_assertions, track_caller)] // only in debug builds due to perf (see #98980)
     pub fn assert_mem_place(&self) -> MPlaceTy<'tcx, Prov> {
-        self.as_mplace_or_local().left().unwrap()
+        self.as_mplace_or_local().left().unwrap_or_else(|| {
+            bug!(
+                "PlaceTy of type {} was a local when it was expected to be an MPlace",
+                self.layout.ty
+            )
+        })
     }
 }
 
@@ -807,11 +820,16 @@ where
     }
 
     /// Turn a place with a `dyn Trait` type into a place with the actual dynamic type.
+    /// Aso returns the vtable.
     pub(super) fn unpack_dyn_trait(
         &self,
         mplace: &MPlaceTy<'tcx, M::Provenance>,
-    ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
-        let vtable = mplace.vtable().to_pointer(self)?; // also sanity checks the type
+    ) -> InterpResult<'tcx, (MPlaceTy<'tcx, M::Provenance>, Pointer<Option<M::Provenance>>)> {
+        assert!(
+            matches!(mplace.layout.ty.kind(), ty::Dynamic(_, _, ty::Dyn)),
+            "`unpack_dyn_trait` only makes sense on `dyn*` types"
+        );
+        let vtable = mplace.meta.unwrap_meta().to_pointer(self)?;
         let (ty, _) = self.get_ptr_vtable(vtable)?;
         let layout = self.layout_of(ty)?;
 
@@ -820,7 +838,26 @@ where
             layout,
             align: layout.align.abi,
         };
-        Ok(mplace)
+        Ok((mplace, vtable))
+    }
+
+    /// Turn an operand with a `dyn* Trait` type into an operand with the actual dynamic type.
+    /// Aso returns the vtable.
+    pub(super) fn unpack_dyn_star(
+        &self,
+        op: &OpTy<'tcx, M::Provenance>,
+    ) -> InterpResult<'tcx, (OpTy<'tcx, M::Provenance>, Pointer<Option<M::Provenance>>)> {
+        assert!(
+            matches!(op.layout.ty.kind(), ty::Dynamic(_, _, ty::DynStar)),
+            "`unpack_dyn_star` only makes sense on `dyn*` types"
+        );
+        let data = self.operand_field(&op, 0)?;
+        let vtable = self.operand_field(&op, 1)?;
+        let vtable = self.read_pointer(&vtable)?;
+        let (ty, _) = self.get_ptr_vtable(vtable)?;
+        let layout = self.layout_of(ty)?;
+        let data = data.transmute(layout);
+        Ok((data, vtable))
     }
 }
 

compiler/rustc_const_eval/src/interpret/terminator.rs

@@ -547,7 +547,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 let receiver_place = loop {
                     match receiver.layout.ty.kind() {
                         ty::Ref(..) | ty::RawPtr(..) => break self.deref_operand(&receiver)?,
-                        ty::Dynamic(..) => break receiver.assert_mem_place(), // no immediate unsized values
+                        ty::Dynamic(.., ty::Dyn) => break receiver.assert_mem_place(), // no immediate unsized values
+                        ty::Dynamic(.., ty::DynStar) => {
+                            // Not clear how to handle this, so far we assume the receiver is always a pointer.
+                            span_bug!(
+                                self.cur_span(),
+                                "by-value calls on a `dyn*`... are those a thing?"
+                            );
+                        }
                         _ => {
                             // Not there yet, search for the only non-ZST field.
                             let mut non_zst_field = None;
@@ -573,39 +580,59 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                         }
                     }
                 };
-                // Obtain the underlying trait we are working on.
-                let receiver_tail = self
-                    .tcx
-                    .struct_tail_erasing_lifetimes(receiver_place.layout.ty, self.param_env);
-                let ty::Dynamic(data, ..) = receiver_tail.kind() else {
-                    span_bug!(self.cur_span(), "dynamic call on non-`dyn` type {}", receiver_tail)
-                };
 
-                // Get the required information from the vtable.
-                let vptr = receiver_place.meta.unwrap_meta().to_pointer(self)?;
-                let (dyn_ty, dyn_trait) = self.get_ptr_vtable(vptr)?;
-                if dyn_trait != data.principal() {
-                    throw_ub_format!(
-                        "`dyn` call on a pointer whose vtable does not match its type"
-                    );
-                }
+                // Obtain the underlying trait we are working on, and the adjusted receiver argument.
+                let (vptr, dyn_ty, adjusted_receiver) = if let ty::Dynamic(data, _, ty::DynStar) =
+                    receiver_place.layout.ty.kind()
+                {
+                    let (recv, vptr) = self.unpack_dyn_star(&receiver_place.into())?;
+                    let (dyn_ty, dyn_trait) = self.get_ptr_vtable(vptr)?;
+                    if dyn_trait != data.principal() {
+                        throw_ub_format!(
+                            "`dyn*` call on a pointer whose vtable does not match its type"
+                        );
+                    }
+                    let recv = recv.assert_mem_place(); // we passed an MPlaceTy to `unpack_dyn_star` so we definitely still have one
+
+                    (vptr, dyn_ty, recv.ptr)
+                } else {
+                    // Doesn't have to be a `dyn Trait`, but the unsized tail must be `dyn Trait`.
+                    // (For that reason we also cannot use `unpack_dyn_trait`.)
+                    let receiver_tail = self
+                        .tcx
+                        .struct_tail_erasing_lifetimes(receiver_place.layout.ty, self.param_env);
+                    let ty::Dynamic(data, _, ty::Dyn) = receiver_tail.kind() else {
+                            span_bug!(self.cur_span(), "dynamic call on non-`dyn` type {}", receiver_tail)
+                        };
+                    assert!(receiver_place.layout.is_unsized());
+
+                    // Get the required information from the vtable.
+                    let vptr = receiver_place.meta.unwrap_meta().to_pointer(self)?;
+                    let (dyn_ty, dyn_trait) = self.get_ptr_vtable(vptr)?;
+                    if dyn_trait != data.principal() {
+                        throw_ub_format!(
+                            "`dyn` call on a pointer whose vtable does not match its type"
+                        );
+                    }
+
+                    // It might be surprising that we use a pointer as the receiver even if this
+                    // is a by-val case; this works because by-val passing of an unsized `dyn
+                    // Trait` to a function is actually desugared to a pointer.
+                    (vptr, dyn_ty, receiver_place.ptr)
+                };
 
                 // Now determine the actual method to call. We can do that in two different ways and
                 // compare them to ensure everything fits.
                 let Some(ty::VtblEntry::Method(fn_inst)) = self.get_vtable_entries(vptr)?.get(idx).copied() else {
                     throw_ub_format!("`dyn` call trying to call something that is not a method")
                 };
+                trace!("Virtual call dispatches to {fn_inst:#?}");
                 if cfg!(debug_assertions) {
                     let tcx = *self.tcx;
 
                     let trait_def_id = tcx.trait_of_item(def_id).unwrap();
                     let virtual_trait_ref =
                         ty::TraitRef::from_method(tcx, trait_def_id, instance.substs);
-                    assert_eq!(
-                        receiver_tail,
-                        virtual_trait_ref.self_ty(),
-                        "mismatch in underlying dyn trait computation within Miri and MIR building",
-                    );
                     let existential_trait_ref =
                         ty::ExistentialTraitRef::erase_self_ty(tcx, virtual_trait_ref);
                     let concrete_trait_ref = existential_trait_ref.with_self_ty(tcx, dyn_ty);
@@ -620,17 +647,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     assert_eq!(fn_inst, concrete_method);
                 }
 
-                // `*mut receiver_place.layout.ty` is almost the layout that we
-                // want for args[0]: We have to project to field 0 because we want
-                // a thin pointer.
-                assert!(receiver_place.layout.is_unsized());
-                let receiver_ptr_ty = self.tcx.mk_mut_ptr(receiver_place.layout.ty);
-                let this_receiver_ptr = self.layout_of(receiver_ptr_ty)?.field(self, 0);
-                // Adjust receiver argument.
-                args[0] = OpTy::from(ImmTy::from_immediate(
-                    Scalar::from_maybe_pointer(receiver_place.ptr, self).into(),
-                    this_receiver_ptr,
-                ));
+                // Adjust receiver argument. Layout can be any (thin) ptr.
+                args[0] = ImmTy::from_immediate(
+                    Scalar::from_maybe_pointer(adjusted_receiver, self).into(),
+                    self.layout_of(self.tcx.mk_mut_ptr(dyn_ty))?,
+                )
+                .into();
                 trace!("Patched receiver operand to {:#?}", args[0]);
                 // recurse with concrete function
                 self.eval_fn_call(
@@ -659,15 +681,24 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // implementation fail -- a problem shared by rustc.
         let place = self.force_allocation(place)?;
 
-        let (instance, place) = match place.layout.ty.kind() {
-            ty::Dynamic(..) => {
+        let place = match place.layout.ty.kind() {
+            ty::Dynamic(_, _, ty::Dyn) => {
                 // Dropping a trait object. Need to find actual drop fn.
-                let place = self.unpack_dyn_trait(&place)?;
-                let instance = ty::Instance::resolve_drop_in_place(*self.tcx, place.layout.ty);
-                (instance, place)
+                self.unpack_dyn_trait(&place)?.0
+            }
+            ty::Dynamic(_, _, ty::DynStar) => {
+                // Dropping a `dyn*`. Need to find actual drop fn.
+                self.unpack_dyn_star(&place.into())?.0.assert_mem_place()
+            }
+            _ => {
+                debug_assert_eq!(
+                    instance,
+                    ty::Instance::resolve_drop_in_place(*self.tcx, place.layout.ty)
+                );
+                place
             }
-            _ => (instance, place),
         };
+        let instance = ty::Instance::resolve_drop_in_place(*self.tcx, place.layout.ty);
         let fn_abi = self.fn_abi_of_instance(instance, ty::List::empty())?;
 
         let arg = ImmTy::from_immediate(