From 6c78fa822a311e30e6421525570f472bf19e32fd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 26 Aug 2018 12:59:59 +0200 Subject: [PATCH] use associated const for machine controlling mutable statics So get rid of the IsStatic trait again --- src/librustc_mir/const_eval.rs | 26 ++---------- src/librustc_mir/interpret/machine.rs | 23 ++++------ src/librustc_mir/interpret/memory.rs | 53 +++++++++++------------- src/librustc_mir/interpret/mod.rs | 2 +- src/librustc_mir/interpret/terminator.rs | 2 +- 5 files changed, 39 insertions(+), 67 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 77fd893b516b..75298a8fcda2 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -26,11 +26,11 @@ use syntax::source_map::Span; use rustc::mir::interpret::{ EvalResult, EvalError, EvalErrorKind, GlobalId, - Scalar, AllocId, Allocation, ConstValue, AllocType, + Scalar, Allocation, ConstValue, }; use interpret::{self, Place, PlaceTy, MemPlace, OpTy, Operand, Value, - EvalContext, StackPopCleanup, MemoryKind, Memory, + EvalContext, StackPopCleanup, MemoryKind, }; pub fn mk_borrowck_eval_cx<'a, 'mir, 'tcx>( @@ -232,17 +232,12 @@ impl Error for ConstEvalError { } } -impl interpret::IsStatic for ! { - fn is_static(self) -> bool { - // unreachable - self - } -} - impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeEvaluator { type MemoryData = (); type MemoryKinds = !; + const MUT_STATIC_KIND: Option = None; // no mutating of statics allowed + fn find_fn<'a>( ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, @@ -308,19 +303,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeEvaluator { } } - fn access_static_mut<'a, 'm>( - mem: &'m mut Memory<'a, 'mir, 'tcx, Self>, - id: AllocId, - ) -> EvalResult<'tcx, &'m mut Allocation> { - // This is always an error, we do not allow mutating statics - match mem.tcx.alloc_map.lock().get(id) { - Some(AllocType::Memory(..)) | - Some(AllocType::Static(..)) => err!(ModifiedConstantMemory), - Some(AllocType::Function(..)) => err!(DerefFunctionPointer), - None => err!(DanglingPointerDeref), - } - } - fn find_foreign_static<'a>( _tcx: TyCtxtAt<'a, 'tcx, 'tcx>, _def_id: DefId, diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index b3e7370b66b5..a8fae2b4871e 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -15,16 +15,11 @@ use std::hash::Hash; use rustc::hir::def_id::DefId; -use rustc::mir::interpret::{AllocId, Allocation, EvalResult, Scalar}; +use rustc::mir::interpret::{Allocation, EvalResult, Scalar}; use rustc::mir; use rustc::ty::{self, layout::TyLayout, query::TyCtxtAt}; -use super::{EvalContext, PlaceTy, OpTy, Memory}; - -/// Used by the machine to tell if a certain allocation is for static memory -pub trait IsStatic { - fn is_static(self) -> bool; -} +use super::{EvalContext, PlaceTy, OpTy}; /// Methods of this trait signifies a point where CTFE evaluation would fail /// and some use case dependent behaviour can instead be applied @@ -33,7 +28,10 @@ pub trait Machine<'mir, 'tcx>: Clone + Eq + Hash { type MemoryData: Clone + Eq + Hash; /// Additional memory kinds a machine wishes to distinguish from the builtin ones - type MemoryKinds: ::std::fmt::Debug + Copy + Clone + Eq + Hash + IsStatic; + type MemoryKinds: ::std::fmt::Debug + Copy + Clone + Eq + Hash; + + /// The memory kind to use for mutated statics -- or None if those are not supported. + const MUT_STATIC_KIND: Option; /// Entry point to all function calls. /// @@ -63,6 +61,9 @@ pub trait Machine<'mir, 'tcx>: Clone + Eq + Hash { ) -> EvalResult<'tcx>; /// Called for read access to a foreign static item. + /// This can be called multiple times for the same static item and should return consistent + /// results. Once the item is *written* the first time, as usual for statics a copy is + /// made and this function is not called again. fn find_foreign_static<'a>( tcx: TyCtxtAt<'a, 'tcx, 'tcx>, def_id: DefId, @@ -83,12 +84,6 @@ pub trait Machine<'mir, 'tcx>: Clone + Eq + Hash { right_layout: TyLayout<'tcx>, ) -> EvalResult<'tcx, Option<(Scalar, bool)>>; - /// Called when requiring mutable access to data in a static. - fn access_static_mut<'a, 'm>( - mem: &'m mut Memory<'a, 'mir, 'tcx, Self>, - id: AllocId, - ) -> EvalResult<'tcx, &'m mut Allocation>; - /// Heap allocations via the `box` keyword /// /// Returns a pointer to the allocated memory diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 85c1ab4edcbd..240f977a5a0e 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -29,7 +29,7 @@ use rustc_data_structures::fx::{FxHashSet, FxHashMap, FxHasher}; use syntax::ast::Mutability; -use super::{Machine, IsStatic}; +use super::Machine; #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] pub enum MemoryKind { @@ -39,15 +39,6 @@ pub enum MemoryKind { Machine(T), } -impl IsStatic for MemoryKind { - fn is_static(self) -> bool { - match self { - MemoryKind::Stack => false, - MemoryKind::Machine(kind) => kind.is_static(), - } - } -} - #[derive(Clone)] pub struct Memory<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'mir, 'tcx>> { /// Additional data required by the Machine @@ -55,7 +46,9 @@ pub struct Memory<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'mir, 'tcx>> { /// Allocations local to this instance of the miri engine. The kind /// helps ensure that the same mechanism is used for allocation and - /// deallocation. + /// deallocation. When an allocation is not found here, it is a + /// static and looked up in the `tcx` for read access. Writing to + /// a static creates a copy here, in the machine. alloc_map: FxHashMap, Allocation)>, pub tcx: TyCtxtAt<'a, 'tcx, 'tcx>, @@ -223,10 +216,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Ok(new_ptr) } - pub fn is_static(&self, alloc_id: AllocId) -> bool { - self.alloc_map.get(&alloc_id).map_or(true, |&(kind, _)| kind.is_static()) - } - /// Deallocate a local, or do nothing if that local has been made into a static pub fn deallocate_local(&mut self, ptr: Pointer) -> EvalResult<'tcx> { // The allocation might be already removed by static interning. @@ -354,10 +343,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { /// Allocation accessors 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) { + // Normal alloc? Some(alloc) => Ok(&alloc.1), - // No need to make any copies, just provide read access to the global static + // Static. No need to make any copies, just provide read access to the global static // memory in tcx. None => const_eval_static::(self.tcx, id), } @@ -368,14 +357,18 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { id: AllocId, ) -> EvalResult<'tcx, &mut Allocation> { // Static? - let alloc = if self.alloc_map.contains_key(&id) { - &mut self.alloc_map.get_mut(&id).unwrap().1 - } else { - // The machine controls to what extend we are allowed to mutate global - // statics. (We do not want to allow that during CTFE, but miri needs it.) - M::access_static_mut(self, id)? - }; - // See if we can use this + if !self.alloc_map.contains_key(&id) { + // Ask the machine for what to do + if let Some(kind) = M::MUT_STATIC_KIND { + // The machine supports mutating statics. Make a copy, use that. + self.deep_copy_static(id, MemoryKind::Machine(kind))?; + } else { + return err!(ModifiedConstantMemory) + } + } + // If we come here, we know the allocation is in our map + let alloc = &mut self.alloc_map.get_mut(&id).unwrap().1; + // See if we are allowed to mutate this if alloc.mutability == Mutability::Immutable { err!(ModifiedConstantMemory) } else { @@ -489,10 +482,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { pub fn leak_report(&self) -> usize { trace!("### LEAK REPORT ###"); + let mut_static_kind = M::MUT_STATIC_KIND.map(|k| MemoryKind::Machine(k)); let leaks: Vec<_> = self.alloc_map .iter() - .filter_map(|(&id, (kind, _))| - if kind.is_static() { None } else { Some(id) } ) + .filter_map(|(&id, &(kind, _))| + // exclude mutable statics + if Some(kind) == mut_static_kind { None } else { Some(id) } ) .collect(); let n = leaks.len(); self.dump_allocs(leaks); @@ -609,7 +604,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { /// The alloc_id must refer to a (mutable) static; a deep copy of that /// static is made into this memory. - pub fn deep_copy_static( + fn deep_copy_static( &mut self, id: AllocId, kind: MemoryKind, @@ -619,7 +614,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { return err!(ModifiedConstantMemory); } let old = self.alloc_map.insert(id, (kind, alloc.clone())); - assert!(old.is_none(), "deep_copy_static: must not overwrite memory with"); + assert!(old.is_none(), "deep_copy_static: must not overwrite existing memory"); Ok(()) } diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index 5f58fdf589c1..462c4b8889dd 100644 --- a/src/librustc_mir/interpret/mod.rs +++ b/src/librustc_mir/interpret/mod.rs @@ -31,7 +31,7 @@ pub use self::place::{Place, PlaceTy, MemPlace, MPlaceTy}; pub use self::memory::{Memory, MemoryKind}; -pub use self::machine::{Machine, IsStatic}; +pub use self::machine::Machine; pub use self::operand::{Value, ValTy, Operand, OpTy}; diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 87ee94cbe30f..d2a91bd3ac28 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -110,7 +110,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { } (instance, sig) } - ref other => bug!("instance def ty: {:?}", other), + _ => bug!("unexpected fn ptr to ty: {:?}", instance_ty), } } ty::FnDef(def_id, substs) => {