Auto merge of #69408 - RalfJung:canonical-alloc-id, r=oli-obk

Miri: let machine canonicalize AllocIDs

This implements the rustc side of the plan I laid out [here](rust-lang/miri#1147 (comment)).

Miri PR: rust-lang/miri#1190

Author: bors

src/librustc_mir/const_eval/machine.rs

@@ -1,7 +1,6 @@
 use rustc::mir;
 use rustc::ty::layout::HasTyCtxt;
-use rustc::ty::{self, Ty, TyCtxt};
-use rustc_hir::def_id::DefId;
+use rustc::ty::{self, Ty};
 use std::borrow::{Borrow, Cow};
 use std::collections::hash_map::Entry;
 use std::hash::Hash;
@@ -320,13 +319,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
         Err(ConstEvalErrKind::NeedsRfc("pointer arithmetic or comparison".to_string()).into())
     }
 
-    fn find_foreign_static(
-        _tcx: TyCtxt<'tcx>,
-        _def_id: DefId,
-    ) -> InterpResult<'tcx, Cow<'tcx, Allocation<Self::PointerTag>>> {
-        throw_unsup!(ReadForeignStatic)
-    }
-
     #[inline(always)]
     fn init_allocation_extra<'b>(
         _memory_extra: &MemoryExtra,

src/librustc_mir/interpret/machine.rs

@@ -6,8 +6,7 @@ use std::borrow::{Borrow, Cow};
 use std::hash::Hash;
 
 use rustc::mir;
