clean up suspensions when function ends

diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index f97132c0b0f54a4c16862379d996ab4db523a566..bd7a42cca1fe29dbaf3e8e235a874c867d3dac64 100644 (file)
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -508,8 +508,7 @@ fn collect_storage_annotations<'tcx>(mir: &'tcx mir::Mir<'tcx>) -> HashSet<mir::
             stmt: 0,
         });
 
-        let cur_frame = self.cur_frame();
-        self.memory.set_cur_frame(cur_frame);
+        self.memory.cur_frame = self.cur_frame();
 
         if self.stack.len() > self.stack_limit {
             err!(StackFrameLimitReached)
@@ -520,14 +519,13 @@ fn collect_storage_annotations<'tcx>(mir: &'tcx mir::Mir<'tcx>) -> HashSet<mir::
 
     pub(super) fn pop_stack_frame(&mut self) -> EvalResult<'tcx> {
         ::log_settings::settings().indentation -= 1;
-        self.memory.locks_lifetime_ended(None);
+        self.end_region(None)?;
         let frame = self.stack.pop().expect(
             "tried to pop a stack frame, but there were none",
         );
         if !self.stack.is_empty() {
-            // TODO: IS this the correct time to start considering these accesses as originating from the returned-to stack frame?
-            let cur_frame = self.cur_frame();
-            self.memory.set_cur_frame(cur_frame);
+            // TODO: Is this the correct time to start considering these accesses as originating from the returned-to stack frame?
+            self.memory.cur_frame = self.cur_frame();
         }
         match frame.return_to_block {
             StackPopCleanup::MarkStatic(mutable) => {

diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index 7454f1c908e0f1d34b5f16986308a355af6e8513..71bb4b4ecd14e8d6a3f08a08ad6d99a37636fc7a 100644 (file)
--- a/src/librustc_mir/interpret/memory.rs
+++ b/src/librustc_mir/interpret/memory.rs
@@ -268,7 +268,7 @@ pub struct Memory<'a, 'tcx, M: Machine<'tcx>> {
     writes_are_aligned: Cell<bool>,
 
     /// The current stack frame.  Used to check accesses against locks.
-    cur_frame: usize,
+    pub cur_frame: usize,
 }
 
 impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> {
@@ -530,10 +530,6 @@ pub fn check_bounds(&self, ptr: MemoryPointer, access: bool) -> EvalResult<'tcx>
         }
         Ok(())
     }
-
-    pub(crate) fn set_cur_frame(&mut self, cur_frame: usize) {
-        self.cur_frame = cur_frame;
-    }
 }
 
 /// Locking

diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs
index e7d5a83532b312b79fbf0a2ac516dac79e6f27ee..3dc74368fe830ef9bf051819ddadfd3623a67b99 100644 (file)
--- a/src/librustc_mir/interpret/step.rs
+++ b/src/librustc_mir/interpret/step.rs
@@ -164,7 +164,7 @@ fn statement(&mut self, stmt: &mir::Statement<'tcx>) -> EvalResult<'tcx> {
                 }
             }
             EndRegion(ce) => {
-                self.end_region(ce)?;
+                self.end_region(Some(ce))?;
             }
 
             // Defined to do nothing. These are added by optimization passes, to avoid changing the

diff --git a/src/librustc_mir/interpret/validation.rs b/src/librustc_mir/interpret/validation.rs
index 490f3b3fbcee08fba9c94e9fcefe2680229215c1..878ec3e191112c5b47bd8a62e6c5e3288beebae2 100644 (file)
--- a/src/librustc_mir/interpret/validation.rs
+++ b/src/librustc_mir/interpret/validation.rs
@@ -45,6 +45,7 @@ pub(crate) fn validation_op(
         if self.tcx.sess.opts.debugging_opts.mir_emit_validate == 0 {
             return Ok(());
         }
+        debug_assert!(self.memory.cur_frame == self.cur_frame());
 
         // HACK: Determine if this method is whitelisted and hence we do not perform any validation.
         // We currently insta-UB on anything passing around uninitialized memory, so we have to whitelist
@@ -93,7 +94,8 @@ pub(crate) fn validation_op(
                 if query.mutbl == MutMutable {
                     let lft = DynamicLifetime {
                         frame: self.cur_frame(),
-                        region: Some(scope),
+                        region: Some(scope), // Notably, we only ever suspend things for given regions.
+                        // Suspending for the entire function does not make any sense.
                     };
                     trace!("Suspending {:?} until {:?}", query, scope);
                     self.suspended.entry(lft).or_insert_with(Vec::new).push(
@@ -106,17 +108,30 @@ pub(crate) fn validation_op(
         self.validate(query, mode)
     }
 
-    pub(crate) fn end_region(&mut self, scope: region::Scope) -> EvalResult<'tcx> {
-        self.memory.locks_lifetime_ended(Some(scope));
-        // Recover suspended lvals
-        let lft = DynamicLifetime {
-            frame: self.cur_frame(),
-            region: Some(scope),
-        };
-        if let Some(queries) = self.suspended.remove(&lft) {
-            for query in queries {
-                trace!("Recovering {:?} from suspension", query);
-                self.validate(query, ValidationMode::Recover(scope))?;
+    /// Release locks and executes suspensions of the given region (or the entire fn, in case of None).
+    pub(crate) fn end_region(&mut self, scope: Option<region::Scope>) -> EvalResult<'tcx> {
+        debug_assert!(self.memory.cur_frame == self.cur_frame());
+        self.memory.locks_lifetime_ended(scope);
+        match scope {
+            Some(scope) => {
+                // Recover suspended lvals
+                let lft = DynamicLifetime {
+                    frame: self.cur_frame(),
+                    region: Some(scope),
+                };
+                if let Some(queries) = self.suspended.remove(&lft) {
+                    for query in queries {
+                        trace!("Recovering {:?} from suspension", query);
+                        self.validate(query, ValidationMode::Recover(scope))?;
+                    }
+                }
+            }
+            None => {
+                // Clean suspension table of current frame
+                let cur_frame = self.cur_frame();
+                self.suspended.retain(|lft, _| {
+                    lft.frame != cur_frame // keep only what is in the other (lower) frames
+                });
             }
         }
         Ok(())