Encapsulate interpreter stack(s) a bit better

Author: saethlin

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -18,7 +18,7 @@ use rustc_target::spec::abi::Abi as CallAbi;
 
 use crate::interpret::{
     self, compile_time_machine, AllocId, ConstAllocation, Frame, ImmTy, InterpCx, InterpResult,
-    OpTy, PlaceTy, Pointer, Scalar, StackPopUnwind,
+    OpTy, Operand, PlaceTy, Pointer, Scalar, StackPopUnwind,
 };
 
 use super::error::*;
@@ -471,11 +471,40 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
         &ecx.machine.stack
     }
 
-    #[inline(always)]
-    fn stack_mut<'a>(
+    fn push_frame(
+        ecx: &mut InterpCx<'mir, 'tcx, Self>,
+        frame: Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>,
+    ) {
+        ecx.machine.stack.push(frame);
+    }
+
+    fn pop_frame(
+        ecx: &mut InterpCx<'mir, 'tcx, Self>,
+    ) -> Option<Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>> {
+        ecx.machine.stack.pop()
+    }
+
+    fn frame<'a>(
+        ecx: &'a InterpCx<'mir, 'tcx, Self>,
+    ) -> &'a Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra> {
+        ecx.machine.stack.last().expect("no call frames exist")
+    }
+
+    fn frame_mut<'a>(
         ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
-    ) -> &'a mut Vec<Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>> {
-        &mut ecx.machine.stack
+    ) -> &'a mut Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra> {
+        ecx.machine.stack.last_mut().expect("no call frames exist")
+    }
+
+    fn access_local_mut<'a>(
+        ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
+        frame: usize,
+        local: mir::Local,
+    ) -> InterpResult<'tcx, &'a mut Operand<Self::Provenance>>
+    where
+        'tcx: 'mir,
+    {
+        ecx.machine.stack[frame].locals[local].access_mut()
     }
 
     fn before_access_global(

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -426,11 +426,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         M::stack(self)
     }
 
-    #[inline(always)]
-    pub(crate) fn stack_mut(
-        &mut self,
-    ) -> &mut Vec<Frame<'mir, 'tcx, M::Provenance, M::FrameExtra>> {
-        M::stack_mut(self)
+    pub(crate) fn push_frame(&mut self, frame: Frame<'mir, 'tcx, M::Provenance, M::FrameExtra>) {
+        M::push_frame(self, frame)
+    }
+
+    pub(crate) fn pop_frame(&mut self) -> Option<Frame<'mir, 'tcx, M::Provenance, M::FrameExtra>> {
+        M::pop_frame(self)
     }
 
     #[inline(always)]
@@ -442,12 +443,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
     #[inline(always)]
     pub fn frame(&self) -> &Frame<'mir, 'tcx, M::Provenance, M::FrameExtra> {
-        self.stack().last().expect("no call frames exist")
+        M::frame(self)
     }
 
     #[inline(always)]
     pub fn frame_mut(&mut self) -> &mut Frame<'mir, 'tcx, M::Provenance, M::FrameExtra> {
-        self.stack_mut().last_mut().expect("no call frames exist")
+        M::frame_mut(self)
     }
 
     #[inline(always)]
@@ -690,7 +691,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             extra: (),
         };
         let frame = M::init_frame_extra(self, pre_frame)?;
-        self.stack_mut().push(frame);
+        self.push_frame(frame);
 
         // Make sure all the constants required by this frame evaluate successfully (post-monomorphization check).
         for ct in &body.required_consts {
@@ -830,8 +831,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
         // All right, now it is time to actually pop the frame.
         // Note that its locals are gone already, but that's fine.
-        let frame =
-            self.stack_mut().pop().expect("tried to pop a stack frame, but there were none");
+        let frame = self.pop_frame().expect("tried to pop a stack frame, but there were none");
         // Report error from return value copy, if any.
         copy_ret_result?;
 

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -222,17 +222,13 @@ pub trait Machine<'mir, 'tcx>: Sized {
     ///
     /// Due to borrow checker trouble, we indicate the `frame` as an index rather than an `&mut
     /// Frame`.
-    #[inline]
     fn access_local_mut<'a>(
         ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
         frame: usize,
         local: mir::Local,
     ) -> InterpResult<'tcx, &'a mut Operand<Self::Provenance>>
     where
-        'tcx: 'mir,
-    {
-        ecx.stack_mut()[frame].locals[local].access_mut()
-    }
+        'tcx: 'mir;
 
     /// Called before a basic block terminator is executed.
     /// You can use this to detect endlessly running programs.
@@ -394,10 +390,22 @@ pub trait Machine<'mir, 'tcx>: Sized {
         ecx: &'a InterpCx<'mir, 'tcx, Self>,
     ) -> &'a [Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>];
 
-    /// Mutably borrow the current thread's stack.
-    fn stack_mut<'a>(
+    fn push_frame(
+        ecx: &mut InterpCx<'mir, 'tcx, Self>,
+        frame: Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>,
+    );
+
+    fn pop_frame(
+        ecx: &mut InterpCx<'mir, 'tcx, Self>,
+    ) -> Option<Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>>;
+
+    fn frame<'a>(
+        ecx: &'a InterpCx<'mir, 'tcx, Self>,
+    ) -> &'a Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>;
+
+    fn frame_mut<'a>(
         ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