-use rustc::ty::{self, Ty, TyCtxt};
-use rustc_hir::def_id::DefId;
+use rustc::ty::{self, Ty};
 use rustc_span::Span;
 
 use super::{
@@ -123,10 +122,6 @@ pub trait Machine<'mir, 'tcx>: Sized {
     /// Whether to enforce the validity invariant
     fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
 
-    /// Called before a basic block terminator is executed.
-    /// You can use this to detect endlessly running programs.
-    fn before_terminator(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx>;
-
     /// Entry point to all function calls.
     ///
     /// Returns either the mir to use for the call, or `None` if execution should
@@ -175,18 +170,6 @@ pub trait Machine<'mir, 'tcx>: Sized {
         unwind: Option<mir::BasicBlock>,
     ) -> InterpResult<'tcx>;
 
-    /// Called for read access to a foreign static item.
-    ///
-    /// This will only be called once per static and machine; the result is cached in
-    /// the machine memory. (This relies on `AllocMap::get_or` being able to add the
-    /// owned allocation to the map even when the map is shared.)
-    ///
-    /// This allocation will then be fed to `tag_allocation` to initialize the "extra" state.
-    fn find_foreign_static(
-        tcx: TyCtxt<'tcx>,
-        def_id: DefId,
-    ) -> InterpResult<'tcx, Cow<'tcx, Allocation>>;
-
     /// Called for all binary operations where the LHS has pointer type.
     ///
     /// Returns a (value, overflowed) pair if the operation succeeded
@@ -204,6 +187,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
     ) -> InterpResult<'tcx>;
 
     /// Called to read the specified `local` from the `frame`.
+    #[inline]
     fn access_local(
         _ecx: &InterpCx<'mir, 'tcx, Self>,
         frame: &Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>,
@@ -212,14 +196,33 @@ pub trait Machine<'mir, 'tcx>: Sized {
         frame.locals[local].access()
     }
 
+    /// Called before a basic block terminator is executed.
+    /// You can use this to detect endlessly running programs.
+    #[inline]
+    fn before_terminator(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
+        Ok(())
+    }
+
     /// Called before a `Static` value is accessed.
+    #[inline]
     fn before_access_static(
         _memory_extra: &Self::MemoryExtra,
         _allocation: &Allocation,
     ) -> InterpResult<'tcx> {
         Ok(())
     }
 
+    /// Called for *every* memory access to determine the real ID of the given allocation.
+    /// This provides a way for the machine to "redirect" certain allocations as it sees fit.
+    ///
+    /// This is used by Miri to redirect extern statics to real allocations.
+    ///
+    /// This function must be idempotent.
+    #[inline]
+    fn canonical_alloc_id(_mem: &Memory<'mir, 'tcx, Self>, id: AllocId) -> AllocId {
+        id
+    }
+
     /// Called to initialize the "extra" state of an allocation and make the pointers
     /// it contains (in relocations) tagged.  The way we construct allocations is
     /// to always first construct it without extra and then add the extra.
@@ -247,6 +250,8 @@ pub trait Machine<'mir, 'tcx>: Sized {
     /// Return the "base" tag for the given *static* allocation: the one that is used for direct
     /// accesses to this static/const/fn allocation. If `id` is not a static allocation,
     /// this will return an unusable tag (i.e., accesses will be UB)!
+    ///
+    /// Expects `id` to be already canonical, if needed.
     fn tag_static_base_pointer(memory_extra: &Self::MemoryExtra, id: AllocId) -> Self::PointerTag;
 
     /// Executes a retagging operation
@@ -259,7 +264,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
         Ok(())
     }
 
-    /// Called immediately before a new stack frame got pushed
+    /// Called immediately before a new stack frame got pushed.
     fn stack_push(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx, Self::FrameExtra>;
 
     /// Called immediately after a stack frame gets popped

src/librustc_mir/interpret/memory.rs

@@ -150,7 +150,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
     /// through a pointer that was created by the program.
     #[inline]
     pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer<M::PointerTag> {
-        ptr.with_tag(M::tag_static_base_pointer(&self.extra, ptr.alloc_id))
+        let id = M::canonical_alloc_id(self, ptr.alloc_id);
+        ptr.with_tag(M::tag_static_base_pointer(&self.extra, id))
     }
 
     pub fn create_fn_alloc(
@@ -421,6 +422,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
     /// The `GlobalAlloc::Memory` branch here is still reachable though; when a static
     /// contains a reference to memory that was created during its evaluation (i.e., not to
     /// another static), those inner references only exist in "resolved" form.
+    ///
+    /// Assumes `id` is already canonical.
     fn get_static_alloc(
         memory_extra: &M::MemoryExtra,
         tcx: TyCtxtAt<'tcx>,
@@ -434,31 +437,30 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
             Some(GlobalAlloc::Static(def_id)) => {
                 // We got a "lazy" static that has not been computed yet.
                 if tcx.is_foreign_item(def_id) {
-                    trace!("static_alloc: foreign item {:?}", def_id);
-                    M::find_foreign_static(tcx.tcx, def_id)?
-                } else {
-                    trace!("static_alloc: Need to compute {:?}", def_id);
-                    let instance = Instance::mono(tcx.tcx, def_id);
-                    let gid = GlobalId { instance, promoted: None };
-                    // use the raw query here to break validation cycles. Later uses of the static
-                    // will call the full query anyway
-                    let raw_const =
-                        tcx.const_eval_raw(ty::ParamEnv::reveal_all().and(gid)).map_err(|err| {
-                            // no need to report anything, the const_eval call takes care of that
-                            // for statics
-                            assert!(tcx.is_static(def_id));
-                            match err {
-                                ErrorHandled::Reported => err_inval!(ReferencedConstant),
-                                ErrorHandled::TooGeneric => err_inval!(TooGeneric),
-                            }
-                        })?;
-                    // Make sure we use the ID of the resolved memory, not the lazy one!
-                    let id = raw_const.alloc_id;
-                    let allocation = tcx.alloc_map.lock().unwrap_memory(id);
-
-                    M::before_access_static(memory_extra, allocation)?;
-                    Cow::Borrowed(allocation)
+                    trace!("get_static_alloc: foreign item {:?}", def_id);
+                    throw_unsup!(ReadForeignStatic)
                 }
+                trace!("get_static_alloc: Need to compute {:?}", def_id);
+                let instance = Instance::mono(tcx.tcx, def_id);
+                let gid = GlobalId { instance, promoted: None };
+                // use the raw query here to break validation cycles. Later uses of the static
+                // will call the full query anyway
+                let raw_const =
+                    tcx.const_eval_raw(ty::ParamEnv::reveal_all().and(gid)).map_err(|err| {
+                        // no need to report anything, the const_eval call takes care of that
+                        // for statics
+                        assert!(tcx.is_static(def_id));
+                        match err {
+                            ErrorHandled::Reported => err_inval!(ReferencedConstant),
+                            ErrorHandled::TooGeneric => err_inval!(TooGeneric),
+                        }
+                    })?;
+                // Make sure we use the ID of the resolved memory, not the lazy one!
+                let id = raw_const.alloc_id;
+                let allocation = tcx.alloc_map.lock().unwrap_memory(id);
+
+                M::before_access_static(memory_extra, allocation)?;
+                Cow::Borrowed(allocation)
             }
         };
         // We got tcx memory. Let the machine initialize its "extra" stuff.
@@ -478,6 +480,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
         &self,
         id: AllocId,
     ) -> InterpResult<'tcx, &Allocation<M::PointerTag, M::AllocExtra>> {
+        let id = M::canonical_alloc_id(self, id);
         // The error type of the inner closure here is somewhat funny.  We have two
         // ways of "erroring": An actual error, or because we got a reference from
         // `get_static_alloc` that we can actually use directly without inserting anything anywhere.
@@ -513,6 +516,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
         &mut self,
         id: AllocId,
     ) -> InterpResult<'tcx, &mut Allocation<M::PointerTag, M::AllocExtra>> {
+        let id = M::canonical_alloc_id(self, id);
         let tcx = self.tcx;
         let memory_extra = &self.extra;
         let a = self.alloc_map.get_mut_or(id, || {
@@ -550,6 +554,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
         id: AllocId,
         liveness: AllocCheck,
     ) -> InterpResult<'static, (Size, Align)> {
+        let id = M::canonical_alloc_id(self, id);
         // # Regular allocations
         // Don't use `self.get_raw` here as that will
         // a) cause cycles in case `id` refers to a static
@@ -602,6 +607,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
         }
     }
 
+    /// Assumes `id` is already canonical.
     fn get_fn_alloc(&self, id: AllocId) -> Option<FnVal<'tcx, M::ExtraFnVal>> {
         trace!("reading fn ptr: {}", id);
         if let Some(extra) = self.extra_fn_ptr_map.get(&id) {
@@ -622,7 +628,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
         if ptr.offset.bytes() != 0 {
             throw_unsup!(InvalidFunctionPointer)
         }
-        self.get_fn_alloc(ptr.alloc_id).ok_or_else(|| err_unsup!(ExecuteMemory).into())
+        let id = M::canonical_alloc_id(self, ptr.alloc_id);
+        self.get_fn_alloc(id).ok_or_else(|| err_unsup!(ExecuteMemory).into())
     }
 
     pub fn mark_immutable(&mut self, id: AllocId) -> InterpResult<'tcx> {

src/librustc_mir/transform/const_prop.rs

@@ -23,7 +23,6 @@ use rustc::ty::{self, ConstKind, Instance, ParamEnv, Ty, TyCtxt, TypeFoldable};
 use rustc_ast::ast::Mutability;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::def::DefKind;
-use rustc_hir::def_id::DefId;
 use rustc_hir::HirId;
 use rustc_index::vec::IndexVec;
 use rustc_infer::traits;
@@ -222,13 +221,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine {
         ));
     }
 
-    fn find_foreign_static(
-        _tcx: TyCtxt<'tcx>,
-        _def_id: DefId,
-    ) -> InterpResult<'tcx, Cow<'tcx, Allocation<Self::PointerTag>>> {
-        throw_unsup!(ReadForeignStatic)
-    }
-
     #[inline(always)]
     fn init_allocation_extra<'b>(
         _memory_extra: &(),
@@ -279,10 +271,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine {
         Ok(())
     }
 
-    fn before_terminator(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
-        Ok(())
-    }
-
     #[inline(always)]
     fn stack_push(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
         Ok(())