Merge pull request rust-lang#122 from oli-obk/master

prevent more deallocations of statics

Author: solson

src/eval_context.rs

@@ -172,7 +172,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
         // FIXME: cache these allocs
         let ptr = self.memory.allocate(s.len() as u64, 1)?;
         self.memory.write_bytes(ptr, s.as_bytes())?;
-        self.memory.mark_static(ptr.alloc_id, false)?;
+        self.memory.mark_static_initalized(ptr.alloc_id, false)?;
         Ok(Value::ByValPair(PrimVal::Ptr(ptr), PrimVal::from_u128(s.len() as u128)))
     }
 
@@ -194,9 +194,10 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             Str(ref s) => return self.str_to_value(s),
 
             ByteStr(ref bs) => {
+                // FIXME: cache these allocs
                 let ptr = self.memory.allocate(bs.len() as u64, 1)?;
                 self.memory.write_bytes(ptr, bs)?;
-                self.memory.mark_static(ptr.alloc_id, false)?;
+                self.memory.mark_static_initalized(ptr.alloc_id, false)?;
                 PrimVal::Ptr(ptr)
             }
 
@@ -316,19 +317,22 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 let global_value = self.globals.get_mut(&id)
                     .expect("global should have been cached (static)");
                 match global_value.value {
-                    Value::ByRef(ptr) => self.memory.mark_static(ptr.alloc_id, mutable)?,
+                    Value::ByRef(ptr) => self.memory.mark_static_initalized(ptr.alloc_id, mutable)?,
                     Value::ByVal(val) => if let PrimVal::Ptr(ptr) = val {
-                        self.memory.mark_static(ptr.alloc_id, mutable)?;
+                        self.memory.mark_static_initalized(ptr.alloc_id, mutable)?;
                     },
                     Value::ByValPair(val1, val2) => {
                         if let PrimVal::Ptr(ptr) = val1 {
-                            self.memory.mark_static(ptr.alloc_id, mutable)?;
+                            self.memory.mark_static_initalized(ptr.alloc_id, mutable)?;
                         }
                         if let PrimVal::Ptr(ptr) = val2 {
-                            self.memory.mark_static(ptr.alloc_id, mutable)?;
+                            self.memory.mark_static_initalized(ptr.alloc_id, mutable)?;
                         }
                     },
                 }
+                // see comment on `initialized` field
+                assert!(!global_value.initialized);
+                global_value.initialized = true;
                 assert!(global_value.mutable);
                 global_value.mutable = mutable;
             } else {
@@ -867,8 +871,12 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                     Value::ByRef(ptr) => Lvalue::from_ptr(ptr),
                     _ => {
                         let ptr = self.alloc_ptr_with_substs(global_val.ty, cid.substs)?;
+                        self.memory.mark_static(ptr.alloc_id);
                         self.write_value_to_ptr(global_val.value, ptr, global_val.ty)?;
-                        self.memory.mark_static(ptr.alloc_id, global_val.mutable)?;
+                        // see comment on `initialized` field
+                        if global_val.initialized {
+                            self.memory.mark_static_initalized(ptr.alloc_id, global_val.mutable)?;
+                        }
                         let lval = self.globals.get_mut(&cid).expect("already checked");
                         *lval = Global {
                             value: Value::ByRef(ptr),

src/lvalue.rs

@@ -57,6 +57,11 @@ pub struct GlobalId<'tcx> {
 #[derive(Copy, Clone, Debug)]
 pub struct Global<'tcx> {
     pub(super) value: Value,
+    /// Only used in `force_allocation` to ensure we don't mark the memory
+    /// before the static is initialized. It is possible to convert a
+    /// global which initially is `Value::ByVal(PrimVal::Undef)` and gets
+    /// lifted to an allocation before the static is fully initialized
+    pub(super) initialized: bool,
     pub(super) mutable: bool,
     pub(super) ty: Ty<'tcx>,
 }
@@ -102,6 +107,7 @@ impl<'tcx> Global<'tcx> {
             value: Value::ByVal(PrimVal::Undef),
             mutable: true,
             ty,
+            initialized: false,
         }
     }
 }

src/memory.rs

