Rollup merge of rust-lang#124030 - RalfJung:adjust_alloc_base_pointer, r=oli-obk

interpret: pass MemoryKind to adjust_alloc_base_pointer

Another puzzle piece for rust-lang/miri#3475.

The 2nd commit renames base_pointer -> root_pointer; that's how Tree Borrows already calls them and I think the term is more clear than "base pointer". In particular, this distinguishes it from "base address", since a root pointer can point anywhere into an allocation, not just its base address.

rust-lang#124018 has been rolled up already so I couldn't add it there any more.

r? `@oli-obk`

Author: RalfJung

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -288,28 +288,19 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
     }
 
     /// Return the `AllocId` for the given thread-local static in the current thread.
-    fn thread_local_static_base_pointer(
+    fn thread_local_static_pointer(
         _ecx: &mut InterpCx<'mir, 'tcx, Self>,
         def_id: DefId,
     ) -> InterpResult<'tcx, Pointer<Self::Provenance>> {
         throw_unsup!(ThreadLocalStatic(def_id))
     }
 
-    /// Return the root pointer for the given `extern static`.
-    fn extern_static_base_pointer(
+    /// Return the `AllocId` for the given `extern static`.
+    fn extern_static_pointer(
         ecx: &InterpCx<'mir, 'tcx, Self>,
         def_id: DefId,
     ) -> InterpResult<'tcx, Pointer<Self::Provenance>>;
 
-    /// Return a "base" pointer for the given allocation: the one that is used for direct
-    /// accesses to this static/const/fn allocation, or the one returned from the heap allocator.
-    ///
-    /// Not called on `extern` or thread-local statics (those use the methods above).
-    fn adjust_alloc_base_pointer(
-        ecx: &InterpCx<'mir, 'tcx, Self>,
-        ptr: Pointer,
-    ) -> InterpResult<'tcx, Pointer<Self::Provenance>>;
-
     /// "Int-to-pointer cast"
     fn ptr_from_addr_cast(
         ecx: &InterpCx<'mir, 'tcx, Self>,
@@ -336,6 +327,8 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
 
     /// Called to adjust allocations to the Provenance and AllocExtra of this machine.
     ///
+    /// If `alloc` contains pointers, then they are all pointing to globals.
+    ///
     /// The way we construct allocations is to always first construct it without extra and then add
     /// the extra. This keeps uniform code paths for handling both allocations created by CTFE for
     /// globals, and allocations created by Miri during evaluation.
@@ -354,6 +347,19 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
         kind: Option<MemoryKind<Self::MemoryKind>>,
     ) -> InterpResult<'tcx, Cow<'b, Allocation<Self::Provenance, Self::AllocExtra, Self::Bytes>>>;
 
+    /// Return a "root" pointer for the given allocation: the one that is used for direct
+    /// accesses to this static/const/fn allocation, or the one returned from the heap allocator.
+    ///
+    /// Not called on `extern` or thread-local statics (those use the methods above).
+    ///
+    /// `kind` is the kind of the allocation the pointer points to; it can be `None` when
+    /// it's a global and `GLOBAL_KIND` is `None`.
+    fn adjust_alloc_root_pointer(
+        ecx: &InterpCx<'mir, 'tcx, Self>,
+        ptr: Pointer,
+        kind: Option<MemoryKind<Self::MemoryKind>>,
+    ) -> InterpResult<'tcx, Pointer<Self::Provenance>>;
+
     /// Evaluate the inline assembly.
     ///
     /// This should take care of jumping to the next block (one of `targets`) when asm goto
@@ -592,7 +598,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
         Ok(alloc)
     }
 
-    fn extern_static_base_pointer(
+    fn extern_static_pointer(
         ecx: &InterpCx<$mir, $tcx, Self>,
         def_id: DefId,
     ) -> InterpResult<$tcx, Pointer> {
@@ -601,9 +607,10 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
     }
 
     #[inline(always)]
-    fn adjust_alloc_base_pointer(
+    fn adjust_alloc_root_pointer(
         _ecx: &InterpCx<$mir, $tcx, Self>,
         ptr: Pointer<CtfeProvenance>,
+        _kind: Option<MemoryKind<Self::MemoryKind>>,
     ) -> InterpResult<$tcx, Pointer<CtfeProvenance>> {
         Ok(ptr)
     }

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -165,7 +165,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     ///
     /// This function can fail only if `ptr` points to an `extern static`.
     #[inline]
-    pub fn global_base_pointer(
+    pub fn global_root_pointer(
         &self,
         ptr: Pointer<CtfeProvenance>,
     ) -> InterpResult<'tcx, Pointer<M::Provenance>> {
@@ -178,12 +178,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 bug!("global memory cannot point to thread-local static")
             }
             Some(GlobalAlloc::Static(def_id)) if self.tcx.is_foreign_item(def_id) => {
-                return M::extern_static_base_pointer(self, def_id);
+                return M::extern_static_pointer(self, def_id);
+            }
+            None => {
+                assert!(
+                    self.memory.extra_fn_ptr_map.contains_key(&alloc_id),
+                    "{alloc_id:?} is neither global nor a function pointer"
+                );
             }
             _ => {}
         }
         // And we need to get the provenance.
-        M::adjust_alloc_base_pointer(self, ptr)
+        M::adjust_alloc_root_pointer(self, ptr, M::GLOBAL_KIND.map(MemoryKind::Machine))
     }
 
     pub fn fn_ptr(&mut self, fn_val: FnVal<'tcx, M::ExtraFnVal>) -> Pointer<M::Provenance> {
@@ -197,9 +203,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 id
             }
         };