-    ) -> &'a mut Vec<Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>>;
+    ) -> &'a mut Frame<'mir, 'tcx, Self::Provenance, 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> {

compiler/rustc_const_eval/src/interpret/terminator.rs

@@ -517,7 +517,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 };
                 match res {
                     Err(err) => {
-                        self.stack_mut().pop();
+                        self.pop_frame();
                         Err(err)
                     }
                     Ok(()) => Ok(()),

compiler/rustc_mir_transform/src/const_prop.rs

@@ -305,11 +305,29 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
         &ecx.machine.stack
     }
 
-    #[inline(always)]
-    fn stack_mut<'a>(
+    fn push_frame(
+        ecx: &mut InterpCx<'mir, 'tcx, Self>,
+        frame: Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>,
+    ) {
+        ecx.machine.stack.push(frame);
+    }
+
+    fn pop_frame(
+        ecx: &mut InterpCx<'mir, 'tcx, Self>,
+    ) -> Option<Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>> {
+        ecx.machine.stack.pop()
+    }
+
+    fn frame<'a>(
+        ecx: &'a InterpCx<'mir, 'tcx, Self>,
+    ) -> &'a Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra> {
+        ecx.machine.stack.last().expect("no call frames exist")
+    }
+
+    fn frame_mut<'a>(
         ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
-    ) -> &'a mut Vec<Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>> {
-        &mut ecx.machine.stack
+    ) -> &'a mut Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra> {
+        ecx.machine.stack.last_mut().expect("no call frames exist")
     }
 }
 

src/tools/miri/src/concurrency/mod.rs

@@ -6,3 +6,4 @@ pub mod init_once;
 pub mod thread;
 mod vector_clock;
 pub mod weak_memory;
+mod stack;

src/tools/miri/src/concurrency/stack.rs

@@ -0,0 +1,47 @@
+use crate::*;
+
+/// An encapsulated call stack, so encapsulated to enable efficient caching of metadata about the
+/// contained `Frame`.
+pub struct Stack<'mir, 'tcx> {
+    frames: Vec<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>>,
+}
+
+impl<'mir, 'tcx> Stack<'mir, 'tcx> {
+    pub fn new() -> Self {
+        Stack { frames: Vec::new() }
+    }
+
+    /// Does this `Stack` contain any `Frame`s?
+    pub fn is_empty(&self) -> bool {
+        self.frames.is_empty()
+    }
+
+    /// Borrow the call frames of a `Stack`.
+    pub fn frames(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>] {
+        &self.frames[..]
+    }
+
+    /// Mutably borrow the call frames of a `Stack`.
+    pub fn frames_mut(&mut self) -> &mut [Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>] {
+        &mut self.frames[..]
+    }
+
+    /// Push a new `Frame` onto a `Stack`.
+    pub fn push(&mut self, frame: Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>) {
+        self.frames.push(frame);
+    }
+
+    /// Try to pop a `Frame` from a `Stack`.
+    pub fn pop(&mut self) -> Option<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>> {
+        self.frames.pop()
+    }
+}
+
+impl VisitTags for Stack<'_, '_> {
+    fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) {
+        let Stack { frames } = self;
+        for frame in frames {
+            frame.visit_tags(visit);
+        }
+    }
+}

src/tools/miri/src/concurrency/thread.rs

@@ -16,6 +16,7 @@ use rustc_target::spec::abi::Abi;
 
 use crate::concurrency::data_race;
 use crate::concurrency::sync::SynchronizationState;
