Rollup merge of #70241 - RalfJung:global-mem, r=oli-obk

Miri: move ModifiedStatic to ConstEval errors

Also generally adjust terminology from "static" to "global" where appropriate (to avoid confusion with specifically `static` items).

Author: Centril

src/librustc/mir/interpret/error.rs

@@ -453,9 +453,6 @@ pub enum UnsupportedOpInfo {
     ReadForeignStatic(DefId),
     /// Could not find MIR for a function.
     NoMirFor(DefId),
-    /// Modified a static during const-eval.
-    /// FIXME: move this to `ConstEvalErrKind` through a machine hook.
-    ModifiedStatic,
     /// Encountered a pointer where we needed raw bytes.
     ReadPointerAsBytes,
     /// Encountered raw bytes where we needed a pointer.
@@ -471,12 +468,6 @@ impl fmt::Debug for UnsupportedOpInfo {
                 write!(f, "tried to read from foreign (extern) static {:?}", did)
             }
             NoMirFor(did) => write!(f, "could not load MIR for {:?}", did),
-            ModifiedStatic => write!(
-                f,
-                "tried to modify a static's initial value from another static's \
-                    initializer"
-            ),
-
             ReadPointerAsBytes => write!(f, "unable to turn pointer into raw bytes",),
             ReadBytesAsPointer => write!(f, "unable to turn bytes into a pointer"),
         }

src/librustc/ty/context.rs

@@ -984,6 +984,7 @@ pub struct GlobalCtxt<'tcx> {
     /// Stores the value of constants (and deduplicates the actual memory)
     allocation_interner: ShardedHashMap<&'tcx Allocation, ()>,
 
+    /// Stores memory for globals (statics/consts).
     pub alloc_map: Lock<interpret::AllocMap<'tcx>>,
 
     layout_interner: ShardedHashMap<&'tcx LayoutDetails, ()>,

src/librustc_mir/const_eval/error.rs

@@ -12,6 +12,7 @@ use crate::interpret::{ConstEvalErr, InterpErrorInfo, Machine};
 pub enum ConstEvalErrKind {
     NeedsRfc(String),
     ConstAccessesStatic,
+    ModifiedGlobal,
     AssertFailure(AssertKind<u64>),
     Panic { msg: Symbol, line: u32, col: u32, file: Symbol },
 }
@@ -33,6 +34,9 @@ impl fmt::Display for ConstEvalErrKind {
                 write!(f, "\"{}\" needs an rfc before being allowed inside constants", msg)
             }
             ConstAccessesStatic => write!(f, "constant accesses static"),