-        // Functions are global allocations, so make sure we get the right base pointer.
+        // Functions are global allocations, so make sure we get the right root pointer.
         // We know this is not an `extern static` so this cannot fail.
-        self.global_base_pointer(Pointer::from(id)).unwrap()
+        self.global_root_pointer(Pointer::from(id)).unwrap()
     }
 
     pub fn allocate_ptr(
@@ -240,7 +246,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         );
         let alloc = M::adjust_allocation(self, id, Cow::Owned(alloc), Some(kind))?;
         self.memory.alloc_map.insert(id, (kind, alloc.into_owned()));
-        M::adjust_alloc_base_pointer(self, Pointer::from(id))
+        M::adjust_alloc_root_pointer(self, Pointer::from(id), Some(kind))
     }
 
     pub fn reallocate_ptr(

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -764,15 +764,15 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // Other cases need layout.
         let adjust_scalar = |scalar| -> InterpResult<'tcx, _> {
             Ok(match scalar {
-                Scalar::Ptr(ptr, size) => Scalar::Ptr(self.global_base_pointer(ptr)?, size),
+                Scalar::Ptr(ptr, size) => Scalar::Ptr(self.global_root_pointer(ptr)?, size),
                 Scalar::Int(int) => Scalar::Int(int),
             })
         };
         let layout = from_known_layout(self.tcx, self.param_env, layout, || self.layout_of(ty))?;
         let imm = match val_val {
             mir::ConstValue::Indirect { alloc_id, offset } => {
                 // This is const data, no mutation allowed.
-                let ptr = self.global_base_pointer(Pointer::new(
+                let ptr = self.global_root_pointer(Pointer::new(
                     CtfeProvenance::from(alloc_id).as_immutable(),
                     offset,
                 ))?;
@@ -784,7 +784,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 // This is const data, no mutation allowed.
                 let alloc_id = self.tcx.reserve_and_set_memory_alloc(data);
                 let ptr = Pointer::new(CtfeProvenance::from(alloc_id).as_immutable(), Size::ZERO);
-                Immediate::new_slice(self.global_base_pointer(ptr)?.into(), meta, self)
+                Immediate::new_slice(self.global_root_pointer(ptr)?.into(), meta, self)
             }
         };
         Ok(OpTy { op: Operand::Immediate(imm), layout })

compiler/rustc_const_eval/src/interpret/place.rs

@@ -1010,7 +1010,7 @@ where
     ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
         // This must be an allocation in `tcx`
         let _ = self.tcx.global_alloc(raw.alloc_id);
-        let ptr = self.global_base_pointer(Pointer::from(raw.alloc_id))?;
+        let ptr = self.global_root_pointer(Pointer::from(raw.alloc_id))?;
         let layout = self.layout_of(raw.ty)?;
         Ok(self.ptr_to_mplace(ptr.into(), layout))
     }

compiler/rustc_const_eval/src/interpret/step.rs

@@ -144,7 +144,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         use rustc_middle::mir::Rvalue::*;
         match *rvalue {
             ThreadLocalRef(did) => {
-                let ptr = M::thread_local_static_base_pointer(self, did)?;
+                let ptr = M::thread_local_static_pointer(self, did)?;
                 self.write_pointer(ptr, &dest)?;
             }
 

compiler/rustc_const_eval/src/interpret/traits.rs

@@ -28,7 +28,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         ensure_monomorphic_enough(*self.tcx, poly_trait_ref)?;
 
         let vtable_symbolic_allocation = self.tcx.reserve_and_set_vtable_alloc(ty, poly_trait_ref);
-        let vtable_ptr = self.global_base_pointer(Pointer::from(vtable_symbolic_allocation))?;
+        let vtable_ptr = self.global_root_pointer(Pointer::from(vtable_symbolic_allocation))?;
         Ok(vtable_ptr.into())
     }
 

src/tools/miri/src/alloc_addresses/mod.rs

@@ -141,7 +141,11 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         }
     }
 
-    fn addr_from_alloc_id(&self, alloc_id: AllocId) -> InterpResult<'tcx, u64> {
+    fn addr_from_alloc_id(
+        &self,
+        alloc_id: AllocId,
+        _kind: MemoryKind,
+    ) -> InterpResult<'tcx, u64> {
         let ecx = self.eval_context_ref();
         let mut global_state = ecx.machine.alloc_addresses.borrow_mut();
         let global_state = &mut *global_state;
@@ -283,16 +287,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     }
 
     /// Convert a relative (tcx) pointer to a Miri pointer.
-    fn ptr_from_rel_ptr(
+    fn adjust_alloc_root_pointer(
         &self,
         ptr: Pointer<CtfeProvenance>,
         tag: BorTag,
+        kind: MemoryKind,
     ) -> InterpResult<'tcx, Pointer<Provenance>> {
         let ecx = self.eval_context_ref();
 
         let (prov, offset) = ptr.into_parts(); // offset is relative (AllocId provenance)
         let alloc_id = prov.alloc_id();
-        let base_addr = ecx.addr_from_alloc_id(alloc_id)?;
+        let base_addr = ecx.addr_from_alloc_id(alloc_id, kind)?;
 
         // Add offset with the right kind of pointer-overflowing arithmetic.
         let dl = ecx.data_layout();
@@ -314,9 +319,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             ecx.alloc_id_from_addr(addr.bytes())?
         };
 
-        // This cannot fail: since we already have a pointer with that provenance, rel_ptr_to_addr
+        // This cannot fail: since we already have a pointer with that provenance, adjust_alloc_root_pointer
         // must have been called in the past, so we can just look up the address in the map.
-        let base_addr = ecx.addr_from_alloc_id(alloc_id).unwrap();
+        let base_addr = *ecx.machine.alloc_addresses.borrow().base_addr.get(&alloc_id).unwrap();
 
         // Wrapping "addr - base_addr"
         #[allow(clippy::cast_possible_wrap)] // we want to wrap here

src/tools/miri/src/borrow_tracker/mod.rs

