From ea806e78155864e5e19c6b78e40810d3ce9a7719 Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Fri, 8 Jun 2018 03:47:26 +0100 Subject: [PATCH 1/2] Removed `uninitialized_statics` field from `Memory` struct in miri. Refactored code accordingly. --- src/librustc_mir/interpret/const_eval.rs | 12 +- src/librustc_mir/interpret/eval_context.rs | 14 +-- src/librustc_mir/interpret/memory.rs | 124 ++++++++------------- src/librustc_mir/interpret/traits.rs | 7 +- 4 files changed, 64 insertions(+), 93 deletions(-) diff --git a/src/librustc_mir/interpret/const_eval.rs b/src/librustc_mir/interpret/const_eval.rs index 749c0d04ae91f..ea09bab5d1411 100644 --- a/src/librustc_mir/interpret/const_eval.rs +++ b/src/librustc_mir/interpret/const_eval.rs @@ -1,3 +1,6 @@ +use std::fmt; +use std::error::Error; + use rustc::hir; use rustc::mir::interpret::{ConstEvalErr}; use rustc::mir; @@ -15,9 +18,6 @@ use rustc::mir::interpret::{ }; use super::{Place, EvalContext, StackPopCleanup, ValTy, PlaceExtra, Memory, MemoryKind}; -use std::fmt; -use std::error::Error; - pub fn mk_borrowck_eval_cx<'a, 'mir, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, instance: Instance<'tcx>, @@ -152,7 +152,7 @@ fn eval_body_using_ecx<'a, 'mir, 'tcx>( let ptr = ecx.memory.allocate( layout.size, layout.align, - None, + MemoryKind::Stack, )?; let internally_mutable = !layout.ty.is_freeze(tcx, param_env, mir.span); let is_static = tcx.is_static(cid.instance.def_id()); @@ -486,7 +486,7 @@ pub fn const_variant_index<'a, 'tcx>( let (ptr, align) = match value { Value::ScalarPair(..) | Value::Scalar(_) => { let layout = ecx.layout_of(val.ty)?; - let ptr = ecx.memory.allocate(layout.size, layout.align, Some(MemoryKind::Stack))?.into(); + let ptr = ecx.memory.allocate(layout.size, layout.align, MemoryKind::Stack)?.into(); ecx.write_value_to_ptr(value, ptr, layout.align, val.ty)?; (ptr, layout.align) }, @@ -515,7 +515,7 @@ pub fn const_value_to_allocation_provider<'a, 'tcx>( ()); let value = ecx.const_to_value(val.val)?; let layout = ecx.layout_of(val.ty)?; - let ptr = ecx.memory.allocate(layout.size, layout.align, Some(MemoryKind::Stack))?; + let ptr = ecx.memory.allocate(layout.size, layout.align, MemoryKind::Stack)?; ecx.write_value_to_ptr(value, ptr.into(), layout.align, val.ty)?; let alloc = ecx.memory.get(ptr.alloc_id)?; Ok(tcx.intern_const_alloc(alloc.clone())) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 15d8b9c36215a..031c75013a27b 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -1,4 +1,5 @@ use std::fmt::Write; +use std::mem; use rustc::hir::def_id::DefId; use rustc::hir::def::Def; @@ -9,14 +10,13 @@ use rustc::ty::subst::{Subst, Substs}; use rustc::ty::{self, Ty, TyCtxt, TypeAndMut}; use rustc::ty::query::TyCtxtAt; use rustc_data_structures::indexed_vec::{IndexVec, Idx}; -use rustc::mir::interpret::FrameInfo; -use syntax::codemap::{self, Span}; -use syntax::ast::Mutability; use rustc::mir::interpret::{ - GlobalId, Value, Scalar, + FrameInfo, GlobalId, Value, Scalar, EvalResult, EvalErrorKind, Pointer, ConstValue, }; -use std::mem; + +use syntax::codemap::{self, Span}; +use syntax::ast::Mutability; use super::{Place, PlaceExtra, Memory, HasMemory, MemoryKind, @@ -206,7 +206,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M let layout = self.layout_of(ty)?; assert!(!layout.is_unsized(), "cannot alloc memory for unsized type"); - self.memory.allocate(layout.size, layout.align, Some(MemoryKind::Stack)) + self.memory.allocate(layout.size, layout.align, MemoryKind::Stack) } pub fn memory(&self) -> &Memory<'a, 'mir, 'tcx, M> { @@ -246,7 +246,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M } ConstValue::ByRef(alloc, offset) => { // FIXME: Allocate new AllocId for all constants inside - let id = self.memory.allocate_value(alloc.clone(), Some(MemoryKind::Stack))?; + let id = self.memory.allocate_value(alloc.clone(), MemoryKind::Stack)?; Ok(Value::ByRef(Pointer::new(id, offset).into(), alloc.align)) }, ConstValue::ScalarPair(a, b) => Ok(Value::ScalarPair(a, b)), diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index f5da65ae44db7..5cf734cce8a3f 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -6,12 +6,12 @@ use rustc::ty::Instance; use rustc::ty::ParamEnv; use rustc::ty::query::TyCtxtAt; use rustc::ty::layout::{self, Align, TargetDataLayout, Size}; -use syntax::ast::Mutability; - -use rustc_data_structures::fx::{FxHashSet, FxHashMap}; use rustc::mir::interpret::{Pointer, AllocId, Allocation, AccessKind, Value, EvalResult, Scalar, EvalErrorKind, GlobalId, AllocType}; pub use rustc::mir::interpret::{write_target_uint, write_target_int, read_target_uint}; +use rustc_data_structures::fx::{FxHashSet, FxHashMap}; + +use syntax::ast::Mutability; use super::{EvalContext, Machine}; @@ -41,11 +41,6 @@ pub struct Memory<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'mir, 'tcx>> { /// Actual memory allocations (arbitrary bytes, may contain pointers into other allocations). alloc_map: FxHashMap, - /// Actual memory allocations (arbitrary bytes, may contain pointers into other allocations). - /// - /// Stores statics while they are being processed, before they are interned and thus frozen - uninitialized_statics: FxHashMap, - /// The current stack frame. Used to check accesses against locks. pub cur_frame: usize, @@ -58,7 +53,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { data, alloc_kind: FxHashMap::default(), alloc_map: FxHashMap::default(), - uninitialized_statics: FxHashMap::default(), tcx, cur_frame: usize::max_value(), } @@ -82,20 +76,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { pub fn allocate_value( &mut self, alloc: Allocation, - kind: Option>, + kind: MemoryKind, ) -> EvalResult<'tcx, AllocId> { let id = self.tcx.alloc_map.lock().reserve(); M::add_lock(self, id); - match kind { - Some(kind @ MemoryKind::Stack) | - Some(kind @ MemoryKind::Machine(_)) => { - self.alloc_map.insert(id, alloc); - self.alloc_kind.insert(id, kind); - }, - None => { - self.uninitialized_statics.insert(id, alloc); - }, - } + self.alloc_map.insert(id, alloc); + self.alloc_kind.insert(id, kind); Ok(id) } @@ -104,7 +90,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { &mut self, size: Size, align: Align, - kind: Option>, + kind: MemoryKind, ) -> EvalResult<'tcx, Pointer> { self.allocate_value(Allocation::undef(size, align), kind).map(Pointer::from) } @@ -132,7 +118,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } // For simplicities' sake, we implement reallocate as "alloc, copy, dealloc" - let new_ptr = self.allocate(new_size, new_align, Some(kind))?; + let new_ptr = self.allocate(new_size, new_align, kind)?; self.copy( ptr.into(), old_align, @@ -168,12 +154,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { let alloc = match self.alloc_map.remove(&ptr.alloc_id) { Some(alloc) => alloc, - None => if self.uninitialized_statics.contains_key(&ptr.alloc_id) { - return err!(DeallocatedWrongMemoryKind( - "uninitializedstatic".to_string(), - format!("{:?}", kind), - )) - } else { + None => { return match self.tcx.alloc_map.lock().get(ptr.alloc_id) { Some(AllocType::Function(..)) => err!(DeallocatedWrongMemoryKind( "function".to_string(), @@ -299,24 +280,21 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { pub fn get(&self, id: AllocId) -> EvalResult<'tcx, &Allocation> { // normal alloc? match self.alloc_map.get(&id) { - Some(alloc) => Ok(alloc), + Some(alloc) => Ok(alloc), // uninitialized static alloc? - None => match self.uninitialized_statics.get(&id) { - Some(alloc) => Ok(alloc), - None => { - // static alloc? - let alloc = self.tcx.alloc_map.lock().get(id); - match alloc { - Some(AllocType::Memory(mem)) => Ok(mem), - Some(AllocType::Function(..)) => { - Err(EvalErrorKind::DerefFunctionPointer.into()) - } - Some(AllocType::Static(did)) => { - self.const_eval_static(did) - } - None => Err(EvalErrorKind::DanglingPointerDeref.into()), + None => { + // static alloc? + let alloc = self.tcx.alloc_map.lock().get(id); + match alloc { + Some(AllocType::Memory(mem)) => Ok(mem), + Some(AllocType::Function(..)) => { + Err(EvalErrorKind::DerefFunctionPointer.into()) + } + Some(AllocType::Static(did)) => { + self.const_eval_static(did) } - }, + None => Err(EvalErrorKind::DanglingPointerDeref.into()), + } }, } } @@ -329,17 +307,14 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { match self.alloc_map.get_mut(&id) { Some(alloc) => Ok(alloc), // uninitialized static alloc? - None => match self.uninitialized_statics.get_mut(&id) { - Some(alloc) => Ok(alloc), - None => { - // no alloc or immutable alloc? produce an error - match self.tcx.alloc_map.lock().get(id) { - Some(AllocType::Memory(..)) | - Some(AllocType::Static(..)) => err!(ModifiedConstantMemory), - Some(AllocType::Function(..)) => err!(DerefFunctionPointer), - None => err!(DanglingPointerDeref), - } - }, + None => { + // no alloc or immutable alloc? produce an error + match self.tcx.alloc_map.lock().get(id) { + Some(AllocType::Memory(..)) | + Some(AllocType::Static(..)) => err!(ModifiedConstantMemory), + Some(AllocType::Function(..)) => err!(DerefFunctionPointer), + None => err!(DanglingPointerDeref), + } }, } } @@ -390,27 +365,23 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { MemoryKind::Stack => " (stack)".to_owned(), MemoryKind::Machine(m) => format!(" ({:?})", m), }), - // uninitialized static alloc? - None => match self.uninitialized_statics.get(&id) { - Some(a) => (a, " (static in the process of initialization)".to_owned()), - None => { - // static alloc? - match self.tcx.alloc_map.lock().get(id) { - Some(AllocType::Memory(a)) => (a, "(immutable)".to_owned()), - Some(AllocType::Function(func)) => { - trace!("{} {}", msg, func); - continue; - } - Some(AllocType::Static(did)) => { - trace!("{} {:?}", msg, did); - continue; - } - None => { - trace!("{} (deallocated)", msg); - continue; - } + None => { + // static alloc? + match self.tcx.alloc_map.lock().get(id) { + Some(AllocType::Memory(a)) => (a, "(immutable)".to_owned()), + Some(AllocType::Function(func)) => { + trace!("{} {}", msg, func); + continue; } - }, + Some(AllocType::Static(did)) => { + trace!("{} {:?}", msg, did); + continue; + } + None => { + trace!("{} (deallocated)", msg); + continue; + } + } }, }; @@ -569,8 +540,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Some(MemoryKind::Machine(_)) => bug!("machine didn't handle machine alloc"), Some(MemoryKind::Stack) => {}, } - let uninit = self.uninitialized_statics.remove(&alloc_id); - if let Some(mut alloc) = alloc.or(uninit) { + if let Some(mut alloc) = alloc { // ensure llvm knows not to put this into immutable memroy alloc.runtime_mutability = mutability; let alloc = self.tcx.intern_const_alloc(alloc); diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 373a0b0d0bfed..b6c7feda19fa8 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -1,9 +1,10 @@ use rustc::ty::{self, Ty}; use rustc::ty::layout::{Size, Align, LayoutOf}; +use rustc::mir::interpret::{Scalar, Value, Pointer, EvalResult}; + use syntax::ast::Mutability; -use rustc::mir::interpret::{Scalar, Value, Pointer, EvalResult}; -use super::{EvalContext, Machine}; +use super::{EvalContext, Machine, MemoryKind}; impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { /// Creates a dynamic vtable for the given type and vtable origin. This is used only for @@ -30,7 +31,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { let vtable = self.memory.allocate( ptr_size * (3 + methods.len() as u64), ptr_align, - None, + MemoryKind::Stack, )?; let drop = ::monomorphize::resolve_drop_in_place(*self.tcx, ty); From 6660c25045ec198d80ada12e621634e22d1da619 Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Mon, 2 Jul 2018 17:18:38 +0100 Subject: [PATCH 2/2] Updated miri submodule. --- src/tools/miri | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri b/src/tools/miri index 9143a69f4b3ef..5b7bb32b0e46d 160000 --- a/src/tools/miri +++ b/src/tools/miri @@ -1 +1 @@ -Subproject commit 9143a69f4b3ef4bda77afddefe934be363e39f31 +Subproject commit 5b7bb32b0e46d195b80c4da09b560ac7fc92015d