Merge pull request #513 from RalfJung/new-interior-mut

New Stacked Borrows, now with better support for interior mutability
This commit is contained in:
Oliver S̶c̶h̶n̶e̶i̶d̶e̶r Scherer 2018-11-08 20:22:54 +01:00 committed by GitHub
commit d136fdb701
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
30 changed files with 473 additions and 347 deletions

View File

@ -10,9 +10,8 @@ import sys, subprocess
def test_cargo_miri():
print("==> Testing `cargo miri` <==")
## Call `cargo miri`, capture all output
# FIXME: Disabling validation, still investigating whether there is UB here
p = subprocess.Popen(
["cargo", "miri", "-q", "--", "-Zmiri-disable-validation"],
["cargo", "miri", "-q"],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE
)

View File

@ -1 +1 @@
nightly-2018-11-07
nightly-2018-11-08

View File

@ -1,6 +1,6 @@
use std::mem;
use rustc::ty;
use rustc::ty::{self, layout};
use rustc::hir::def_id::{DefId, CRATE_DEF_INDEX};
use crate::*;
@ -32,6 +32,15 @@ fn to_bytes(self) -> EvalResult<'static, u128> {
pub trait EvalContextExt<'tcx> {
fn resolve_path(&self, path: &[&str]) -> EvalResult<'tcx, ty::Instance<'tcx>>;
/// Visit the memory covered by `place` that is frozen -- i.e., NOT
/// what is inside an `UnsafeCell`.
fn visit_frozen(
&self,
place: MPlaceTy<'tcx, Borrow>,
size: Size,
action: impl FnMut(Pointer<Borrow>, Size) -> EvalResult<'tcx>,
) -> EvalResult<'tcx>;
}
@ -69,4 +78,153 @@ fn resolve_path(&self, path: &[&str]) -> EvalResult<'tcx, ty::Instance<'tcx>> {
EvalErrorKind::PathNotFound(path).into()
})
}
/// Visit the memory covered by `place` that is frozen -- i.e., NOT
/// what is inside an `UnsafeCell`.
fn visit_frozen(
&self,
place: MPlaceTy<'tcx, Borrow>,
size: Size,
mut frozen_action: impl FnMut(Pointer<Borrow>, Size) -> EvalResult<'tcx>,
) -> EvalResult<'tcx> {
trace!("visit_frozen(place={:?}, size={:?})", *place, size);
debug_assert_eq!(size,
self.size_and_align_of_mplace(place)?
.map(|(size, _)| size)
.unwrap_or_else(|| place.layout.size)
);
// Store how far we proceeded into the place so far. Everything to the left of
// this offset has already been handled, in the sense that the frozen parts
// have had `action` called on them.
let mut end_ptr = place.ptr;
// Called when we detected an `UnsafeCell` at the given offset and size.
// Calls `action` and advances `end_ptr`.
let mut unsafe_cell_action = |unsafe_cell_offset, unsafe_cell_size| {
// We assume that we are given the fields in increasing offset order,
// and nothing else changes.
let end_offset = end_ptr.get_ptr_offset(self);
assert!(unsafe_cell_offset >= end_offset);
let frozen_size = unsafe_cell_offset - end_offset;
// Everything between the end_ptr and this `UnsafeCell` is frozen.
if frozen_size != Size::ZERO {
frozen_action(end_ptr.to_ptr()?, frozen_size)?;
}
// Update end end_ptr.
end_ptr = end_ptr.ptr_wrapping_offset(frozen_size+unsafe_cell_size, self);
// Done
Ok(())
};
// Run a visitor
{
let mut visitor = UnsafeCellVisitor {
ecx: self,
unsafe_cell_action: |place| {
trace!("unsafe_cell_action on {:?}", place.ptr);
// We need a size to go on.
let (unsafe_cell_size, _) = self.size_and_align_of_mplace(place)?
// for extern types, just cover what we can
.unwrap_or_else(|| place.layout.size_and_align());
// Now handle this `UnsafeCell`, unless it is empty.
if unsafe_cell_size != Size::ZERO {
unsafe_cell_action(place.ptr.get_ptr_offset(self), unsafe_cell_size)
} else {
Ok(())
}
},
};
visitor.visit_value(place)?;
}
// The part between the end_ptr and the end of the place is also frozen.
// So pretend there is a 0-sized `UnsafeCell` at the end.
unsafe_cell_action(place.ptr.get_ptr_offset(self) + size, Size::ZERO)?;
// Done!
return Ok(());
/// Visiting the memory covered by a `MemPlace`, being aware of
/// whether we are inside an `UnsafeCell` or not.
struct UnsafeCellVisitor<'ecx, 'a, 'mir, 'tcx, F>
where F: FnMut(MPlaceTy<'tcx, Borrow>) -> EvalResult<'tcx>
{
ecx: &'ecx MiriEvalContext<'a, 'mir, 'tcx>,
unsafe_cell_action: F,
}
impl<'ecx, 'a, 'mir, 'tcx, F> ValueVisitor<'a, 'mir, 'tcx, Evaluator<'tcx>>
for UnsafeCellVisitor<'ecx, 'a, 'mir, 'tcx, F>
where
F: FnMut(MPlaceTy<'tcx, Borrow>) -> EvalResult<'tcx>
{
type V = MPlaceTy<'tcx, Borrow>;
#[inline(always)]
fn ecx(&self) -> &MiriEvalContext<'a, 'mir, 'tcx> {
&self.ecx
}
// Hook to detect `UnsafeCell`
fn visit_value(&mut self, v: MPlaceTy<'tcx, Borrow>) -> EvalResult<'tcx>
{
trace!("UnsafeCellVisitor: {:?} {:?}", *v, v.layout.ty);
let is_unsafe_cell = match v.layout.ty.sty {
ty::Adt(adt, _) => Some(adt.did) == self.ecx.tcx.lang_items().unsafe_cell_type(),
_ => false,
};
if is_unsafe_cell {
// We do not have to recurse further, this is an `UnsafeCell`.
(self.unsafe_cell_action)(v)
} else if self.ecx.type_is_freeze(v.layout.ty) {
// This is `Freeze`, there cannot be an `UnsafeCell`
Ok(())
} else {
// Proceed further
self.walk_value(v)
}
}
// Make sure we visit aggregrates in increasing offset order
fn visit_aggregate(
&mut self,
place: MPlaceTy<'tcx, Borrow>,
fields: impl Iterator<Item=EvalResult<'tcx, MPlaceTy<'tcx, Borrow>>>,
) -> EvalResult<'tcx> {
match place.layout.fields {
layout::FieldPlacement::Array { .. } => {
// For the array layout, we know the iterator will yield sorted elements so
// we can avoid the allocation.
self.walk_aggregate(place, fields)
}
layout::FieldPlacement::Arbitrary { .. } => {
// Gather the subplaces and sort them before visiting.
let mut places = fields.collect::<EvalResult<'tcx, Vec<MPlaceTy<'tcx, Borrow>>>>()?;
places[..].sort_by_key(|place| place.ptr.get_ptr_offset(self.ecx()));
self.walk_aggregate(place, places.into_iter().map(Ok))
}
layout::FieldPlacement::Union { .. } => {
// Uh, what?
bug!("A union is not an aggregate we should ever visit")
}
}
}
// We have to do *something* for unions
fn visit_union(&mut self, v: MPlaceTy<'tcx, Borrow>) -> EvalResult<'tcx>
{
// With unions, we fall back to whatever the type says, to hopefully be consistent
// with LLVM IR.
// FIXME Are we consistent? And is this really the behavior we want?
let frozen = self.ecx.type_is_freeze(v.layout.ty);
if frozen {
Ok(())
} else {
(self.unsafe_cell_action)(v)
}
}
// We should never get to a primitive, but always short-circuit somewhere above
fn visit_primitive(&mut self, _val: ImmTy<'tcx, Borrow>) -> EvalResult<'tcx>
{
bug!("We should always short-circit before coming to a primitive")
}
}
}
}

View File

@ -17,7 +17,7 @@
use std::borrow::Cow;
use std::env;
use rustc::ty::{self, Ty, TyCtxt, query::TyCtxtAt};
use rustc::ty::{self, TyCtxt, query::TyCtxtAt};
use rustc::ty::layout::{TyLayout, LayoutOf, Size};
use rustc::hir::{self, def_id::DefId};
use rustc::mir;
@ -48,7 +48,7 @@
use crate::stacked_borrows::{EvalContextExt as StackedBorEvalContextExt};
// Used by priroda
pub use crate::stacked_borrows::{Borrow, Stack, Stacks, Mut as MutBorrow, BorStackItem};
pub use crate::stacked_borrows::{Borrow, Stack, Stacks, BorStackItem};
/// Insert rustc arguments at the beginning of the argument list that miri wants to be
/// set per default, for maximal validation power.
@ -476,38 +476,38 @@ fn memory_deallocated(
#[inline(always)]
fn tag_reference(
ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,
place: MemPlace<Borrow>,
ty: Ty<'tcx>,
size: Size,
place: MPlaceTy<'tcx, Borrow>,
mutability: Option<hir::Mutability>,
) -> EvalResult<'tcx, MemPlace<Borrow>> {
) -> EvalResult<'tcx, Scalar<Borrow>> {
let (size, _) = ecx.size_and_align_of_mplace(place)?
// for extern types, just cover what we can
.unwrap_or_else(|| place.layout.size_and_align());
if !ecx.machine.validate || size == Size::ZERO {
// No tracking
Ok(place)
Ok(place.ptr)
} else {
let ptr = place.ptr.to_ptr()?;
let tag = ecx.tag_reference(ptr, ty, size, mutability.into())?;
let ptr = Scalar::Ptr(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag));
Ok(MemPlace { ptr, ..place })
let tag = ecx.tag_reference(place, size, mutability.into())?;
Ok(Scalar::Ptr(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag)))
}
}
#[inline(always)]
fn tag_dereference(
ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
place: MemPlace<Borrow>,
ty: Ty<'tcx>,
size: Size,
place: MPlaceTy<'tcx, Borrow>,
mutability: Option<hir::Mutability>,
) -> EvalResult<'tcx, MemPlace<Borrow>> {
) -> EvalResult<'tcx, Scalar<Borrow>> {
let (size, _) = ecx.size_and_align_of_mplace(place)?
// for extern types, just cover what we can
.unwrap_or_else(|| place.layout.size_and_align());
if !ecx.machine.validate || size == Size::ZERO {
// No tracking
Ok(place)
Ok(place.ptr)
} else {
let ptr = place.ptr.to_ptr()?;
let tag = ecx.tag_dereference(ptr, ty, size, mutability.into())?;
let ptr = Scalar::Ptr(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag));
Ok(MemPlace { ptr, ..place })
let tag = ecx.tag_dereference(place, size, mutability.into())?;
Ok(Scalar::Ptr(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag)))
}
}