@@ -38,7 +38,7 @@ pub struct Allocation {
     /// The alignment of the allocation to detect unaligned reads.
     pub align: u64,
     /// Whether the allocation may be modified.
-    /// Use the `mark_static` method of `Memory` to ensure that an error occurs, if the memory of this
+    /// Use the `mark_static_initalized` method of `Memory` to ensure that an error occurs, if the memory of this
     /// allocation is modified or deallocated in the future.
     pub static_kind: StaticKind,
 }
@@ -152,6 +152,11 @@ impl<'tcx> Function<'tcx> {
 pub struct Memory<'a, 'tcx> {
     /// Actual memory allocations (arbitrary bytes, may contain pointers into other allocations)
     alloc_map: HashMap<AllocId, Allocation>,
+    /// Set of statics, constants, promoteds, vtables, ... to prevent `mark_static_initalized` from stepping
+    /// out of its own allocations.
+    /// This set only contains statics backed by an allocation. If they are ByVal or ByValPair they
+    /// are not here, but will be inserted once they become ByRef.
+    static_alloc: HashSet<AllocId>,
     /// Number of virtual bytes allocated
     memory_usage: u64,
     /// Maximum number of virtual bytes that may be allocated
@@ -189,6 +194,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
             memory_size: max_memory,
             memory_usage: 0,
             packed: BTreeSet::new(),
+            static_alloc: HashSet::new(),
         }
     }
 
@@ -624,8 +630,15 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
 
 /// Reading and writing
 impl<'a, 'tcx> Memory<'a, 'tcx> {
-    /// mark an allocation as static, either mutable or not
-    pub fn mark_static(&mut self, alloc_id: AllocId, mutable: bool) -> EvalResult<'tcx> {
+    /// mark an allocation as being the entry point to a static (see `static_alloc` field)
+    pub fn mark_static(&mut self, alloc_id: AllocId) {
+        if alloc_id != NEVER_ALLOC_ID && alloc_id != ZST_ALLOC_ID && !self.static_alloc.insert(alloc_id) {
+            bug!("tried to mark an allocation ({:?}) as static twice", alloc_id);
+        }
+    }
+
+    /// mark an allocation as static and initialized, either mutable or not
+    pub fn mark_static_initalized(&mut self, alloc_id: AllocId, mutable: bool) -> EvalResult<'tcx> {
         // do not use `self.get_mut(alloc_id)` here, because we might have already marked a
         // sub-element or have circular pointers (e.g. `Rc`-cycles)
         let relocations = match self.alloc_map.get_mut(&alloc_id) {
@@ -645,7 +658,10 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
         };
         // recurse into inner allocations
         for &alloc in relocations.values() {
-            self.mark_static(alloc, mutable)?;
+            // relocations into other statics are not "inner allocations"
+            if !self.static_alloc.contains(&alloc) {
+                self.mark_static_initalized(alloc, mutable)?;
+            }
         }
         // put back the relocations
         self.alloc_map.get_mut(&alloc_id).expect("checked above").relocations = relocations;

src/vtable.rs

@@ -112,7 +112,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             }
         }
 
-        self.memory.mark_static(vtable.alloc_id, false)?;
+        self.memory.mark_static_initalized(vtable.alloc_id, false)?;
 
         Ok(vtable)
     }

tests/run-pass/issue-31267-additional.rs

@@ -0,0 +1,31 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![allow(unused_variables)]
+
+#![feature(associated_consts)]
+
+#[derive(Clone, Copy, Debug)]
+struct Bar;
+
+const BAZ: Bar = Bar;
+
+#[derive(Debug)]
+struct Foo([Bar; 1]);
+
+struct Biz;
+
+impl Biz {
+    const BAZ: Foo = Foo([BAZ; 1]);
+}
+
+fn main() {
+    let foo = Biz::BAZ;
+}

tests/run-pass/issue-5917.rs

@@ -0,0 +1,17 @@
+// Copyright 2013 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+
+struct T (&'static [isize]);
+static STATIC : T = T (&[5, 4, 3]);
+pub fn main () {
+    let T(ref v) = STATIC;
+    assert_eq!(v[0], 5);
+}