Rollup merge of #116650 - RalfJung:miri-intptrcast, r=oli-obk

add some comments and some cleanup around Miri intptrcast

`@saethlin` maybe this helps a bit?

Author: matthiaskrgr

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -1235,6 +1235,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
     /// Turning a "maybe pointer" into a proper pointer (and some information
     /// about where it points), or an absolute address.
+    ///
+    /// The result must be used immediately; it is not allowed to convert
+    /// the returned data back into a `Pointer` and store that in machine state.
+    /// (In fact that's not even possible since `M::ProvenanceExtra` is generic and
+    /// we don't have an operation to turn it back into `M::Provenance`.)
     pub fn ptr_try_get_alloc_id(
         &self,
         ptr: Pointer<Option<M::Provenance>>,
@@ -1253,6 +1258,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     }
 
     /// Turning a "maybe pointer" into a proper pointer (and some information about where it points).
+    ///
+    /// The result must be used immediately; it is not allowed to convert
+    /// the returned data back into a `Pointer` and store that in machine state.
+    /// (In fact that's not even possible since `M::ProvenanceExtra` is generic and
+    /// we don't have an operation to turn it back into `M::Provenance`.)
     #[inline(always)]
     pub fn ptr_get_alloc_id(
         &self,

src/tools/miri/src/intptrcast.rs

@@ -119,24 +119,14 @@ impl<'mir, 'tcx> GlobalStateInner {
         Ok(())
     }
 
-    pub fn ptr_from_addr_transmute(
-        _ecx: &MiriInterpCx<'mir, 'tcx>,
-        addr: u64,
-    ) -> Pointer<Option<Provenance>> {
-        trace!("Transmuting {:#x} to a pointer", addr);
-
-        // We consider transmuted pointers to be "invalid" (`None` provenance).
-        Pointer::new(None, Size::from_bytes(addr))
-    }
-
     pub fn ptr_from_addr_cast(
         ecx: &MiriInterpCx<'mir, 'tcx>,
         addr: u64,
     ) -> InterpResult<'tcx, Pointer<Option<Provenance>>> {
         trace!("Casting {:#x} to a pointer", addr);
 
+        // Potentially emit a warning.
         let global_state = ecx.machine.intptrcast.borrow();
-
         match global_state.provenance_mode {
             ProvenanceMode::Default => {
                 // The first time this happens at a particular location, print a warning.
@@ -158,7 +148,12 @@ impl<'mir, 'tcx> GlobalStateInner {
             ProvenanceMode::Permissive => {}
         }
 
-        // This is how wildcard pointers are born.
+        // We do *not* look up the `AllocId` here! This is a `ptr as usize` cast, and it is
+        // completely legal to do a cast and then `wrapping_offset` to another allocation and only
+        // *then* do a memory access. So the allocation that the pointer happens to point to on a
+        // cast is fairly irrelevant. Instead we generate this as a "wildcard" pointer, such that
+        // *every time the pointer is used*, we do an `AllocId` lookup to find the (exposed)
+        // allocation it might be referencing.
         Ok(Pointer::new(Some(Provenance::Wildcard), Size::from_bytes(addr)))
     }
 
@@ -219,22 +214,27 @@ impl<'mir, 'tcx> GlobalStateInner {
         })
     }
 
-    /// Convert a relative (tcx) pointer to an absolute address.
-    pub fn rel_ptr_to_addr(
+    /// Convert a relative (tcx) pointer to a Miri pointer.
+    pub fn ptr_from_rel_ptr(
         ecx: &MiriInterpCx<'mir, 'tcx>,
         ptr: Pointer<AllocId>,
-    ) -> InterpResult<'tcx, u64> {
+        tag: BorTag,
+    ) -> InterpResult<'tcx, Pointer<Provenance>> {
         let (alloc_id, offset) = ptr.into_parts(); // offset is relative (AllocId provenance)
         let base_addr = GlobalStateInner::alloc_base_addr(ecx, alloc_id)?;
 
         // Add offset with the right kind of pointer-overflowing arithmetic.
         let dl = ecx.data_layout();
-        Ok(dl.overflowing_offset(base_addr, offset.bytes()).0)
+        let absolute_addr = dl.overflowing_offset(base_addr, offset.bytes()).0;
+        Ok(Pointer::new(
+            Provenance::Concrete { alloc_id, tag },
+            Size::from_bytes(absolute_addr),
+        ))
     }
 
     /// When a pointer is used for a memory access, this computes where in which allocation the
     /// access is going.