+use crate::concurrency::stack::Stack;
 use crate::*;
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -116,7 +117,7 @@ pub struct Thread<'mir, 'tcx> {
     thread_name: Option<Vec<u8>>,
 
     /// The virtual call stack.
-    stack: Vec<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>>,
+    stack: Stack<'mir, 'tcx>,
 
     /// The join status.
     join_status: ThreadJoinStatus,
@@ -166,7 +167,7 @@ impl<'mir, 'tcx> Default for Thread<'mir, 'tcx> {
         Self {
             state: ThreadState::Enabled,
             thread_name: None,
-            stack: Vec::new(),
+            stack: Stack::new(),
             join_status: ThreadJoinStatus::Joinable,
             panic_payload: None,
             last_error: None,
@@ -189,9 +190,7 @@ impl VisitTags for Thread<'_, '_> {
 
         panic_payload.visit_tags(visit);
         last_error.visit_tags(visit);
-        for frame in stack {
-            frame.visit_tags(visit)
-        }
+        stack.visit_tags(visit);
     }
 }
 
@@ -345,20 +344,18 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
 
     /// Borrow the stack of the active thread.
     pub fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>] {
-        &self.threads[self.active_thread].stack
+        self.threads[self.active_thread].stack.frames()
     }
 
     /// Mutably borrow the stack of the active thread.
-    fn active_thread_stack_mut(
-        &mut self,
-    ) -> &mut Vec<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>> {
-        &mut self.threads[self.active_thread].stack
+    pub fn active_thread_stack_mut(&mut self) -> &mut [Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>] {
+        self.threads[self.active_thread].stack.frames_mut()
     }
 
     pub fn all_stacks(
         &self,
     ) -> impl Iterator<Item = &[Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>]> {
-        self.threads.iter().map(|t| &t.stack[..])
+        self.threads.iter().map(|t| t.stack.frames())
     }
 
     /// Create a new thread and returns its id.
@@ -561,10 +558,11 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
         // this allows us to have a deterministic scheduler.
         for thread in self.threads.indices() {
             match self.timeout_callbacks.entry(thread) {
-                Entry::Occupied(entry) =>
+                Entry::Occupied(entry) => {
                     if entry.get().call_time.get_wait_time(clock) == Duration::new(0, 0) {
                         return Some((thread, entry.remove().callback));
-                    },
+                    }
+                }
                 Entry::Vacant(_) => {}
             }
         }
@@ -863,13 +861,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     }
 
     #[inline]
-    fn active_thread_stack_mut(
-        &mut self,
-    ) -> &mut Vec<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>> {
+    fn active_thread_stack_mut(&mut self) -> &mut [Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>] {
         let this = self.eval_context_mut();
         this.machine.threads.active_thread_stack_mut()
     }
 
+    fn push_frame(&mut self, frame: Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>) {
+        let this = self.eval_context_mut();
+        this.machine.threads.active_thread_mut().stack.push(frame);
+    }
+
+    fn pop_frame(&mut self) -> Option<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>> {
+        let this = self.eval_context_mut();
+        this.machine.threads.active_thread_mut().stack.pop()
+    }
+
     /// Set the name of the current thread. The buffer must not include the null terminator.
     #[inline]
     fn set_thread_name(&mut self, thread: ThreadId, new_thread_name: Vec<u8>) {

src/tools/miri/src/machine.rs

@@ -1138,10 +1138,40 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
         ecx.active_thread_stack()
     }
 
-    fn stack_mut<'a>(
+    fn push_frame(
+        ecx: &mut InterpCx<'mir, 'tcx, Self>,
+        frame: Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>,
+    ) {
+        ecx.push_frame(frame);
+    }
+
+    fn pop_frame(
+        ecx: &mut InterpCx<'mir, 'tcx, Self>,
+    ) -> Option<Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>> {
+        ecx.pop_frame()
+    }
+
+    fn frame<'a>(
+        ecx: &'a InterpCx<'mir, 'tcx, Self>,
+    ) -> &'a Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra> {
+        ecx.active_thread_stack().last().expect("no call frames exist")
+    }
+
+    fn frame_mut<'a>(
+        ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
+    ) -> &'a mut Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra> {
+        ecx.active_thread_stack_mut().last_mut().expect("no call frames exist")
+    }
+
+    fn access_local_mut<'a>(
         ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
-    ) -> &'a mut Vec<Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>> {
-        ecx.active_thread_stack_mut()
+        frame: usize,
+        local: mir::Local,
+    ) -> InterpResult<'tcx, &'a mut Operand<Self::Provenance>>
+    where
+        'tcx: 'mir,
+    {
+        ecx.active_thread_stack_mut()[frame].locals[local].access_mut()
     }
 
     fn before_terminator(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {