From 3db6ec3f11385c17e5f712cadfb3d31417ac166a Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Wed, 8 Feb 2017 15:32:49 +0100 Subject: [PATCH 1/4] prevent more deallocations of statics --- src/eval_context.rs | 8 +++++++- src/lvalue.rs | 6 ++++++ tests/run-pass/issue-5917.rs | 17 +++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 tests/run-pass/issue-5917.rs diff --git a/src/eval_context.rs b/src/eval_context.rs index 1996bbf9dcb..7ea4003b0f7 100644 --- a/src/eval_context.rs +++ b/src/eval_context.rs @@ -329,6 +329,9 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } }, } + // see comment on `initialized` field + assert!(!global_value.initialized); + global_value.initialized = true; assert!(global_value.mutable); global_value.mutable = mutable; } else { @@ -868,7 +871,10 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { _ => { let ptr = self.alloc_ptr_with_substs(global_val.ty, cid.substs)?; 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(ptr.alloc_id, global_val.mutable)?; + } let lval = self.globals.get_mut(&cid).expect("already checked"); *lval = Global { value: Value::ByRef(ptr), diff --git a/src/lvalue.rs b/src/lvalue.rs index df1f166c074..d54c2790476 100644 --- a/src/lvalue.rs +++ b/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, } } } diff --git a/tests/run-pass/issue-5917.rs b/tests/run-pass/issue-5917.rs new file mode 100644 index 00000000000..112ad0185d8 --- /dev/null +++ b/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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + + +struct T (&'static [isize]); +static t : T = T (&[5, 4, 3]); +pub fn main () { + let T(ref v) = t; + assert_eq!(v[0], 5); +} From e23fc79d257095b1347dab58e369d55063857f15 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Wed, 8 Feb 2017 15:38:48 +0100 Subject: [PATCH 2/4] silence some style warning --- tests/run-pass/issue-5917.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/run-pass/issue-5917.rs b/tests/run-pass/issue-5917.rs index 112ad0185d8..69b95f2cd7e 100644 --- a/tests/run-pass/issue-5917.rs +++ b/tests/run-pass/issue-5917.rs @@ -10,8 +10,8 @@ struct T (&'static [isize]); -static t : T = T (&[5, 4, 3]); +static STATIC : T = T (&[5, 4, 3]); pub fn main () { - let T(ref v) = t; + let T(ref v) = STATIC; assert_eq!(v[0], 5); } From 080d3e435519e2665502c8bae6c3168ae4232e33 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Wed, 8 Feb 2017 16:27:28 +0100 Subject: [PATCH 3/4] properly prevent recursive statics from marking each other --- src/eval_context.rs | 16 +++++++++------- src/memory.rs | 24 ++++++++++++++++++++---- src/vtable.rs | 2 +- 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/eval_context.rs b/src/eval_context.rs index 7ea4003b0f7..dedb8a53b71 100644 --- a/src/eval_context.rs +++ b/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,16 +317,16 @@ 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)?; } }, } @@ -870,10 +871,11 @@ 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)?; // see comment on `initialized` field if global_val.initialized { - self.memory.mark_static(ptr.alloc_id, global_val.mutable)?; + self.memory.mark_static_initalized(ptr.alloc_id, global_val.mutable)?; } let lval = self.globals.get_mut(&cid).expect("already checked"); *lval = Global { diff --git a/src/memory.rs b/src/memory.rs index 8f0c3a5622c..6dee7ce49a1 100644 --- a/src/memory.rs +++ b/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, + /// 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, /// 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 !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; diff --git a/src/vtable.rs b/src/vtable.rs index 0b43b349443..bfae608ecbc 100644 --- a/src/vtable.rs +++ b/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) } From 4beb774caac83e3a80662c7f141b16cce0404cb1 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Wed, 8 Feb 2017 17:24:20 +0100 Subject: [PATCH 4/4] don't mark the zst allocation as static --- src/memory.rs | 2 +- tests/run-pass/issue-31267-additional.rs | 31 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 tests/run-pass/issue-31267-additional.rs diff --git a/src/memory.rs b/src/memory.rs index 6dee7ce49a1..cbef71c68f6 100644 --- a/src/memory.rs +++ b/src/memory.rs @@ -632,7 +632,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> { impl<'a, 'tcx> Memory<'a, '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 !self.static_alloc.insert(alloc_id) { + 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); } } diff --git a/tests/run-pass/issue-31267-additional.rs b/tests/run-pass/issue-31267-additional.rs new file mode 100644 index 00000000000..90160ebcdf9 --- /dev/null +++ b/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 or the MIT license +// , 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; +}