+            ModifiedGlobal => {
+                write!(f, "modifying a static's initial value from another static's initializer")
+            }
             AssertFailure(ref msg) => write!(f, "{:?}", msg),
             Panic { msg, line, col, file } => {
                 write!(f, "the evaluated program panicked at '{}', {}:{}:{}", msg, file, line, col)

src/librustc_mir/const_eval/machine.rs

@@ -8,8 +8,9 @@ use std::hash::Hash;
 use rustc_data_structures::fx::FxHashMap;
 
 use rustc::mir::AssertMessage;
-use rustc_span::source_map::Span;
+use rustc_ast::ast::Mutability;
 use rustc_span::symbol::Symbol;
+use rustc_span::{def_id::DefId, Span};
 
 use crate::interpret::{
     self, AllocId, Allocation, GlobalId, ImmTy, InterpCx, InterpResult, Memory, MemoryKind, OpTy,
@@ -167,7 +168,7 @@ impl interpret::MayLeak for ! {
 }
 
 impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter {
-    type MemoryKinds = !;
+    type MemoryKind = !;
     type PointerTag = ();
     type ExtraFnVal = !;
 
@@ -177,7 +178,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter {
 
     type MemoryMap = FxHashMap<AllocId, (MemoryKind<!>, Allocation)>;
 
-    const STATIC_KIND: Option<!> = None; // no copying of statics allowed
+    const GLOBAL_KIND: Option<!> = None; // no copying of globals allowed
 
     // We do not check for alignment to avoid having to carry an `Align`
     // in `ConstValue::ByRef`.
@@ -317,7 +318,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter {
     }
 
     #[inline(always)]
-    fn tag_static_base_pointer(_memory_extra: &MemoryExtra, _id: AllocId) -> Self::PointerTag {}
+    fn tag_global_base_pointer(_memory_extra: &MemoryExtra, _id: AllocId) -> Self::PointerTag {}
 
     fn box_alloc(
         _ecx: &mut InterpCx<'mir, 'tcx, Self>,
@@ -345,11 +346,19 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter {
         Ok(())
     }
 
-    fn before_access_static(
+    fn before_access_global(
         memory_extra: &MemoryExtra,
-        _allocation: &Allocation,
+        alloc_id: AllocId,
+        allocation: &Allocation,
+        def_id: Option<DefId>,
+        is_write: bool,
     ) -> InterpResult<'tcx> {
-        if memory_extra.can_access_statics {
+        if is_write && allocation.mutability == Mutability::Not {
+            Err(err_ub!(WriteToReadOnly(alloc_id)).into())
+        } else if is_write {
+            Err(ConstEvalErrKind::ModifiedGlobal.into())
+        } else if memory_extra.can_access_statics || def_id.is_none() {
+            // `def_id.is_none()` indicates this is not a static, but a const or so.
             Ok(())
         } else {
             Err(ConstEvalErrKind::ConstAccessesStatic.into())

src/librustc_mir/interpret/eval_context.rs

@@ -253,8 +253,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     /// This represents a *direct* access to that memory, as opposed to access
     /// through a pointer that was created by the program.
     #[inline(always)]
-    pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer<M::PointerTag> {
-        self.memory.tag_static_base_pointer(ptr)
+    pub fn tag_global_base_pointer(&self, ptr: Pointer) -> Pointer<M::PointerTag> {
+        self.memory.tag_global_base_pointer(ptr)
     }
 
     #[inline(always)]

src/librustc_mir/interpret/intern.rs

@@ -16,7 +16,7 @@ use super::{AllocId, Allocation, InterpCx, MPlaceTy, Machine, MemoryKind, Scalar
 pub trait CompileTimeMachine<'mir, 'tcx> = Machine<
     'mir,
     'tcx,
-    MemoryKinds = !,
+    MemoryKind = !,
     PointerTag = (),
     ExtraFnVal = !,
     FrameExtra = (),
@@ -104,7 +104,7 @@ fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>(
         MemoryKind::Stack | MemoryKind::Vtable | MemoryKind::CallerLocation => {}
     }
     // Set allocation mutability as appropriate. This is used by LLVM to put things into
-    // read-only memory, and also by Miri when evluating other constants/statics that
+    // read-only memory, and also by Miri when evaluating other globals that
     // access this one.
     if mode == InternMode::Static {
         // When `ty` is `None`, we assume no interior mutability.

src/librustc_mir/interpret/machine.rs

@@ -7,7 +7,7 @@ use std::hash::Hash;
 
 use rustc::mir;
 use rustc::ty::{self, Ty};
-use rustc_span::Span;
+use rustc_span::{def_id::DefId, Span};
 
 use super::{
     AllocId, Allocation, AllocationExtra, Frame, ImmTy, InterpCx, InterpResult, Memory, MemoryKind,
@@ -79,7 +79,7 @@ pub trait AllocMap<K: Hash + Eq, V> {
 /// and some use case dependent behaviour can instead be applied.
 pub trait Machine<'mir, 'tcx>: Sized {
     /// Additional memory kinds a machine wishes to distinguish from the builtin ones
-    type MemoryKinds: ::std::fmt::Debug + MayLeak + Eq + 'static;
+    type MemoryKind: ::std::fmt::Debug + MayLeak + Eq + 'static;
 
     /// Tag tracked alongside every pointer. This is used to implement "Stacked Borrows"
     /// <https://www.ralfj.de/blog/2018/08/07/stacked-borrows.html>.
@@ -105,16 +105,17 @@ pub trait Machine<'mir, 'tcx>: Sized {
     /// Memory's allocation map
     type MemoryMap: AllocMap<
             AllocId,
-            (MemoryKind<Self::MemoryKinds>, Allocation<Self::PointerTag, Self::AllocExtra>),
+            (MemoryKind<Self::MemoryKind>, Allocation<Self::PointerTag, Self::AllocExtra>),
         > + Default
         + Clone;
 
-    /// The memory kind to use for copied statics -- or None if statics should not be mutated
-    /// and thus any such attempt will cause a `ModifiedStatic` error to be raised.
+    /// The memory kind to use for copied global memory (held in `tcx`) --
+    /// or None if such memory should not be mutated and thus any such attempt will cause
+    /// a `ModifiedStatic` error to be raised.
     /// Statics are copied under two circumstances: When they are mutated, and when
-    /// `tag_allocation` or `find_foreign_static` (see below) returns an owned allocation
+    /// `tag_allocation` (see below) returns an owned allocation
     /// that is added to the memory so that the work is not done twice.
-    const STATIC_KIND: Option<Self::MemoryKinds>;
+    const GLOBAL_KIND: Option<Self::MemoryKind>;
 
     /// Whether memory accesses should be alignment-checked.
     const CHECK_ALIGN: bool;
@@ -207,11 +208,15 @@ pub trait Machine<'mir, 'tcx>: Sized {
         Ok(())
     }
 
-    /// Called before a `Static` value is accessed.
+    /// Called before a global allocation is accessed.
+    /// `def_id` is `Some` if this is the "lazy" allocation of a static.
     #[inline]
-    fn before_access_static(
+    fn before_access_global(
         _memory_extra: &Self::MemoryExtra,
+        _alloc_id: AllocId,
         _allocation: &Allocation,
+        _def_id: Option<DefId>,
+        _is_write: bool,
     ) -> InterpResult<'tcx> {
         Ok(())
     }
@@ -231,10 +236,10 @@ pub trait Machine<'mir, 'tcx>: Sized {
     /// it contains (in relocations) tagged.  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 statics, and allocations created by Miri during evaluation.
+    /// for globals, and allocations created by Miri during evaluation.
     ///
     /// `kind` is the kind of the allocation being tagged; it can be `None` when
-    /// it's a static and `STATIC_KIND` is `None`.
+    /// it's a global and `GLOBAL_KIND` is `None`.
     ///
     /// This should avoid copying if no work has to be done! If this returns an owned
     /// allocation (because a copy had to be done to add tags or metadata), machine memory will
@@ -243,20 +248,20 @@ pub trait Machine<'mir, 'tcx>: Sized {
     ///
     /// Also return the "base" tag to use for this allocation: the one that is used for direct
     /// accesses to this allocation. If `kind == STATIC_KIND`, this tag must be consistent
-    /// with `tag_static_base_pointer`.
+    /// with `tag_global_base_pointer`.
     fn init_allocation_extra<'b>(
         memory_extra: &Self::MemoryExtra,
         id: AllocId,
         alloc: Cow<'b, Allocation>,
-        kind: Option<MemoryKind<Self::MemoryKinds>>,
+        kind: Option<MemoryKind<Self::MemoryKind>>,
     ) -> (Cow<'b, Allocation<Self::PointerTag, Self::AllocExtra>>, Self::PointerTag);
 
-    /// 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,
+    /// Return the "base" tag for the given *global* allocation: the one that is used for direct
+    /// accesses to this static/const/fn allocation. If `id` is not a global 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;
+    fn tag_global_base_pointer(memory_extra: &Self::MemoryExtra, id: AllocId) -> Self::PointerTag;
 
     /// Executes a retagging operation
     #[inline]