Merge pull request #121 from oli-obk/master

fix `static mut` dealloc or freeze
This commit is contained in:
Scott Olson 2017-02-08 09:41:10 +01:00 committed by GitHub
commit 4f3fc85525
9 changed files with 126 additions and 53 deletions

View File

@ -49,8 +49,8 @@ pub enum EvalError<'tcx> {
AssumptionNotHeld,
InlineAsm,
TypeNotPrimitive(Ty<'tcx>),
ReallocatedFrozenMemory,
DeallocatedFrozenMemory,
ReallocatedStaticMemory,
DeallocatedStaticMemory,
Layout(layout::LayoutError<'tcx>),
Unreachable,
ExpectedConcreteFunction(Function<'tcx>),
@ -118,10 +118,10 @@ fn description(&self) -> &str {
"cannot evaluate inline assembly",
EvalError::TypeNotPrimitive(_) =>
"expected primitive type, got nonprimitive",
EvalError::ReallocatedFrozenMemory =>
"tried to reallocate frozen memory",
EvalError::DeallocatedFrozenMemory =>
"tried to deallocate frozen memory",
EvalError::ReallocatedStaticMemory =>
"tried to reallocate static memory",
EvalError::DeallocatedStaticMemory =>
"tried to deallocate static memory",
EvalError::Layout(_) =>
"rustc layout computation failed",
EvalError::UnterminatedCString(_) =>

View File

@ -100,9 +100,11 @@ pub struct Frame<'tcx> {
#[derive(Clone, Debug, Eq, PartialEq, Hash)]
pub enum StackPopCleanup {
/// The stackframe existed to compute the initial value of a static/constant, make sure it
/// isn't modifyable afterwards. The allocation of the result is frozen iff it's an
/// actual allocation. `PrimVal`s are unmodifyable anyway.
Freeze,
/// isn't modifyable afterwards in case of constants.
/// In case of `static mut`, mark the memory to ensure it's never marked as immutable through
/// references or deallocated
/// The bool decides whether the value is mutable (true) or not (false)
MarkStatic(bool),
/// A regular stackframe added due to a function call will need to get forwarded to the next
/// block
Goto(mir::BasicBlock),
@ -170,7 +172,7 @@ pub(super) fn str_to_value(&mut self, s: &str) -> EvalResult<'tcx, Value> {
// FIXME: cache these allocs
let ptr = self.memory.allocate(s.len() as u64, 1)?;
self.memory.write_bytes(ptr, s.as_bytes())?;
self.memory.freeze(ptr.alloc_id)?;
self.memory.mark_static(ptr.alloc_id, false)?;
Ok(Value::ByValPair(PrimVal::Ptr(ptr), PrimVal::from_u128(s.len() as u128)))
}
@ -194,7 +196,7 @@ pub(super) fn const_to_value(&mut self, const_val: &ConstVal) -> EvalResult<'tcx
ByteStr(ref bs) => {
let ptr = self.memory.allocate(bs.len() as u64, 1)?;
self.memory.write_bytes(ptr, bs)?;
self.memory.freeze(ptr.alloc_id)?;
self.memory.mark_static(ptr.alloc_id, false)?;
PrimVal::Ptr(ptr)
}
@ -310,27 +312,27 @@ pub(super) fn pop_stack_frame(&mut self) -> EvalResult<'tcx> {
::log_settings::settings().indentation -= 1;
let frame = self.stack.pop().expect("tried to pop a stack frame, but there were none");
match frame.return_to_block {
StackPopCleanup::Freeze => if let Lvalue::Global(id) = frame.return_lvalue {
StackPopCleanup::MarkStatic(mutable) => if let Lvalue::Global(id) = frame.return_lvalue {
let global_value = self.globals.get_mut(&id)
.expect("global should have been cached (freeze)");
.expect("global should have been cached (static)");
match global_value.value {
Value::ByRef(ptr) => self.memory.freeze(ptr.alloc_id)?,
Value::ByRef(ptr) => self.memory.mark_static(ptr.alloc_id, mutable)?,
Value::ByVal(val) => if let PrimVal::Ptr(ptr) = val {
self.memory.freeze(ptr.alloc_id)?;
self.memory.mark_static(ptr.alloc_id, mutable)?;
},
Value::ByValPair(val1, val2) => {
if let PrimVal::Ptr(ptr) = val1 {
self.memory.freeze(ptr.alloc_id)?;
self.memory.mark_static(ptr.alloc_id, mutable)?;
}
if let PrimVal::Ptr(ptr) = val2 {
self.memory.freeze(ptr.alloc_id)?;
self.memory.mark_static(ptr.alloc_id, mutable)?;
}
},
}
assert!(global_value.mutable);
global_value.mutable = false;
global_value.mutable = mutable;
} else {
bug!("StackPopCleanup::Freeze on: {:?}", frame.return_lvalue);
bug!("StackPopCleanup::MarkStatic on: {:?}", frame.return_lvalue);
},
StackPopCleanup::Goto(target) => self.goto_block(target),
StackPopCleanup::None => {},
@ -341,11 +343,9 @@ pub(super) fn pop_stack_frame(&mut self) -> EvalResult<'tcx> {
trace!("deallocating local");
self.memory.dump_alloc(ptr.alloc_id);
match self.memory.deallocate(ptr) {
// Any frozen memory means that it belongs to a constant or something referenced
// by a constant. We could alternatively check whether the alloc_id is frozen
// before calling deallocate, but this is much simpler and is probably the
// rare case.
Ok(()) | Err(EvalError::DeallocatedFrozenMemory) => {},
// We could alternatively check whether the alloc_id is static before calling
// deallocate, but this is much simpler and is probably the rare case.
Ok(()) | Err(EvalError::DeallocatedStaticMemory) => {},
other => return other,
}
}
@ -868,9 +868,7 @@ pub(super) fn force_allocation(
_ => {
let ptr = self.alloc_ptr_with_substs(global_val.ty, cid.substs)?;
self.write_value_to_ptr(global_val.value, ptr, global_val.ty)?;
if !global_val.mutable {
self.memory.freeze(ptr.alloc_id)?;
}
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),

View File

@ -38,9 +38,19 @@ pub struct Allocation {
/// The alignment of the allocation to detect unaligned reads.
pub align: u64,
/// Whether the allocation may be modified.
/// Use the `freeze` method of `Memory` to ensure that an error occurs, if the memory of this
/// allocation is modified in the future.
pub immutable: bool,
/// Use the `mark_static` 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,
}
#[derive(Debug, PartialEq, Copy, Clone)]
pub enum StaticKind {
/// may be deallocated without breaking miri's invariants
NotStatic,
/// may be modified, but never deallocated
Mutable,
/// may neither be modified nor deallocated
Immutable,
}
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
@ -272,7 +282,7 @@ pub fn allocate(&mut self, size: u64, align: u64) -> EvalResult<'tcx, Pointer> {
relocations: BTreeMap::new(),
undef_mask: UndefMask::new(size),
align,
immutable: false,
static_kind: StaticKind::NotStatic,
};
let id = self.next_id;
self.next_id.0 += 1;
@ -290,8 +300,8 @@ pub fn reallocate(&mut self, ptr: Pointer, new_size: u64, align: u64) -> EvalRes
if ptr.points_to_zst() {
return self.allocate(new_size, align);
}
if self.get(ptr.alloc_id).map(|alloc| alloc.immutable).ok() == Some(true) {
return Err(EvalError::ReallocatedFrozenMemory);
if self.get(ptr.alloc_id).ok().map_or(false, |alloc| alloc.static_kind != StaticKind::NotStatic) {
return Err(EvalError::ReallocatedStaticMemory);
}
let size = self.get(ptr.alloc_id)?.bytes.len() as u64;
@ -325,8 +335,8 @@ pub fn deallocate(&mut self, ptr: Pointer) -> EvalResult<'tcx> {
// TODO(solson): Report error about non-__rust_allocate'd pointer.
return Err(EvalError::Unimplemented(format!("bad pointer offset: {}", ptr.offset)));
}
if self.get(ptr.alloc_id).map(|alloc| alloc.immutable).ok() == Some(true) {
return Err(EvalError::DeallocatedFrozenMemory);
if self.get(ptr.alloc_id).ok().map_or(false, |alloc| alloc.static_kind != StaticKind::NotStatic) {
return Err(EvalError::DeallocatedStaticMemory);
}
if let Some(alloc) = self.alloc_map.remove(&ptr.alloc_id) {
@ -430,8 +440,11 @@ pub fn get(&self, id: AllocId) -> EvalResult<'tcx, &Allocation> {
pub fn get_mut(&mut self, id: AllocId) -> EvalResult<'tcx, &mut Allocation> {
match self.alloc_map.get_mut(&id) {
Some(ref alloc) if alloc.immutable => Err(EvalError::ModifiedConstantMemory),
Some(alloc) => Ok(alloc),
Some(alloc) => match alloc.static_kind {
StaticKind::Mutable |
StaticKind::NotStatic => Ok(alloc),
StaticKind::Immutable => Err(EvalError::ModifiedConstantMemory),
},
None => match self.functions.get(&id) {
Some(_) => Err(EvalError::DerefFunctionPointer),
None if id == NEVER_ALLOC_ID || id == ZST_ALLOC_ID => Err(EvalError::InvalidMemoryAccess),
@ -514,7 +527,11 @@ pub fn dump_allocs(&self, mut allocs: Vec<AllocId>) {
}
}
let immutable = if alloc.immutable { " (immutable)" } else { "" };
let immutable = match alloc.static_kind {
StaticKind::Mutable => " (static mut)",
StaticKind::Immutable => " (immutable)",
StaticKind::NotStatic => "",
};
trace!("{}({} bytes){}", msg, alloc.bytes.len(), immutable);
if !relocations.is_empty() {
@ -607,15 +624,20 @@ fn get_bytes_mut(&mut self, ptr: Pointer, size: u64, align: u64) -> EvalResult<'
/// Reading and writing
impl<'a, 'tcx> Memory<'a, 'tcx> {
pub fn freeze(&mut self, alloc_id: AllocId) -> EvalResult<'tcx> {
// do not use `self.get_mut(alloc_id)` here, because we might have already frozen a
/// mark an allocation as static, either mutable or not
pub fn mark_static(&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) {
Some(ref mut alloc) if !alloc.immutable => {
alloc.immutable = true;
Some(&mut Allocation { ref mut relocations, static_kind: ref mut kind @ StaticKind::NotStatic, .. }) => {
*kind = if mutable {
StaticKind::Mutable
} else {
StaticKind::Immutable
};
// take out the relocations vector to free the borrow on self, so we can call
// freeze recursively
mem::replace(&mut alloc.relocations, Default::default())
// mark recursively
mem::replace(relocations, Default::default())
},
None if alloc_id == NEVER_ALLOC_ID || alloc_id == ZST_ALLOC_ID => return Ok(()),
None if !self.functions.contains_key(&alloc_id) => return Err(EvalError::DanglingPointerDeref),
@ -623,7 +645,7 @@ pub fn freeze(&mut self, alloc_id: AllocId) -> EvalResult<'tcx> {
};
// recurse into inner allocations
for &alloc in relocations.values() {
self.freeze(alloc)?;
self.mark_static(alloc, mutable)?;
}
// put back the relocations
self.alloc_map.get_mut(&alloc_id).expect("checked above").relocations = relocations;

View File

@ -156,11 +156,7 @@ fn global_item(&mut self, def_id: DefId, substs: &'tcx subst::Substs<'tcx>, span
self.try(|this| {
let mir = this.ecx.load_mir(def_id)?;
this.ecx.globals.insert(cid, Global::uninitialized(mir.return_ty));
let cleanup = if immutable && !mir.return_ty.type_contents(this.ecx.tcx).interior_unsafe() {
StackPopCleanup::Freeze
} else {
StackPopCleanup::None
};
let cleanup = StackPopCleanup::MarkStatic(!immutable || mir.return_ty.type_contents(this.ecx.tcx).interior_unsafe());
let name = ty::tls::with(|tcx| tcx.item_path_str(def_id));
trace!("pushing stack frame for global: {}", name);
this.ecx.push_stack_frame(def_id, span, mir, substs, Lvalue::Global(cid), cleanup, Vec::new())
@ -213,7 +209,7 @@ fn visit_constant(&mut self, constant: &mir::Constant<'tcx>, location: mir::Loca
mir,
this.substs,
Lvalue::Global(cid),
StackPopCleanup::Freeze,
StackPopCleanup::MarkStatic(false),
Vec::new())
});
}

View File

@ -112,7 +112,7 @@ pub fn get_vtable(&mut self, trait_ref: ty::PolyTraitRef<'tcx>) -> EvalResult<'t
}
}
self.memory.freeze(vtable.alloc_id)?;
self.memory.mark_static(vtable.alloc_id, false)?;
Ok(vtable)
}

View File

@ -1,5 +1,5 @@
fn main() {
let x = &1; // the `&1` is promoted to a constant, but it used to be that only the pointer is frozen, not the pointee
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
let y = unsafe { &mut *(x as *const i32 as *mut i32) };
*y = 42; //~ ERROR tried to modify constant memory
assert_eq!(*x, 42);

View File

@ -0,0 +1,29 @@
// 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.
// pretty-expanded FIXME #23616
/*!
* Try to double-check that static fns have the right size (with or
* without dummy env ptr, as appropriate) by iterating a size-2 array.
* If the static size differs from the runtime size, the second element
* should be read as a null or otherwise wrong pointer and crash.
*/
fn f() { }
static mut CLOSURES: &'static mut [fn()] = &mut [f as fn(), f as fn()];
pub fn main() {
unsafe {
for closure in &mut *CLOSURES {
(*closure)()
}
}
}

View File

@ -0,0 +1,11 @@
#![feature(static_recursion)]
struct S(&'static S);
static S1: S = S(&S2);
static S2: S = S(&S1);
fn main() {
let p: *const S = S2.0;
let q: *const S = &S1;
assert_eq!(p, q);
}

View File

@ -0,0 +1,17 @@
#![allow(dead_code)]
static mut FOO: i32 = 42;
static BAR: Foo = Foo(unsafe { &FOO as *const _} );
struct Foo(*const i32);
unsafe impl Sync for Foo {}
fn main() {
unsafe {
assert_eq!(*BAR.0, 42);
FOO = 5;
assert_eq!(FOO, 5);
assert_eq!(*BAR.0, 5);
}
}