Skip to content

Commit

Permalink
Auto merge of #70598 - vakaras:add-threads-cr3, r=oli-obk,RalfJung
Browse files Browse the repository at this point in the history
Make the necessary changes to support concurrency in Miri.

This pull request makes the necessary changes to the Rust compiler to allow Miri to support concurrency:

1.  Move stack from the interpretation context (`InterpCx`) to machine, so that the machine can switch the stacks when it changes the thread being executed.
2.  Add the callbacks that allow the machine to generate fresh allocation ids for each thread local allocation and to translate them back to original allocations when needed. This allows the machine to ensure the property that allocation ids are unique, which allows using a simpler representation of the memory.

r? @oli-obk

cc @RalfJung
  • Loading branch information
bors committed Apr 19, 2020
2 parents dbf8b6b + cbd9222 commit c6b55ee
Show file tree
Hide file tree
Showing 17 changed files with 172 additions and 70 deletions.
2 changes: 1 addition & 1 deletion src/librustc_mir/const_eval/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl Error for ConstEvalErrKind {}
/// Turn an interpreter error into something to report to the user.
/// As a side-effect, if RUSTC_CTFE_BACKTRACE is set, this prints the backtrace.
/// Should be called only if the error is actually going to to be reported!
pub fn error_to_const_error<'mir, 'tcx, M: Machine<'mir, 'tcx>>(
pub fn error_to_const_error<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>(
ecx: &InterpCx<'mir, 'tcx, M>,
mut error: InterpErrorInfo<'tcx>,
) -> ConstEvalErr<'tcx> {
Expand Down
30 changes: 24 additions & 6 deletions src/librustc_mir/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::interpret::{

use super::error::*;

impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter> {
impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>> {
/// Evaluate a const function where all arguments (if any) are zero-sized types.
/// The evaluation is memoized thanks to the query system.
///
Expand Down Expand Up @@ -86,12 +86,15 @@ impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter> {
}

/// Extra machine state for CTFE, and the Machine instance
pub struct CompileTimeInterpreter {
pub struct CompileTimeInterpreter<'mir, 'tcx> {
/// For now, the number of terminators that can be evaluated before we throw a resource
/// exhuastion error.
///
/// Setting this to `0` disables the limit and allows the interpreter to run forever.
pub steps_remaining: usize,

/// The virtual call stack.
pub(crate) stack: Vec<Frame<'mir, 'tcx, (), ()>>,
}

#[derive(Copy, Clone, Debug)]
Expand All @@ -100,9 +103,9 @@ pub struct MemoryExtra {
pub(super) can_access_statics: bool,
}

impl CompileTimeInterpreter {
impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {
pub(super) fn new(const_eval_limit: usize) -> Self {
CompileTimeInterpreter { steps_remaining: const_eval_limit }
CompileTimeInterpreter { steps_remaining: const_eval_limit, stack: Vec::new() }
}
}

Expand Down Expand Up @@ -156,7 +159,8 @@ impl<K: Hash + Eq, V> interpret::AllocMap<K, V> for FxHashMap<K, V> {
}
}

crate type CompileTimeEvalContext<'mir, 'tcx> = InterpCx<'mir, 'tcx, CompileTimeInterpreter>;
crate type CompileTimeEvalContext<'mir, 'tcx> =
InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>;

impl interpret::MayLeak for ! {
#[inline(always)]
Expand All @@ -166,7 +170,7 @@ impl interpret::MayLeak for ! {
}
}

impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter {
impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, 'tcx> {
type MemoryKind = !;
type PointerTag = ();
type ExtraFnVal = !;
Expand Down Expand Up @@ -349,6 +353,20 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter {
Ok(frame)
}

#[inline(always)]
fn stack(
ecx: &'a InterpCx<'mir, 'tcx, Self>,
) -> &'a [Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>] {
&ecx.machine.stack
}

#[inline(always)]
fn stack_mut(
ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
) -> &'a mut Vec<Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>> {
&mut ecx.machine.stack
}

fn before_access_global(
memory_extra: &MemoryExtra,
alloc_id: AllocId,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_middle::ty::{self, Ty, TypeAndMut, TypeFoldable};
use rustc_span::symbol::sym;
use rustc_target::abi::{LayoutOf, Size, Variants};

impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
pub fn cast(
&mut self,
src: OpTy<'tcx, M::PointerTag>,
Expand Down
57 changes: 32 additions & 25 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ use crate::util::storage::AlwaysLiveLocals;

pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> {
/// Stores the `Machine` instance.
///
/// Note: the stack is provided by the machine.
pub machine: M,

/// The results of the type checker, from rustc.
Expand All @@ -39,9 +41,6 @@ pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> {
/// The virtual memory system.
pub memory: Memory<'mir, 'tcx, M>,

/// The virtual call stack.
pub(crate) stack: Vec<Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>>,

/// A cache for deduplicating vtables
pub(super) vtables:
FxHashMap<(Ty<'tcx>, Option<ty::PolyExistentialTraitRef<'tcx>>), Pointer<M::PointerTag>>,
Expand Down Expand Up @@ -297,7 +296,7 @@ pub(super) fn from_known_layout<'tcx>(
}
}

impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
pub fn new(
tcx: TyCtxtAt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
Expand All @@ -309,7 +308,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
tcx,
param_env,
memory: Memory::new(tcx, memory_extra),
stack: Vec::new(),
vtables: FxHashMap::default(),
}
}
Expand Down Expand Up @@ -349,24 +347,32 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

#[inline(always)]
pub fn stack(&self) -> &[Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>] {
&self.stack
pub(crate) fn stack(&self) -> &[Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>] {
M::stack(self)
}

#[inline(always)]
pub(crate) fn stack_mut(
&mut self,
) -> &mut Vec<Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>> {
M::stack_mut(self)
}

#[inline(always)]
pub fn cur_frame(&self) -> usize {
assert!(!self.stack.is_empty());
self.stack.len() - 1
pub fn frame_idx(&self) -> usize {
let stack = self.stack();
assert!(!stack.is_empty());
stack.len() - 1
}

#[inline(always)]
pub fn frame(&self) -> &Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra> {
self.stack.last().expect("no call frames exist")
self.stack().last().expect("no call frames exist")
}

#[inline(always)]
pub fn frame_mut(&mut self) -> &mut Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra> {
self.stack.last_mut().expect("no call frames exist")
self.stack_mut().last_mut().expect("no call frames exist")
}

#[inline(always)]
Expand Down Expand Up @@ -596,8 +602,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
return_place: Option<PlaceTy<'tcx, M::PointerTag>>,
return_to_block: StackPopCleanup,
) -> InterpResult<'tcx> {
if !self.stack.is_empty() {
info!("PAUSING({}) {}", self.cur_frame(), self.frame().instance);
if !self.stack().is_empty() {
info!("PAUSING({}) {}", self.frame_idx(), self.frame().instance);
}
::log_settings::settings().indentation += 1;

Expand All @@ -615,7 +621,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
extra: (),
};
let frame = M::init_frame_extra(self, pre_frame)?;
self.stack.push(frame);
self.stack_mut().push(frame);

// don't allocate at all for trivial constants
if body.local_decls.len() > 1 {
Expand Down Expand Up @@ -648,9 +654,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

M::after_stack_push(self)?;
info!("ENTERING({}) {}", self.cur_frame(), self.frame().instance);
info!("ENTERING({}) {}", self.frame_idx(), self.frame().instance);

if self.stack.len() > *self.tcx.sess.recursion_limit.get() {
if self.stack().len() > *self.tcx.sess.recursion_limit.get() {
throw_exhaust!(StackFrameLimitReached)
} else {
Ok(())
Expand Down Expand Up @@ -705,7 +711,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
pub(super) fn pop_stack_frame(&mut self, unwinding: bool) -> InterpResult<'tcx> {
info!(
"LEAVING({}) {} (unwinding = {})",
self.cur_frame(),
self.frame_idx(),
self.frame().instance,
unwinding
);
Expand All @@ -720,7 +726,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
);

::log_settings::settings().indentation -= 1;
let frame = self.stack.pop().expect("tried to pop a stack frame, but there were none");
let frame =
self.stack_mut().pop().expect("tried to pop a stack frame, but there were none");

// Now where do we jump next?

Expand All @@ -735,7 +742,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
};

if !cleanup {
assert!(self.stack.is_empty(), "only the topmost frame should ever be leaked");
assert!(self.stack().is_empty(), "only the topmost frame should ever be leaked");
assert!(next_block.is_none(), "tried to skip cleanup when we have a next block!");
assert!(!unwinding, "tried to skip cleanup during unwinding");
// Leak the locals, skip validation, skip machine hook.
Expand Down Expand Up @@ -784,10 +791,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}

if !self.stack.is_empty() {
if !self.stack().is_empty() {
info!(
"CONTINUING({}) {} (unwinding = {})",
self.cur_frame(),
self.frame_idx(),
self.frame().instance,
unwinding
);
Expand Down Expand Up @@ -895,12 +902,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Place::Local { frame, local } => {
let mut allocs = Vec::new();
let mut msg = format!("{:?}", local);
if frame != self.cur_frame() {
write!(msg, " ({} frames up)", self.cur_frame() - frame).unwrap();
if frame != self.frame_idx() {
write!(msg, " ({} frames up)", self.frame_idx() - frame).unwrap();
}
write!(msg, ":").unwrap();

match self.stack[frame].locals[local].value {
match self.stack()[frame].locals[local].value {
LocalValue::Dead => write!(msg, " is dead").unwrap(),
LocalValue::Uninitialized => write!(msg, " is uninitialized").unwrap(),
LocalValue::Live(Operand::Indirect(mplace)) => match mplace.ptr {
Expand Down
7 changes: 5 additions & 2 deletions src/librustc_mir/interpret/intern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> InternVisitor<'rt, 'mir
}
}

impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
for InternVisitor<'rt, 'mir, 'tcx, M>
{
type V = MPlaceTy<'tcx>;
Expand Down Expand Up @@ -286,7 +286,10 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
intern_kind: InternKind,
ret: MPlaceTy<'tcx>,
ignore_interior_mut_in_const_validation: bool,
) -> InterpResult<'tcx> {
) -> InterpResult<'tcx>
where
'tcx: 'mir,
{
let tcx = ecx.tcx;
let (base_mutability, base_intern_mode) = match intern_kind {
// `static mut` doesn't care about interior mutability, it's mutable anyway
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ crate fn eval_nullary_intrinsic<'tcx>(
})
}

impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// Returns `true` if emulation happened.
pub fn emulate_intrinsic(
&mut self,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/intrinsics/caller_location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ use crate::interpret::{
MPlaceTy, MemoryKind, Scalar,
};

impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// Walks up the callstack from the intrinsic's callsite, searching for the first callsite in a
/// frame which is not `#[track_caller]`.
crate fn find_closest_untracked_caller_location(&self) -> Span {
self.stack
self.stack()
.iter()
.rev()
// Find first non-`#[track_caller]` frame.
Expand Down
38 changes: 38 additions & 0 deletions src/librustc_mir/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,34 @@ pub trait Machine<'mir, 'tcx>: Sized {
id
}

/// Called when converting a `ty::Const` to an operand (in
/// `eval_const_to_op`).
///
/// Miri uses this callback for creating per thread allocations for thread
/// locals. In Rust, one way of creating a thread local is by marking a
/// static with `#[thread_local]`. On supported platforms this gets
/// translated to a LLVM thread local for which LLVM automatically ensures
/// that each thread gets its own copy. Since LLVM automatically handles
/// thread locals, the Rust compiler just treats thread local statics as
/// regular statics even though accessing a thread local static should be an
/// effectful computation that depends on the current thread. The long term
/// plan is to change MIR to make accesses to thread locals explicit
/// (https://github.com/rust-lang/rust/issues/70685). While the issue 70685
/// is not fixed, our current workaround in Miri is to use this function to
/// make per-thread copies of thread locals. Please note that we cannot make
/// these copies in `canonical_alloc_id` because that is too late: for
/// example, if one created a pointer in thread `t1` to a thread local and
/// sent it to another thread `t2`, resolving the access in
/// `canonical_alloc_id` would result in pointer pointing to `t2`'s thread
/// local and not `t1` as it should.
#[inline]
fn adjust_global_const(
_ecx: &InterpCx<'mir, 'tcx, Self>,
val: mir::interpret::ConstValue<'tcx>,
) -> InterpResult<'tcx, mir::interpret::ConstValue<'tcx>> {
Ok(val)
}

/// Called to initialize the "extra" state of an allocation and make the pointers
/// it contains (in relocations) tagged. The way we construct allocations is
/// to always first construct it without extra and then add the extra.
Expand Down Expand Up @@ -285,6 +313,16 @@ pub trait Machine<'mir, 'tcx>: Sized {
frame: Frame<'mir, 'tcx, Self::PointerTag>,
) -> InterpResult<'tcx, Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>>;

/// Borrow the current thread's stack.
fn stack(
ecx: &'a InterpCx<'mir, 'tcx, Self>,
) -> &'a [Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>];

/// Mutably borrow the current thread's stack.
fn stack_mut(
ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
) -> &'a mut Vec<Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>>;

/// Called immediately after a stack frame got pushed and its locals got initialized.
fn after_stack_push(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
Ok(())
Expand Down
14 changes: 12 additions & 2 deletions src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ impl<'tcx, Tag: Copy> ImmTy<'tcx, Tag> {
}
}

impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// Normalice `place.ptr` to a `Pointer` if this is a place and not a ZST.
/// Can be helpful to avoid lots of `force_ptr` calls later, if this place is used a lot.
#[inline]
Expand Down Expand Up @@ -440,7 +440,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
let op = match *place {
Place::Ptr(mplace) => Operand::Indirect(mplace),
Place::Local { frame, local } => *self.access_local(&self.stack[frame], local, None)?,
Place::Local { frame, local } => {
*self.access_local(&self.stack()[frame], local, None)?
}
};
Ok(OpTy { op, layout: place.layout })
}
Expand Down Expand Up @@ -528,6 +530,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// potentially requiring the current static to be evaluated again. This is not a
// problem here, because we are building an operand which means an actual read is
// happening.
//
// The machine callback `adjust_global_const` below is guaranteed to
// be called for all constants because `const_eval` calls
// `eval_const_to_op` recursively.
return Ok(self.const_eval(GlobalId { instance, promoted }, val.ty)?);
}
ty::ConstKind::Infer(..)
Expand All @@ -537,6 +543,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
ty::ConstKind::Value(val_val) => val_val,
};
// This call allows the machine to create fresh allocation ids for
// thread-local statics (see the `adjust_global_const` function
// documentation).
let val_val = M::adjust_global_const(self, val_val)?;
// Other cases need layout.
let layout = from_known_layout(self.tcx, layout, || self.layout_of(val.ty))?;
let op = match val_val {
Expand Down
Loading

0 comments on commit c6b55ee

Please sign in to comment.