Rollup merge of #99644 - RalfJung:interpret-int-ptr-transmute, r=oli-obk

remove some provenance-related machine hooks that Miri no longer needs

Then we can make `scalar_to_ptr` a method on `Scalar`. :)

Fixes rust-lang/miri#2188
r? `@oli-obk`

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -173,18 +173,17 @@ pub(super) fn op_to_const<'tcx>(
             Immediate::ScalarPair(a, b) => {
                 debug!("ScalarPair(a: {:?}, b: {:?})", a, b);
                 // We know `offset` is relative to the allocation, so we can use `into_parts`.
-                let (data, start) =
-                    match ecx.scalar_to_ptr(a.check_init().unwrap()).unwrap().into_parts() {
-                        (Some(alloc_id), offset) => {
-                            (ecx.tcx.global_alloc(alloc_id).unwrap_memory(), offset.bytes())
-                        }
-                        (None, _offset) => (
-                            ecx.tcx.intern_const_alloc(
-                                Allocation::from_bytes_byte_aligned_immutable(b"" as &[u8]),
-                            ),
-                            0,
-                        ),
-                    };
+                let (data, start) = match a.to_pointer(ecx).unwrap().into_parts() {
+                    (Some(alloc_id), offset) => {
+                        (ecx.tcx.global_alloc(alloc_id).unwrap_memory(), offset.bytes())
+                    }
+                    (None, _offset) => (
+                        ecx.tcx.intern_const_alloc(Allocation::from_bytes_byte_aligned_immutable(
+                            b"" as &[u8],
+                        )),
+                        0,
+                    ),
+                };
                 let len = b.to_machine_usize(ecx).unwrap();
                 let start = start.try_into().unwrap();
                 let len: usize = len.try_into().unwrap();

compiler/rustc_const_eval/src/interpret/cast.rs

@@ -180,7 +180,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         assert!(cast_ty.is_integral());
 
         let scalar = src.to_scalar()?;
-        let ptr = self.scalar_to_ptr(scalar)?;
+        let ptr = scalar.to_pointer(self)?;
         match ptr.into_pointer_or_addr() {
             Ok(ptr) => M::expose_ptr(self, ptr)?,
             Err(_) => {} // Do nothing, exposing an invalid pointer (`None` provenance) is a NOP.
@@ -299,7 +299,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }
             (&ty::Dynamic(ref data_a, ..), &ty::Dynamic(ref data_b, ..)) => {
                 let (old_data, old_vptr) = self.read_immediate(src)?.to_scalar_pair()?;
-                let old_vptr = self.scalar_to_ptr(old_vptr)?;
+                let old_vptr = old_vptr.to_pointer(self)?;
                 let (ty, old_trait) = self.get_ptr_vtable(old_vptr)?;
                 if old_trait != data_a.principal() {
                     throw_ub_format!("upcast on a pointer whose vtable does not match its type");

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -629,7 +629,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 Ok(Some((size, align)))
             }
             ty::Dynamic(..) => {
-                let vtable = self.scalar_to_ptr(metadata.unwrap_meta())?;
+                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

@@ -245,7 +245,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
             if let ty::Dynamic(..) =
                 tcx.struct_tail_erasing_lifetimes(referenced_ty, self.ecx.param_env).kind()
             {
-                let ptr = self.ecx.scalar_to_ptr(mplace.meta.unwrap_meta())?;
+                let ptr = mplace.meta.unwrap_meta().to_pointer(&tcx)?;
                 if let Some(alloc_id) = ptr.provenance {
                     // Explicitly choose const mode here, since vtables are immutable, even
                     // if the reference of the fat pointer is mutable.

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -135,9 +135,6 @@ pub trait Machine<'mir, 'tcx>: Sized {
     /// Whether to enforce integers and floats being initialized.
     fn enforce_number_init(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
 
-    /// Whether to enforce integers and floats not having provenance.
-    fn enforce_number_no_provenance(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
-
     /// Whether function calls should be [ABI](CallAbi)-checked.
     fn enforce_abi(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
         true
@@ -300,13 +297,6 @@ pub trait Machine<'mir, 'tcx>: Sized {
         addr: u64,
     ) -> InterpResult<'tcx, Pointer<Option<Self::Provenance>>>;
 
-    /// Hook for returning a pointer from a transmute-like operation on an addr.
-    /// This is only needed to support Miri's (unsound) "allow-ptr-int-transmute" flag.
-    fn ptr_from_addr_transmute(
-        ecx: &InterpCx<'mir, 'tcx, Self>,
-        addr: u64,
-    ) -> Pointer<Option<Self::Provenance>>;
-
     /// Marks a pointer as exposed, allowing it's provenance
     /// to be recovered. "Pointer-to-int cast"
     fn expose_ptr(
@@ -469,11 +459,6 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
         true
     }
 
-    #[inline(always)]
-    fn enforce_number_no_provenance(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
-        true
-    }
-
     #[inline(always)]
     fn checked_binop_checks_overflow(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
         true
@@ -518,14 +503,6 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
         ptr
     }
 
-    #[inline(always)]
-    fn ptr_from_addr_transmute(
-        _ecx: &InterpCx<$mir, $tcx, Self>,
-        addr: u64,
-    ) -> Pointer<Option<AllocId>> {
-        Pointer::from_addr(addr)
-    }
-
     #[inline(always)]
     fn ptr_from_addr_cast(
         _ecx: &InterpCx<$mir, $tcx, Self>,

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -9,7 +9,6 @@
 use std::assert_matches::assert_matches;
 use std::borrow::Cow;
 use std::collections::VecDeque;
-use std::convert::TryFrom;
 use std::fmt;
 use std::ptr;
 
@@ -1172,34 +1171,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
 /// Machine pointer introspection.
 impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
-    pub fn scalar_to_ptr(
-        &self,
-        scalar: Scalar<M::Provenance>,
-    ) -> InterpResult<'tcx, Pointer<Option<M::Provenance>>> {
-        // We use `to_bits_or_ptr_internal` since we are just implementing the method people need to
-        // call to force getting out a pointer.
-        Ok(
-            match scalar
-                .to_bits_or_ptr_internal(self.pointer_size())
-                .map_err(|s| err_ub!(ScalarSizeMismatch(s)))?
-            {
-                Err(ptr) => ptr.into(),
-                Ok(bits) => {
-                    let addr = u64::try_from(bits).unwrap();
-                    M::ptr_from_addr_transmute(&self, addr)
-                }
-            },
-        )
-    }
-
     /// Test if this value might be null.
     /// If the machine does not support ptr-to-int casts, this is conservative.
     pub fn scalar_may_be_null(&self, scalar: Scalar<M::Provenance>) -> InterpResult<'tcx, bool> {
         Ok(match scalar.try_to_int() {
             Ok(int) => int.is_null(),
             Err(_) => {
                 // Can only happen during CTFE.
-                let ptr = self.scalar_to_ptr(scalar)?;
+                let ptr = scalar.to_pointer(self)?;
                 match self.ptr_try_get_alloc_id(ptr) {
                     Ok((alloc_id, offset, _)) => {
                         let (size, _align, _kind) = self.get_alloc_info(alloc_id);

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -14,7 +14,7 @@ use rustc_target::abi::{VariantIdx, Variants};
 use super::{
     alloc_range, from_known_layout, mir_assign_valid_types, AllocId, ConstValue, Frame, GlobalId,
     InterpCx, InterpResult, MPlaceTy, Machine, MemPlace, MemPlaceMeta, Place, PlaceTy, Pointer,
-    PointerArithmetic, Provenance, Scalar, ScalarMaybeUninit,
+    Provenance, Scalar, ScalarMaybeUninit,
 };
 
 /// An `Immediate` represents a single immediate self-contained Rust value.
@@ -363,17 +363,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             Abi::Scalar(s) if force => Some(s.primitive()),
             _ => None,
         };
-        let read_provenance = |s: abi::Primitive, size| {
-            // Should be just `s.is_ptr()`, but we support a Miri flag that accepts more
-            // questionable ptr-int transmutes.
-            let number_may_have_provenance = !M::enforce_number_no_provenance(self);
-            s.is_ptr() || (number_may_have_provenance && size == self.pointer_size())
-        };
         if let Some(s) = scalar_layout {
             let size = s.size(self);
             assert_eq!(size, mplace.layout.size, "abi::Scalar size does not match layout size");
-            let scalar =
-                alloc.read_scalar(alloc_range(Size::ZERO, size), read_provenance(s, size))?;
+            let scalar = alloc
+                .read_scalar(alloc_range(Size::ZERO, size), /*read_provenance*/ s.is_ptr())?;
             return Ok(Some(ImmTy { imm: scalar.into(), layout: mplace.layout }));
         }
         let scalar_pair_layout = match mplace.layout.abi {
@@ -391,10 +385,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             let (a_size, b_size) = (a.size(self), b.size(self));
             let b_offset = a_size.align_to(b.align(self).abi);
             assert!(b_offset.bytes() > 0); // in `operand_field` we use the offset to tell apart the fields
-            let a_val =
-                alloc.read_scalar(alloc_range(Size::ZERO, a_size), read_provenance(a, a_size))?;
-            let b_val =
-                alloc.read_scalar(alloc_range(b_offset, b_size), read_provenance(b, b_size))?;
+            let a_val = alloc.read_scalar(
+                alloc_range(Size::ZERO, a_size),
+                /*read_provenance*/ a.is_ptr(),
+            )?;
+            let b_val = alloc
+                .read_scalar(alloc_range(b_offset, b_size), /*read_provenance*/ b.is_ptr())?;
             return Ok(Some(ImmTy {
                 imm: Immediate::ScalarPair(a_val, b_val),
                 layout: mplace.layout,
@@ -459,7 +455,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         &self,
         op: &OpTy<'tcx, M::Provenance>,
     ) -> InterpResult<'tcx, Pointer<Option<M::Provenance>>> {
-        self.scalar_to_ptr(self.read_scalar(op)?.check_init()?)
+        self.read_scalar(op)?.to_pointer(self)
     }
 
     /// Turn the wide MPlace into a string (must already be dereferenced!)

compiler/rustc_const_eval/src/interpret/place.rs

@@ -331,7 +331,7 @@ where
             Immediate::Uninit => throw_ub!(InvalidUninitBytes(None)),
         };
 
-        let mplace = MemPlace { ptr: self.scalar_to_ptr(ptr.check_init()?)?, meta };
+        let mplace = MemPlace { ptr: ptr.to_pointer(self)?, meta };
         // When deref'ing a pointer, the *static* alignment given by the type is what matters.
         let align = layout.align.abi;
         Ok(MPlaceTy { mplace, layout, align })
@@ -889,7 +889,7 @@ where
         &self,
         mplace: &MPlaceTy<'tcx, M::Provenance>,
     ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
-        let vtable = self.scalar_to_ptr(mplace.vtable())?; // also sanity checks the type
+        let vtable = mplace.vtable().to_pointer(self)?; // also sanity checks the type
         let (ty, _) = self.get_ptr_vtable(vtable)?;
         let layout = self.layout_of(ty)?;
 

compiler/rustc_const_eval/src/interpret/terminator.rs

@@ -561,7 +561,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 };
 
                 // Get the required information from the vtable.
-                let vptr = self.scalar_to_ptr(receiver_place.meta.unwrap_meta())?;
+                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!(

compiler/rustc_const_eval/src/interpret/validity.rs

@@ -312,7 +312,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
         let tail = self.ecx.tcx.struct_tail_erasing_lifetimes(pointee.ty, self.ecx.param_env);
         match tail.kind() {
             ty::Dynamic(..) => {
-                let vtable = self.ecx.scalar_to_ptr(meta.unwrap_meta())?;
+                let vtable = meta.unwrap_meta().to_pointer(self.ecx)?;
                 // Make sure it is a genuine vtable pointer.
                 let (_ty, _trait) = try_validation!(
                     self.ecx.get_ptr_vtable(vtable),
@@ -517,15 +517,13 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
                             { "{:x}", value } expected { "initialized bytes" }
                     );
                 }
-                if M::enforce_number_no_provenance(self.ecx) {
-                    // As a special exception we *do* match on a `Scalar` here, since we truly want
-                    // to know its underlying representation (and *not* cast it to an integer).
-                    let is_ptr = value.check_init().map_or(false, |v| matches!(v, Scalar::Ptr(..)));
-                    if is_ptr {
-                        throw_validation_failure!(self.path,
-                            { "{:x}", value } expected { "plain (non-pointer) bytes" }
-                        )
-                    }
+                // As a special exception we *do* match on a `Scalar` here, since we truly want
+                // to know its underlying representation (and *not* cast it to an integer).
+                let is_ptr = value.check_init().map_or(false, |v| matches!(v, Scalar::Ptr(..)));
+                if is_ptr {
+                    throw_validation_failure!(self.path,
+                        { "{:x}", value } expected { "plain (non-pointer) bytes" }
+                    )
                 }
                 Ok(true)
             }
@@ -568,7 +566,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
 
                 // If we check references recursively, also check that this points to a function.
                 if let Some(_) = self.ref_tracking {
-                    let ptr = self.ecx.scalar_to_ptr(value)?;
+                    let ptr = value.to_pointer(self.ecx)?;
                     let _fn = try_validation!(
                         self.ecx.get_ptr_fn(ptr),
                         self.path,
@@ -906,7 +904,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
                 match alloc.check_bytes(
                     alloc_range(Size::ZERO, size),
                     /*allow_uninit*/ !M::enforce_number_init(self.ecx),
-                    /*allow_ptr*/ !M::enforce_number_no_provenance(self.ecx),
+                    /*allow_ptr*/ false,
                 ) {
                     // In the happy case, we needn't check anything else.
                     Ok(()) => {}

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

@@ -331,6 +331,19 @@ impl<Prov> Scalar<Prov> {
 }
 
 impl<'tcx, Prov: Provenance> Scalar<Prov> {
+    pub fn to_pointer(self, cx: &impl HasDataLayout) -> InterpResult<'tcx, Pointer<Option<Prov>>> {
+        match self
+            .to_bits_or_ptr_internal(cx.pointer_size())
+            .map_err(|s| err_ub!(ScalarSizeMismatch(s)))?
+        {
+            Err(ptr) => Ok(ptr.into()),
+            Ok(bits) => {
+                let addr = u64::try_from(bits).unwrap();
+                Ok(Pointer::from_addr(addr))
+            }
+        }
+    }
+
     /// Fundamental scalar-to-int (cast) operation. Many convenience wrappers exist below, that you
     /// likely want to use instead.
     ///
@@ -546,6 +559,11 @@ impl<Prov> ScalarMaybeUninit<Prov> {
 }
 
 impl<'tcx, Prov: Provenance> ScalarMaybeUninit<Prov> {
+    #[inline(always)]
+    pub fn to_pointer(self, cx: &impl HasDataLayout) -> InterpResult<'tcx, Pointer<Option<Prov>>> {
+        self.check_init()?.to_pointer(cx)
+    }
+
     #[inline(always)]
     pub fn to_bool(self) -> InterpResult<'tcx, bool> {
         self.check_init()?.to_bool()