View File

@ -1,64 +1,43 @@
use std::cell::RefCell;
use rustc::ty::{self, Ty, layout::Size};
use rustc::ty::{self, layout::Size};
use rustc::hir;
use crate::{
MemoryKind, MiriMemoryKind, RangeMap, EvalResult, AllocId,
Pointer, PlaceTy,
EvalResult, MiriEvalContext, HelpersEvalContextExt,
MemoryKind, MiriMemoryKind, RangeMap, AllocId,
Pointer, PlaceTy, MPlaceTy,
};
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,
}
impl Mut {
#[inline(always)]
pub fn is_raw(self) -> bool {
match self {
Mut::Raw => true,
_ => false,
}
}
#[inline(always)]
pub fn is_uniq(self) -> bool {
match self {
Mut::Uniq(_) => true,
_ => false,
}
}
}
/// Information about any kind of borrow
/// Information about which kind of borrow was used to create the reference this is tagged
/// with.
#[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 unique (mutable) reference.
Uniq(Timestamp),
/// A shared reference. This is also used by raw pointers, which do not track details
/// of how or when they were created, hence the timestamp is optional.
/// Shr(Some(_)) does NOT mean that the destination of this reference is frozen;
/// that depends on the type! Only those parts outside of an `UnsafeCell` are actually
/// frozen.
Shr(Option<Timestamp>),
}
impl Borrow {
#[inline(always)]
pub fn is_uniq(self) -> bool {
pub fn is_shr(self) -> bool {
match self {
Borrow::Mut(m) => m.is_uniq(),
Borrow::Shr(_) => true,
_ => false,
}
}
#[inline(always)]
pub fn is_frz(self) -> bool {
pub fn is_uniq(self) -> bool {
match self {
Borrow::Frz(_) => true,
Borrow::Uniq(_) => true,
_ => false,
}
}
@ -66,15 +45,19 @@ pub fn is_frz(self) -> bool {
impl Default for Borrow {
fn default() -> Self {
Borrow::Mut(Mut::Raw)
Borrow::Shr(None)
}
}
/// An item in the borrow stack
/// An item in the per-location 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),
/// Indicates the unique reference that may mutate.
Uniq(Timestamp),
/// Indicates that the location has been shared. Used for raw pointers, but
/// also for shared references. The latter *additionally* get frozen
/// when there is no `UnsafeCell`.
Shr,
/// A barrier, tracking the function it belongs to by its index on the call stack
#[allow(dead_code)] // for future use
FnBarrier(usize)
@ -90,6 +73,29 @@ pub fn is_fn_barrier(self) -> bool {
}
}
/// Extra per-location state
#[derive(Clone, Debug)]
pub struct Stack {
borrows: Vec<BorStackItem>, // used as a stack; never empty
frozen_since: Option<Timestamp>, // virtual frozen "item" on top of the stack
}
impl Default for Stack {
fn default() -> Self {
Stack {
borrows: vec![BorStackItem::Shr],
frozen_since: None,
}
}
}
impl Stack {
#[inline(always)]
pub fn is_frozen(&self) -> bool {
self.frozen_since.is_some()
}
}
/// What kind of usage of the pointer are we talking about?
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
pub enum UsageKind {
@ -97,7 +103,7 @@ pub enum UsageKind {
Write,
/// Read, or create &
Read,
/// Create *
/// Create * (raw ptr)
Raw,
}
@ -123,29 +129,6 @@ pub fn new() -> State {
}
}
/// Extra per-location state
#[derive(Clone, Debug)]
pub struct Stack {
borrows: Vec<BorStackItem>, // used as a stack
frozen_since: Option<Timestamp>,
}
impl Default for Stack {
fn default() -> Self {
Stack {
borrows: vec![BorStackItem::Mut(Mut::Raw)],
frozen_since: None,
}
}
}
impl Stack {
#[inline(always)]
fn is_frozen(&self) -> bool {
self.frozen_since.is_some()
}
}
/// Extra per-allocation state
#[derive(Clone, Debug, Default)]
pub struct Stacks {
@ -156,124 +139,125 @@ pub struct Stacks {
/// Core operations
impl<'tcx> Stack {
/// Check if `bor` could be activated by unfreezing and popping.
/// `usage` indicates whether this is being used to read/write (or, equivalently, to
/// borrow as &/&mut), or to borrow as raw.
/// Returns `Err` if the answer is "no"; otherwise the data says
/// what needs to happen to activate this: `None` = nothing,
/// `Some(n)` = unfreeze and make item `n` the top item of the stack.
fn reactivatable(&self, bor: Borrow, usage: UsageKind) -> Result<Option<usize>, String> {
let mut_borrow = match bor {
Borrow::Frz(since) =>
// The only way to reactivate a `Frz` is if this is already frozen.
return match self.frozen_since {
_ if usage == UsageKind::Write =>
Err(format!("Using a shared borrow for mutation")),
None =>
Err(format!("Location should be frozen but it is not")),
Some(loc) if loc <= since =>
Ok(None),
Some(loc) =>
Err(format!("Location should be frozen since {} but it is only frozen \
since {}", since, loc)),
},
Borrow::Mut(Mut::Raw) if self.is_frozen() && usage != UsageKind::Write =>
// Non-mutating access with a raw from a frozen location is a special case: The
// shared refs do not mind raw reads, and the raw itself does not assume any
// exclusivity. So we do not even require there to be a raw on the stack,
// the raw is instead "matched" by the fact that this location is frozen.
// This does not break the assumption that an `&mut` we own is
// exclusive for reads, because there we have the invariant that
// the location is *not* frozen.
return Ok(None),
Borrow::Mut(mut_borrow) => mut_borrow
};
// See if we can get there via popping.
for (idx, &itm) in self.borrows.iter().enumerate().rev() {
match itm {
BorStackItem::FnBarrier(_) =>
return Err(format!("Trying to reactivate a mutable borrow ({:?}) that lives \
behind a barrier", mut_borrow)),
BorStackItem::Mut(loc) => {
if loc == mut_borrow {
// We found it! This is good to know.
// Yet, maybe we do not really want to pop?
if usage == UsageKind::Read && self.is_frozen() {
// Whoever had exclusive access to this location allowed it
// to become frozen. That can only happen if they reborrowed
// to a shared ref, at which point they gave up on exclusive access.
// Hence we allow more reads, entirely ignoring everything above
// on the stack (but still making sure it is on the stack).
// This does not break the assumption that an `&mut` we own is
// exclusive for reads, because there we have the invariant that
// the location is *not* frozen.
return Ok(None);
} else {
return Ok(Some(idx));
/// `is_write` indicates whether this is being used to write (or, equivalently, to
/// borrow as &mut).
/// Returns `Err` if the answer is "no"; otherwise the return value indicates what to
/// do: With `Some(n)` you need to unfreeze, and then additionally pop `n` items.
fn reactivatable(&self, bor: Borrow, is_write: bool) -> Result<Option<usize>, String> {
// Check if we can match the frozen "item". Not possible on writes!
if !is_write {
// For now, we do NOT check the timestamp. That might be surprising, but
// we cannot even notice when a location should be frozen but is not!
// Those checks are both done in `tag_dereference`, where we have type information.
// Either way, it is crucial that the frozen "item" matches raw pointers:
// Reading through a raw should not unfreeze.
match (self.frozen_since, bor) {
(Some(_), Borrow::Shr(_)) => {
return Ok(None)
}
_ => {},
}
}
// See if we can find this borrow.
for (idx, &itm) in self.borrows.iter().rev().enumerate() {
// Check borrow and stack item for compatibility.
match (itm, bor) {
(BorStackItem::FnBarrier(_), _) => {
return Err(format!("Trying to reactivate a borrow ({:?}) that lives \
behind a barrier", bor))
}
(BorStackItem::Uniq(itm_t), Borrow::Uniq(bor_t)) if itm_t == bor_t => {
// Found matching unique item.
if !is_write {
// As a special case, if we are reading and since we *did* find the `Uniq`,
// we try to pop less: We are happy with making a `Shr` or `Frz` active;
// that one will not mind concurrent reads.
match self.reactivatable(Borrow::default(), is_write) {
// If we got something better that `idx`, use that
Ok(None) => return Ok(None),
Ok(Some(shr_idx)) if shr_idx <= idx => return Ok(Some(shr_idx)),
// Otherwise just go on.
_ => {},
}
}
return Ok(Some(idx))
}
(BorStackItem::Shr, Borrow::Shr(_)) => {
// Found matching shared/raw item.
return Ok(Some(idx))
}
// Go on looking.
_ => {}
}
}
// Nothing to be found.
Err(format!("Mutable borrow-to-reactivate ({:?}) does not exist on the stack", mut_borrow))
Err(format!("Borrow-to-reactivate {:?} does not exist on the stack", bor))
}
/// Reactive `bor` for this stack. `usage` indicates whether this is being
/// used to read/write (or, equivalently, to borrow as &/&mut), or to borrow as raw.
fn reactivate(&mut self, bor: Borrow, usage: UsageKind) -> EvalResult<'tcx> {
let action = match self.reactivatable(bor, usage) {
Ok(action) => action,
/// Reactive `bor` for this stack. `is_write` indicates whether this is being
/// used to write (or, equivalently, to borrow as &mut).
fn reactivate(&mut self, bor: Borrow, is_write: bool) -> EvalResult<'tcx> {
let mut pop = match self.reactivatable(bor, is_write) {
Ok(None) => return Ok(()),
Ok(Some(pop)) => pop,
Err(err) => return err!(MachineError(err)),
};
// Execute what `reactivatable` told us to do.
match action {
None => {}, // nothing to do
Some(top) => {
if self.frozen_since.is_some() {
trace!("reactivate: Unfreezing");
}
self.frozen_since = None;
for itm in self.borrows.drain(top+1..).rev() {
trace!("reactivate: Popping {:?}", itm);
}
}
// Pop what `reactivatable` told us to pop. Always unfreeze.
if self.is_frozen() {
trace!("reactivate: Unfreezing");
}
self.frozen_since = None;
while pop > 0 {
let itm = self.borrows.pop().unwrap();
trace!("reactivate: Popping {:?}", itm);
pop -= 1;
}
Ok(())
}
/// Initiate `bor`; mostly this means freezing or pushing.
/// Initiate `bor`; mostly this means pushing.
/// This operation cannot fail; it is up to the caller to ensure that the precondition
/// is met: We cannot push onto frozen stacks.
fn initiate(&mut self, bor: Borrow) {
match bor {
Borrow::Frz(t) => {
match self.frozen_since {
None => {
trace!("initiate: Freezing");
self.frozen_since = Some(t);
}
Some(since) => {
trace!("initiate: Already frozen");
assert!(since <= t);
}
}
}
Borrow::Mut(m) => {
match self.frozen_since {
None => {
trace!("initiate: Pushing {:?}", bor);
self.borrows.push(BorStackItem::Mut(m))
}
Some(_) if m.is_raw() =>
// We only ever initiate right after activating the ref we come from.
// If the source ref is fine being frozen, then a raw ref we create
// from it is fine with this as well.
trace!("initiate: Initiating a raw on a frozen location, not doing a thing"),
Some(_) =>
bug!("Trying to mutate frozen location")
}
if let Some(_) = self.frozen_since {
// "Pushing" a Shr or Frz on top is redundant.
match bor {
Borrow::Uniq(_) => bug!("Trying to create unique ref to frozen location"),
Borrow::Shr(_) => trace!("initiate: New shared ref to frozen location is a NOP"),
}
} else {
// Just push.
let itm = match bor {
Borrow::Uniq(t) => BorStackItem::Uniq(t),
Borrow::Shr(_) if *self.borrows.last().unwrap() == BorStackItem::Shr => {
// Optimization: Don't push a Shr onto a Shr.
trace!("initiate: New shared ref to already shared location is a NOP");
return
},
Borrow::Shr(_) => BorStackItem::Shr,
};
trace!("initiate: Pushing {:?}", itm);
self.borrows.push(itm)
}
}
/// Check if this location is "frozen enough".
fn check_frozen(&self, bor_t: Timestamp) -> EvalResult<'tcx> {
let frozen = self.frozen_since.map_or(false, |itm_t| itm_t <= bor_t);
if !frozen {
err!(MachineError(format!("Location is not frozen long enough")))
} else {
Ok(())
}
}
/// Freeze this location, since `bor_t`.
fn freeze(&mut self, bor_t: Timestamp) {
if let Some(itm_t) = self.frozen_since {
assert!(itm_t <= bor_t, "Trying to freeze shorter than it was frozen?");
} else {
trace!("Freezing");
self.frozen_since = Some(bor_t);
}
}
}
@ -288,7 +272,7 @@ fn increment_clock(&mut self) -> Timestamp {
/// Higher-level operations
impl<'tcx> Stacks {
/// The single most operation: Make sure that using `ptr` as `usage` is okay,
/// The single most important operation: Make sure that using `ptr` is okay,
/// and if `new_bor` is present then make that the new current borrow.
fn use_and_maybe_re_borrow(
&self,
@ -301,15 +285,65 @@ fn use_and_maybe_re_borrow(
ptr.tag, usage, new_bor, ptr, size.bytes());
let mut stacks = self.stacks.borrow_mut();
for stack in stacks.iter_mut(ptr.offset, size) {
stack.reactivate(ptr.tag, usage)?;
stack.reactivate(ptr.tag, usage == UsageKind::Write)?;
if let Some(new_bor) = new_bor {
stack.initiate(new_bor);
}
}
Ok(())
}
/// Freeze the given memory range.
fn freeze(
&self,
ptr: Pointer<Borrow>,
size: Size,
bor_t: Timestamp
) -> EvalResult<'tcx> {
let mut stacks = self.stacks.borrow_mut();
for stack in stacks.iter_mut(ptr.offset, size) {
stack.freeze(bor_t);
}
Ok(())
}
/// Check that this stack is fine with being dereferenced
fn check_deref(
&self,
ptr: Pointer<Borrow>,
size: Size,
) -> EvalResult<'tcx> {
let mut stacks = self.stacks.borrow_mut();
// We need `iter_mut` because `iter` would skip gaps!
for stack in stacks.iter_mut(ptr.offset, size) {
// Conservatively assume we will just read
if let Err(err) = stack.reactivatable(ptr.tag, /*is_write*/false) {
return err!(MachineError(format!(
"Encountered reference with non-reactivatable tag: {}",
err
)))
}
}
Ok(())
}
/// Check that this stack is appropriately frozen
fn check_frozen(
&self,
ptr: Pointer<Borrow>,
size: Size,
bor_t: Timestamp
) -> EvalResult<'tcx> {
let mut stacks = self.stacks.borrow_mut();
for stack in stacks.iter_mut(ptr.offset, size) {
stack.check_frozen(bor_t)?;
}
Ok(())
}
}
/// Hooks and glue
impl<'tcx> Stacks {
#[inline(always)]
pub fn memory_read(
&self,
@ -340,34 +374,34 @@ pub fn memory_deallocated(
// FIXME: Error out of there are any barriers?
}
/// Pushes the first borrow to the stacks, must be a mutable one.
pub fn first_borrow(
/// Pushes the first item to the stacks.
pub fn first_item(
&mut self,
mut_borrow: Mut,
itm: BorStackItem,
size: Size
) {
assert!(!itm.is_fn_barrier());
for stack in self.stacks.get_mut().iter_mut(Size::ZERO, size) {
assert!(stack.borrows.len() == 1 && stack.frozen_since.is_none());
assert_eq!(stack.borrows.pop().unwrap(), BorStackItem::Mut(Mut::Raw));
stack.borrows.push(BorStackItem::Mut(mut_borrow));
assert!(stack.borrows.len() == 1);
assert_eq!(stack.borrows.pop().unwrap(), BorStackItem::Shr);
stack.borrows.push(itm);
}
}
}
pub trait EvalContextExt<'tcx> {
fn tag_reference(
&mut self,
ptr: Pointer<Borrow>,
pointee_ty: Ty<'tcx>,
place: MPlaceTy<'tcx, Borrow>,
size: Size,
usage: UsageKind,
) -> EvalResult<'tcx, Borrow>;
fn tag_dereference(
&self,
ptr: Pointer<Borrow>,
pointee_ty: Ty<'tcx>,
place: MPlaceTy<'tcx, Borrow>,
size: Size,
usage: UsageKind,
) -> EvalResult<'tcx, Borrow>;
@ -385,40 +419,36 @@ fn retag(
) -> EvalResult<'tcx>;
}
impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, 'tcx> {
impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> {
/// Called for place-to-value conversion.
fn tag_reference(
&mut self,
ptr: Pointer<Borrow>,
pointee_ty: Ty<'tcx>,
place: MPlaceTy<'tcx, Borrow>,
size: Size,
usage: UsageKind,
) -> EvalResult<'tcx, Borrow> {
let ptr = place.ptr.to_ptr()?;
let time = self.machine.stacked_borrows.increment_clock();
let new_bor = match usage {
UsageKind::Write => Borrow::Mut(Mut::Uniq(time)),
UsageKind::Read =>
// FIXME This does not do enough checking when only part of the data has
// interior mutability. When the type is `(i32, Cell<i32>)`, we want the
// first field to be frozen but not the second.
if self.type_is_freeze(pointee_ty) {
Borrow::Frz(time)
} else {
// Shared reference with interior mutability.
Borrow::Mut(Mut::Raw)
},
UsageKind::Raw => Borrow::Mut(Mut::Raw),
UsageKind::Write => Borrow::Uniq(time),
UsageKind::Read => Borrow::Shr(Some(time)),
UsageKind::Raw => Borrow::Shr(None),
};
trace!("tag_reference: Creating new reference ({:?}) for {:?} (pointee {}, size {}): {:?}",
usage, ptr, pointee_ty, size.bytes(), new_bor);
trace!("tag_reference: Creating new reference ({:?}) for {:?} (pointee {}): {:?}",
usage, ptr, place.layout.ty, new_bor);
// Make sure this reference is not dangling or so
// Update the stacks. First create the new ref as usual, then maybe freeze stuff.
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.use_and_maybe_re_borrow(ptr, size, usage, Some(new_bor))?;
// Maybe freeze stuff
if let Borrow::Shr(Some(bor_t)) = new_bor {
self.visit_frozen(place, size, |frz_ptr, size| {
debug_assert_eq!(frz_ptr.alloc_id, ptr.alloc_id);
// Be frozen!
alloc.extra.freeze(frz_ptr, size, bor_t)
})?;
}
Ok(new_bor)
}
@ -429,13 +459,13 @@ fn tag_reference(
/// We could be in the middle of `&(*var).1`.
fn tag_dereference(
&self,
ptr: Pointer<Borrow>,
pointee_ty: Ty<'tcx>,
place: MPlaceTy<'tcx, Borrow>,
size: Size,
usage: UsageKind,
) -> EvalResult<'tcx, Borrow> {
trace!("tag_reference: Accessing reference ({:?}) for {:?} (pointee {}, size {})",
usage, ptr, pointee_ty, size.bytes());
let ptr = place.ptr.to_ptr()?;
trace!("tag_dereference: Accessing reference ({:?}) for {:?} (pointee {})",
usage, ptr, place.layout.ty);
// In principle we should not have to do anything here. However, with transmutes involved,
// it can happen that the tag of `ptr` does not actually match `usage`, and we
// should adjust for that.
@ -446,27 +476,24 @@ fn tag_dereference(
// Don't use the tag, this is a raw access! Even if there is a tag,
// that means transmute happened and we ignore the tag.
// Also don't do any further validation, this is raw after all.
return Ok(Borrow::Mut(Mut::Raw));
return Ok(Borrow::default());
}
(UsageKind::Write, Borrow::Mut(Mut::Uniq(_))) |
(UsageKind::Read, Borrow::Frz(_)) |
(UsageKind::Read, Borrow::Mut(Mut::Raw)) => {
(UsageKind::Write, Borrow::Uniq(_)) |
(UsageKind::Read, Borrow::Shr(_)) => {
// Expected combinations. Nothing to do.
// FIXME: We probably shouldn't accept this if we got a raw shr without
// interior mutability.
}
(UsageKind::Write, Borrow::Mut(Mut::Raw)) => {
(UsageKind::Write, Borrow::Shr(None)) => {
// Raw transmuted to mut ref. Keep this as raw access.
// We cannot reborrow here; there might be a raw in `&(*var).1` where
// `var` is an `&mut`. The other field of the struct might be already frozen,
// also using `var`, and that would be okay.
}
(UsageKind::Read, Borrow::Mut(Mut::Uniq(_))) => {
(UsageKind::Read, Borrow::Uniq(_)) => {
// A mut got transmuted to shr. Can happen even from compiler transformations:
// `&*x` gets optimized to `x` even when `x` is a `&mut`.
}
(UsageKind::Write, Borrow::Frz(_)) => {
// This is just invalid.
(UsageKind::Write, Borrow::Shr(Some(_))) => {
// This is just invalid: A shr got transmuted to a mut.
// If we ever allow this, we have to consider what we do when a turn a
// `Raw`-tagged `&mut` into a raw pointer pointing to a frozen location.
// We probably do not want to allow that, but we have to allow
@ -474,23 +501,21 @@ fn tag_dereference(
return err!(MachineError(format!("Encountered mutable reference with frozen tag {:?}", ptr.tag)))
}
}
// Even if we don't touch the tag, this operation is only okay if we *could*
// activate it. Also it must not be dangling.
// If we got here, we do some checking, *but* we leave the tag unchanged.
self.memory().check_bounds(ptr, size, false)?;
let alloc = self.memory().get(ptr.alloc_id).expect("We checked that the ptr is fine!");
let mut stacks = alloc.extra.stacks.borrow_mut();
// We need `iter_mut` because `iter` would skip gaps!
for stack in stacks.iter_mut(ptr.offset, size) {
// Conservatively assume that we will only read.
if let Err(err) = stack.reactivatable(ptr.tag, UsageKind::Read) {
return err!(MachineError(format!(
"Encountered {} reference with non-reactivatable tag: {}",
if usage == UsageKind::Write { "mutable" } else { "shared" },
err
)))
}
alloc.extra.check_deref(ptr, size)?;
// Maybe check frozen stuff
if let Borrow::Shr(Some(bor_t)) = ptr.tag {
self.visit_frozen(place, size, |frz_ptr, size| {
debug_assert_eq!(frz_ptr.alloc_id, ptr.alloc_id);
// Are you frozen?
alloc.extra.check_frozen(frz_ptr, size, bor_t)
})?;
}
// All is good.
// All is good, and do not change the tag
Ok(ptr.tag)
}
@ -499,7 +524,7 @@ fn tag_new_allocation(
id: AllocId,
kind: MemoryKind<MiriMemoryKind>,
) -> Borrow {
let mut_borrow = match kind {
let time = match kind {
MemoryKind::Stack => {
// New unique borrow. This `Uniq` is not accessible by the program,
// so it will only ever be used when using the local directly (i.e.,
@ -509,19 +534,18 @@ fn tag_new_allocation(
// `reset` which the blog post [1] says to perform when accessing a local.
//
// [1] https://www.ralfj.de/blog/2018/08/07/stacked-borrows.html
let time = self.machine.stacked_borrows.increment_clock();
Mut::Uniq(time)
self.machine.stacked_borrows.increment_clock()
}
_ => {
// Raw for everything else
Mut::Raw
// Nothing to do for everything else
return Borrow::default()
}
};
// Make this the active borrow for this allocation
let alloc = self.memory_mut().get_mut(id).expect("This is a new allocation, it must still exist");
let size = Size::from_bytes(alloc.bytes.len() as u64);
alloc.extra.first_borrow(mut_borrow, size);
Borrow::Mut(mut_borrow)
alloc.extra.first_item(BorStackItem::Uniq(time), size);
Borrow::Uniq(time)
}
fn retag(

View File

@ -133,7 +133,7 @@ fn fetch_tls_dtor(
}
}
impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, 'tcx> {
impl<'a, 'mir, 'tcx> 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

View File

@ -14,6 +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 reference with non-reactivatable tag
v1[1] = 5; //~ ERROR does not exist on the stack
v1[1] = 6;
}

View File

@ -11,7 +11,7 @@ pub fn split_at_mut<T>(self_: &mut [T], mid: usize) -> (&mut [T], &mut [T]) {
assert!(mid <= len);
(from_raw_parts_mut(ptr, len - mid), // BUG: should be "mid" instead of "len - mid"
//~^ ERROR reference with non-reactivatable tag
//~^ ERROR does not exist on the stack
from_raw_parts_mut(ptr.offset(mid as isize), len - mid))
}
}

View File

@ -7,7 +7,7 @@ fn main() {
let xref = unsafe { &mut *xraw }; // derived from raw, so using raw is still okay...
callee(xraw);
let _val = *xref; // ...but any use of raw will invalidate our ref.
//~^ ERROR: mutable reference with non-reactivatable tag
//~^ ERROR: does not exist on the stack
}
fn callee(xraw: *mut i32) {

View File

@ -7,7 +7,7 @@ fn main() {
let xref = unsafe { &mut *xraw }; // derived from raw, so using raw is still okay...
callee(xraw);
let _val = *xref; // ...but any use of raw will invalidate our ref.
//~^ ERROR: mutable reference with non-reactivatable tag
//~^ ERROR: does not exist on the stack
}
fn callee(xraw: *mut i32) {

View File

@ -8,5 +8,5 @@ fn main() {
let target = Box::new(42); // has an implicit raw
let ref_ = &*target;
evil(ref_); // invalidates shared ref, activates raw
let _x = *ref_; //~ ERROR reference with non-reactivatable tag
let _x = *ref_; //~ ERROR is not frozen long enough
}

View File

@ -3,6 +3,6 @@ fn main() {
// Make sure raw ptr with raw tag cannot mutate frozen location without breaking the shared ref.
let r#ref = &target; // freeze
let ptr = r#ref as *const _ as *mut _; // raw ptr, with raw tag
unsafe { *ptr = 42; } //~ ERROR does not exist on the stack
let _val = *r#ref;
unsafe { *ptr = 42; }
let _val = *r#ref; //~ ERROR is not frozen long enough
}

View File

@ -9,5 +9,5 @@ fn main() {
let ptr = reference as *const _ as *mut i32; // raw ptr, with raw tag
let _mut_ref: &mut i32 = unsafe { mem::transmute(ptr) }; // &mut, with raw tag
// Now we retag, making our ref top-of-stack -- and, in particular, unfreezing.
let _val = *reference; //~ ERROR Location should be frozen
let _val = *reference; //~ ERROR is not frozen long enough
}

View File

@ -5,5 +5,5 @@ fn main() {
let xref = unsafe { &mut *xraw };
let xref_in_mem = Box::new(xref);
let _val = *x; // invalidate xraw
let _val = *xref_in_mem; //~ ERROR mutable reference with non-reactivatable tag
let _val = *xref_in_mem; //~ ERROR does not exist on the stack
}

View File

@ -5,5 +5,5 @@ fn main() {
let xref = unsafe { &*xraw };
let xref_in_mem = Box::new(xref);
*x = 42; // invalidate xraw
let _val = *xref_in_mem; //~ ERROR shared reference with non-reactivatable tag: Location should be frozen
let _val = *xref_in_mem; //~ ERROR does not exist on the stack
}

View File

@ -6,5 +6,5 @@ fn main() {
let xraw = x as *mut _;
let xref = unsafe { &mut *xraw };
let _val = *x; // invalidate xraw
foo(xref); //~ ERROR mutable reference with non-reactivatable tag
foo(xref); //~ ERROR does not exist on the stack
}

View File

@ -6,5 +6,5 @@ fn main() {
let xraw = &*x as *const _;
let xref = unsafe { &*xraw };
*x = 42; // invalidate xraw
foo(xref); //~ ERROR shared reference with non-reactivatable tag: Location should be frozen
foo(xref); //~ ERROR does not exist on the stack
}

View File

@ -3,7 +3,7 @@ fn foo(x: &mut (i32, i32)) -> &mut i32 {
let xraw = x as *mut (i32, i32);
let ret = unsafe { &mut (*xraw).1 };
let _val = *x; // invalidate xraw and its children
ret //~ ERROR mutable reference with non-reactivatable tag
ret //~ ERROR does not exist on the stack
}
fn main() {

View File

@ -3,7 +3,7 @@ fn foo(x: &mut (i32, i32)) -> &i32 {
let xraw = x as *mut (i32, i32);
let ret = unsafe { &(*xraw).1 };
x.1 = 42; // invalidate xraw on the 2nd field
ret //~ ERROR shared reference with non-reactivatable tag: Location should be frozen
ret //~ ERROR does not exist on the stack
}
fn main() {

View File

@ -1,29 +0,0 @@
// Optimization kills all the reborrows, enough to make this error go away. There are
// no retags either because we don't retag immediately after a `&[mut]`; we rely on
// that creating a fresh reference.
// See `shared_confusion_opt.rs` for a variant that is caught even with optimizations.
// Keep this test to make sure that without optimizations, we do not have to actually
// use the `x_inner_shr`.
// compile-flags: -Zmir-opt-level=0
#![allow(unused_variables)]
use std::cell::RefCell;
fn test(r: &mut RefCell<i32>) {
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_shr = &*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 reference with non-reactivatable tag
}
fn main() {
test(&mut RefCell::new(0));
}

View File

@ -1,25 +0,0 @@
// A variant of `shared_confusion.rs` that gets flagged even with optimizations.
#![allow(unused_variables)]
use std::cell::RefCell;
fn test(r: &mut RefCell<i32>) {
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 _val = *x_inner_shr;
let y = &*r; // outer ref, not freezing
let x_inner_shr = &*x_inner; // freezing again
let _val = *x_inner_shr;
}
// 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 reference with non-reactivatable tag
}
fn main() {
test(&mut RefCell::new(0));
}

View File

@ -1,3 +1,3 @@
fn main() {
let _b = unsafe { std::mem::transmute::<u8, bool>(2) }; //~ ERROR encountered 2, but expected something in the range 0..=1
let _b = unsafe { std::mem::transmute::<u8, bool>(2) }; //~ ERROR encountered 2, but expected something less or equal to 1
}

View File

@ -1,6 +1,6 @@
fn main() {
assert!(std::char::from_u32(-1_i32 as u32).is_none());
let _ = match unsafe { std::mem::transmute::<i32, char>(-1) } { //~ ERROR encountered 4294967295, but expected something in the range 0..=1114111
let _ = match unsafe { std::mem::transmute::<i32, char>(-1) } { //~ ERROR encountered 4294967295, but expected something less or equal to 1114111
'a' => {true},
'b' => {false},
_ => {true},

View File

@ -4,5 +4,5 @@ pub enum Foo {
}
fn main() {
let _f = unsafe { std::mem::transmute::<i32, Foo>(42) }; //~ ERROR encountered invalid enum discriminant 42
let _f = unsafe { std::mem::transmute::<i32, Foo>(42) }; //~ ERROR encountered 42, but expected a valid enum discriminant
}

View File

@ -12,5 +12,5 @@ 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
//~^ ERROR encountered 44, but expected a valid enum discriminant
}

View File

@ -8,9 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// FIXME: Still investigating whether there is UB here
// compile-flags: -Zmiri-disable-validation
use std::sync::Mutex;
fn par_for<I, F>(iter: I, f: F)

View File

@ -8,9 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// FIXME: Still investigating whether there is UB here
// compile-flags: -Zmiri-disable-validation
fn b<T>(t: T) -> T { t }
fn main() {

View File

@ -1,6 +1,3 @@
// FIXME: Still investigating whether there is UB here
// compile-flags: -Zmiri-disable-validation
use std::collections::VecDeque;
fn main() {

View File

@ -1,6 +1,3 @@
// FIXME: Still investigating whether there is UB here
// compile-flags: -Zmiri-disable-validation
use std::slice;
fn slice_of_zst() {

View File

@ -1,7 +1,8 @@
// Test various stacked-borrows-related things.
fn main() {
deref_partially_dangling_raw();
read_does_not_invalidate();
read_does_not_invalidate1();
read_does_not_invalidate2();
}
// Deref a raw ptr to access a field of a large struct, where the field
@ -15,13 +16,23 @@ fn deref_partially_dangling_raw() {
// Make sure that reading from an `&mut` does, like reborrowing to `&`,
// NOT invalidate other reborrows.
fn read_does_not_invalidate() {
fn read_does_not_invalidate1() {
fn foo(x: &mut (i32, i32)) -> &i32 {
let xraw = x as *mut (i32, i32);
let ret = unsafe { &(*xraw).1 };
let _val = x.1; // we just read, this does NOT invalidate the reborrows.
ret
}
foo(&mut (1, 2));
}
// Same as above, but this time we first create a raw, then read from `&mut`
// and then freeze from the raw.
fn read_does_not_invalidate2() {
fn foo(x: &mut (i32, i32)) -> &i32 {
let xraw = x as *mut (i32, i32);
let _val = x.1; // we just read, this does NOT invalidate the raw reborrow.
let ret = unsafe { &(*xraw).1 };
ret
}
foo(&mut (1, 2));
}