-    pub fn abs_ptr_to_rel(
+    pub fn ptr_get_alloc(
         ecx: &MiriInterpCx<'mir, 'tcx>,
         ptr: Pointer<Provenance>,
     ) -> Option<(AllocId, Size)> {
@@ -252,12 +252,11 @@ impl<'mir, 'tcx> GlobalStateInner {
         let base_addr = GlobalStateInner::alloc_base_addr(ecx, alloc_id).unwrap();
 
         // Wrapping "addr - base_addr"
-        let dl = ecx.data_layout();
         #[allow(clippy::cast_possible_wrap)] // we want to wrap here
         let neg_base_addr = (base_addr as i64).wrapping_neg();
         Some((
             alloc_id,
-            Size::from_bytes(dl.overflowing_signed_offset(addr.bytes(), neg_base_addr).0),
+            Size::from_bytes(ecx.overflowing_signed_offset(addr.bytes(), neg_base_addr).0),
         ))
     }
 

src/tools/miri/src/machine.rs

@@ -168,11 +168,29 @@ impl fmt::Display for MiriMemoryKind {
 /// Pointer provenance.
 #[derive(Clone, Copy)]
 pub enum Provenance {
+    /// For pointers with concrete provenance. we exactly know which allocation they are attached to
+    /// and what their borrow tag is.
     Concrete {
         alloc_id: AllocId,
         /// Borrow Tracker tag.
         tag: BorTag,
     },
+    /// Pointers with wildcard provenance are created on int-to-ptr casts. According to the
+    /// specification, we should at that point angelically "guess" a provenance that will make all
+    /// future uses of this pointer work, if at all possible. Of course such a semantics cannot be
+    /// actually implemented in Miri. So instead, we approximate this, erroring on the side of
+    /// accepting too much code rather than rejecting correct code: a pointer with wildcard
+    /// provenance "acts like" any previously exposed pointer. Each time it is used, we check
+    /// whether *some* exposed pointer could have done what we want to do, and if the answer is yes
+    /// then we allow the access. This allows too much code in two ways:
+    /// - The same wildcard pointer can "take the role" of multiple different exposed pointers on
+    ///   subsequenct memory accesses.
+    /// - In the aliasing model, we don't just have to know the borrow tag of the pointer used for
+    ///   the access, we also have to update the aliasing state -- and that update can be very
+    ///   different depending on which borrow tag we pick! Stacked Borrows has support for this by
+    ///   switching to a stack that is only approximately known, i.e. we overapproximate the effect
+    ///   of using *any* exposed pointer for this access, and only keep information about the borrow
+    ///   stack that would be true with all possible choices.
     Wildcard,
 }
 
@@ -1122,19 +1140,16 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
                 _ => {}
             }
         }
-        let absolute_addr = intptrcast::GlobalStateInner::rel_ptr_to_addr(ecx, ptr)?;
         let tag = if let Some(borrow_tracker) = &ecx.machine.borrow_tracker {
             borrow_tracker.borrow_mut().base_ptr_tag(ptr.provenance, &ecx.machine)
         } else {
             // Value does not matter, SB is disabled
             BorTag::default()
         };
-        Ok(Pointer::new(
-            Provenance::Concrete { alloc_id: ptr.provenance, tag },
-            Size::from_bytes(absolute_addr),
-        ))
+        intptrcast::GlobalStateInner::ptr_from_rel_ptr(ecx, ptr, tag)
     }
 
+    /// Called on `usize as ptr` casts.
     #[inline(always)]
     fn ptr_from_addr_cast(
         ecx: &MiriInterpCx<'mir, 'tcx>,
@@ -1143,6 +1158,9 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
         intptrcast::GlobalStateInner::ptr_from_addr_cast(ecx, addr)
     }
 
+    /// Called on `ptr as usize` casts.
+    /// (Actually computing the resulting `usize` doesn't need machine help,
+    /// that's just `Scalar::try_to_int`.)
     fn expose_ptr(
         ecx: &mut InterpCx<'mir, 'tcx, Self>,
         ptr: Pointer<Self::Provenance>,
@@ -1160,11 +1178,17 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
 
     /// Convert a pointer with provenance into an allocation-offset pair,
     /// or a `None` with an absolute address if that conversion is not possible.
+    ///
+    /// This is called when a pointer is about to be used for memory access,
+    /// an in-bounds check, or anything else that requires knowing which allocation it points to.
+    /// The resulting `AllocId` will just be used for that one step and the forgotten again
+    /// (i.e., we'll never turn the data returned here back into a `Pointer` that might be
+    /// stored in machine state).
     fn ptr_get_alloc(
         ecx: &MiriInterpCx<'mir, 'tcx>,
         ptr: Pointer<Self::Provenance>,
     ) -> Option<(AllocId, Size, Self::ProvenanceExtra)> {
-        let rel = intptrcast::GlobalStateInner::abs_ptr_to_rel(ecx, ptr);
+        let rel = intptrcast::GlobalStateInner::ptr_get_alloc(ecx, ptr);
 
         rel.map(|(alloc_id, size)| {
             let tag = match ptr.provenance {