Tag static/const allocations (#748)

Tag static/const allocations

rust-version

@@ -1 +1 @@
-8b40a188cee5bef97526dfc271afbd2a98008183
+627486af15d222bcba336b12ea92a05237cc9ab1

src/intrinsic.rs

@@ -5,7 +5,7 @@ use rustc::ty;
 
 use crate::{
     PlaceTy, OpTy, ImmTy, Immediate, Scalar, ScalarMaybeUndef, Tag,
-    OperatorEvalContextExt
+    OperatorEvalContextExt, MiriMemoryKind,
 };
 
 impl<'a, 'mir, 'tcx> EvalContextExt<'a, 'mir, 'tcx> for crate::MiriEvalContext<'a, 'mir, 'tcx> {}
@@ -401,7 +401,8 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a,
             "type_name" => {
                 let ty = substs.type_at(0);
                 let ty_name = ty.to_string();
-                let value = this.str_to_immediate(&ty_name)?;
+                let ptr = this.memory_mut().allocate_static_bytes(ty_name.as_bytes(), MiriMemoryKind::Static.into());
+                let value = Immediate::new_slice(Scalar::Ptr(ptr), ty_name.len() as u64, this);
                 this.write_immediate(value, dest)?;
             }
 

src/lib.rs

@@ -30,7 +30,7 @@ use rand::SeedableRng;
 
 use rustc::ty::{self, TyCtxt, query::TyCtxtAt};
 use rustc::ty::layout::{LayoutOf, Size, Align};
-use rustc::hir::{self, def_id::DefId};
+use rustc::hir::def_id::DefId;
 use rustc::mir;
 pub use rustc_mir::interpret::*;
 // Resolve ambiguity.
@@ -113,7 +113,7 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
 
     // Return value (in static memory so that it does not count as leak).
     let ret = ecx.layout_of(start_mir.return_ty())?;
-    let ret_ptr = ecx.allocate(ret, MiriMemoryKind::MutStatic.into());
+    let ret_ptr = ecx.allocate(ret, MiriMemoryKind::Static.into());
 
     // Push our stack frame.
     ecx.push_stack_frame(
@@ -128,7 +128,7 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
     let mut args = ecx.frame().mir.args_iter();
 
     // First argument: pointer to `main()`.
-    let main_ptr = ecx.memory_mut().create_fn_alloc(main_instance).with_default_tag();
+    let main_ptr = ecx.memory_mut().create_fn_alloc(main_instance);
     let dest = ecx.eval_place(&mir::Place::Base(mir::PlaceBase::Local(args.next().unwrap())))?;
     ecx.write_scalar(Scalar::Ptr(main_ptr), dest)?;
 
@@ -162,7 +162,7 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
         // Add `0` terminator.
         let mut arg = arg.into_bytes();
         arg.push(0);
-        argvs.push(ecx.memory_mut().allocate_static_bytes(arg.as_slice()).with_default_tag());
+        argvs.push(ecx.memory_mut().allocate_static_bytes(arg.as_slice(), MiriMemoryKind::Static.into()));
     }
     // Make an array with all these pointers, in the Miri memory.
     let argvs_layout = ecx.layout_of(ecx.tcx.mk_array(ecx.tcx.mk_imm_ptr(ecx.tcx.types.u8), argvs.len() as u64))?;
@@ -299,8 +299,8 @@ pub enum MiriMemoryKind {
     C,
     /// Part of env var emulation.
     Env,
-    /// Mutable statics.
-    MutStatic,
+    /// Statics.
+    Static,
 }
 
 impl Into<MemoryKind<MiriMemoryKind>> for MiriMemoryKind {
@@ -316,7 +316,7 @@ impl MayLeak for MiriMemoryKind {
         use self::MiriMemoryKind::*;
         match self {
             Rust | C => false,
-            Env | MutStatic => true,
+            Env | Static => true,
         }
     }
 }
@@ -392,7 +392,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
 
     type MemoryMap = MonoHashMap<AllocId, (MemoryKind<MiriMemoryKind>, Allocation<Tag, Self::AllocExtra>)>;
 
-    const STATIC_KIND: Option<MiriMemoryKind> = Some(MiriMemoryKind::MutStatic);
+    const STATIC_KIND: Option<MiriMemoryKind> = Some(MiriMemoryKind::Static);
 
     #[inline(always)]
     fn enforce_validity(ecx: &InterpretCx<'a, 'mir, 'tcx, Self>) -> bool {
@@ -476,8 +476,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
     fn find_foreign_static(
         def_id: DefId,
         tcx: TyCtxtAt<'a, 'tcx, 'tcx>,
-        memory_extra: &Self::MemoryExtra,
-    ) -> EvalResult<'tcx, Cow<'tcx, Allocation<Tag, Self::AllocExtra>>> {
+    ) -> EvalResult<'tcx, Cow<'tcx, Allocation>> {
         let attrs = tcx.get_attrs(def_id);
         let link_name = match attr::first_attr_value_str_by_name(&attrs, sym::link_name) {
             Some(name) => name.as_str(),
@@ -489,8 +488,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
                 // This should be all-zero, pointer-sized.
                 let size = tcx.data_layout.pointer_size;
                 let data = vec![0; size.bytes() as usize];
-                let extra = Stacks::new(size, Tag::default(), Rc::clone(memory_extra));
-                Allocation::from_bytes(&data, tcx.data_layout.pointer_align.abi, extra)
+                Allocation::from_bytes(&data, tcx.data_layout.pointer_align.abi)
             }
             _ => return err!(Unimplemented(
                     format!("can't access foreign static: {}", link_name),
@@ -506,47 +504,48 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
         Ok(())
     }
 
-    fn adjust_static_allocation<'b>(
-        alloc: &'b Allocation,
+    fn tag_allocation<'b>(
+        id: AllocId,
+        alloc: Cow<'b, Allocation>,
+        kind: Option<MemoryKind<Self::MemoryKinds>>,
         memory_extra: &Self::MemoryExtra,
-    ) -> Cow<'b, Allocation<Tag, Self::AllocExtra>> {
-        let extra = Stacks::new(
+    ) -> (Cow<'b, Allocation<Self::PointerTag, Self::AllocExtra>>, Self::PointerTag) {
+        let kind = kind.expect("we set our STATIC_KIND so this cannot be None");
+        let alloc = alloc.into_owned();
+        let (extra, base_tag) = Stacks::new_allocation(
+            id,
             Size::from_bytes(alloc.bytes.len() as u64),
-            Tag::default(),
             Rc::clone(memory_extra),
+            kind,
         );
+        if kind != MiriMemoryKind::Static.into() {
+            assert!(alloc.relocations.is_empty(), "Only statics can come initialized with inner pointers");
+            // Now we can rely on the inner pointers being static, too.
+        }
+        let mut memory_extra = memory_extra.borrow_mut();
         let alloc: Allocation<Tag, Self::AllocExtra> = Allocation {
-            bytes: alloc.bytes.clone(),
+            bytes: alloc.bytes,
             relocations: Relocations::from_presorted(
                 alloc.relocations.iter()
-                    .map(|&(offset, ((), alloc))| (offset, (Tag::default(), alloc)))
+                    // The allocations in the relocations (pointers stored *inside* this allocation)
+                    // all get the base pointer tag.
+                    .map(|&(offset, ((), alloc))| (offset, (memory_extra.static_base_ptr(alloc), alloc)))
                     .collect()
             ),
-            undef_mask: alloc.undef_mask.clone(),
+            undef_mask: alloc.undef_mask,
             align: alloc.align,
             mutability: alloc.mutability,
             extra,
         };
-        Cow::Owned(alloc)
-    }
-
-    #[inline(always)]
-    fn new_allocation(
-        size: Size,
-        extra: &Self::MemoryExtra,
-        kind: MemoryKind<MiriMemoryKind>,
-    ) -> (Self::AllocExtra, Self::PointerTag) {
-        Stacks::new_allocation(size, extra, kind)
+        (Cow::Owned(alloc), base_tag)
     }
 
     #[inline(always)]
-    fn tag_dereference(
-        _ecx: &InterpretCx<'a, 'mir, 'tcx, Self>,
-        place: MPlaceTy<'tcx, Tag>,
-        _mutability: Option<hir::Mutability>,
-    ) -> EvalResult<'tcx, Scalar<Tag>> {
-        // Nothing happens.
-        Ok(place.ptr)
+    fn tag_static_base_pointer(
+        id: AllocId,
+        memory_extra: &Self::MemoryExtra,
+    ) -> Self::PointerTag {
+        memory_extra.borrow_mut().static_base_ptr(id)
     }
 
     #[inline(always)]

src/stacked_borrows.rs

@@ -1,5 +1,5 @@
 use std::cell::RefCell;
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 use std::rc::Rc;
 use std::fmt;
 use std::num::NonZeroU64;
@@ -10,7 +10,7 @@ use rustc::mir::RetagKind;
 
 use crate::{
     EvalResult, InterpError, MiriEvalContext, HelpersEvalContextExt, Evaluator, MutValueVisitor,
-    MemoryKind, MiriMemoryKind, RangeMap, Allocation, AllocationExtra, CheckInAllocMsg,
+    MemoryKind, MiriMemoryKind, RangeMap, Allocation, AllocationExtra, AllocId, CheckInAllocMsg,
     Pointer, Immediate, ImmTy, PlaceTy, MPlaceTy,
 };
 
@@ -92,10 +92,18 @@ pub struct Stacks {
 /// Extra global state, available to the memory access hooks.
 #[derive(Debug)]
 pub struct GlobalState {
+    /// Next unused pointer ID (tag).
     next_ptr_id: PtrId,
+    /// Table storing the "base" tag for each allocation.
+    /// The base tag is the one used for the initial pointer.
+    /// We need this in a separate table to handle cyclic statics.
+    base_ptr_ids: HashMap<AllocId, Tag>,
+    /// Next unused call ID (for protectors).
     next_call_id: CallId,
+    /// Those call IDs corresponding to functions that are still running.
     active_calls: HashSet<CallId>,
 }
+/// Memory extra state gives us interior mutable access to the global state.
 pub type MemoryState = Rc<RefCell<GlobalState>>;
 
 /// Indicates which kind of access is being performed.
@@ -144,14 +152,15 @@ impl Default for GlobalState {
     fn default() -> Self {
         GlobalState {
             next_ptr_id: NonZeroU64::new(1).unwrap(),
+            base_ptr_ids: HashMap::default(),
             next_call_id: NonZeroU64::new(1).unwrap(),
             active_calls: HashSet::default(),
         }
     }
 }
 
 impl GlobalState {
-    pub fn new_ptr(&mut self) -> PtrId {
+    fn new_ptr(&mut self) -> PtrId {
         let id = self.next_ptr_id;
         self.next_ptr_id = NonZeroU64::new(id.get() + 1).unwrap();
         id
@@ -172,6 +181,15 @@ impl GlobalState {
     fn is_active(&self, id: CallId) -> bool {
         self.active_calls.contains(&id)
     }
+
+    pub fn static_base_ptr(&mut self, id: AllocId) -> Tag {
+        self.base_ptr_ids.get(&id).copied().unwrap_or_else(|| {
+            let tag = Tag::Tagged(self.new_ptr());
+            trace!("New allocation {:?} has base tag {:?}", id, tag);
+            self.base_ptr_ids.insert(id, tag);
+            tag
+        })
+    }
 }
 
 // # Stacked Borrows Core Begin
@@ -190,14 +208,6 @@ impl GlobalState {
 /// F3: If an access happens with an `&` outside `UnsafeCell`,
 ///     it requires the `SharedReadOnly` to still be in the stack.
 
-impl Default for Tag {
-    #[inline(always)]
-    fn default() -> Tag {
-        Tag::Untagged
-    }
-}
-
-
 /// Core relation on `Permission` to define which accesses are allowed
 impl Permission {
     /// This defines for a given permission, whether it permits the given kind of access.
@@ -431,12 +441,13 @@ impl<'tcx> Stack {
 /// Map per-stack operations to higher-level per-location-range operations.
 impl<'tcx> Stacks {
     /// Creates new stack with initial tag.
-    pub(crate) fn new(
+    fn new(
         size: Size,
+        perm: Permission,
         tag: Tag,
         extra: MemoryState,
     ) -> Self {
-        let item = Item { perm: Permission::Unique, tag, protector: None };
+        let item = Item { perm, tag, protector: None };
         let stack = Stack {
             borrows: vec![item],
         };
@@ -465,27 +476,25 @@ impl<'tcx> Stacks {
 /// Glue code to connect with Miri Machine Hooks
 impl Stacks {
     pub fn new_allocation(
+        id: AllocId,
         size: Size,
-        extra: &MemoryState,
+        extra: MemoryState,
         kind: MemoryKind<MiriMemoryKind>,
     ) -> (Self, Tag) {
-        let tag = match kind {
-            MemoryKind::Stack => {
-                // New unique borrow. This `Uniq` is not accessible by the program,
+        let (tag, perm) = match kind {
+            MemoryKind::Stack =>
+                // New unique borrow. This tag is not accessible by the program,
                 // so it will only ever be used when using the local directly (i.e.,
-                // not through a pointer). That is, whenever we directly use a local, this will pop
+                // 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. This subsumes the explicit
-                // `reset` which the blog post [1] says to perform when accessing a local.
-                //
-                // [1]: <https://www.ralfj.de/blog/2018/08/07/stacked-borrows.html>
-                Tag::Tagged(extra.borrow_mut().new_ptr())
-            }
-            _ => {
-                Tag::Untagged
-            }
+                // and in particular, *all* raw pointers.
+                (Tag::Tagged(extra.borrow_mut().new_ptr()), Permission::Unique),
+            MemoryKind::Machine(MiriMemoryKind::Static) =>
+                (extra.borrow_mut().static_base_ptr(id), Permission::SharedReadWrite),
+            _ =>
+                (Tag::Untagged, Permission::SharedReadWrite),
         };
-        let stack = Stacks::new(size, tag, Rc::clone(extra));
+        let stack = Stacks::new(size, perm, tag, extra);
         (stack, tag)
     }
 }

tests/compile-fail/modifying_constants.rs

@@ -1,3 +1,5 @@
+// This should fail even without validation
+// compile-flags: -Zmiri-disable-validation
 
 fn main() {
     let x = &1; // the `&1` is promoted to a constant, but it used to be that only the pointer is marked static, not the pointee

tests/compile-fail/stacked_borrows/unescaped_static.rs

@@ -0,0 +1,7 @@
+static ARRAY: [u8; 2] = [0, 1];
+
+fn main() {
+    let ptr_to_first = &ARRAY[0] as *const u8;
+    // Illegally use this to access the 2nd element.
+    let _val = unsafe { *ptr_to_first.add(1) }; //~ ERROR borrow stack
+}

tests/run-pass/thread-local.rs

@@ -56,7 +56,7 @@ fn main() {
         create(None); // check that the no-dtor case works
 
         // Initialize the keys we use to check destructor ordering
-        for (key, global) in KEYS.iter_mut().zip(GLOBALS.iter()) {
+        for (key, global) in KEYS.iter_mut().zip(GLOBALS.iter_mut()) {
             *key = create(Some(mem::transmute(dtor as unsafe extern fn(*mut u64))));
             set(*key, global as *const _ as *mut _);
         }