From ed679c3d238b7ece32d5072067d1a192318c0945 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Thu, 20 Oct 2016 13:10:22 +0200 Subject: [PATCH 01/13] make some pieces public that are required by priroda --- src/interpreter/mod.rs | 6 +++--- src/lib.rs | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 463b9bf0063..0b39da9f382 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -17,7 +17,7 @@ use syntax::codemap::{self, DUMMY_SP}; use error::{EvalError, EvalResult}; use memory::{Memory, Pointer, AllocId}; use primval::{self, PrimVal, PrimValKind}; -use self::value::Value; +pub use self::value::Value; use std::collections::HashMap; @@ -1462,7 +1462,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } impl<'a, 'tcx: 'a> Frame<'a, 'tcx> { - fn get_local(&self, local: mir::Local) -> Option { + pub fn get_local(&self, local: mir::Local) -> Option { // Subtract 1 because we don't store a value for the ReturnPointer, the local with index 0. self.locals[local.index() - 1] } @@ -1474,7 +1474,7 @@ impl<'a, 'tcx: 'a> Frame<'a, 'tcx> { } impl Lvalue { - fn from_ptr(ptr: Pointer) -> Self { + pub fn from_ptr(ptr: Pointer) -> Self { Lvalue::Ptr { ptr: ptr, extra: LvalueExtra::None } } diff --git a/src/lib.rs b/src/lib.rs index 2f2894a59ee..3ed75a54c2a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -36,6 +36,9 @@ pub use interpreter::{ eval_main, run_mir_passes, StackPopCleanup, + Value, + Lvalue, + LvalueExtra, }; pub use memory::{ @@ -43,3 +46,8 @@ pub use memory::{ Pointer, AllocId, }; + +pub use primval::{ + PrimVal, + PrimValKind, +}; From 24be49f7dd927d74af8a5adae672fa35234740e0 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 21 Oct 2016 10:29:56 +0200 Subject: [PATCH 02/13] add a 'tcx lifetime to Lvalue in preparation for statics --- src/interpreter/mod.rs | 28 ++++++++++++------------ src/interpreter/terminator/intrinsics.rs | 2 +- src/interpreter/terminator/mod.rs | 4 ++-- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 0b39da9f382..2663ea159dc 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -77,7 +77,7 @@ pub struct Frame<'a, 'tcx: 'a> { pub return_to_block: StackPopCleanup, /// The location where the result of the current stack frame should be written to. - pub return_lvalue: Lvalue, + pub return_lvalue: Lvalue<'tcx>, /// The list of locals for this stack frame, stored in order as /// `[arguments..., variables..., temporaries...]`. The locals are stored as `Value`s, which @@ -99,7 +99,7 @@ pub struct Frame<'a, 'tcx: 'a> { } #[derive(Copy, Clone, Debug, Eq, PartialEq)] -pub enum Lvalue { +pub enum Lvalue<'tcx> { /// An lvalue referring to a value allocated in the `Memory` system. Ptr { ptr: Pointer, @@ -347,7 +347,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { span: codemap::Span, mir: CachedMir<'a, 'tcx>, substs: &'tcx Substs<'tcx>, - return_lvalue: Lvalue, + return_lvalue: Lvalue<'tcx>, return_to_block: StackPopCleanup, ) -> EvalResult<'tcx, ()> { ::log_settings::settings().indentation += 1; @@ -406,7 +406,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { op: mir::BinOp, left: &mir::Operand<'tcx>, right: &mir::Operand<'tcx>, - dest: Lvalue, + dest: Lvalue<'tcx>, dest_ty: Ty<'tcx>, ) -> EvalResult<'tcx, ()> { let (val, overflowed) = self.binop_with_overflow(op, left, right)?; @@ -421,7 +421,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { op: mir::BinOp, left: &mir::Operand<'tcx>, right: &mir::Operand<'tcx>, - dest: Lvalue, + dest: Lvalue<'tcx>, ) -> EvalResult<'tcx, bool> { let (val, overflowed) = self.binop_with_overflow(op, left, right)?; self.write_primval(dest, val)?; @@ -430,7 +430,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { fn assign_fields>( &mut self, - dest: Lvalue, + dest: Lvalue<'tcx>, offsets: I, operands: &[mir::Operand<'tcx>], ) -> EvalResult<'tcx, ()> { @@ -863,7 +863,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } } - fn eval_lvalue(&mut self, mir_lvalue: &mir::Lvalue<'tcx>) -> EvalResult<'tcx, Lvalue> { + fn eval_lvalue(&mut self, mir_lvalue: &mir::Lvalue<'tcx>) -> EvalResult<'tcx, Lvalue<'tcx>> { use rustc::mir::repr::Lvalue::*; let lvalue = match *mir_lvalue { Local(mir::RETURN_POINTER) => self.frame().return_lvalue, @@ -900,7 +900,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { fn eval_lvalue_projection( &mut self, proj: &mir::LvalueProjection<'tcx>, - ) -> EvalResult<'tcx, Lvalue> { + ) -> EvalResult<'tcx, Lvalue<'tcx>> { let base = self.eval_lvalue(&proj.base)?; let base_ty = self.lvalue_ty(&proj.base); let base_layout = self.type_layout(base_ty); @@ -1071,7 +1071,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Ok(()) } - fn force_allocation(&mut self, lvalue: Lvalue) -> EvalResult<'tcx, Lvalue> { + fn force_allocation(&mut self, lvalue: Lvalue<'tcx>) -> EvalResult<'tcx, Lvalue<'tcx>> { let new_lvalue = match lvalue { Lvalue::Local { frame, local } => { let ptr = match self.stack[frame].get_local(local) { @@ -1157,7 +1157,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { fn write_primval( &mut self, - dest: Lvalue, + dest: Lvalue<'tcx>, val: PrimVal, ) -> EvalResult<'tcx, ()> { match dest { @@ -1175,7 +1175,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { fn write_value( &mut self, src_val: Value, - dest: Lvalue, + dest: Lvalue<'tcx>, dest_ty: Ty<'tcx>, ) -> EvalResult<'tcx, ()> { match dest { @@ -1441,7 +1441,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Ok(()) } - fn dump_local(&self, lvalue: Lvalue) { + fn dump_local(&self, lvalue: Lvalue<'tcx>) { if let Lvalue::Local { frame, local } = lvalue { if let Some(val) = self.stack[frame].get_local(local) { match val { @@ -1473,7 +1473,7 @@ impl<'a, 'tcx: 'a> Frame<'a, 'tcx> { } } -impl Lvalue { +impl<'tcx> Lvalue<'tcx> { pub fn from_ptr(ptr: Pointer) -> Self { Lvalue::Ptr { ptr: ptr, extra: LvalueExtra::None } } @@ -1492,7 +1492,7 @@ impl Lvalue { ptr } - fn elem_ty_and_len<'tcx>(self, ty: Ty<'tcx>) -> (Ty<'tcx>, u64) { + fn elem_ty_and_len(self, ty: Ty<'tcx>) -> (Ty<'tcx>, u64) { match ty.sty { ty::TyArray(elem, n) => (elem, n as u64), diff --git a/src/interpreter/terminator/intrinsics.rs b/src/interpreter/terminator/intrinsics.rs index c3327dd76f9..c4289375432 100644 --- a/src/interpreter/terminator/intrinsics.rs +++ b/src/interpreter/terminator/intrinsics.rs @@ -15,7 +15,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { def_id: DefId, substs: &'tcx Substs<'tcx>, args: &[mir::Operand<'tcx>], - dest: Lvalue, + dest: Lvalue<'tcx>, dest_ty: Ty<'tcx>, dest_layout: &'tcx Layout, ) -> EvalResult<'tcx, ()> { diff --git a/src/interpreter/terminator/mod.rs b/src/interpreter/terminator/mod.rs index be6147f8b74..e8be90cf91a 100644 --- a/src/interpreter/terminator/mod.rs +++ b/src/interpreter/terminator/mod.rs @@ -150,7 +150,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { def_id: DefId, substs: &'tcx Substs<'tcx>, fn_ty: &'tcx BareFnTy, - destination: Option<(Lvalue, mir::BasicBlock)>, + destination: Option<(Lvalue<'tcx>, mir::BasicBlock)>, arg_operands: &[mir::Operand<'tcx>], span: Span, ) -> EvalResult<'tcx, ()> { @@ -264,7 +264,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { &mut self, def_id: DefId, args: &[mir::Operand<'tcx>], - dest: Lvalue, + dest: Lvalue<'tcx>, dest_size: usize, ) -> EvalResult<'tcx, ()> { let name = self.tcx.item_name(def_id); From bef879083e10461b5a3897e85958a289299594c9 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 21 Oct 2016 10:31:13 +0200 Subject: [PATCH 03/13] split eval_and_read_lvalue into two functions --- src/interpreter/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 2663ea159dc..a7cc27a8e5f 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -851,8 +851,12 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } } } + let lvalue = self.eval_lvalue(lvalue)?; + self.read_lvalue(lvalue) + } - match self.eval_lvalue(lvalue)? { + fn read_lvalue(&mut self, lvalue: Lvalue<'tcx>) -> EvalResult<'tcx, Value> { + match lvalue { Lvalue::Ptr { ptr, extra } => { assert_eq!(extra, LvalueExtra::None); Ok(Value::ByRef(ptr)) From a75e7f76865b5359b72bd8400a30083364ef180d Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 21 Oct 2016 10:32:27 +0200 Subject: [PATCH 04/13] don't allocate statics unless a reference to them is created --- src/interpreter/mod.rs | 200 ++++++++++++++++++++++++++++------------ src/interpreter/step.rs | 21 ++--- 2 files changed, 149 insertions(+), 72 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index a7cc27a8e5f..405a8b41c6c 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -15,7 +15,7 @@ use std::rc::Rc; use syntax::codemap::{self, DUMMY_SP}; use error::{EvalError, EvalResult}; -use memory::{Memory, Pointer, AllocId}; +use memory::{Memory, Pointer}; use primval::{self, PrimVal, PrimValKind}; pub use self::value::Value; @@ -41,8 +41,7 @@ pub struct EvalContext<'a, 'tcx: 'a> { memory: Memory<'a, 'tcx>, /// Precomputed statics, constants and promoteds. - // FIXME(solson): Change from Pointer to Value. - statics: HashMap, Pointer>, + statics: HashMap, Constant<'tcx>>, /// The virtual call stack. stack: Vec>, @@ -111,9 +110,11 @@ pub enum Lvalue<'tcx> { Local { frame: usize, local: mir::Local, - } + }, - // TODO(solson): Static/Const? None/Never? + Static(ConstantId<'tcx>), + + // TODO(solson): None/Never? } #[derive(Copy, Clone, Debug, Eq, PartialEq)] @@ -130,9 +131,9 @@ pub enum CachedMir<'mir, 'tcx: 'mir> { Owned(Rc>) } -#[derive(Clone, Debug, Eq, PartialEq, Hash)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] /// Uniquely identifies a specific constant or static -struct ConstantId<'tcx> { +pub struct ConstantId<'tcx> { /// the def id of the constant/static or in case of promoteds, the def id of the function they belong to def_id: DefId, /// In case of statics and constants this is `Substs::empty()`, so only promoteds and associated @@ -143,18 +144,36 @@ struct ConstantId<'tcx> { kind: ConstantKind, } -#[derive(Clone, Debug, Eq, PartialEq, Hash)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] enum ConstantKind { Promoted(mir::Promoted), /// Statics, constants and associated constants Global, } +#[derive(Copy, Clone, Debug)] +pub struct Constant<'tcx> { + data: Option, + mutable: bool, + ty: Ty<'tcx>, +} + +impl<'tcx> Constant<'tcx> { + fn uninitialized(ty: Ty<'tcx>) -> Self { + Constant { + data: None, + mutable: true, + ty: ty, + } + } +} + #[derive(Clone, Debug, Eq, PartialEq, Hash)] pub enum StackPopCleanup { /// The stackframe existed to compute the initial value of a static/constant, make sure the - /// static isn't modifyable afterwards - Freeze(AllocId), + /// static isn't modifyable afterwards. The allocation of the result is frozen iff it's an + /// actual allocation. `PrimVal`s are unmodifyable anyway. + Freeze, /// A regular stackframe added due to a function call will need to get forwarded to the next /// block Goto(mir::BasicBlock), @@ -380,7 +399,18 @@ impl<'a, 'tcx> EvalContext<'a, '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(alloc_id) => self.memory.freeze(alloc_id)?, + StackPopCleanup::Freeze => if let Lvalue::Static(id) = frame.return_lvalue { + let static_value = self.statics + .get_mut(&id) + .expect("static should have been cached (freeze)"); + if let Value::ByRef(ptr) = static_value.data.expect("static should have been initialized") { + self.memory.freeze(ptr.alloc_id)?; + } + assert!(static_value.mutable); + static_value.mutable = false; + } else { + bug!("StackPopCleanup::Freeze on: {:?}", frame.return_lvalue); + }, StackPopCleanup::Goto(target) => self.goto_block(target), StackPopCleanup::None => {}, } @@ -817,9 +847,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { substs: substs, kind: ConstantKind::Global, }; - let static_ptr = *self.statics.get(&cid) - .expect("static should have been cached (rvalue)"); - Value::ByRef(static_ptr) + self.read_lvalue(Lvalue::Static(cid))? } } @@ -829,9 +857,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { substs: self.substs(), kind: ConstantKind::Promoted(index), }; - let static_ptr = *self.statics.get(&cid) - .expect("a promoted constant hasn't been precomputed"); - Value::ByRef(static_ptr) + self.read_lvalue(Lvalue::Static(cid))? } }; @@ -864,6 +890,11 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Lvalue::Local { frame, local } => { self.stack[frame].get_local(local).ok_or(EvalError::ReadUndefBytes) } + Lvalue::Static(cid) => Ok(self.statics + .get(&cid) + .expect("static not cached") + .data + .expect("static not initialized")), } } @@ -886,9 +917,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { substs: substs, kind: ConstantKind::Global, }; - let ptr = *self.statics.get(&cid) - .expect("static should have been cached (lvalue)"); - Lvalue::Ptr { ptr: ptr, extra: LvalueExtra::None } + Lvalue::Static(cid) } Projection(ref proj) => return self.eval_lvalue_projection(proj), @@ -1078,8 +1107,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { fn force_allocation(&mut self, lvalue: Lvalue<'tcx>) -> EvalResult<'tcx, Lvalue<'tcx>> { let new_lvalue = match lvalue { Lvalue::Local { frame, local } => { - let ptr = match self.stack[frame].get_local(local) { - Some(Value::ByRef(ptr)) => ptr, + match self.stack[frame].get_local(local) { + Some(Value::ByRef(ptr)) => Lvalue::from_ptr(ptr), opt_val => { let ty = self.stack[frame].mir.local_decls[local].ty; let substs = self.stack[frame].substs; @@ -1088,12 +1117,32 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { if let Some(val) = opt_val { self.write_value_to_ptr(val, ptr, ty)?; } - ptr + Lvalue::from_ptr(ptr) } - }; - Lvalue::Ptr { ptr: ptr, extra: LvalueExtra::None } + } } Lvalue::Ptr { .. } => lvalue, + Lvalue::Static(cid) => { + let static_val = *self.statics.get(&cid).expect("static not cached"); + match static_val.data { + Some(Value::ByRef(ptr)) => Lvalue::from_ptr(ptr), + _ => { + let ptr = self.alloc_ptr_with_substs(static_val.ty, cid.substs)?; + if let Some(val) = static_val.data { + self.write_value_to_ptr(val, ptr, static_val.ty)?; + } + if !static_val.mutable { + self.memory.freeze(ptr.alloc_id)?; + } + let lval = self.statics.get_mut(&cid).expect("already checked"); + *lval = Constant { + data: Some(Value::ByRef(ptr)), + .. static_val + }; + Lvalue::from_ptr(ptr) + }, + } + } }; Ok(new_lvalue) } @@ -1173,6 +1222,12 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { self.stack[frame].set_local(local, Value::ByVal(val)); Ok(()) } + Lvalue::Static(cid) => { + let static_val = self.statics.get_mut(&cid).expect("static not cached"); + assert!(static_val.mutable); + static_val.data = Some(Value::ByVal(val)); + Ok(()) + } } } @@ -1183,48 +1238,73 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { dest_ty: Ty<'tcx>, ) -> EvalResult<'tcx, ()> { match dest { + Lvalue::Static(cid) => { + let dest = *self.statics.get_mut(&cid).expect("static should be cached"); + assert!(dest.mutable); + self.write_value_to_dest( + src_val, + |this, val| *this.statics.get_mut(&cid).expect("already checked") = Constant { data: Some(val), ..dest }, + dest.data, + dest_ty, + ) + }, + Lvalue::Ptr { ptr, extra } => { assert_eq!(extra, LvalueExtra::None); - self.write_value_to_ptr(src_val, ptr, dest_ty)?; + self.write_value_to_ptr(src_val, ptr, dest_ty) } - // The cases here can be a bit subtle. Read carefully! Lvalue::Local { frame, local } => { - let dest_val = self.stack[frame].get_local(local); - - if let Some(Value::ByRef(dest_ptr)) = dest_val { - // If the local value is already `ByRef` (that is, backed by an `Allocation`), - // then we must write the new value into this allocation, because there may be - // other pointers into the allocation. These other pointers are logically - // pointers into the local variable, and must be able to observe the change. - // - // Thus, it would be an error to replace the `ByRef` with a `ByVal`, unless we - // knew for certain that there were no outstanding pointers to this local. - self.write_value_to_ptr(src_val, dest_ptr, dest_ty)?; - - } else if let Value::ByRef(src_ptr) = src_val { - // If the local value is not `ByRef`, then we know there are no pointers to it - // and we can simply overwrite the `Value` in the locals array directly. - // - // In this specific case, where the source value is `ByRef`, we must duplicate - // the allocation, because this is a by-value operation. It would be incorrect - // if they referred to the same allocation, since then a change to one would - // implicitly change the other. - // - // TODO(solson): It would be valid to attempt reading a primitive value out of - // the source and writing that into the destination without making an - // allocation. This would be a pure optimization. - let dest_ptr = self.alloc_ptr(dest_ty)?; - self.copy(src_ptr, dest_ptr, dest_ty)?; - self.stack[frame].set_local(local, Value::ByRef(dest_ptr)); - - } else { - // Finally, we have the simple case where neither source nor destination are - // `ByRef`. We may simply copy the source value over the the destintion local. - self.stack[frame].set_local(local, src_val); - } + let dest = self.stack[frame].get_local(local); + self.write_value_to_dest( + src_val, + |this, val| this.stack[frame].set_local(local, val), + dest, + dest_ty, + ) } } + } + + // The cases here can be a bit subtle. Read carefully! + fn write_value_to_dest( + &mut self, + src_val: Value, + write_dest: F, + dest_val: Option, + dest_ty: Ty<'tcx>, + ) -> EvalResult<'tcx, ()> { + if let Some(Value::ByRef(dest_ptr)) = dest_val { + // If the local value is already `ByRef` (that is, backed by an `Allocation`), + // then we must write the new value into this allocation, because there may be + // other pointers into the allocation. These other pointers are logically + // pointers into the local variable, and must be able to observe the change. + // + // Thus, it would be an error to replace the `ByRef` with a `ByVal`, unless we + // knew for certain that there were no outstanding pointers to this local. + self.write_value_to_ptr(src_val, dest_ptr, dest_ty)?; + + } else if let Value::ByRef(src_ptr) = src_val { + // If the local value is not `ByRef`, then we know there are no pointers to it + // and we can simply overwrite the `Value` in the locals array directly. + // + // In this specific case, where the source value is `ByRef`, we must duplicate + // the allocation, because this is a by-value operation. It would be incorrect + // if they referred to the same allocation, since then a change to one would + // implicitly change the other. + // + // TODO(solson): It would be valid to attempt reading a primitive value out of + // the source and writing that into the destination without making an + // allocation. This would be a pure optimization. + let dest_ptr = self.alloc_ptr(dest_ty)?; + self.copy(src_ptr, dest_ptr, dest_ty)?; + write_dest(self, Value::ByRef(dest_ptr)); + + } else { + // Finally, we have the simple case where neither source nor destination are + // `ByRef`. We may simply copy the source value over the the destintion local. + write_dest(self, src_val); + } Ok(()) } diff --git a/src/interpreter/step.rs b/src/interpreter/step.rs index c109b7cd489..6c7d61b54e4 100644 --- a/src/interpreter/step.rs +++ b/src/interpreter/step.rs @@ -9,6 +9,7 @@ use super::{ Lvalue, ConstantKind, StackPopCleanup, + Constant, }; use error::EvalResult; use rustc::mir::repr as mir; @@ -128,15 +129,13 @@ impl<'a, 'b, 'tcx> ConstantExtractor<'a, 'b, 'tcx> { } self.try(|this| { let mir = this.ecx.load_mir(def_id)?; - // FIXME(solson): Don't allocate a pointer unconditionally. - let ptr = this.ecx.alloc_ptr_with_substs(mir.return_ty, substs)?; - this.ecx.statics.insert(cid.clone(), ptr); + this.ecx.statics.insert(cid.clone(), Constant::uninitialized(mir.return_ty)); let cleanup = if immutable && !mir.return_ty.type_contents(this.ecx.tcx).interior_unsafe() { - StackPopCleanup::Freeze(ptr.alloc_id) + StackPopCleanup::Freeze } else { StackPopCleanup::None }; - this.ecx.push_stack_frame(def_id, span, mir, substs, Lvalue::from_ptr(ptr), cleanup) + this.ecx.push_stack_frame(def_id, span, mir, substs, Lvalue::Static(cid.clone()), cleanup) }); } fn try EvalResult<'tcx, ()>>(&mut self, f: F) { @@ -167,6 +166,7 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for ConstantExtractor<'a, 'b, 'tcx> { } }, mir::Literal::Promoted { index } => { + let mir = self.mir.promoted[index].clone(); let cid = ConstantId { def_id: self.def_id, substs: self.substs, @@ -175,19 +175,16 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for ConstantExtractor<'a, 'b, 'tcx> { if self.ecx.statics.contains_key(&cid) { return; } - let mir = self.mir.promoted[index].clone(); - let return_ty = mir.return_ty; self.try(|this| { - // FIXME(solson): Don't allocate a pointer unconditionally. - let return_ptr = this.ecx.alloc_ptr_with_substs(return_ty, cid.substs)?; let mir = CachedMir::Owned(Rc::new(mir)); - this.ecx.statics.insert(cid.clone(), return_ptr); + let ty = this.ecx.monomorphize(mir.return_ty, this.substs); + this.ecx.statics.insert(cid.clone(), Constant::uninitialized(ty)); this.ecx.push_stack_frame(this.def_id, constant.span, mir, this.substs, - Lvalue::from_ptr(return_ptr), - StackPopCleanup::Freeze(return_ptr.alloc_id)) + Lvalue::Static(cid.clone()), + StackPopCleanup::Freeze) }); } } From b8842b25e8d96d877bdef1eba6ef1116bef56077 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 21 Oct 2016 10:44:48 +0200 Subject: [PATCH 05/13] yield a miri error instead of panicking on uninitialized statics --- src/interpreter/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 405a8b41c6c..46b4c1c6d6f 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -890,11 +890,11 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Lvalue::Local { frame, local } => { self.stack[frame].get_local(local).ok_or(EvalError::ReadUndefBytes) } - Lvalue::Static(cid) => Ok(self.statics - .get(&cid) - .expect("static not cached") - .data - .expect("static not initialized")), + Lvalue::Static(cid) => self.statics + .get(&cid) + .expect("static not cached") + .data + .ok_or(EvalError::ReadUndefBytes), } } From f81c4ac91b7296dd306f1625fc995d55060a86d7 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 21 Oct 2016 10:45:01 +0200 Subject: [PATCH 06/13] more priroda requirements --- src/interpreter/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 46b4c1c6d6f..23b1b390c69 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -881,7 +881,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { self.read_lvalue(lvalue) } - fn read_lvalue(&mut self, lvalue: Lvalue<'tcx>) -> EvalResult<'tcx, Value> { + pub fn read_lvalue(&self, lvalue: Lvalue<'tcx>) -> EvalResult<'tcx, Value> { match lvalue { Lvalue::Ptr { ptr, extra } => { assert_eq!(extra, LvalueExtra::None); From 9e9586da9555d779be81bd27f2390dced5ff78f4 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 21 Oct 2016 11:21:52 +0200 Subject: [PATCH 07/13] constant ids are `Copy` now --- src/interpreter/step.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/interpreter/step.rs b/src/interpreter/step.rs index 6c7d61b54e4..1207bed97a3 100644 --- a/src/interpreter/step.rs +++ b/src/interpreter/step.rs @@ -129,13 +129,13 @@ impl<'a, 'b, 'tcx> ConstantExtractor<'a, 'b, 'tcx> { } self.try(|this| { let mir = this.ecx.load_mir(def_id)?; - this.ecx.statics.insert(cid.clone(), Constant::uninitialized(mir.return_ty)); + this.ecx.statics.insert(cid, Constant::uninitialized(mir.return_ty)); let cleanup = if immutable && !mir.return_ty.type_contents(this.ecx.tcx).interior_unsafe() { StackPopCleanup::Freeze } else { StackPopCleanup::None }; - this.ecx.push_stack_frame(def_id, span, mir, substs, Lvalue::Static(cid.clone()), cleanup) + this.ecx.push_stack_frame(def_id, span, mir, substs, Lvalue::Static(cid), cleanup) }); } fn try EvalResult<'tcx, ()>>(&mut self, f: F) { @@ -178,12 +178,12 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for ConstantExtractor<'a, 'b, 'tcx> { self.try(|this| { let mir = CachedMir::Owned(Rc::new(mir)); let ty = this.ecx.monomorphize(mir.return_ty, this.substs); - this.ecx.statics.insert(cid.clone(), Constant::uninitialized(ty)); + this.ecx.statics.insert(cid, Constant::uninitialized(ty)); this.ecx.push_stack_frame(this.def_id, constant.span, mir, this.substs, - Lvalue::Static(cid.clone()), + Lvalue::Static(cid), StackPopCleanup::Freeze) }); } From f6bbea0f08469fde3e7b9f1b7b4e81690a9482a8 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 21 Oct 2016 11:24:10 +0200 Subject: [PATCH 08/13] choose better function and argument names --- src/interpreter/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 23b1b390c69..7069a72389c 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -1241,7 +1241,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Lvalue::Static(cid) => { let dest = *self.statics.get_mut(&cid).expect("static should be cached"); assert!(dest.mutable); - self.write_value_to_dest( + self.write_value_possibly_by_val( src_val, |this, val| *this.statics.get_mut(&cid).expect("already checked") = Constant { data: Some(val), ..dest }, dest.data, @@ -1256,7 +1256,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Lvalue::Local { frame, local } => { let dest = self.stack[frame].get_local(local); - self.write_value_to_dest( + self.write_value_possibly_by_val( src_val, |this, val| this.stack[frame].set_local(local, val), dest, @@ -1267,14 +1267,14 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } // The cases here can be a bit subtle. Read carefully! - fn write_value_to_dest( + fn write_value_possibly_by_val( &mut self, src_val: Value, write_dest: F, - dest_val: Option, + old_dest_val: Option, dest_ty: Ty<'tcx>, ) -> EvalResult<'tcx, ()> { - if let Some(Value::ByRef(dest_ptr)) = dest_val { + if let Some(Value::ByRef(dest_ptr)) = old_dest_val { // If the local value is already `ByRef` (that is, backed by an `Allocation`), // then we must write the new value into this allocation, because there may be // other pointers into the allocation. These other pointers are logically From d3b3c56b07ee480ea4633e21e33207b4e239950d Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 21 Oct 2016 11:39:39 +0200 Subject: [PATCH 09/13] rename statics/Constant/ConstantId/ConstantKind to [gG]lobal* --- src/interpreter/mod.rs | 46 ++++++++++++++++++++--------------------- src/interpreter/step.rs | 22 ++++++++++---------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 7069a72389c..4e6c80f9aab 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -41,7 +41,7 @@ pub struct EvalContext<'a, 'tcx: 'a> { memory: Memory<'a, 'tcx>, /// Precomputed statics, constants and promoteds. - statics: HashMap, Constant<'tcx>>, + globals: HashMap, Global<'tcx>>, /// The virtual call stack. stack: Vec>, @@ -112,7 +112,7 @@ pub enum Lvalue<'tcx> { local: mir::Local, }, - Static(ConstantId<'tcx>), + Static(GlobalId<'tcx>), // TODO(solson): None/Never? } @@ -133,7 +133,7 @@ pub enum CachedMir<'mir, 'tcx: 'mir> { #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] /// Uniquely identifies a specific constant or static -pub struct ConstantId<'tcx> { +pub struct GlobalId<'tcx> { /// the def id of the constant/static or in case of promoteds, the def id of the function they belong to def_id: DefId, /// In case of statics and constants this is `Substs::empty()`, so only promoteds and associated @@ -141,26 +141,26 @@ pub struct ConstantId<'tcx> { /// but that would only require more branching when working with constants, and not bring any /// real benefits. substs: &'tcx Substs<'tcx>, - kind: ConstantKind, + kind: GlobalKind, } #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] -enum ConstantKind { +enum GlobalKind { Promoted(mir::Promoted), /// Statics, constants and associated constants Global, } #[derive(Copy, Clone, Debug)] -pub struct Constant<'tcx> { +pub struct Global<'tcx> { data: Option, mutable: bool, ty: Ty<'tcx>, } -impl<'tcx> Constant<'tcx> { +impl<'tcx> Global<'tcx> { fn uninitialized(ty: Ty<'tcx>) -> Self { - Constant { + Global { data: None, mutable: true, ty: ty, @@ -188,7 +188,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { mir_map: mir_map, mir_cache: RefCell::new(DefIdMap()), memory: Memory::new(&tcx.data_layout, memory_size), - statics: HashMap::new(), + globals: HashMap::new(), stack: Vec::new(), stack_limit: stack_limit, } @@ -400,7 +400,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { 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::Static(id) = frame.return_lvalue { - let static_value = self.statics + let static_value = self.globals .get_mut(&id) .expect("static should have been cached (freeze)"); if let Value::ByRef(ptr) = static_value.data.expect("static should have been initialized") { @@ -842,20 +842,20 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { // function items are zero sized Value::ByRef(self.memory.allocate(0, 0)?) } else { - let cid = ConstantId { + let cid = GlobalId { def_id: def_id, substs: substs, - kind: ConstantKind::Global, + kind: GlobalKind::Global, }; self.read_lvalue(Lvalue::Static(cid))? } } Literal::Promoted { index } => { - let cid = ConstantId { + let cid = GlobalId { def_id: self.frame().def_id, substs: self.substs(), - kind: ConstantKind::Promoted(index), + kind: GlobalKind::Promoted(index), }; self.read_lvalue(Lvalue::Static(cid))? } @@ -890,7 +890,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Lvalue::Local { frame, local } => { self.stack[frame].get_local(local).ok_or(EvalError::ReadUndefBytes) } - Lvalue::Static(cid) => self.statics + Lvalue::Static(cid) => self.globals .get(&cid) .expect("static not cached") .data @@ -912,10 +912,10 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Static(def_id) => { let substs = subst::Substs::empty(self.tcx); - let cid = ConstantId { + let cid = GlobalId { def_id: def_id, substs: substs, - kind: ConstantKind::Global, + kind: GlobalKind::Global, }; Lvalue::Static(cid) } @@ -1123,7 +1123,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } Lvalue::Ptr { .. } => lvalue, Lvalue::Static(cid) => { - let static_val = *self.statics.get(&cid).expect("static not cached"); + let static_val = *self.globals.get(&cid).expect("static not cached"); match static_val.data { Some(Value::ByRef(ptr)) => Lvalue::from_ptr(ptr), _ => { @@ -1134,8 +1134,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { if !static_val.mutable { self.memory.freeze(ptr.alloc_id)?; } - let lval = self.statics.get_mut(&cid).expect("already checked"); - *lval = Constant { + let lval = self.globals.get_mut(&cid).expect("already checked"); + *lval = Global { data: Some(Value::ByRef(ptr)), .. static_val }; @@ -1223,7 +1223,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Ok(()) } Lvalue::Static(cid) => { - let static_val = self.statics.get_mut(&cid).expect("static not cached"); + let static_val = self.globals.get_mut(&cid).expect("static not cached"); assert!(static_val.mutable); static_val.data = Some(Value::ByVal(val)); Ok(()) @@ -1239,11 +1239,11 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { ) -> EvalResult<'tcx, ()> { match dest { Lvalue::Static(cid) => { - let dest = *self.statics.get_mut(&cid).expect("static should be cached"); + let dest = *self.globals.get_mut(&cid).expect("static should be cached"); assert!(dest.mutable); self.write_value_possibly_by_val( src_val, - |this, val| *this.statics.get_mut(&cid).expect("already checked") = Constant { data: Some(val), ..dest }, + |this, val| *this.globals.get_mut(&cid).expect("already checked") = Global { data: Some(val), ..dest }, dest.data, dest_ty, ) diff --git a/src/interpreter/step.rs b/src/interpreter/step.rs index 1207bed97a3..f25eea875fc 100644 --- a/src/interpreter/step.rs +++ b/src/interpreter/step.rs @@ -4,12 +4,12 @@ use super::{ CachedMir, - ConstantId, + GlobalId, EvalContext, Lvalue, - ConstantKind, + GlobalKind, StackPopCleanup, - Constant, + Global, }; use error::EvalResult; use rustc::mir::repr as mir; @@ -119,17 +119,17 @@ struct ConstantExtractor<'a, 'b: 'a, 'tcx: 'b> { impl<'a, 'b, 'tcx> ConstantExtractor<'a, 'b, 'tcx> { fn global_item(&mut self, def_id: DefId, substs: &'tcx subst::Substs<'tcx>, span: Span, immutable: bool) { - let cid = ConstantId { + let cid = GlobalId { def_id: def_id, substs: substs, - kind: ConstantKind::Global, + kind: GlobalKind::Global, }; - if self.ecx.statics.contains_key(&cid) { + if self.ecx.globals.contains_key(&cid) { return; } self.try(|this| { let mir = this.ecx.load_mir(def_id)?; - this.ecx.statics.insert(cid, Constant::uninitialized(mir.return_ty)); + 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 { @@ -167,18 +167,18 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for ConstantExtractor<'a, 'b, 'tcx> { }, mir::Literal::Promoted { index } => { let mir = self.mir.promoted[index].clone(); - let cid = ConstantId { + let cid = GlobalId { def_id: self.def_id, substs: self.substs, - kind: ConstantKind::Promoted(index), + kind: GlobalKind::Promoted(index), }; - if self.ecx.statics.contains_key(&cid) { + if self.ecx.globals.contains_key(&cid) { return; } self.try(|this| { let mir = CachedMir::Owned(Rc::new(mir)); let ty = this.ecx.monomorphize(mir.return_ty, this.substs); - this.ecx.statics.insert(cid, Constant::uninitialized(ty)); + this.ecx.globals.insert(cid, Global::uninitialized(ty)); this.ecx.push_stack_frame(this.def_id, constant.span, mir, From 2f81729e76e4bbd9acd2b1e5fb9bbc21eed3ba0f Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 21 Oct 2016 11:48:56 +0200 Subject: [PATCH 10/13] rename more [Ss]tatic* to [Gg]lobal* --- src/interpreter/mod.rs | 56 ++++++++++++++++++++--------------------- src/interpreter/step.rs | 4 +-- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 4e6c80f9aab..bcab8be53e9 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -112,7 +112,7 @@ pub enum Lvalue<'tcx> { local: mir::Local, }, - Static(GlobalId<'tcx>), + Global(GlobalId<'tcx>), // TODO(solson): None/Never? } @@ -170,8 +170,8 @@ impl<'tcx> Global<'tcx> { #[derive(Clone, Debug, Eq, PartialEq, Hash)] pub enum StackPopCleanup { - /// The stackframe existed to compute the initial value of a static/constant, make sure the - /// static isn't modifyable afterwards. The allocation of the result is frozen iff it's an + /// 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, /// A regular stackframe added due to a function call will need to get forwarded to the next @@ -399,15 +399,15 @@ impl<'a, 'tcx> EvalContext<'a, '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::Static(id) = frame.return_lvalue { - let static_value = self.globals + StackPopCleanup::Freeze => if let Lvalue::Global(id) = frame.return_lvalue { + let global_value = self.globals .get_mut(&id) - .expect("static should have been cached (freeze)"); - if let Value::ByRef(ptr) = static_value.data.expect("static should have been initialized") { + .expect("global should have been cached (freeze)"); + if let Value::ByRef(ptr) = global_value.data.expect("global should have been initialized") { self.memory.freeze(ptr.alloc_id)?; } - assert!(static_value.mutable); - static_value.mutable = false; + assert!(global_value.mutable); + global_value.mutable = false; } else { bug!("StackPopCleanup::Freeze on: {:?}", frame.return_lvalue); }, @@ -847,7 +847,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { substs: substs, kind: GlobalKind::Global, }; - self.read_lvalue(Lvalue::Static(cid))? + self.read_lvalue(Lvalue::Global(cid))? } } @@ -857,7 +857,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { substs: self.substs(), kind: GlobalKind::Promoted(index), }; - self.read_lvalue(Lvalue::Static(cid))? + self.read_lvalue(Lvalue::Global(cid))? } }; @@ -890,9 +890,9 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Lvalue::Local { frame, local } => { self.stack[frame].get_local(local).ok_or(EvalError::ReadUndefBytes) } - Lvalue::Static(cid) => self.globals + Lvalue::Global(cid) => self.globals .get(&cid) - .expect("static not cached") + .expect("global not cached") .data .ok_or(EvalError::ReadUndefBytes), } @@ -917,7 +917,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { substs: substs, kind: GlobalKind::Global, }; - Lvalue::Static(cid) + Lvalue::Global(cid) } Projection(ref proj) => return self.eval_lvalue_projection(proj), @@ -1122,22 +1122,22 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } } Lvalue::Ptr { .. } => lvalue, - Lvalue::Static(cid) => { - let static_val = *self.globals.get(&cid).expect("static not cached"); - match static_val.data { + Lvalue::Global(cid) => { + let global_val = *self.globals.get(&cid).expect("global not cached"); + match global_val.data { Some(Value::ByRef(ptr)) => Lvalue::from_ptr(ptr), _ => { - let ptr = self.alloc_ptr_with_substs(static_val.ty, cid.substs)?; - if let Some(val) = static_val.data { - self.write_value_to_ptr(val, ptr, static_val.ty)?; + let ptr = self.alloc_ptr_with_substs(global_val.ty, cid.substs)?; + if let Some(val) = global_val.data { + self.write_value_to_ptr(val, ptr, global_val.ty)?; } - if !static_val.mutable { + if !global_val.mutable { self.memory.freeze(ptr.alloc_id)?; } let lval = self.globals.get_mut(&cid).expect("already checked"); *lval = Global { data: Some(Value::ByRef(ptr)), - .. static_val + .. global_val }; Lvalue::from_ptr(ptr) }, @@ -1222,10 +1222,10 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { self.stack[frame].set_local(local, Value::ByVal(val)); Ok(()) } - Lvalue::Static(cid) => { - let static_val = self.globals.get_mut(&cid).expect("static not cached"); - assert!(static_val.mutable); - static_val.data = Some(Value::ByVal(val)); + Lvalue::Global(cid) => { + let global_val = self.globals.get_mut(&cid).expect("global not cached"); + assert!(global_val.mutable); + global_val.data = Some(Value::ByVal(val)); Ok(()) } } @@ -1238,8 +1238,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { dest_ty: Ty<'tcx>, ) -> EvalResult<'tcx, ()> { match dest { - Lvalue::Static(cid) => { - let dest = *self.globals.get_mut(&cid).expect("static should be cached"); + Lvalue::Global(cid) => { + let dest = *self.globals.get_mut(&cid).expect("global should be cached"); assert!(dest.mutable); self.write_value_possibly_by_val( src_val, diff --git a/src/interpreter/step.rs b/src/interpreter/step.rs index f25eea875fc..e745fe64d71 100644 --- a/src/interpreter/step.rs +++ b/src/interpreter/step.rs @@ -135,7 +135,7 @@ impl<'a, 'b, 'tcx> ConstantExtractor<'a, 'b, 'tcx> { } else { StackPopCleanup::None }; - this.ecx.push_stack_frame(def_id, span, mir, substs, Lvalue::Static(cid), cleanup) + this.ecx.push_stack_frame(def_id, span, mir, substs, Lvalue::Global(cid), cleanup) }); } fn try EvalResult<'tcx, ()>>(&mut self, f: F) { @@ -183,7 +183,7 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for ConstantExtractor<'a, 'b, 'tcx> { constant.span, mir, this.substs, - Lvalue::Static(cid), + Lvalue::Global(cid), StackPopCleanup::Freeze) }); } From e82e6132ecb4c1c789a6e41e61aba551bb6003df Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 21 Oct 2016 11:51:24 +0200 Subject: [PATCH 11/13] preemptively change some assertions into errors --- src/interpreter/mod.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index bcab8be53e9..66053e9dd59 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -1224,9 +1224,12 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } Lvalue::Global(cid) => { let global_val = self.globals.get_mut(&cid).expect("global not cached"); - assert!(global_val.mutable); - global_val.data = Some(Value::ByVal(val)); - Ok(()) + if global_val.mutable { + global_val.data = Some(Value::ByVal(val)); + Ok(()) + } else { + Err(EvalError::ModifiedConstantMemory) + } } } } @@ -1240,7 +1243,9 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { match dest { Lvalue::Global(cid) => { let dest = *self.globals.get_mut(&cid).expect("global should be cached"); - assert!(dest.mutable); + if !dest.mutable { + return Err(EvalError::ModifiedConstantMemory); + } self.write_value_possibly_by_val( src_val, |this, val| *this.globals.get_mut(&cid).expect("already checked") = Global { data: Some(val), ..dest }, From 9d0b903d9db7baa07c3b1bac274e1b071005f44d Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 21 Oct 2016 11:54:38 +0200 Subject: [PATCH 12/13] remove GlobalKind --- src/interpreter/mod.rs | 16 +++++----------- src/interpreter/step.rs | 5 ++--- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 66053e9dd59..1cbfafd10b2 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -141,14 +141,8 @@ pub struct GlobalId<'tcx> { /// but that would only require more branching when working with constants, and not bring any /// real benefits. substs: &'tcx Substs<'tcx>, - kind: GlobalKind, -} - -#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] -enum GlobalKind { - Promoted(mir::Promoted), - /// Statics, constants and associated constants - Global, + /// this `Option` is `Some` for promoted constants + promoted: Option, } #[derive(Copy, Clone, Debug)] @@ -845,7 +839,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let cid = GlobalId { def_id: def_id, substs: substs, - kind: GlobalKind::Global, + promoted: None, }; self.read_lvalue(Lvalue::Global(cid))? } @@ -855,7 +849,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let cid = GlobalId { def_id: self.frame().def_id, substs: self.substs(), - kind: GlobalKind::Promoted(index), + promoted: Some(index), }; self.read_lvalue(Lvalue::Global(cid))? } @@ -915,7 +909,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let cid = GlobalId { def_id: def_id, substs: substs, - kind: GlobalKind::Global, + promoted: None, }; Lvalue::Global(cid) } diff --git a/src/interpreter/step.rs b/src/interpreter/step.rs index e745fe64d71..eac5494f431 100644 --- a/src/interpreter/step.rs +++ b/src/interpreter/step.rs @@ -7,7 +7,6 @@ use super::{ GlobalId, EvalContext, Lvalue, - GlobalKind, StackPopCleanup, Global, }; @@ -122,7 +121,7 @@ impl<'a, 'b, 'tcx> ConstantExtractor<'a, 'b, 'tcx> { let cid = GlobalId { def_id: def_id, substs: substs, - kind: GlobalKind::Global, + promoted: None, }; if self.ecx.globals.contains_key(&cid) { return; @@ -170,7 +169,7 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for ConstantExtractor<'a, 'b, 'tcx> { let cid = GlobalId { def_id: self.def_id, substs: self.substs, - kind: GlobalKind::Promoted(index), + promoted: Some(index), }; if self.ecx.globals.contains_key(&cid) { return; From edc6b93b855ef342e0f3f5935eef60b3218a9b97 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 21 Oct 2016 12:03:34 +0200 Subject: [PATCH 13/13] adjust some comments referencing locals --- src/interpreter/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 1cbfafd10b2..ed5dc96458c 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -1274,17 +1274,17 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { dest_ty: Ty<'tcx>, ) -> EvalResult<'tcx, ()> { if let Some(Value::ByRef(dest_ptr)) = old_dest_val { - // If the local value is already `ByRef` (that is, backed by an `Allocation`), + // If the value is already `ByRef` (that is, backed by an `Allocation`), // then we must write the new value into this allocation, because there may be // other pointers into the allocation. These other pointers are logically // pointers into the local variable, and must be able to observe the change. // // Thus, it would be an error to replace the `ByRef` with a `ByVal`, unless we - // knew for certain that there were no outstanding pointers to this local. + // knew for certain that there were no outstanding pointers to this allocation. self.write_value_to_ptr(src_val, dest_ptr, dest_ty)?; } else if let Value::ByRef(src_ptr) = src_val { - // If the local value is not `ByRef`, then we know there are no pointers to it + // If the value is not `ByRef`, then we know there are no pointers to it // and we can simply overwrite the `Value` in the locals array directly. // // In this specific case, where the source value is `ByRef`, we must duplicate @@ -1301,7 +1301,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } else { // Finally, we have the simple case where neither source nor destination are - // `ByRef`. We may simply copy the source value over the the destintion local. + // `ByRef`. We may simply copy the source value over the the destintion. write_dest(self, src_val); } Ok(())