@@ -89,10 +89,10 @@ pub struct GlobalStateInner {
     borrow_tracker_method: BorrowTrackerMethod,
     /// Next unused pointer ID (tag).
     next_ptr_tag: BorTag,
-    /// Table storing the "base" tag for each allocation.
-    /// The base tag is the one used for the initial pointer.
+    /// Table storing the "root" tag for each allocation.
+    /// The root tag is the one used for the initial pointer.
     /// We need this in a separate table to handle cyclic statics.
-    base_ptr_tags: FxHashMap<AllocId, BorTag>,
+    root_ptr_tags: FxHashMap<AllocId, BorTag>,
     /// Next unused call ID (for protectors).
     next_call_id: CallId,
     /// All currently protected tags.
@@ -175,7 +175,7 @@ impl GlobalStateInner {
         GlobalStateInner {
             borrow_tracker_method,
             next_ptr_tag: BorTag::one(),
-            base_ptr_tags: FxHashMap::default(),
+            root_ptr_tags: FxHashMap::default(),
             next_call_id: NonZero::new(1).unwrap(),
             protected_tags: FxHashMap::default(),
             tracked_pointer_tags,
@@ -213,8 +213,8 @@ impl GlobalStateInner {
         }
     }
 
-    pub fn base_ptr_tag(&mut self, id: AllocId, machine: &MiriMachine<'_, '_>) -> BorTag {
-        self.base_ptr_tags.get(&id).copied().unwrap_or_else(|| {
+    pub fn root_ptr_tag(&mut self, id: AllocId, machine: &MiriMachine<'_, '_>) -> BorTag {
+        self.root_ptr_tags.get(&id).copied().unwrap_or_else(|| {
             let tag = self.new_ptr();
             if self.tracked_pointer_tags.contains(&tag) {
                 machine.emit_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(
@@ -223,14 +223,14 @@ impl GlobalStateInner {
                     None,
                 ));
             }
-            trace!("New allocation {:?} has base tag {:?}", id, tag);
-            self.base_ptr_tags.try_insert(id, tag).unwrap();
+            trace!("New allocation {:?} has rpot tag {:?}", id, tag);
+            self.root_ptr_tags.try_insert(id, tag).unwrap();
             tag
         })
     }
 
     pub fn remove_unreachable_allocs(&mut self, allocs: &LiveAllocs<'_, '_, '_>) {
-        self.base_ptr_tags.retain(|id, _| allocs.is_live(*id));
+        self.root_ptr_tags.retain(|id, _| allocs.is_live(*id));
     }
 }
 

src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs

@@ -20,7 +20,7 @@ fn err_sb_ub<'tcx>(
 #[derive(Clone, Debug)]
 pub struct AllocHistory {
     id: AllocId,
-    base: (Item, Span),
+    root: (Item, Span),
     creations: smallvec::SmallVec<[Creation; 1]>,
     invalidations: smallvec::SmallVec<[Invalidation; 1]>,
     protectors: smallvec::SmallVec<[Protection; 1]>,
@@ -225,7 +225,7 @@ impl AllocHistory {
     pub fn new(id: AllocId, item: Item, machine: &MiriMachine<'_, '_>) -> Self {
         Self {
             id,
-            base: (item, machine.current_span()),
+            root: (item, machine.current_span()),
             creations: SmallVec::new(),
             invalidations: SmallVec::new(),
             protectors: SmallVec::new(),
@@ -342,15 +342,15 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> {
                 })
             })
             .or_else(|| {
-                // If we didn't find a retag that created this tag, it might be the base tag of
+                // If we didn't find a retag that created this tag, it might be the root tag of
                 // this allocation.
-                if self.history.base.0.tag() == tag {
+                if self.history.root.0.tag() == tag {
                     Some((
                         format!(
-                            "{tag:?} was created here, as the base tag for {:?}",
+                            "{tag:?} was created here, as the root tag for {:?}",
                             self.history.id
                         ),
-                        self.history.base.1.data(),
+                        self.history.root.1.data(),
                     ))
                 } else {
                     None

src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs

@@ -518,9 +518,9 @@ impl Stacks {
             // not through a pointer). That is, whenever we directly write to a local, this will pop
             // everything else off the stack, invalidating all previous pointers,
             // and in particular, *all* raw pointers.
-            MemoryKind::Stack => (state.base_ptr_tag(id, machine), Permission::Unique),
+            MemoryKind::Stack => (state.root_ptr_tag(id, machine), Permission::Unique),
             // Everything else is shared by default.
-            _ => (state.base_ptr_tag(id, machine), Permission::SharedReadWrite),
+            _ => (state.root_ptr_tag(id, machine), Permission::SharedReadWrite),
         };
         Stacks::new(size, perm, base_tag, id, machine)
     }

src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs

@@ -47,7 +47,7 @@ impl Stack {
         let mut first_removed = None;
 
         // We never consider removing the bottom-most tag. For stacks without an unknown
-        // bottom this preserves the base tag.
+        // bottom this preserves the root tag.
         // Note that the algorithm below is based on considering the tag at read_idx - 1,
         // so precisely considering the tag at index 0 for removal when we have an unknown
         // bottom would complicate the implementation. The simplification of not considering
@@ -93,7 +93,7 @@ impl Stack {
                 self.unique_range = 0..self.len();
             }
 
-            // Replace any Items which have been collected with the base item, a known-good value.
+            // Replace any Items which have been collected with the root item, a known-good value.
             for i in 0..CACHE_LEN {
                 if self.cache.idx[i] >= first_removed {
                     self.cache.items[i] = self.borrows[0];
@@ -331,7 +331,7 @@ impl<'tcx> Stack {
         self.verify_cache_consistency();
     }
 
-    /// Construct a new `Stack` using the passed `Item` as the base tag.
+    /// Construct a new `Stack` using the passed `Item` as the root tag.
     pub fn new(item: Item) -> Self {
         Stack {
             borrows: vec![item],
@@ -438,8 +438,8 @@ impl<'tcx> Stack {
             let mut removed = 0;
             let mut cursor = 0;
             // Remove invalid entries from the cache by rotating them to the end of the cache, then
-            // keep track of how many invalid elements there are and overwrite them with the base tag.
-            // The base tag here serves as a harmless default value.
+            // keep track of how many invalid elements there are and overwrite them with the root tag.
+            // The root tag here serves as a harmless default value.
             for _ in 0..CACHE_LEN - 1 {
                 if self.cache.idx[cursor] >= start {
                     self.cache.idx[cursor..CACHE_LEN - removed].rotate_left(1);

src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs

@@ -37,7 +37,7 @@ impl<'tcx> Tree {
         _kind: MemoryKind,
         machine: &MiriMachine<'_, 'tcx>,
     ) -> Self {
-        let tag = state.base_ptr_tag(id, machine); // Fresh tag for the root
+        let tag = state.root_ptr_tag(id, machine); // Fresh tag for the root
         let span = machine.current_span();
         Tree::new(tag, size, span)
     }