From b84f7e20293cd1cebee011dbb485583c79f7ede3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 16 Oct 2018 11:21:38 +0200 Subject: [PATCH 01/12] add Borrow tag to pointers; remove old locking code --- src/fn_call.rs | 28 ++++++------- src/helpers.rs | 4 +- src/intrinsic.rs | 10 ++--- src/lib.rs | 61 ++++++++++++++------------- src/locks.rs | 94 ------------------------------------------ src/operator.rs | 47 ++++++++++++--------- src/stacked_borrows.rs | 36 ++++++++++++++++ src/tls.rs | 10 ++--- 8 files changed, 119 insertions(+), 171 deletions(-) delete mode 100644 src/locks.rs create mode 100644 src/stacked_borrows.rs diff --git a/src/fn_call.rs b/src/fn_call.rs index 812df49b0b..280cc2bdea 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -14,8 +14,8 @@ pub trait EvalContextExt<'tcx, 'mir> { fn emulate_foreign_item( &mut self, def_id: DefId, - args: &[OpTy<'tcx>], - dest: PlaceTy<'tcx>, + args: &[OpTy<'tcx, Borrow>], + dest: PlaceTy<'tcx, Borrow>, ret: mir::BasicBlock, ) -> EvalResult<'tcx>; @@ -28,28 +28,28 @@ pub trait EvalContextExt<'tcx, 'mir> { fn emulate_missing_fn( &mut self, path: String, - args: &[OpTy<'tcx>], - dest: Option>, + args: &[OpTy<'tcx, Borrow>], + dest: Option>, ret: Option, ) -> EvalResult<'tcx>; fn find_fn( &mut self, instance: ty::Instance<'tcx>, - args: &[OpTy<'tcx>], - dest: Option>, + args: &[OpTy<'tcx, Borrow>], + dest: Option>, ret: Option, ) -> EvalResult<'tcx, Option<&'mir mir::Mir<'tcx>>>; - fn write_null(&mut self, dest: PlaceTy<'tcx>) -> EvalResult<'tcx>; + fn write_null(&mut self, dest: PlaceTy<'tcx, Borrow>) -> EvalResult<'tcx>; } impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for EvalContext<'a, 'mir, 'tcx, super::Evaluator<'tcx>> { fn find_fn( &mut self, instance: ty::Instance<'tcx>, - args: &[OpTy<'tcx>], - dest: Option>, + args: &[OpTy<'tcx, Borrow>], + dest: Option>, ret: Option, ) -> EvalResult<'tcx, Option<&'mir mir::Mir<'tcx>>> { trace!("eval_fn_call: {:#?}, {:?}", instance, dest.map(|place| *place)); @@ -108,8 +108,8 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for EvalContext<'a, ' fn emulate_foreign_item( &mut self, def_id: DefId, - args: &[OpTy<'tcx>], - dest: PlaceTy<'tcx>, + args: &[OpTy<'tcx, Borrow>], + dest: PlaceTy<'tcx, Borrow>, ret: mir::BasicBlock, ) -> EvalResult<'tcx> { let attrs = self.tcx.get_attrs(def_id); @@ -675,8 +675,8 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for EvalContext<'a, ' fn emulate_missing_fn( &mut self, path: String, - _args: &[OpTy<'tcx>], - dest: Option>, + _args: &[OpTy<'tcx, Borrow>], + dest: Option>, ret: Option, ) -> EvalResult<'tcx> { // In some cases in non-MIR libstd-mode, not having a destination is legit. Handle these early. @@ -724,7 +724,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for EvalContext<'a, ' Ok(()) } - fn write_null(&mut self, dest: PlaceTy<'tcx>) -> EvalResult<'tcx> { + fn write_null(&mut self, dest: PlaceTy<'tcx, Borrow>) -> EvalResult<'tcx> { self.write_scalar(Scalar::from_int(0, dest.layout.size), dest) } } diff --git a/src/helpers.rs b/src/helpers.rs index 27b2109d18..de787145e2 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -7,7 +7,7 @@ pub trait FalibleScalarExt { fn to_bytes(self) -> EvalResult<'static, u128>; } -impl FalibleScalarExt for Scalar { +impl FalibleScalarExt for Scalar { fn to_bytes(self) -> EvalResult<'static, u128> { match self { Scalar::Bits { bits, size } => { @@ -19,7 +19,7 @@ impl FalibleScalarExt for Scalar { } } -impl FalibleScalarExt for ScalarMaybeUndef { +impl FalibleScalarExt for ScalarMaybeUndef { fn to_bytes(self) -> EvalResult<'static, u128> { self.not_undef()?.to_bytes() } diff --git a/src/intrinsic.rs b/src/intrinsic.rs index 19c4f04f48..11c0bd7907 100644 --- a/src/intrinsic.rs +++ b/src/intrinsic.rs @@ -6,7 +6,7 @@ use rustc::mir::interpret::{EvalResult, PointerArithmetic}; use rustc_mir::interpret::{EvalContext, PlaceTy, OpTy}; use super::{ - Value, Scalar, ScalarMaybeUndef, + Value, Scalar, ScalarMaybeUndef, Borrow, FalibleScalarExt, OperatorEvalContextExt }; @@ -14,8 +14,8 @@ pub trait EvalContextExt<'tcx> { fn call_intrinsic( &mut self, instance: ty::Instance<'tcx>, - args: &[OpTy<'tcx>], - dest: PlaceTy<'tcx>, + args: &[OpTy<'tcx, Borrow>], + dest: PlaceTy<'tcx, Borrow>, ) -> EvalResult<'tcx>; } @@ -23,8 +23,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: fn call_intrinsic( &mut self, instance: ty::Instance<'tcx>, - args: &[OpTy<'tcx>], - dest: PlaceTy<'tcx>, + args: &[OpTy<'tcx, Borrow>], + dest: PlaceTy<'tcx, Borrow>, ) -> EvalResult<'tcx> { if self.emulate_intrinsic(instance, args, dest)? { return Ok(()); diff --git a/src/lib.rs b/src/lib.rs index 0bebe40d52..9ac703e267 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -21,11 +21,9 @@ use rustc::ty::layout::{TyLayout, LayoutOf, Size}; use rustc::hir::def_id::DefId; use rustc::mir; -use syntax::ast::Mutability; use syntax::attr; -pub use rustc::mir::interpret::*; pub use rustc_mir::interpret::*; pub use rustc_mir::interpret::{self, AllocMap}; // resolve ambiguity @@ -34,9 +32,9 @@ mod operator; mod intrinsic; mod helpers; mod tls; -mod locks; mod range_map; mod mono_hash_map; +mod stacked_borrows; use fn_call::EvalContextExt as MissingFnsEvalContextExt; use operator::EvalContextExt as OperatorEvalContextExt; @@ -45,6 +43,7 @@ use tls::{EvalContextExt as TlsEvalContextExt, TlsData}; use range_map::RangeMap; use helpers::FalibleScalarExt; use mono_hash_map::MonoHashMap; +use stacked_borrows::Borrow; pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>( tcx: TyCtxt<'a, 'tcx, 'tcx>, @@ -56,7 +55,6 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>( tcx.at(syntax::source_map::DUMMY_SP), ty::ParamEnv::reveal_all(), Evaluator::new(validate), - Default::default(), ); let main_instance = ty::Instance::mono(ecx.tcx.tcx, main_id); @@ -118,9 +116,9 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>( let foo = ecx.memory.allocate_static_bytes(b"foo\0"); let foo_ty = ecx.tcx.mk_imm_ptr(ecx.tcx.types.u8); let foo_layout = ecx.layout_of(foo_ty)?; - let foo_place = ecx.allocate(foo_layout, MemoryKind::Stack)?; // will be interned in just a second + let foo_place = ecx.allocate(foo_layout, MiriMemoryKind::Env.into())?; ecx.write_scalar(Scalar::Ptr(foo), foo_place.into())?; - ecx.memory.intern_static(foo_place.to_ptr()?.alloc_id, Mutability::Immutable)?; + ecx.memory.mark_immutable(foo_place.to_ptr()?.alloc_id)?; ecx.write_scalar(foo_place.ptr, dest)?; assert!(args.next().is_none(), "start lang item has more arguments than expected"); @@ -227,7 +225,7 @@ impl Into> for MiriMemoryKind { pub struct Evaluator<'tcx> { /// Environment variables set by `setenv` /// Miri does not expose env vars from the host to the emulated program - pub(crate) env_vars: HashMap, Pointer>, + pub(crate) env_vars: HashMap, Pointer>, /// TLS state pub(crate) tls: TlsData<'tcx>, @@ -247,11 +245,11 @@ impl<'tcx> Evaluator<'tcx> { } impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { - type MemoryData = (); type MemoryKinds = MiriMemoryKind; - type PointerTag = (); // still WIP + type AllocExtra = (); + type PointerTag = Borrow; - type MemoryMap = MonoHashMap, Allocation<()>)>; + type MemoryMap = MonoHashMap, Allocation)>; const STATIC_KIND: Option = Some(MiriMemoryKind::MutStatic); @@ -282,8 +280,8 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { fn find_fn( ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, - args: &[OpTy<'tcx>], - dest: Option>, + args: &[OpTy<'tcx, Borrow>], + dest: Option>, ret: Option, ) -> EvalResult<'tcx, Option<&'mir mir::Mir<'tcx>>> { ecx.find_fn(instance, args, dest, ret) @@ -292,8 +290,8 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { fn call_intrinsic( ecx: &mut rustc_mir::interpret::EvalContext<'a, 'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, - args: &[OpTy<'tcx>], - dest: PlaceTy<'tcx>, + args: &[OpTy<'tcx, Borrow>], + dest: PlaceTy<'tcx, Borrow>, ) -> EvalResult<'tcx> { ecx.call_intrinsic(instance, args, dest) } @@ -301,17 +299,17 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { fn ptr_op( ecx: &rustc_mir::interpret::EvalContext<'a, 'mir, 'tcx, Self>, bin_op: mir::BinOp, - left: Scalar, + left: Scalar, left_layout: TyLayout<'tcx>, - right: Scalar, + right: Scalar, right_layout: TyLayout<'tcx>, - ) -> EvalResult<'tcx, (Scalar, bool)> { + ) -> EvalResult<'tcx, (Scalar, bool)> { ecx.ptr_op(bin_op, left, left_layout, right, right_layout) } fn box_alloc( ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, - dest: PlaceTy<'tcx>, + dest: PlaceTy<'tcx, Borrow>, ) -> EvalResult<'tcx> { trace!("box_alloc for {:?}", dest.layout.ty); // Call the `exchange_malloc` lang item @@ -351,7 +349,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { fn find_foreign_static( tcx: TyCtxtAt<'a, 'tcx, 'tcx>, def_id: DefId, - ) -> EvalResult<'tcx, Cow<'tcx, Allocation>> { + ) -> EvalResult<'tcx, Cow<'tcx, Allocation>> { let attrs = tcx.get_attrs(def_id); let link_name = match attr::first_attr_value_str_by_name(&attrs, "link_name") { Some(name) => name.as_str(), @@ -371,16 +369,6 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { Ok(Cow::Owned(alloc)) } - fn validation_op( - _ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, - _op: ::rustc::mir::ValidationOp, - _operand: &::rustc::mir::ValidationOperand<'tcx, ::rustc::mir::Place<'tcx>>, - ) -> EvalResult<'tcx> { - // FIXME: prevent this from ICEing - //ecx.validation_op(op, operand) - Ok(()) - } - fn before_terminator(_ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>) -> EvalResult<'tcx> { // We are not interested in detecting loops @@ -389,8 +377,19 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { fn static_with_default_tag( alloc: &'_ Allocation - ) -> Cow<'_, Allocation> { - let alloc = alloc.clone(); + ) -> Cow<'_, Allocation> { + let alloc: Allocation = Allocation { + bytes: alloc.bytes.clone(), + relocations: Relocations::from_presorted( + alloc.relocations.iter() + .map(|&(offset, ((), alloc))| (offset, (Borrow::default(), alloc))) + .collect() + ), + undef_mask: alloc.undef_mask.clone(), + align: alloc.align, + mutability: alloc.mutability, + extra: Self::AllocExtra::default(), + }; Cow::Owned(alloc) } } diff --git a/src/locks.rs b/src/locks.rs deleted file mode 100644 index a87ff6367e..0000000000 --- a/src/locks.rs +++ /dev/null @@ -1,94 +0,0 @@ -#![allow(unused)] - -use super::*; -use rustc::middle::region; -use rustc::ty::layout::Size; - -//////////////////////////////////////////////////////////////////////////////// -// Locks -//////////////////////////////////////////////////////////////////////////////// - -// Just some dummy to keep this compiling; I think some of this will be useful later -type AbsPlace<'tcx> = ::rustc::ty::Ty<'tcx>; - -/// Information about a lock that is currently held. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct LockInfo<'tcx> { - /// Stores for which lifetimes (of the original write lock) we got - /// which suspensions. - suspended: HashMap, Vec>, - /// The current state of the lock that's actually effective. - pub active: Lock, -} - -/// Write locks are identified by a stack frame and an "abstract" (untyped) place. -/// It may be tempting to use the lifetime as identifier, but that does not work -/// for two reasons: -/// * First of all, due to subtyping, the same lock may be referred to with different -/// lifetimes. -/// * Secondly, different write locks may actually have the same lifetime. See `test2` -/// in `run-pass/many_shr_bor.rs`. -/// The Id is "captured" when the lock is first suspended; at that point, the borrow checker -/// considers the path frozen and hence the Id remains stable. -#[derive(Clone, Debug, PartialEq, Eq, Hash)] -pub struct WriteLockId<'tcx> { - frame: usize, - path: AbsPlace<'tcx>, -} - - -use rustc::mir::interpret::Lock::*; -use rustc::mir::interpret::Lock; - -impl<'tcx> Default for LockInfo<'tcx> { - fn default() -> Self { - LockInfo::new(NoLock) - } -} - -impl<'tcx> LockInfo<'tcx> { - fn new(lock: Lock) -> LockInfo<'tcx> { - LockInfo { - suspended: HashMap::new(), - active: lock, - } - } - - fn access_permitted(&self, frame: Option, access: AccessKind) -> bool { - use super::AccessKind::*; - match (&self.active, access) { - (&NoLock, _) => true, - (&ReadLock(ref lfts), Read) => { - assert!(!lfts.is_empty(), "Someone left an empty read lock behind."); - // Read access to read-locked region is okay, no matter who's holding the read lock. - true - } - (&WriteLock(ref lft), _) => { - // All access is okay if we are the ones holding it - Some(lft.frame) == frame - } - _ => false, // Nothing else is okay. - } - } -} - -impl<'tcx> RangeMap> { - pub fn check( - &self, - frame: Option, - offset: u64, - len: u64, - access: AccessKind, - ) -> Result<(), LockInfo<'tcx>> { - if len == 0 { - return Ok(()); - } - for lock in self.iter(offset, len) { - // Check if the lock is in conflict with the access. - if !lock.access_permitted(frame, access) { - return Err(lock.clone()); - } - } - Ok(()) - } -} diff --git a/src/operator.rs b/src/operator.rs index 4662a16267..dd79f29313 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -7,44 +7,44 @@ pub trait EvalContextExt<'tcx> { fn ptr_op( &self, bin_op: mir::BinOp, - left: Scalar, + left: Scalar, left_layout: TyLayout<'tcx>, - right: Scalar, + right: Scalar, right_layout: TyLayout<'tcx>, - ) -> EvalResult<'tcx, (Scalar, bool)>; + ) -> EvalResult<'tcx, (Scalar, bool)>; fn ptr_int_arithmetic( &self, bin_op: mir::BinOp, - left: Pointer, + left: Pointer, right: u128, signed: bool, - ) -> EvalResult<'tcx, (Scalar, bool)>; + ) -> EvalResult<'tcx, (Scalar, bool)>; fn ptr_eq( &self, - left: Scalar, - right: Scalar, + left: Scalar, + right: Scalar, size: Size, ) -> EvalResult<'tcx, bool>; fn pointer_offset_inbounds( &self, - ptr: Scalar, + ptr: Scalar, pointee_ty: Ty<'tcx>, offset: i64, - ) -> EvalResult<'tcx, Scalar>; + ) -> EvalResult<'tcx, Scalar>; } impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super::Evaluator<'tcx>> { fn ptr_op( &self, bin_op: mir::BinOp, - left: Scalar, + left: Scalar, left_layout: TyLayout<'tcx>, - right: Scalar, + right: Scalar, right_layout: TyLayout<'tcx>, - ) -> EvalResult<'tcx, (Scalar, bool)> { + ) -> EvalResult<'tcx, (Scalar, bool)> { use rustc::mir::BinOp::*; trace!("ptr_op: {:?} {:?} {:?}", left, bin_op, right); @@ -124,8 +124,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: fn ptr_eq( &self, - left: Scalar, - right: Scalar, + left: Scalar, + right: Scalar, size: Size, ) -> EvalResult<'tcx, bool> { Ok(match (left, right) { @@ -203,13 +203,13 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: fn ptr_int_arithmetic( &self, bin_op: mir::BinOp, - left: Pointer, + left: Pointer, right: u128, signed: bool, - ) -> EvalResult<'tcx, (Scalar, bool)> { + ) -> EvalResult<'tcx, (Scalar, bool)> { use rustc::mir::BinOp::*; - fn map_to_primval((res, over): (Pointer, bool)) -> (Scalar, bool) { + fn map_to_primval((res, over): (Pointer, bool)) -> (Scalar, bool) { (Scalar::Ptr(res), over) } @@ -237,7 +237,14 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: if right & base_mask == base_mask { // Case 1: The base address bits are all preserved, i.e., right is all-1 there let offset = (left.offset.bytes() as u128 & right) as u64; - (Scalar::Ptr(Pointer::new(left.alloc_id, Size::from_bytes(offset))), false) + ( + Scalar::Ptr(Pointer::new_with_tag( + left.alloc_id, + Size::from_bytes(offset), + left.tag, + )), + false, + ) } else if right & base_mask == 0 { // Case 2: The base address bits are all taken away, i.e., right is all-0 there (Scalar::Bits { bits: (left.offset.bytes() as u128) & right, size: ptr_size }, false) @@ -277,10 +284,10 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: /// allocation. fn pointer_offset_inbounds( &self, - ptr: Scalar, + ptr: Scalar, pointee_ty: Ty<'tcx>, offset: i64, - ) -> EvalResult<'tcx, Scalar> { + ) -> EvalResult<'tcx, Scalar> { // FIXME: assuming here that type size is < i64::max_value() let pointee_size = self.layout_of(pointee_ty)?.size.bytes() as i64; let offset = offset.checked_mul(pointee_size).ok_or_else(|| EvalErrorKind::Overflow(mir::BinOp::Mul))?; diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs new file mode 100644 index 0000000000..dd9f1b3708 --- /dev/null +++ b/src/stacked_borrows.rs @@ -0,0 +1,36 @@ +use super::RangeMap; + +pub type Timestamp = u64; + +/// Information about a potentially mutable borrow +#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] +pub enum Mut { + /// A unique, mutable reference + Uniq(Timestamp), + /// Any raw pointer, or a shared borrow with interior mutability + Raw, +} + +/// Information about any kind of borrow +#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] +pub enum Borrow { + /// A mutable borrow, a raw pointer, or a shared borrow with interior mutability + Mut(Mut), + /// A shared borrow without interior mutability + Frz(Timestamp) +} + +/// An item in the borrow stack +#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] +pub enum BorStackItem { + /// Defines which references are permitted to mutate *if* the location is not frozen + Mut(Mut), + /// A barrier, tracking the function it belongs to by its index on the call stack + FnBarrier(usize) +} + +impl Default for Borrow { + fn default() -> Self { + Borrow::Mut(Mut::Raw) + } +} diff --git a/src/tls.rs b/src/tls.rs index c04f7a9c35..c5d119ec7f 100644 --- a/src/tls.rs +++ b/src/tls.rs @@ -5,14 +5,14 @@ use rustc::{ty, ty::layout::HasDataLayout, mir}; use super::{ EvalResult, EvalErrorKind, StackPopCleanup, EvalContext, Evaluator, - MPlaceTy, Scalar, + MPlaceTy, Scalar, Borrow, }; pub type TlsKey = u128; #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub struct TlsEntry<'tcx> { - pub(crate) data: Scalar, // Will eventually become a map from thread IDs to `Scalar`s, if we ever support more than one thread. + pub(crate) data: Scalar, // Will eventually become a map from thread IDs to `Scalar`s, if we ever support more than one thread. pub(crate) dtor: Option>, } @@ -67,7 +67,7 @@ impl<'tcx> TlsData<'tcx> { } } - pub fn load_tls(&mut self, key: TlsKey) -> EvalResult<'tcx, Scalar> { + pub fn load_tls(&mut self, key: TlsKey) -> EvalResult<'tcx, Scalar> { match self.keys.get(&key) { Some(&TlsEntry { data, .. }) => { trace!("TLS key {} loaded: {:?}", key, data); @@ -77,7 +77,7 @@ impl<'tcx> TlsData<'tcx> { } } - pub fn store_tls(&mut self, key: TlsKey, new_data: Scalar) -> EvalResult<'tcx> { + pub fn store_tls(&mut self, key: TlsKey, new_data: Scalar) -> EvalResult<'tcx> { match self.keys.get_mut(&key) { Some(&mut TlsEntry { ref mut data, .. }) => { trace!("TLS key {} stored: {:?}", key, new_data); @@ -110,7 +110,7 @@ impl<'tcx> TlsData<'tcx> { &mut self, key: Option, cx: impl HasDataLayout, - ) -> Option<(ty::Instance<'tcx>, Scalar, TlsKey)> { + ) -> Option<(ty::Instance<'tcx>, Scalar, TlsKey)> { use std::collections::Bound::*; let thread_local = &mut self.keys; From 348f782085ea60cbda5a1bd0beb8cd62b0ebd142 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 16 Oct 2018 12:44:04 +0200 Subject: [PATCH 02/12] add env var emulation test, and fix it complaining about leaks --- src/lib.rs | 11 +++++++++++ tests/run-pass/env.rs | 7 +++++++ 2 files changed, 18 insertions(+) create mode 100644 tests/run-pass/env.rs diff --git a/src/lib.rs b/src/lib.rs index 9ac703e267..47eaee6dfa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -215,11 +215,22 @@ pub enum MiriMemoryKind { } impl Into> for MiriMemoryKind { + #[inline(always)] fn into(self) -> MemoryKind { MemoryKind::Machine(self) } } +impl MayLeak for MiriMemoryKind { + #[inline(always)] + fn may_leak(self) -> bool { + use MiriMemoryKind::*; + match self { + Rust | C => false, + Env | MutStatic => true, + } + } +} #[derive(Clone, PartialEq, Eq)] pub struct Evaluator<'tcx> { diff --git a/tests/run-pass/env.rs b/tests/run-pass/env.rs new file mode 100644 index 0000000000..c0bf883daf --- /dev/null +++ b/tests/run-pass/env.rs @@ -0,0 +1,7 @@ +use std::env; + +fn main() { + assert_eq!(env::var("MIRI_TEST"), Err(env::VarError::NotPresent)); + env::set_var("MIRI_TEST", "the answer"); + assert_eq!(env::var("MIRI_TEST"), Ok("the answer".to_owned())); +} From 66b4bb7cf2e7136ca0a31d04a2abe1754e89a5b6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 16 Oct 2018 18:01:50 +0200 Subject: [PATCH 03/12] stacked borrows: track refs and derefs --- src/fn_call.rs | 2 +- src/intrinsic.rs | 5 +- src/lib.rs | 49 ++++++++- src/operator.rs | 2 +- src/range_map.rs | 150 +++++++++++++++++--------- src/stacked_borrows.rs | 238 ++++++++++++++++++++++++++++++++++++++++- src/tls.rs | 8 +- 7 files changed, 388 insertions(+), 66 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index 280cc2bdea..1613da4a48 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -44,7 +44,7 @@ pub trait EvalContextExt<'tcx, 'mir> { fn write_null(&mut self, dest: PlaceTy<'tcx, Borrow>) -> EvalResult<'tcx>; } -impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for EvalContext<'a, 'mir, 'tcx, super::Evaluator<'tcx>> { +impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalContext<'a, 'mir, 'tcx> { fn find_fn( &mut self, instance: ty::Instance<'tcx>, diff --git a/src/intrinsic.rs b/src/intrinsic.rs index 11c0bd7907..719bc93939 100644 --- a/src/intrinsic.rs +++ b/src/intrinsic.rs @@ -3,10 +3,9 @@ use rustc::ty::layout::{self, LayoutOf, Size}; use rustc::ty; use rustc::mir::interpret::{EvalResult, PointerArithmetic}; -use rustc_mir::interpret::{EvalContext, PlaceTy, OpTy}; use super::{ - Value, Scalar, ScalarMaybeUndef, Borrow, + PlaceTy, OpTy, Value, Scalar, ScalarMaybeUndef, Borrow, FalibleScalarExt, OperatorEvalContextExt }; @@ -19,7 +18,7 @@ pub trait EvalContextExt<'tcx> { ) -> EvalResult<'tcx>; } -impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super::Evaluator<'tcx>> { +impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, 'tcx> { fn call_intrinsic( &mut self, instance: ty::Instance<'tcx>, diff --git a/src/lib.rs b/src/lib.rs index 47eaee6dfa..97b73e6069 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,7 +16,7 @@ extern crate syntax; use std::collections::HashMap; use std::borrow::Cow; -use rustc::ty::{self, TyCtxt, query::TyCtxtAt}; +use rustc::ty::{self, Ty, TyCtxt, query::TyCtxtAt}; use rustc::ty::layout::{TyLayout, LayoutOf, Size}; use rustc::hir::def_id::DefId; use rustc::mir; @@ -43,7 +43,7 @@ use tls::{EvalContextExt as TlsEvalContextExt, TlsData}; use range_map::RangeMap; use helpers::FalibleScalarExt; use mono_hash_map::MonoHashMap; -use stacked_borrows::Borrow; +use stacked_borrows::{EvalContextExt as StackedBorEvalContextExt, Borrow}; pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>( tcx: TyCtxt<'a, 'tcx, 'tcx>, @@ -232,7 +232,6 @@ impl MayLeak for MiriMemoryKind { } } -#[derive(Clone, PartialEq, Eq)] pub struct Evaluator<'tcx> { /// Environment variables set by `setenv` /// Miri does not expose env vars from the host to the emulated program @@ -243,6 +242,9 @@ pub struct Evaluator<'tcx> { /// Whether to enforce the validity invariant pub(crate) validate: bool, + + /// Stacked Borrows state + pub(crate) stacked_borrows: stacked_borrows::State, } impl<'tcx> Evaluator<'tcx> { @@ -251,13 +253,18 @@ impl<'tcx> Evaluator<'tcx> { env_vars: HashMap::default(), tls: TlsData::default(), validate, + stacked_borrows: stacked_borrows::State::new(), } } } +#[allow(dead_code)] // FIXME https://github.com/rust-lang/rust/issues/47131 +type MiriEvalContext<'a, 'mir, 'tcx> = EvalContext<'a, 'mir, 'tcx, Evaluator<'tcx>>; + + impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { type MemoryKinds = MiriMemoryKind; - type AllocExtra = (); + type AllocExtra = stacked_borrows::Stacks; type PointerTag = Borrow; type MemoryMap = MonoHashMap, Allocation)>; @@ -288,6 +295,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { } /// Returns Ok() when the function was handled, fail otherwise + #[inline(always)] fn find_fn( ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, @@ -298,6 +306,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { ecx.find_fn(instance, args, dest, ret) } + #[inline(always)] fn call_intrinsic( ecx: &mut rustc_mir::interpret::EvalContext<'a, 'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, @@ -307,6 +316,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { ecx.call_intrinsic(instance, args, dest) } + #[inline(always)] fn ptr_op( ecx: &rustc_mir::interpret::EvalContext<'a, 'mir, 'tcx, Self>, bin_op: mir::BinOp, @@ -380,6 +390,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { Ok(Cow::Owned(alloc)) } + #[inline(always)] fn before_terminator(_ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>) -> EvalResult<'tcx> { // We are not interested in detecting loops @@ -403,4 +414,34 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { }; Cow::Owned(alloc) } + + #[inline(always)] + fn tag_reference( + ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, + ptr: Pointer, + pointee_ty: Ty<'tcx>, + pointee_size: Size, + borrow_kind: mir::BorrowKind, + ) -> EvalResult<'tcx, Self::PointerTag> { + if !ecx.machine.validate { + // No tracking + Ok(Borrow::default()) + } else { + ecx.tag_reference(ptr, pointee_ty, pointee_size, borrow_kind) + } + } + + #[inline(always)] + fn tag_dereference( + ecx: &EvalContext<'a, 'mir, 'tcx, Self>, + ptr: Pointer, + ptr_ty: Ty<'tcx>, + ) -> EvalResult<'tcx, Self::PointerTag> { + if !ecx.machine.validate { + // No tracking + Ok(Borrow::default()) + } else { + ecx.tag_dereference(ptr, ptr_ty) + } + } } diff --git a/src/operator.rs b/src/operator.rs index dd79f29313..a11f8f34e7 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -36,7 +36,7 @@ pub trait EvalContextExt<'tcx> { ) -> EvalResult<'tcx, Scalar>; } -impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super::Evaluator<'tcx>> { +impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, 'tcx> { fn ptr_op( &self, bin_op: mir::BinOp, diff --git a/src/range_map.rs b/src/range_map.rs index e55534e36f..1d9a38cb5e 100644 --- a/src/range_map.rs +++ b/src/range_map.rs @@ -9,11 +9,20 @@ use std::collections::BTreeMap; use std::ops; +use rustc::ty::layout::Size; + #[derive(Clone, Debug, PartialEq, Eq)] pub struct RangeMap { map: BTreeMap, } +impl Default for RangeMap { + #[inline(always)] + fn default() -> Self { + RangeMap::new() + } +} + // The derived `Ord` impl sorts first by the first field, then, if the fields are the same, // by the second field. // This is exactly what we need for our purposes, since a range query on a BTReeSet/BTreeMap will give us all @@ -21,14 +30,19 @@ pub struct RangeMap { // At the same time the `end` is irrelevant for the sorting and range searching, but used for the check. // This kind of search breaks, if `end < start`, so don't do that! #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Debug)] -pub struct Range { +struct Range { start: u64, end: u64, // Invariant: end > start } impl Range { + /// Compute a range of ranges that contains all ranges overlaping with [offset, offset+len) fn range(offset: u64, len: u64) -> ops::Range { - assert!(len > 0); + if len == 0 { + // We can produce an empty range, nothing overlaps with this. + let r = Range { start: 0, end: 1 }; + return r..r; + } // We select all elements that are within // the range given by the offset into the allocation and the length. // This is sound if all ranges that intersect with the argument range, are in the @@ -46,14 +60,20 @@ impl Range { left..right } - /// Tests if all of [offset, offset+len) are contained in this range. + /// Tests if any element of [offset, offset+len) is contained in this range. + #[inline(always)] fn overlaps(&self, offset: u64, len: u64) -> bool { - assert!(len > 0); - offset < self.end && offset + len >= self.start + if len == 0 { + // `offset` totally does not matter, we cannot overlap with an empty interval + false + } else { + offset < self.end && offset.checked_add(len).unwrap() >= self.start + } } } impl RangeMap { + #[inline(always)] pub fn new() -> RangeMap { RangeMap { map: BTreeMap::new() } } @@ -63,10 +83,9 @@ impl RangeMap { offset: u64, len: u64, ) -> impl Iterator + 'a { - assert!(len > 0); self.map.range(Range::range(offset, len)).filter_map( - move |(range, - data)| { + move |(range, data)| { + debug_assert!(len > 0); if range.overlaps(offset, len) { Some((range, data)) } else { @@ -76,8 +95,8 @@ impl RangeMap { ) } - pub fn iter<'a>(&'a self, offset: u64, len: u64) -> impl Iterator + 'a { - self.iter_with_range(offset, len).map(|(_, data)| data) + pub fn iter<'a>(&'a self, offset: Size, len: Size) -> impl Iterator + 'a { + self.iter_with_range(offset.bytes(), len.bytes()).map(|(_, data)| data) } fn split_entry_at(&mut self, offset: u64) @@ -114,28 +133,30 @@ impl RangeMap { } } - pub fn iter_mut_all<'a>(&'a mut self) -> impl Iterator + 'a { - self.map.values_mut() - } - /// Provide mutable iteration over everything in the given range. As a side-effect, /// this will split entries in the map that are only partially hit by the given range, /// to make sure that when they are mutated, the effect is constrained to the given range. + /// If there are gaps, leave them be. pub fn iter_mut_with_gaps<'a>( &'a mut self, - offset: u64, - len: u64, + offset: Size, + len: Size, ) -> impl Iterator + 'a where T: Clone, { - assert!(len > 0); - // Preparation: Split first and last entry as needed. - self.split_entry_at(offset); - self.split_entry_at(offset + len); + let offset = offset.bytes(); + let len = len.bytes(); + + if len > 0 { + // Preparation: Split first and last entry as needed. + self.split_entry_at(offset); + self.split_entry_at(offset + len); + } // Now we can provide a mutable iterator self.map.range_mut(Range::range(offset, len)).filter_map( move |(&range, data)| { + debug_assert!(len > 0); if range.overlaps(offset, len) { assert!( offset <= range.start && offset + len >= range.end, @@ -151,35 +172,41 @@ impl RangeMap { } /// Provide a mutable iterator over everything in the given range, with the same side-effects as - /// iter_mut_with_gaps. Furthermore, if there are gaps between ranges, fill them with the given default. + /// iter_mut_with_gaps. Furthermore, if there are gaps between ranges, fill them with the given default + /// before yielding them in the iterator. /// This is also how you insert. - pub fn iter_mut<'a>(&'a mut self, offset: u64, len: u64) -> impl Iterator + 'a + pub fn iter_mut<'a>(&'a mut self, offset: Size, len: Size) -> impl Iterator + 'a where T: Clone + Default, { - // Do a first iteration to collect the gaps - let mut gaps = Vec::new(); - let mut last_end = offset; - for (range, _) in self.iter_with_range(offset, len) { - if last_end < range.start { + if len.bytes() > 0 { + let offset = offset.bytes(); + let len = len.bytes(); + + // Do a first iteration to collect the gaps + let mut gaps = Vec::new(); + let mut last_end = offset; + for (range, _) in self.iter_with_range(offset, len) { + if last_end < range.start { + gaps.push(Range { + start: last_end, + end: range.start, + }); + } + last_end = range.end; + } + if last_end < offset + len { gaps.push(Range { start: last_end, - end: range.start, + end: offset + len, }); } - last_end = range.end; - } - if last_end < offset + len { - gaps.push(Range { - start: last_end, - end: offset + len, - }); - } - // Add default for all gaps - for gap in gaps { - let old = self.map.insert(gap, Default::default()); - assert!(old.is_none()); + // Add default for all gaps + for gap in gaps { + let old = self.map.insert(gap, Default::default()); + assert!(old.is_none()); + } } // Now provide mutable iteration @@ -208,10 +235,16 @@ mod tests { use super::*; /// Query the map at every offset in the range and collect the results. - fn to_vec(map: &RangeMap, offset: u64, len: u64) -> Vec { + fn to_vec(map: &RangeMap, offset: u64, len: u64, default: Option) -> Vec { (offset..offset + len) .into_iter() - .map(|i| *map.iter(i, 1).next().unwrap()) + .map(|i| map + .iter(Size::from_bytes(i), Size::from_bytes(1)) + .next() + .map(|&t| t) + .or(default) + .unwrap() + ) .collect() } @@ -219,34 +252,47 @@ mod tests { fn basic_insert() { let mut map = RangeMap::::new(); // Insert - for x in map.iter_mut(10, 1) { + for x in map.iter_mut(Size::from_bytes(10), Size::from_bytes(1)) { *x = 42; } // Check - assert_eq!(to_vec(&map, 10, 1), vec![42]); + assert_eq!(to_vec(&map, 10, 1, None), vec![42]); + + // Insert with size 0 + for x in map.iter_mut(Size::from_bytes(10), Size::from_bytes(0)) { + *x = 19; + } + for x in map.iter_mut(Size::from_bytes(11), Size::from_bytes(0)) { + *x = 19; + } + assert_eq!(to_vec(&map, 10, 2, Some(-1)), vec![42, -1]); } #[test] fn gaps() { let mut map = RangeMap::::new(); - for x in map.iter_mut(11, 1) { + for x in map.iter_mut(Size::from_bytes(11), Size::from_bytes(1)) { *x = 42; } - for x in map.iter_mut(15, 1) { - *x = 42; + for x in map.iter_mut(Size::from_bytes(15), Size::from_bytes(1)) { + *x = 43; } + assert_eq!( + to_vec(&map, 10, 10, Some(-1)), + vec![-1, 42, -1, -1, -1, 43, -1, -1, -1, -1] + ); // Now request a range that needs three gaps filled - for x in map.iter_mut(10, 10) { - if *x != 42 { + for x in map.iter_mut(Size::from_bytes(10), Size::from_bytes(10)) { + if *x < 42 { *x = 23; } } assert_eq!( - to_vec(&map, 10, 10), - vec![23, 42, 23, 23, 23, 42, 23, 23, 23, 23] + to_vec(&map, 10, 10, None), + vec![23, 42, 23, 23, 23, 43, 23, 23, 23, 23] ); - assert_eq!(to_vec(&map, 13, 5), vec![23, 23, 42, 23, 23]); + assert_eq!(to_vec(&map, 13, 5, None), vec![23, 23, 43, 23, 23]); } } diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index dd9f1b3708..c3514efdbe 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -1,4 +1,12 @@ -use super::RangeMap; +use std::cell::RefCell; + +use rustc::ty::{Ty, layout::Size}; +use rustc::mir; + +use super::{ + RangeMap, EvalResult, + Pointer, +}; pub type Timestamp = u64; @@ -11,6 +19,24 @@ pub enum Mut { Raw, } +impl Mut { + #[inline(always)] + fn is_raw(self) -> bool { + match self { + Mut::Raw => true, + _ => false, + } + } + + #[inline(always)] + fn is_uniq(self) -> bool { + match self { + Mut::Uniq(_) => true, + _ => false, + } + } +} + /// Information about any kind of borrow #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] pub enum Borrow { @@ -20,6 +46,24 @@ pub enum Borrow { Frz(Timestamp) } +impl Borrow { + #[inline(always)] + fn is_mut(self) -> bool { + match self { + Borrow::Mut(_) => true, + _ => false, + } + } + + #[inline(always)] + fn is_uniq(self) -> bool { + match self { + Borrow::Mut(Mut::Uniq(_)) => true, + _ => false, + } + } +} + /// An item in the borrow stack #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] pub enum BorStackItem { @@ -34,3 +78,195 @@ impl Default for Borrow { Borrow::Mut(Mut::Raw) } } + +/// Extra global machine state +#[derive(Clone, Debug)] +pub struct State { + clock: Timestamp +} + +impl State { + pub fn new() -> State { + State { clock: 0 } + } +} + +/// Extra per-location state +#[derive(Clone, Debug)] +struct Stack { + borrows: Vec, // used as a stack + frozen_since: Option, +} + +impl Default for Stack { + fn default() -> Self { + Stack { + borrows: Vec::new(), + frozen_since: None, + } + } +} + +/// Extra per-allocation state +#[derive(Clone, Debug, Default)] +pub struct Stacks { + stacks: RefCell>, +} + +/// Core operations +impl<'tcx> Stack { + fn check(&self, bor: Borrow) -> bool { + match bor { + Borrow::Frz(acc_t) => + // Must be frozen at least as long as the `acc_t` says. + self.frozen_since.map_or(false, |loc_t| loc_t <= acc_t), + Borrow::Mut(acc_m) => + // Raw pointers are fine with frozen locations. This is important because &Cell is raw! + if self.frozen_since.is_some() { + acc_m.is_raw() + } else { + self.borrows.last().map_or(false, |&loc_itm| loc_itm == BorStackItem::Mut(acc_m)) + } + } + } + + /// Reactive `bor` for this stack. If `force_mut` is set, we want to aggressively + /// unfreeze this location (because we are about to push a `Uniq`). + fn reactivate(&mut self, bor: Borrow, force_mut: bool) -> EvalResult<'tcx> { + assert!(!force_mut || bor.is_mut()); // if `force_mut` is set, this must be a mutable borrow + // Do NOT change anything if `bor` is already active -- in particular, if + // it is a `Mut(Raw)` and we are frozen. + if !force_mut && self.check(bor) { + return Ok(()); + } + + let acc_m = match bor { + Borrow::Frz(_) => return err!(MachineError(format!("Location should be frozen but it is not"))), + Borrow::Mut(acc_m) => acc_m, + }; + // We definitely have to unfreeze this, even if we use the topmost item. + self.frozen_since = None; + // Pop until we see the one we are looking for. + while let Some(&itm) = self.borrows.last() { + match itm { + BorStackItem::FnBarrier(_) => { + return err!(MachineError(format!("Trying to reactivate a borrow that lives behind a barrier"))); + } + BorStackItem::Mut(loc_m) => { + if loc_m == acc_m { return Ok(()); } + self.borrows.pop(); + } + } + } + // Nothing to be found. Simulate a "virtual raw" element at the bottom of the stack. + if acc_m.is_raw() { + Ok(()) + } else { + err!(MachineError(format!("Borrow-to-reactivate does not exist on the stack"))) + } + } + + fn initiate(&mut self, bor: Borrow) -> EvalResult<'tcx> { + match bor { + Borrow::Frz(t) => { + match self.frozen_since { + None => self.frozen_since = Some(t), + Some(since) => assert!(since <= t), + } + } + Borrow::Mut(m) => { + match self.frozen_since { + None => self.borrows.push(BorStackItem::Mut(m)), + Some(_) => + // FIXME: Do we want an exception for raw borrows? + return err!(MachineError(format!("Trying to mutate frozen location"))) + } + } + } + Ok(()) + } +} + +impl State { + fn increment_clock(&mut self) -> Timestamp { + self.clock += 1; + self.clock + } +} + +/// Machine hooks +pub trait EvalContextExt<'tcx> { + fn tag_reference( + &mut self, + ptr: Pointer, + pointee_ty: Ty<'tcx>, + size: Size, + borrow_kind: mir::BorrowKind, + ) -> EvalResult<'tcx, Borrow>; + + fn tag_dereference( + &self, + ptr: Pointer, + ptr_ty: Ty<'tcx>, + ) -> EvalResult<'tcx, Borrow>; +} + +impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, 'tcx> { + fn tag_reference( + &mut self, + ptr: Pointer, + pointee_ty: Ty<'tcx>, + size: Size, + borrow_kind: mir::BorrowKind, + ) -> EvalResult<'tcx, Borrow> { + let old_bor = ptr.tag; + let time = self.machine.stacked_borrows.increment_clock(); + // FIXME This does not do enough checking when only part of the data lacks + // interior mutability. + let new_bor = match borrow_kind { + mir::BorrowKind::Mut { .. } => Borrow::Mut(Mut::Uniq(time)), + _ => + if self.type_is_freeze(pointee_ty) { + Borrow::Frz(time) + } else { + Borrow::Mut(Mut::Raw) + } + }; + trace!("tag_reference: Creating new tag for {:?} (pointee {}, size {}): {:?}", ptr, pointee_ty, size.bytes(), new_bor); + + // Make sure this reference is not dangling or so + self.memory.check_bounds(ptr, size, false)?; + + // Update the stacks. We cannot use `get_mut` becuse this might be immutable + // memory. + let alloc = self.memory.get(ptr.alloc_id).expect("We checked that the ptr is fine!"); + let mut stacks = alloc.extra.stacks.borrow_mut(); + for stack in stacks.iter_mut(ptr.offset, size) { + if stack.check(new_bor) { + // The new borrow is already active! This can happen when creating multiple + // shared references from the same mutable reference. Do nothing. + } else { + // FIXME: The blog post says we should `reset` if this is a local. + stack.reactivate(old_bor, /*force_mut*/new_bor.is_uniq())?; + stack.initiate(new_bor)?; + } + } + + Ok(new_bor) + } + + fn tag_dereference( + &self, + ptr: Pointer, + ptr_ty: Ty<'tcx>, + ) -> EvalResult<'tcx, Borrow> { + // If this is a raw ptr, forget about the tag. + Ok(if ptr_ty.is_unsafe_ptr() { + trace!("tag_dereference: Erasing tag for {:?} ({})", ptr, ptr_ty); + Borrow::Mut(Mut::Raw) + } else { + // FIXME: Do we want to adjust the tag if it does not match the type? + ptr.tag + }) + } +} diff --git a/src/tls.rs b/src/tls.rs index c5d119ec7f..2bddc43df8 100644 --- a/src/tls.rs +++ b/src/tls.rs @@ -4,19 +4,19 @@ use rustc_target::abi::LayoutOf; use rustc::{ty, ty::layout::HasDataLayout, mir}; use super::{ - EvalResult, EvalErrorKind, StackPopCleanup, EvalContext, Evaluator, + EvalResult, EvalErrorKind, StackPopCleanup, MPlaceTy, Scalar, Borrow, }; pub type TlsKey = u128; -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug)] pub struct TlsEntry<'tcx> { pub(crate) data: Scalar, // Will eventually become a map from thread IDs to `Scalar`s, if we ever support more than one thread. pub(crate) dtor: Option>, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug)] pub struct TlsData<'tcx> { /// The Key to use for the next thread-local allocation. pub(crate) next_key: TlsKey, @@ -133,7 +133,7 @@ impl<'tcx> TlsData<'tcx> { } } -impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, Evaluator<'tcx>> { +impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, 'tcx> { fn run_tls_dtors(&mut self) -> EvalResult<'tcx> { let mut dtor = self.machine.tls.fetch_tls_dtor(None, *self.tcx); // FIXME: replace loop by some structure that works with stepping From d4b78b36abe896556ba04785610148b51aa67572 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 17 Oct 2018 15:15:53 +0200 Subject: [PATCH 04/12] stacked borrows: enforcement at memory accesses --- src/fn_call.rs | 67 +++++++++------- src/intrinsic.rs | 15 ++-- src/lib.rs | 44 ++++++++-- src/range_map.rs | 4 + src/stacked_borrows.rs | 165 ++++++++++++++++++++++++++------------ tests/run-pass/raw.rs | 21 +++++ tests/run-pass/refcell.rs | 33 ++++++++ tests/run-pass/std.rs | 5 +- 8 files changed, 259 insertions(+), 95 deletions(-) create mode 100644 tests/run-pass/raw.rs create mode 100644 tests/run-pass/refcell.rs diff --git a/src/fn_call.rs b/src/fn_call.rs index 1613da4a48..eb889c1d49 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -118,6 +118,10 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo None => self.tcx.item_name(def_id).as_str(), }; + // All these functions take raw pointers, so if we access memory directly + // (as opposed to through a place), we have to remember to erase any tag + // that might still hang around! + match &link_name[..] { "malloc" => { let size = self.read_scalar(args[0])?.to_usize(&self)?; @@ -131,10 +135,10 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo } "free" => { - let ptr = self.read_scalar(args[0])?.not_undef()?; + let ptr = self.read_scalar(args[0])?.not_undef()?.erase_tag(); // raw ptr operation, no tag if !ptr.is_null() { self.memory.deallocate( - ptr.to_ptr()?, + ptr.to_ptr()?.with_default_tag(), None, MiriMemoryKind::C.into(), )?; @@ -171,7 +175,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo self.write_scalar(Scalar::Ptr(ptr), dest)?; } "__rust_dealloc" => { - let ptr = self.read_scalar(args[0])?.to_ptr()?; + let ptr = self.read_scalar(args[0])?.to_ptr()?.erase_tag(); // raw ptr operation, no tag let old_size = self.read_scalar(args[1])?.to_usize(&self)?; let align = self.read_scalar(args[2])?.to_usize(&self)?; if old_size == 0 { @@ -181,13 +185,13 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo return err!(HeapAllocNonPowerOfTwoAlignment(align)); } self.memory.deallocate( - ptr, + ptr.with_default_tag(), Some((Size::from_bytes(old_size), Align::from_bytes(align, align).unwrap())), MiriMemoryKind::Rust.into(), )?; } "__rust_realloc" => { - let ptr = self.read_scalar(args[0])?.to_ptr()?; + let ptr = self.read_scalar(args[0])?.to_ptr()?.erase_tag(); // raw ptr operation, no tag let old_size = self.read_scalar(args[1])?.to_usize(&self)?; let align = self.read_scalar(args[2])?.to_usize(&self)?; let new_size = self.read_scalar(args[3])?.to_usize(&self)?; @@ -198,7 +202,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo return err!(HeapAllocNonPowerOfTwoAlignment(align)); } let new_ptr = self.memory.reallocate( - ptr, + ptr.with_default_tag(), Size::from_bytes(old_size), Align::from_bytes(align, align).unwrap(), Size::from_bytes(new_size), @@ -230,8 +234,8 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo "dlsym" => { let _handle = self.read_scalar(args[0])?; - let symbol = self.read_scalar(args[1])?.to_ptr()?; - let symbol_name = self.memory.read_c_str(symbol)?; + let symbol = self.read_scalar(args[1])?.to_ptr()?.erase_tag(); + let symbol_name = self.memory.read_c_str(symbol.with_default_tag())?; let err = format!("bad c unicode symbol: {:?}", symbol_name); let symbol_name = ::std::str::from_utf8(symbol_name).unwrap_or(&err); return err!(Unimplemented(format!( @@ -284,13 +288,13 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo return err!(MachineError("the evaluated program panicked".to_string())), "memcmp" => { - let left = self.read_scalar(args[0])?.not_undef()?; - let right = self.read_scalar(args[1])?.not_undef()?; + let left = self.read_scalar(args[0])?.not_undef()?.erase_tag(); // raw ptr operation + let right = self.read_scalar(args[1])?.not_undef()?.erase_tag(); // raw ptr operation let n = Size::from_bytes(self.read_scalar(args[2])?.to_usize(&self)?); let result = { - let left_bytes = self.memory.read_bytes(left, n)?; - let right_bytes = self.memory.read_bytes(right, n)?; + let left_bytes = self.memory.read_bytes(left.with_default_tag(), n)?; + let right_bytes = self.memory.read_bytes(right.with_default_tag(), n)?; use std::cmp::Ordering::*; match left_bytes.cmp(right_bytes) { @@ -307,12 +311,12 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo } "memrchr" => { - let ptr = self.read_scalar(args[0])?.not_undef()?; + let ptr = self.read_scalar(args[0])?.not_undef()?.erase_tag(); // raw ptr operation + let ptr = ptr.with_default_tag(); let val = self.read_scalar(args[1])?.to_bytes()? as u8; let num = self.read_scalar(args[2])?.to_usize(&self)?; - if let Some(idx) = self.memory.read_bytes(ptr, Size::from_bytes(num))?.iter().rev().position( - |&c| c == val, - ) + if let Some(idx) = self.memory.read_bytes(ptr, Size::from_bytes(num))? + .iter().rev().position(|&c| c == val) { let new_ptr = ptr.ptr_offset(Size::from_bytes(num - idx as u64 - 1), &self)?; self.write_scalar(new_ptr, dest)?; @@ -322,7 +326,8 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo } "memchr" => { - let ptr = self.read_scalar(args[0])?.not_undef()?; + let ptr = self.read_scalar(args[0])?.not_undef()?.erase_tag(); // raw ptr operation + let ptr = ptr.with_default_tag(); let val = self.read_scalar(args[1])?.to_bytes()? as u8; let num = self.read_scalar(args[2])?.to_usize(&self)?; if let Some(idx) = self.memory.read_bytes(ptr, Size::from_bytes(num))?.iter().position( @@ -338,8 +343,8 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo "getenv" => { let result = { - let name_ptr = self.read_scalar(args[0])?.to_ptr()?; - let name = self.memory.read_c_str(name_ptr)?; + let name_ptr = self.read_scalar(args[0])?.to_ptr()?.erase_tag(); // raw ptr operation + let name = self.memory.read_c_str(name_ptr.with_default_tag())?; match self.machine.env_vars.get(name) { Some(&var) => Scalar::Ptr(var), None => Scalar::ptr_null(*self.tcx), @@ -351,9 +356,9 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo "unsetenv" => { let mut success = None; { - let name_ptr = self.read_scalar(args[0])?.not_undef()?; + let name_ptr = self.read_scalar(args[0])?.not_undef()?.erase_tag(); // raw ptr operation if !name_ptr.is_null() { - let name = self.memory.read_c_str(name_ptr.to_ptr()?)?; + let name = self.memory.read_c_str(name_ptr.to_ptr()?.with_default_tag())?; if !name.is_empty() && !name.contains(&b'=') { success = Some(self.machine.env_vars.remove(name)); } @@ -372,11 +377,11 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo "setenv" => { let mut new = None; { - let name_ptr = self.read_scalar(args[0])?.not_undef()?; - let value_ptr = self.read_scalar(args[1])?.to_ptr()?; - let value = self.memory.read_c_str(value_ptr)?; + let name_ptr = self.read_scalar(args[0])?.not_undef()?.erase_tag(); // raw ptr operation + let value_ptr = self.read_scalar(args[1])?.to_ptr()?.erase_tag(); // raw ptr operation + let value = self.memory.read_c_str(value_ptr.with_default_tag())?; if !name_ptr.is_null() { - let name = self.memory.read_c_str(name_ptr.to_ptr()?)?; + let name = self.memory.read_c_str(name_ptr.to_ptr()?.with_default_tag())?; if !name.is_empty() && !name.contains(&b'=') { new = Some((name.to_owned(), value.to_owned())); } @@ -407,14 +412,14 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo "write" => { let fd = self.read_scalar(args[0])?.to_bytes()?; - let buf = self.read_scalar(args[1])?.not_undef()?; + let buf = self.read_scalar(args[1])?.not_undef()?.erase_tag(); let n = self.read_scalar(args[2])?.to_bytes()? as u64; trace!("Called write({:?}, {:?}, {:?})", fd, buf, n); let result = if fd == 1 || fd == 2 { // stdout/stderr use std::io::{self, Write}; - let buf_cont = self.memory.read_bytes(buf, Size::from_bytes(n))?; + let buf_cont = self.memory.read_bytes(buf.with_default_tag(), Size::from_bytes(n))?; let res = if fd == 1 { io::stdout().write(buf_cont) } else { @@ -435,8 +440,8 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo } "strlen" => { - let ptr = self.read_scalar(args[0])?.to_ptr()?; - let n = self.memory.read_c_str(ptr)?.len(); + let ptr = self.read_scalar(args[0])?.to_ptr()?.erase_tag(); + let n = self.memory.read_c_str(ptr.with_default_tag())?.len(); self.write_scalar(Scalar::from_uint(n as u64, dest.layout.size), dest)?; } @@ -482,7 +487,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo // Hook pthread calls that go to the thread-local storage memory subsystem "pthread_key_create" => { - let key_ptr = self.read_scalar(args[0])?.to_ptr()?; + let key_ptr = self.read_scalar(args[0])?.to_ptr()?.erase_tag(); // raw ptr operation // Extract the function type out of the signature (that seems easier than constructing it ourselves...) let dtor = match self.read_scalar(args[1])?.not_undef()? { @@ -505,7 +510,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo return err!(OutOfTls); } self.memory.write_scalar( - key_ptr, + key_ptr.with_default_tag(), key_layout.align, Scalar::from_uint(key, key_layout.size).into(), key_layout.size, diff --git a/src/intrinsic.rs b/src/intrinsic.rs index 719bc93939..2b02f22a38 100644 --- a/src/intrinsic.rs +++ b/src/intrinsic.rs @@ -31,6 +31,10 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' let substs = instance.substs; + // All these intrinsics take raw pointers, so if we access memory directly + // (as opposed to through a place), we have to remember to erase any tag + // that might still hang around! + let intrinsic_name = &self.tcx.item_name(instance.def_id()).as_str()[..]; match intrinsic_name { "arith_offset" => { @@ -146,12 +150,13 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' let elem_size = elem_layout.size.bytes(); let count = self.read_scalar(args[2])?.to_usize(&self)?; let elem_align = elem_layout.align; - let src = self.read_scalar(args[0])?.not_undef()?; - let dest = self.read_scalar(args[1])?.not_undef()?; + // erase tags: this is a raw ptr operation + let src = self.read_scalar(args[0])?.not_undef()?.erase_tag(); + let dest = self.read_scalar(args[1])?.not_undef()?.erase_tag(); self.memory.copy( - src, + src.with_default_tag(), elem_align, - dest, + dest.with_default_tag(), elem_align, Size::from_bytes(count * elem_size), intrinsic_name.ends_with("_nonoverlapping"), @@ -428,7 +433,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' let ty = substs.type_at(0); let ty_layout = self.layout_of(ty)?; let val_byte = self.read_scalar(args[1])?.to_u8()?; - let ptr = self.read_scalar(args[0])?.not_undef()?; + let ptr = self.read_scalar(args[0])?.not_undef()?.erase_tag().with_default_tag(); let count = self.read_scalar(args[2])?.to_usize(&self)?; self.memory.check_align(ptr, ty_layout.align)?; self.memory.write_repeat(ptr, val_byte, ty_layout.size * count)?; diff --git a/src/lib.rs b/src/lib.rs index 97b73e6069..c910911f57 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -415,14 +415,48 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { Cow::Owned(alloc) } + #[inline(always)] + fn memory_accessed( + alloc: &Allocation, + ptr: Pointer, + size: Size, + access: MemoryAccess, + ) -> EvalResult<'tcx> { + alloc.extra.memory_accessed(ptr, size, access) + } + + #[inline(always)] + fn memory_deallocated( + alloc: &mut Allocation, + ptr: Pointer, + ) -> EvalResult<'tcx> { + alloc.extra.memory_deallocated(ptr) + } + + /*/// Hook for when a reference is cast to a raw pointer + #[inline(always)] + fn ref_to_raw_cast( + ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, + ptr: Pointer, + ptr_ty: Ty<'tcx>, + size: Size, + ) -> EvalResult<'tcx> { + if !ecx.machine.validate { + // No tracking. + Ok(()) + } else { + ecx.ref_to_raw_cast(ptr, ptr_ty, size) + } + }*/ + #[inline(always)] fn tag_reference( ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, - ptr: Pointer, + ptr: Pointer, pointee_ty: Ty<'tcx>, pointee_size: Size, - borrow_kind: mir::BorrowKind, - ) -> EvalResult<'tcx, Self::PointerTag> { + borrow_kind: Option, + ) -> EvalResult<'tcx, Borrow> { if !ecx.machine.validate { // No tracking Ok(Borrow::default()) @@ -434,9 +468,9 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { #[inline(always)] fn tag_dereference( ecx: &EvalContext<'a, 'mir, 'tcx, Self>, - ptr: Pointer, + ptr: Pointer, ptr_ty: Ty<'tcx>, - ) -> EvalResult<'tcx, Self::PointerTag> { + ) -> EvalResult<'tcx, Borrow> { if !ecx.machine.validate { // No tracking Ok(Borrow::default()) diff --git a/src/range_map.rs b/src/range_map.rs index 1d9a38cb5e..74a49bf83b 100644 --- a/src/range_map.rs +++ b/src/range_map.rs @@ -99,6 +99,10 @@ impl RangeMap { self.iter_with_range(offset.bytes(), len.bytes()).map(|(_, data)| data) } + pub fn iter_mut_all<'a>(&'a mut self) -> impl Iterator + 'a { + self.map.values_mut() + } + fn split_entry_at(&mut self, offset: u64) where T: Clone, diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index c3514efdbe..e4d2289bf3 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -4,7 +4,7 @@ use rustc::ty::{Ty, layout::Size}; use rustc::mir; use super::{ - RangeMap, EvalResult, + MemoryAccess, RangeMap, EvalResult, Pointer, }; @@ -13,10 +13,10 @@ pub type Timestamp = u64; /// Information about a potentially mutable borrow #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] pub enum Mut { - /// A unique, mutable reference - Uniq(Timestamp), - /// Any raw pointer, or a shared borrow with interior mutability - Raw, + /// A unique, mutable reference + Uniq(Timestamp), + /// Any raw pointer, or a shared borrow with interior mutability + Raw, } impl Mut { @@ -27,34 +27,18 @@ impl Mut { _ => false, } } - - #[inline(always)] - fn is_uniq(self) -> bool { - match self { - Mut::Uniq(_) => true, - _ => false, - } - } } /// Information about any kind of borrow #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] pub enum Borrow { - /// A mutable borrow, a raw pointer, or a shared borrow with interior mutability - Mut(Mut), - /// A shared borrow without interior mutability - Frz(Timestamp) + /// A mutable borrow, a raw pointer, or a shared borrow with interior mutability + Mut(Mut), + /// A shared borrow without interior mutability + Frz(Timestamp) } impl Borrow { - #[inline(always)] - fn is_mut(self) -> bool { - match self { - Borrow::Mut(_) => true, - _ => false, - } - } - #[inline(always)] fn is_uniq(self) -> bool { match self { @@ -67,10 +51,11 @@ impl Borrow { /// An item in the borrow stack #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] pub enum BorStackItem { - /// Defines which references are permitted to mutate *if* the location is not frozen - Mut(Mut), - /// A barrier, tracking the function it belongs to by its index on the call stack - FnBarrier(usize) + /// Defines which references are permitted to mutate *if* the location is not frozen + Mut(Mut), + /// A barrier, tracking the function it belongs to by its index on the call stack + #[allow(dead_code)] // for future use + FnBarrier(usize) } impl Default for Borrow { @@ -133,15 +118,19 @@ impl<'tcx> Stack { /// Reactive `bor` for this stack. If `force_mut` is set, we want to aggressively /// unfreeze this location (because we are about to push a `Uniq`). fn reactivate(&mut self, bor: Borrow, force_mut: bool) -> EvalResult<'tcx> { - assert!(!force_mut || bor.is_mut()); // if `force_mut` is set, this must be a mutable borrow - // Do NOT change anything if `bor` is already active -- in particular, if - // it is a `Mut(Raw)` and we are frozen. + // Unless mutation is bound to happen, do NOT change anything if `bor` is already active. + // In particular, if it is a `Mut(Raw)` and we are frozen, this should be a NOP. if !force_mut && self.check(bor) { return Ok(()); } let acc_m = match bor { - Borrow::Frz(_) => return err!(MachineError(format!("Location should be frozen but it is not"))), + Borrow::Frz(_) => + if force_mut { + return err!(MachineError(format!("Using a shared borrow for mutation"))) + } else { + return err!(MachineError(format!("Location should be frozen but it is not"))) + } Borrow::Mut(acc_m) => acc_m, }; // We definitely have to unfreeze this, even if we use the topmost item. @@ -154,6 +143,7 @@ impl<'tcx> Stack { } BorStackItem::Mut(loc_m) => { if loc_m == acc_m { return Ok(()); } + trace!("reactivate: Popping {:?}", itm); self.borrows.pop(); } } @@ -169,12 +159,14 @@ impl<'tcx> Stack { fn initiate(&mut self, bor: Borrow) -> EvalResult<'tcx> { match bor { Borrow::Frz(t) => { + trace!("initiate: Freezing"); match self.frozen_since { None => self.frozen_since = Some(t), Some(since) => assert!(since <= t), } } Borrow::Mut(m) => { + trace!("initiate: Pushing {:?}", bor); match self.frozen_since { None => self.borrows.push(BorStackItem::Mut(m)), Some(_) => @@ -194,6 +186,58 @@ impl State { } } +/// Higher-level operations +impl<'tcx> Stacks { + pub fn memory_accessed( + &self, + ptr: Pointer, + size: Size, + access: MemoryAccess, + ) -> EvalResult<'tcx> { + trace!("memory_accessed({:?}) with tag {:?}: {:?}, size {}", access, ptr.tag, ptr, size.bytes()); + let mut stacks = self.stacks.borrow_mut(); + for stack in stacks.iter_mut(ptr.offset, size) { + // FIXME: Compare this with what the blog post says. + stack.reactivate(ptr.tag, /*force_mut*/access == MemoryAccess::Write)?; + } + Ok(()) + } + + pub fn memory_deallocated( + &mut self, + ptr: Pointer, + ) -> EvalResult<'tcx> { + trace!("memory_deallocated with tag {:?}: {:?}", ptr.tag, ptr); + let stacks = self.stacks.get_mut(); + for stack in stacks.iter_mut_all() { + // This is like mutating. + stack.reactivate(ptr.tag, /*force_mut*/true)?; + } + Ok(()) + } + + fn reborrow( + &self, + ptr: Pointer, + size: Size, + new_bor: Borrow, + ) -> EvalResult<'tcx> { + let mut stacks = self.stacks.borrow_mut(); + for stack in stacks.iter_mut(ptr.offset, size) { + if stack.check(new_bor) { + // The new borrow is already active! This can happen when creating multiple + // shared references from the same mutable reference. Do nothing. + } else { + // FIXME: The blog post says we should `reset` if this is a local. + stack.reactivate(ptr.tag, /*force_mut*/new_bor.is_uniq())?; + stack.initiate(new_bor)?; + } + } + + Ok(()) + } +} + /// Machine hooks pub trait EvalContextExt<'tcx> { fn tag_reference( @@ -201,7 +245,7 @@ pub trait EvalContextExt<'tcx> { ptr: Pointer, pointee_ty: Ty<'tcx>, size: Size, - borrow_kind: mir::BorrowKind, + borrow_kind: Option, ) -> EvalResult<'tcx, Borrow>; fn tag_dereference( @@ -209,6 +253,13 @@ pub trait EvalContextExt<'tcx> { ptr: Pointer, ptr_ty: Ty<'tcx>, ) -> EvalResult<'tcx, Borrow>; + + fn ref_to_raw_cast( + &mut self, + ptr: Pointer, + ptr_ty: Ty<'tcx>, + size: Size, + ) -> EvalResult<'tcx>; } impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, 'tcx> { @@ -217,22 +268,23 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' ptr: Pointer, pointee_ty: Ty<'tcx>, size: Size, - borrow_kind: mir::BorrowKind, + borrow_kind: Option, ) -> EvalResult<'tcx, Borrow> { - let old_bor = ptr.tag; let time = self.machine.stacked_borrows.increment_clock(); - // FIXME This does not do enough checking when only part of the data lacks + // FIXME This does not do enough checking when only part of the data has // interior mutability. let new_bor = match borrow_kind { - mir::BorrowKind::Mut { .. } => Borrow::Mut(Mut::Uniq(time)), - _ => + Some(mir::BorrowKind::Mut { .. }) => Borrow::Mut(Mut::Uniq(time)), + Some(_) => if self.type_is_freeze(pointee_ty) { Borrow::Frz(time) } else { Borrow::Mut(Mut::Raw) - } + }, + None => Borrow::Mut(Mut::Raw), }; - trace!("tag_reference: Creating new tag for {:?} (pointee {}, size {}): {:?}", ptr, pointee_ty, size.bytes(), new_bor); + trace!("tag_reference: Creating new reference ({:?}) for {:?} (pointee {}, size {}): {:?}", + borrow_kind, ptr, pointee_ty, size.bytes(), new_bor); // Make sure this reference is not dangling or so self.memory.check_bounds(ptr, size, false)?; @@ -240,17 +292,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' // Update the stacks. We cannot use `get_mut` becuse this might be immutable // memory. let alloc = self.memory.get(ptr.alloc_id).expect("We checked that the ptr is fine!"); - let mut stacks = alloc.extra.stacks.borrow_mut(); - for stack in stacks.iter_mut(ptr.offset, size) { - if stack.check(new_bor) { - // The new borrow is already active! This can happen when creating multiple - // shared references from the same mutable reference. Do nothing. - } else { - // FIXME: The blog post says we should `reset` if this is a local. - stack.reactivate(old_bor, /*force_mut*/new_bor.is_uniq())?; - stack.initiate(new_bor)?; - } - } + alloc.extra.reborrow(ptr, size, new_bor)?; Ok(new_bor) } @@ -269,4 +311,23 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' ptr.tag }) } + + fn ref_to_raw_cast( + &mut self, + ptr: Pointer, + _ptr_ty: Ty<'tcx>, + size: Size, + ) -> EvalResult<'tcx> { + trace!("ref_to_raw_cast: Escaping {:?}", ptr); + + // Make sure this reference is not dangling or so + self.memory.check_bounds(ptr, size, false)?; + + // Update the stacks. We cannot use `get_mut` becuse this might be immutable + // memory. + let alloc = self.memory.get(ptr.alloc_id).expect("We checked that the ptr is fine!"); + alloc.extra.reborrow(ptr, size, Borrow::Mut(Mut::Raw))?; + + Ok(()) + } } diff --git a/tests/run-pass/raw.rs b/tests/run-pass/raw.rs new file mode 100644 index 0000000000..4fbbb27095 --- /dev/null +++ b/tests/run-pass/raw.rs @@ -0,0 +1,21 @@ +fn basic_raw() { + let mut x = 12; + let x = &mut x; + + assert_eq!(*x, 12); + + let raw = x as *mut i32; + unsafe { *raw = 42; } + + assert_eq!(*x, 42); + + let raw = x as *mut i32; + unsafe { *raw = 12; } + *x = 23; + + assert_eq!(*x, 23); +} + +fn main() { + basic_raw(); +} diff --git a/tests/run-pass/refcell.rs b/tests/run-pass/refcell.rs new file mode 100644 index 0000000000..93cef1572a --- /dev/null +++ b/tests/run-pass/refcell.rs @@ -0,0 +1,33 @@ +use std::cell::RefCell; + +fn main() { + let c = RefCell::new(42); + { + let s1 = c.borrow(); + let _x: i32 = *s1; + let s2 = c.borrow(); + let _x: i32 = *s1; + let _y: i32 = *s2; + let _x: i32 = *s1; + let _y: i32 = *s2; + } + { + let mut m = c.borrow_mut(); + let _z: i32 = *m; + { + let s: &i32 = &*m; + let _x = *s; + } + *m = 23; + let _z: i32 = *m; + } + { + let s1 = c.borrow(); + let _x: i32 = *s1; + let s2 = c.borrow(); + let _x: i32 = *s1; + let _y: i32 = *s2; + let _x: i32 = *s1; + let _y: i32 = *s2; + } +} diff --git a/tests/run-pass/std.rs b/tests/run-pass/std.rs index e0e23812d2..7ff967b29f 100644 --- a/tests/run-pass/std.rs +++ b/tests/run-pass/std.rs @@ -12,8 +12,9 @@ fn rc_cell() -> Rc> { fn rc_refcell() -> i32 { let r = Rc::new(RefCell::new(42)); *r.borrow_mut() += 10; - let x = *r.borrow(); - x + let x = r.borrow(); + let y = r.borrow(); + (*x + *y)/2 } fn arc() -> Arc { From 1907782b64c36f41802a41f1f784f877870293ff Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 17 Oct 2018 16:55:59 +0200 Subject: [PATCH 05/12] reenable some tests that work now, and organize them better with directories --- .../stacked_borrows/alias_through_mutation.rs | 15 +++++++++++ .../aliasing_mut1.rs} | 0 .../aliasing_mut2.rs} | 0 .../aliasing_mut3.rs} | 0 .../aliasing_mut4.rs} | 0 .../buggy_as_mut_slice.rs} | 11 +++----- .../buggy_split_at_mut.rs} | 6 ++--- .../stacked_borrows/illegal_write.rs | 11 ++++++++ .../stacked_borrows/illegal_write2.rs | 10 +++++++ .../stacked_borrows/pointer_smuggling.rs | 22 ++++++++++++++++ .../stacked_borrows/shared_confusion.rs | 21 +++++++++++++++ .../compile-fail/validation_illegal_write.rs | 17 ------------ .../compile-fail/validation_lock_confusion.rs | 26 ------------------- .../validation_pointer_smuggling.rs | 22 ---------------- tests/compile-fail/validation_recover1.rs | 18 ------------- tests/compile-fail/validation_recover2.rs | 16 ------------ tests/compile-fail/validation_recover3.rs | 17 ------------ tests/compile-fail/validation_undef.rs | 16 ------------ .../cast_fn_ptr1.rs} | 0 .../cast_fn_ptr2.rs} | 0 .../{ => validity}/invalid_bool.rs | 0 .../{ => validity}/invalid_char.rs | 0 .../invalid_enum_discriminant.rs | 0 .../validity/transmute_through_ptr.rs | 16 ++++++++++++ tests/compile-fail/validity/undef.rs | 12 +++++++++ tests/compiletest.rs | 1 + 26 files changed, 114 insertions(+), 143 deletions(-) create mode 100644 tests/compile-fail/stacked_borrows/alias_through_mutation.rs rename tests/compile-fail/{validation_aliasing_mut1.rs => stacked_borrows/aliasing_mut1.rs} (100%) rename tests/compile-fail/{validation_aliasing_mut2.rs => stacked_borrows/aliasing_mut2.rs} (100%) rename tests/compile-fail/{validation_aliasing_mut3.rs => stacked_borrows/aliasing_mut3.rs} (100%) rename tests/compile-fail/{validation_aliasing_mut4.rs => stacked_borrows/aliasing_mut4.rs} (100%) rename tests/compile-fail/{validation_buggy_as_mut_slice.rs => stacked_borrows/buggy_as_mut_slice.rs} (52%) rename tests/compile-fail/{validation_buggy_split_at_mut.rs => stacked_borrows/buggy_split_at_mut.rs} (79%) create mode 100644 tests/compile-fail/stacked_borrows/illegal_write.rs create mode 100644 tests/compile-fail/stacked_borrows/illegal_write2.rs create mode 100644 tests/compile-fail/stacked_borrows/pointer_smuggling.rs create mode 100644 tests/compile-fail/stacked_borrows/shared_confusion.rs delete mode 100644 tests/compile-fail/validation_illegal_write.rs delete mode 100644 tests/compile-fail/validation_lock_confusion.rs delete mode 100644 tests/compile-fail/validation_pointer_smuggling.rs delete mode 100644 tests/compile-fail/validation_recover1.rs delete mode 100644 tests/compile-fail/validation_recover2.rs delete mode 100644 tests/compile-fail/validation_recover3.rs delete mode 100644 tests/compile-fail/validation_undef.rs rename tests/compile-fail/{validation_cast_fn_ptr1.rs => validity/cast_fn_ptr1.rs} (100%) rename tests/compile-fail/{validation_cast_fn_ptr2.rs => validity/cast_fn_ptr2.rs} (100%) rename tests/compile-fail/{ => validity}/invalid_bool.rs (100%) rename tests/compile-fail/{ => validity}/invalid_char.rs (100%) rename tests/compile-fail/{ => validity}/invalid_enum_discriminant.rs (100%) create mode 100644 tests/compile-fail/validity/transmute_through_ptr.rs create mode 100644 tests/compile-fail/validity/undef.rs diff --git a/tests/compile-fail/stacked_borrows/alias_through_mutation.rs b/tests/compile-fail/stacked_borrows/alias_through_mutation.rs new file mode 100644 index 0000000000..9fa50da45b --- /dev/null +++ b/tests/compile-fail/stacked_borrows/alias_through_mutation.rs @@ -0,0 +1,15 @@ +#![allow(unused_variables)] + +// This makes a ref that was passed to us via &mut alias with things it should not alias with +fn retarget(x: &mut &u32, target: &mut u32) { + unsafe { *x = &mut *(target as *mut _); } +} + +fn main() { + let target = &mut 42; + let mut target_alias = &42; // initial dummy value + retarget(&mut target_alias, target); + // now `target_alias` points to the same thing as `target` + *target = 13; + let _val = *target_alias; //~ ERROR should be frozen +} diff --git a/tests/compile-fail/validation_aliasing_mut1.rs b/tests/compile-fail/stacked_borrows/aliasing_mut1.rs similarity index 100% rename from tests/compile-fail/validation_aliasing_mut1.rs rename to tests/compile-fail/stacked_borrows/aliasing_mut1.rs diff --git a/tests/compile-fail/validation_aliasing_mut2.rs b/tests/compile-fail/stacked_borrows/aliasing_mut2.rs similarity index 100% rename from tests/compile-fail/validation_aliasing_mut2.rs rename to tests/compile-fail/stacked_borrows/aliasing_mut2.rs diff --git a/tests/compile-fail/validation_aliasing_mut3.rs b/tests/compile-fail/stacked_borrows/aliasing_mut3.rs similarity index 100% rename from tests/compile-fail/validation_aliasing_mut3.rs rename to tests/compile-fail/stacked_borrows/aliasing_mut3.rs diff --git a/tests/compile-fail/validation_aliasing_mut4.rs b/tests/compile-fail/stacked_borrows/aliasing_mut4.rs similarity index 100% rename from tests/compile-fail/validation_aliasing_mut4.rs rename to tests/compile-fail/stacked_borrows/aliasing_mut4.rs diff --git a/tests/compile-fail/validation_buggy_as_mut_slice.rs b/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs similarity index 52% rename from tests/compile-fail/validation_buggy_as_mut_slice.rs rename to tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs index 282e536ce9..3345668cee 100644 --- a/tests/compile-fail/validation_buggy_as_mut_slice.rs +++ b/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs @@ -1,22 +1,17 @@ -// ignore-test validation_op is disabled - #![allow(unused_variables)] -// For some reason, the error location is different when using fullmir -// error-pattern: in conflict with lock WriteLock - mod safe { use std::slice::from_raw_parts_mut; pub fn as_mut_slice(self_: &Vec) -> &mut [T] { unsafe { - from_raw_parts_mut(self_.as_ptr() as *mut T, self_.len()) + from_raw_parts_mut(self_.as_ptr() as *mut T, self_.len()) //~ ERROR shared borrow for mutation } } } fn main() { let v = vec![0,1,2]; - let v1_ = safe::as_mut_slice(&v); - let v2_ = safe::as_mut_slice(&v); + let v1 = safe::as_mut_slice(&v); + let v2 = safe::as_mut_slice(&v); } diff --git a/tests/compile-fail/validation_buggy_split_at_mut.rs b/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs similarity index 79% rename from tests/compile-fail/validation_buggy_split_at_mut.rs rename to tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs index a750f1466f..d7f4300f82 100644 --- a/tests/compile-fail/validation_buggy_split_at_mut.rs +++ b/tests/compile-fail/stacked_borrows/buggy_split_at_mut.rs @@ -1,5 +1,3 @@ -// ignore-test validation_op is disabled - #![allow(unused_variables)] mod safe { @@ -20,5 +18,7 @@ mod safe { fn main() { let mut array = [1,2,3,4]; - let _x = safe::split_at_mut(&mut array, 0); //~ ERROR: in conflict with lock WriteLock + let (a, b) = safe::split_at_mut(&mut array, 0); + a[1] = 5; //~ ERROR does not exist on the stack + b[1] = 6; } diff --git a/tests/compile-fail/stacked_borrows/illegal_write.rs b/tests/compile-fail/stacked_borrows/illegal_write.rs new file mode 100644 index 0000000000..6a7ccc8401 --- /dev/null +++ b/tests/compile-fail/stacked_borrows/illegal_write.rs @@ -0,0 +1,11 @@ +fn evil(x: &u32) { + let x : &mut u32 = unsafe { &mut *(x as *const _ as *mut _) }; + *x = 42; // mutating shared ref without `UnsafeCell` +} + +fn main() { + let target = 42; + let ref_ = ⌖ + evil(ref_); // invalidates shared ref + let _x = *ref_; //~ ERROR should be frozen +} diff --git a/tests/compile-fail/stacked_borrows/illegal_write2.rs b/tests/compile-fail/stacked_borrows/illegal_write2.rs new file mode 100644 index 0000000000..1d61b1b988 --- /dev/null +++ b/tests/compile-fail/stacked_borrows/illegal_write2.rs @@ -0,0 +1,10 @@ +#![allow(unused_variables)] + +fn main() { + let target = &mut 42; + let target2 = target as *mut _; + drop(&mut *target); // reborrow + // Now make sure our ref is still the only one + unsafe { *target2 = 13; } // invalidate our ref + let _val = *target; //~ ERROR does not exist on the stack +} diff --git a/tests/compile-fail/stacked_borrows/pointer_smuggling.rs b/tests/compile-fail/stacked_borrows/pointer_smuggling.rs new file mode 100644 index 0000000000..3576aa52b7 --- /dev/null +++ b/tests/compile-fail/stacked_borrows/pointer_smuggling.rs @@ -0,0 +1,22 @@ +#![allow(unused_variables)] + +static mut PTR: *mut u8 = 0 as *mut _; + +fn fun1(x: &mut u8) { + unsafe { + PTR = x; + } +} + +fn fun2() { + // Now we use a pointer we are not allowed to use + let _x = unsafe { *PTR }; +} + +fn main() { + let val = &mut 0; // FIXME: This should also work with a local variable, but currently it does not. + fun1(val); + *val = 2; // this invalidates any raw ptrs `fun1` might have created. + fun2(); // if they now use a raw ptr they break our reference + *val = 3; //~ ERROR does not exist on the stack +} diff --git a/tests/compile-fail/stacked_borrows/shared_confusion.rs b/tests/compile-fail/stacked_borrows/shared_confusion.rs new file mode 100644 index 0000000000..584053f593 --- /dev/null +++ b/tests/compile-fail/stacked_borrows/shared_confusion.rs @@ -0,0 +1,21 @@ +#![allow(unused_variables)] +use std::cell::RefCell; + +fn test(r: &mut RefCell) { + let x = &*r; // not freezing because interior mutability + let mut x_ref = x.borrow_mut(); + let x_inner : &mut i32 = &mut *x_ref; // Uniq reference + let x_evil = x_inner as *mut _; + { + let x_inner_shr = &*x_inner; // frozen + let y = &*r; // outer ref, not freezing + let x_inner_shr2 = &*x_inner; // freezing again + } + // Our old raw should be dead by now + unsafe { *x_evil = 0; } // this falls back to some Raw higher up the stack + *x_inner = 12; //~ ERROR does not exist on the stack +} + +fn main() { + test(&mut RefCell::new(0)); +} diff --git a/tests/compile-fail/validation_illegal_write.rs b/tests/compile-fail/validation_illegal_write.rs deleted file mode 100644 index cb3e4b3c1a..0000000000 --- a/tests/compile-fail/validation_illegal_write.rs +++ /dev/null @@ -1,17 +0,0 @@ -// ignore-test validation_op is disabled - -#![allow(unused_variables)] - -mod safe { - pub(crate) fn safe(x: &u32) { - let x : &mut u32 = unsafe { &mut *(x as *const _ as *mut _) }; - *x = 42; //~ ERROR: in conflict with lock ReadLock - } -} - -fn main() { - let target = &mut 42; - let target_ref = ⌖ - // do a reborrow, but we keep the lock - safe::safe(&*target); -} diff --git a/tests/compile-fail/validation_lock_confusion.rs b/tests/compile-fail/validation_lock_confusion.rs deleted file mode 100644 index 2a08576596..0000000000 --- a/tests/compile-fail/validation_lock_confusion.rs +++ /dev/null @@ -1,26 +0,0 @@ -// ignore-test validation_op is disabled - -// Make sure validation can handle many overlapping shared borrows for different parts of a data structure -#![allow(unused_variables)] -use std::cell::RefCell; - -fn evil(x: *mut i32) { - unsafe { *x = 0; } //~ ERROR: in conflict with lock WriteLock -} - -fn test(r: &mut RefCell) { - let x = &*r; // releasing write lock, first suspension recorded - let mut x_ref = x.borrow_mut(); - let x_inner : &mut i32 = &mut *x_ref; // new inner write lock, with same lifetime as outer lock - { - let x_inner_shr = &*x_inner; // releasing inner write lock, recording suspension - let y = &*r; // second suspension for the outer write lock - let x_inner_shr2 = &*x_inner; // 2nd suspension for inner write lock - } - // If the two locks are mixed up, here we should have a write lock, but we do not. - evil(x_inner as *mut _); -} - -fn main() { - test(&mut RefCell::new(0)); -} diff --git a/tests/compile-fail/validation_pointer_smuggling.rs b/tests/compile-fail/validation_pointer_smuggling.rs deleted file mode 100644 index 14d6242860..0000000000 --- a/tests/compile-fail/validation_pointer_smuggling.rs +++ /dev/null @@ -1,22 +0,0 @@ -// ignore-test validation_op is disabled - -#![allow(unused_variables)] - -static mut PTR: *mut u8 = 0 as *mut _; - -fn fun1(x: &mut u8) { - unsafe { - PTR = x; - } -} - -fn fun2() { - // Now we use a pointer we are not allowed to use - let _x = unsafe { *PTR }; //~ ERROR: in conflict with lock WriteLock -} - -fn main() { - let mut val = 0; - fun1(&mut val); - fun2(); -} diff --git a/tests/compile-fail/validation_recover1.rs b/tests/compile-fail/validation_recover1.rs deleted file mode 100644 index 9061070ef6..0000000000 --- a/tests/compile-fail/validation_recover1.rs +++ /dev/null @@ -1,18 +0,0 @@ -// ignore-test validation_op is disabled - -#![allow(unused_variables)] - -#[repr(u32)] -enum Bool { True } - -mod safe { - pub(crate) fn safe(x: &mut super::Bool) { - let x = x as *mut _ as *mut u32; - unsafe { *x = 44; } // out-of-bounds enum discriminant - } -} - -fn main() { - let mut x = Bool::True; - safe::safe(&mut x); //~ ERROR: invalid enum discriminant -} diff --git a/tests/compile-fail/validation_recover2.rs b/tests/compile-fail/validation_recover2.rs deleted file mode 100644 index 7a4a417ab1..0000000000 --- a/tests/compile-fail/validation_recover2.rs +++ /dev/null @@ -1,16 +0,0 @@ -// ignore-test validation_op is disabled - -#![allow(unused_variables)] - -mod safe { - // This makes a ref that was passed to us via &mut alias with things it should not alias with - pub(crate) fn safe(x: &mut &u32, target: &mut u32) { - unsafe { *x = &mut *(target as *mut _); } - } -} - -fn main() { - let target = &mut 42; - let mut target_alias = &42; // initial dummy value - safe::safe(&mut target_alias, target); //~ ERROR: in conflict with lock ReadLock -} diff --git a/tests/compile-fail/validation_recover3.rs b/tests/compile-fail/validation_recover3.rs deleted file mode 100644 index 5cfc8aaa66..0000000000 --- a/tests/compile-fail/validation_recover3.rs +++ /dev/null @@ -1,17 +0,0 @@ -// ignore-test validation_op is disabled - -#![allow(unused_variables)] - -mod safe { - pub(crate) fn safe(x: *mut u32) { - unsafe { *x = 42; } //~ ERROR: in conflict with lock WriteLock - } -} - -fn main() { - let target = &mut 42u32; - let target2 = target as *mut _; - drop(&mut *target); // reborrow - // Now make sure we still got the lock - safe::safe(target2); -} diff --git a/tests/compile-fail/validation_undef.rs b/tests/compile-fail/validation_undef.rs deleted file mode 100644 index 939e93a264..0000000000 --- a/tests/compile-fail/validation_undef.rs +++ /dev/null @@ -1,16 +0,0 @@ -// ignore-test validation_op is disabled - -#![allow(unused_variables)] -// error-pattern: attempted to read undefined bytes - -mod safe { - use std::mem; - - pub(crate) fn make_float() -> f32 { - unsafe { mem::uninitialized() } - } -} - -fn main() { - let _x = safe::make_float(); -} diff --git a/tests/compile-fail/validation_cast_fn_ptr1.rs b/tests/compile-fail/validity/cast_fn_ptr1.rs similarity index 100% rename from tests/compile-fail/validation_cast_fn_ptr1.rs rename to tests/compile-fail/validity/cast_fn_ptr1.rs diff --git a/tests/compile-fail/validation_cast_fn_ptr2.rs b/tests/compile-fail/validity/cast_fn_ptr2.rs similarity index 100% rename from tests/compile-fail/validation_cast_fn_ptr2.rs rename to tests/compile-fail/validity/cast_fn_ptr2.rs diff --git a/tests/compile-fail/invalid_bool.rs b/tests/compile-fail/validity/invalid_bool.rs similarity index 100% rename from tests/compile-fail/invalid_bool.rs rename to tests/compile-fail/validity/invalid_bool.rs diff --git a/tests/compile-fail/invalid_char.rs b/tests/compile-fail/validity/invalid_char.rs similarity index 100% rename from tests/compile-fail/invalid_char.rs rename to tests/compile-fail/validity/invalid_char.rs diff --git a/tests/compile-fail/invalid_enum_discriminant.rs b/tests/compile-fail/validity/invalid_enum_discriminant.rs similarity index 100% rename from tests/compile-fail/invalid_enum_discriminant.rs rename to tests/compile-fail/validity/invalid_enum_discriminant.rs diff --git a/tests/compile-fail/validity/transmute_through_ptr.rs b/tests/compile-fail/validity/transmute_through_ptr.rs new file mode 100644 index 0000000000..0a4c64bb9b --- /dev/null +++ b/tests/compile-fail/validity/transmute_through_ptr.rs @@ -0,0 +1,16 @@ +#![allow(unused_variables)] + +#[repr(u32)] +enum Bool { True } + +fn evil(x: &mut Bool) { + let x = x as *mut _ as *mut u32; + unsafe { *x = 44; } // out-of-bounds enum discriminant +} + +fn main() { + let mut x = Bool::True; + evil(&mut x); + let _y = x; // reading this ought to be enough to trigger validation + //~^ ERROR invalid enum discriminant 44 +} diff --git a/tests/compile-fail/validity/undef.rs b/tests/compile-fail/validity/undef.rs new file mode 100644 index 0000000000..58d3926dad --- /dev/null +++ b/tests/compile-fail/validity/undef.rs @@ -0,0 +1,12 @@ +#![allow(unused_variables)] +// error-pattern: encountered undefined data in pointer + +use std::mem; + +fn make_raw() -> *const f32 { + unsafe { mem::uninitialized() } +} + +fn main() { + let _x = make_raw(); +} diff --git a/tests/compiletest.rs b/tests/compiletest.rs index b240fb31d2..7bec3fa8de 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -63,6 +63,7 @@ fn compile_fail(sysroot: &Path, path: &str, target: &str, host: &str, need_fullm flags.push(format!("--sysroot {}", sysroot.display())); flags.push("-Dwarnings -Dunused".to_owned()); // overwrite the -Aunused in compiletest-rs config.src_base = PathBuf::from(path.to_string()); + flags.push("-Zmir-opt-level=0".to_owned()); // optimization circumvents some stacked borrow checks flags.push("-Zmir-emit-validate=1".to_owned()); config.target_rustcflags = Some(flags.join(" ")); config.target = target.to_owned(); From b259512c5783f1ebda367a3094aecc5f9b7da7fd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 17 Oct 2018 17:36:07 +0200 Subject: [PATCH 06/12] bump for ENABLE_PTR_TRACKING_HOOKS, and remove some dead code --- src/lib.rs | 18 ++---------------- src/stacked_borrows.rs | 26 -------------------------- 2 files changed, 2 insertions(+), 42 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c910911f57..1ff29828da 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -264,8 +264,10 @@ type MiriEvalContext<'a, 'mir, 'tcx> = EvalContext<'a, 'mir, 'tcx, Evaluator<'tc impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { type MemoryKinds = MiriMemoryKind; + type AllocExtra = stacked_borrows::Stacks; type PointerTag = Borrow; + const ENABLE_PTR_TRACKING_HOOKS: bool = true; type MemoryMap = MonoHashMap, Allocation)>; @@ -433,22 +435,6 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { alloc.extra.memory_deallocated(ptr) } - /*/// Hook for when a reference is cast to a raw pointer - #[inline(always)] - fn ref_to_raw_cast( - ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, - ptr: Pointer, - ptr_ty: Ty<'tcx>, - size: Size, - ) -> EvalResult<'tcx> { - if !ecx.machine.validate { - // No tracking. - Ok(()) - } else { - ecx.ref_to_raw_cast(ptr, ptr_ty, size) - } - }*/ - #[inline(always)] fn tag_reference( ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index e4d2289bf3..19d5c723d4 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -253,13 +253,6 @@ pub trait EvalContextExt<'tcx> { ptr: Pointer, ptr_ty: Ty<'tcx>, ) -> EvalResult<'tcx, Borrow>; - - fn ref_to_raw_cast( - &mut self, - ptr: Pointer, - ptr_ty: Ty<'tcx>, - size: Size, - ) -> EvalResult<'tcx>; } impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, 'tcx> { @@ -311,23 +304,4 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' ptr.tag }) } - - fn ref_to_raw_cast( - &mut self, - ptr: Pointer, - _ptr_ty: Ty<'tcx>, - size: Size, - ) -> EvalResult<'tcx> { - trace!("ref_to_raw_cast: Escaping {:?}", ptr); - - // Make sure this reference is not dangling or so - self.memory.check_bounds(ptr, size, false)?; - - // Update the stacks. We cannot use `get_mut` becuse this might be immutable - // memory. - let alloc = self.memory.get(ptr.alloc_id).expect("We checked that the ptr is fine!"); - alloc.extra.reborrow(ptr, size, Borrow::Mut(Mut::Raw))?; - - Ok(()) - } } From b9fe91e48627cde55aaf9b55d016746db0b4907f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 18 Oct 2018 12:04:41 +0200 Subject: [PATCH 07/12] fix for ptr-to-raw casts properly erasing the tag --- tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs b/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs index 3345668cee..4857ada7fb 100644 --- a/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs +++ b/tests/compile-fail/stacked_borrows/buggy_as_mut_slice.rs @@ -5,7 +5,7 @@ mod safe { pub fn as_mut_slice(self_: &Vec) -> &mut [T] { unsafe { - from_raw_parts_mut(self_.as_ptr() as *mut T, self_.len()) //~ ERROR shared borrow for mutation + from_raw_parts_mut(self_.as_ptr() as *mut T, self_.len()) } } } @@ -14,4 +14,6 @@ fn main() { let v = vec![0,1,2]; let v1 = safe::as_mut_slice(&v); let v2 = safe::as_mut_slice(&v); + v1[1] = 5; //~ ERROR does not exist on the stack + v1[1] = 6; } From bbb1d80703f272a5592ceeb3832a489776512251 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 21 Oct 2018 01:31:46 +0200 Subject: [PATCH 08/12] disable env var test on macOS, win --- tests/run-pass/env.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/run-pass/env.rs b/tests/run-pass/env.rs index c0bf883daf..8c655b953f 100644 --- a/tests/run-pass/env.rs +++ b/tests/run-pass/env.rs @@ -1,3 +1,6 @@ +//ignore-windows: env var emulation not implemented on Windows +//ignore-macos: env var emulation not implemented on macOS + use std::env; fn main() { From 41eabb658e7a27ba77500f36341a108804c8e0d0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 22 Oct 2018 08:41:06 +0200 Subject: [PATCH 09/12] bump Rust version --- rust-toolchain | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-toolchain b/rust-toolchain index abbc6c9045..cc0787f877 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1 +1 @@ -nightly-2018-10-14 +nightly-2018-10-22 From fdb3022a1195965acf26746bf6d4c8dcae5608af Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 22 Oct 2018 08:41:48 +0200 Subject: [PATCH 10/12] env vars are only available with full MIR --- tests/{run-pass => run-pass-fullmir}/env.rs | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{run-pass => run-pass-fullmir}/env.rs (100%) diff --git a/tests/run-pass/env.rs b/tests/run-pass-fullmir/env.rs similarity index 100% rename from tests/run-pass/env.rs rename to tests/run-pass-fullmir/env.rs From 0b22a1c9d950f741db8f144a61891166df69dbbd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 22 Oct 2018 09:04:32 +0200 Subject: [PATCH 11/12] env vars should work on macOS --- tests/run-pass-fullmir/env.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/run-pass-fullmir/env.rs b/tests/run-pass-fullmir/env.rs index 8c655b953f..0ca63e148f 100644 --- a/tests/run-pass-fullmir/env.rs +++ b/tests/run-pass-fullmir/env.rs @@ -1,5 +1,4 @@ //ignore-windows: env var emulation not implemented on Windows -//ignore-macos: env var emulation not implemented on macOS use std::env; From 1a7fb7ec3ce5be7205fc1b3fb40642792985d501 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 22 Oct 2018 12:51:30 +0200 Subject: [PATCH 12/12] expand comment about incomplete support for interior mutability --- src/stacked_borrows.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 19d5c723d4..077deceecf 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -264,11 +264,12 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' borrow_kind: Option, ) -> EvalResult<'tcx, Borrow> { let time = self.machine.stacked_borrows.increment_clock(); - // FIXME This does not do enough checking when only part of the data has - // interior mutability. let new_bor = match borrow_kind { Some(mir::BorrowKind::Mut { .. }) => Borrow::Mut(Mut::Uniq(time)), Some(_) => + // FIXME This does not do enough checking when only part of the data has + // interior mutability. When the type is `(i32, Cell)`, we want the + // first field to be frozen but not the second. if self.type_is_freeze(pointee_ty) { Borrow::Frz(time) } else {