Skip to content

Miri engine cleanup #53671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Aug 29, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c141ccf
Miri Memory Work
RalfJung Aug 23, 2018
2592b20
without all those copies of constants, we can finally make eval_opera…
RalfJung Aug 23, 2018
286fc5c
allow Machine to hook into foreign statics; remove unused HasMemory t…
RalfJung Aug 23, 2018
66d64ba
simplify const_to_allocation_provider because it is used for statics …
RalfJung Aug 23, 2018
aa645f3
Clean up function calling
RalfJung Aug 24, 2018
a5baea6
terminator/drop.rs is just one fn... merge it together with the other…
RalfJung Aug 24, 2018
035c69f
switch validation to use operand, not mplace
RalfJung Aug 24, 2018
ef96a60
move const_eval out of rustc_mir::interpret
RalfJung Aug 24, 2018
9cfc9f0
get rid of FinishStatic hack from stack clenaup; const_eval can do th…
RalfJung Aug 24, 2018
548b373
dedicated handling for binops on bool and char (UB if they are not va…
RalfJung Aug 24, 2018
89cfd08
validate enum discriminant whenever it is read
RalfJung Aug 25, 2018
c898e19
fix handling of unsized types in validation; validate str to be UTF-8
RalfJung Aug 25, 2018
07bdd48
expand comment on how statics work
RalfJung Aug 25, 2018
c38cc89
fix len() on non-array but array-layout types (e.g. SIMD)
RalfJung Aug 25, 2018
5b737db
get rid of *most* of the fn call hack by honoring mir.spread_arg
RalfJung Aug 25, 2018
6c78fa8
use associated const for machine controlling mutable statics
RalfJung Aug 26, 2018
f96208c
address nits
RalfJung Aug 26, 2018
066d2ee
fix unsized extern types
RalfJung Aug 26, 2018
e6a5a94
restructure unary_op to also dispatch on type first; fix promotion wi…
RalfJung Aug 26, 2018
506dd70
fix const_prop detecting unary neg underflows
RalfJung Aug 26, 2018
c9b5fac
first test const-ness, then hook fn call
RalfJung Aug 27, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
use associated const for machine controlling mutable statics
So get rid of the IsStatic trait again
  • Loading branch information
RalfJung committed Aug 27, 2018
commit 6c78fa822a311e30e6421525570f472bf19e32fd
26 changes: 4 additions & 22 deletions src/librustc_mir/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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,
Expand Down
23 changes: 9 additions & 14 deletions src/librustc_mir/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<Self::MemoryKinds>;

/// Entry point to all function calls.
///
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
53 changes: 24 additions & 29 deletions src/librustc_mir/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
Expand All @@ -39,23 +39,16 @@ pub enum MemoryKind<T> {
Machine(T),
}

impl<T: IsStatic> IsStatic for MemoryKind<T> {
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
pub data: M::MemoryData,

/// 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<AllocId, (MemoryKind<M::MemoryKinds>, Allocation)>,

pub tcx: TyCtxtAt<'a, 'tcx, 'tcx>,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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::<M>(self.tcx, id),
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<M::MemoryKinds>,
Expand All @@ -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(())
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down