Address comments

Author: oli-obk

miri/lib.rs

@@ -38,7 +38,7 @@ mod tls;
 use fn_call::EvalContextExt as MissingFnsEvalContextExt;
 use operator::EvalContextExt as OperatorEvalContextExt;
 use intrinsic::EvalContextExt as IntrinsicEvalContextExt;
-use tls::MemoryExt as TlsMemoryExt;
+use tls::EvalContextExt as TlsEvalContextExt;
 
 pub fn eval_main<'a, 'tcx: 'a>(
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
@@ -111,7 +111,7 @@ pub fn eval_main<'a, 'tcx: 'a>(
         }
 
         while ecx.step()? {}
-        ecx.finish()?;
+        ecx.run_tls_dtors()?;
         if let Some(cleanup_ptr) = cleanup_ptr {
             ecx.memory_mut().deallocate(cleanup_ptr, None, Kind::Stack)?;
         }
@@ -157,43 +157,6 @@ struct MemoryData<'tcx> {
     thread_local: BTreeMap<TlsKey, TlsEntry<'tcx>>,
 }
 
-trait EvalContextExt<'tcx> {
-    fn finish(&mut self) -> EvalResult<'tcx>;
-}
-
-impl<'a, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'tcx, Evaluator> {
-    fn finish(&mut self) -> EvalResult<'tcx> {
-        let mut dtor = self.memory.fetch_tls_dtor(None)?;
-        // FIXME: replace loop by some structure that works with stepping
-        while let Some((instance, ptr, key)) = dtor {
-            trace!("Running TLS dtor {:?} on {:?}", instance, ptr);
-            // TODO: Potentially, this has to support all the other possible instances?
-            // See eval_fn_call in interpret/terminator/mod.rs
-            let mir = self.load_mir(instance.def)?;
-            self.push_stack_frame(
-                instance,
-                mir.span,
-                mir,
-                Lvalue::undef(),
-                StackPopCleanup::None,
-            )?;
-            let arg_local = self.frame().mir.args_iter().next().ok_or(EvalError::AbiViolation("TLS dtor does not take enough arguments.".to_owned()))?;
-            let dest = self.eval_lvalue(&mir::Lvalue::Local(arg_local))?;
-            let ty = self.tcx.mk_mut_ptr(self.tcx.types.u8);
-            self.write_ptr(dest, ptr, ty)?;
-
-            // step until out of stackframes
-            while self.step()? {}
-
-            dtor = match self.memory.fetch_tls_dtor(Some(key))? {
-                dtor @ Some(_) => dtor,
-                None => self.memory.fetch_tls_dtor(None)?,
-            };
-        }
-        Ok(())
-    }
-}
-
 impl<'tcx> Machine<'tcx> for Evaluator {
     type Data = EvaluatorData;
     type MemoryData = MemoryData<'tcx>;
@@ -223,7 +186,7 @@ impl<'tcx> Machine<'tcx> for Evaluator {
         ecx.call_intrinsic(instance, args, dest, dest_ty, dest_layout, target)
     }
 
-    fn ptr_op<'a>(
+    fn try_ptr_op<'a>(
         ecx: &rustc_miri::interpret::EvalContext<'a, 'tcx, Self>,
         bin_op: mir::BinOp,
         left: PrimVal,

miri/tls.rs

@@ -1,11 +1,13 @@
-use rustc::ty;
+use rustc::{ty, mir};
 
 use super::{
     TlsKey, TlsEntry,
     EvalResult, EvalError,
     Pointer,
     Memory,
     Evaluator,
+    Lvalue,
+    StackPopCleanup, EvalContext,
 };
 
 pub trait MemoryExt<'tcx> {
@@ -16,6 +18,10 @@ pub trait MemoryExt<'tcx> {
     fn fetch_tls_dtor(&mut self, key: Option<TlsKey>) -> EvalResult<'tcx, Option<(ty::Instance<'tcx>, Pointer, TlsKey)>>;
 }
 
+pub trait EvalContextExt<'tcx> {
+    fn run_tls_dtors(&mut self) -> EvalResult<'tcx>;
+}
+
 impl<'a, 'tcx: 'a> MemoryExt<'tcx> for Memory<'a, 'tcx, Evaluator> {
     fn create_tls_key(&mut self, dtor: Option<ty::Instance<'tcx>>) -> TlsKey {
         let new_key = self.data.next_thread_local;
@@ -92,3 +98,36 @@ impl<'a, 'tcx: 'a> MemoryExt<'tcx> for Memory<'a, 'tcx, Evaluator> {
         return Ok(None);
     }
 }
+
+impl<'a, 'tcx: 'a> EvalContextExt<'tcx> for EvalContext<'a, 'tcx, Evaluator> {
+    fn run_tls_dtors(&mut self) -> EvalResult<'tcx> {
+        let mut dtor = self.memory.fetch_tls_dtor(None)?;
+        // FIXME: replace loop by some structure that works with stepping
+        while let Some((instance, ptr, key)) = dtor {
+            trace!("Running TLS dtor {:?} on {:?}", instance, ptr);
+            // TODO: Potentially, this has to support all the other possible instances?
+            // See eval_fn_call in interpret/terminator/mod.rs
+            let mir = self.load_mir(instance.def)?;
+            self.push_stack_frame(
+                instance,
+                mir.span,
+                mir,
+                Lvalue::undef(),
+                StackPopCleanup::None,
+            )?;
+            let arg_local = self.frame().mir.args_iter().next().ok_or(EvalError::AbiViolation("TLS dtor does not take enough arguments.".to_owned()))?;
+            let dest = self.eval_lvalue(&mir::Lvalue::Local(arg_local))?;
+            let ty = self.tcx.mk_mut_ptr(self.tcx.types.u8);
+            self.write_ptr(dest, ptr, ty)?;
+
+            // step until out of stackframes
+            while self.step()? {}
+
+            dtor = match self.memory.fetch_tls_dtor(Some(key))? {
+                dtor @ Some(_) => dtor,
+                None => self.memory.fetch_tls_dtor(None)?,
+            };
+        }
+        Ok(())
+    }
+}

src/librustc_mir/interpret/const_eval.rs

@@ -176,7 +176,7 @@ impl<'tcx> super::Machine<'tcx> for CompileTimeFunctionEvaluator {
         Err(ConstEvalError::NeedsRfc("calling intrinsics".to_string()).into())
     }
 
-    fn ptr_op<'a>(
+    fn try_ptr_op<'a>(
         _ecx: &EvalContext<'a, 'tcx, Self>,
         _bin_op: mir::BinOp,
         left: PrimVal,

src/librustc_mir/interpret/machine.rs

@@ -50,13 +50,13 @@ pub trait Machine<'tcx>: Sized {
         target: mir::BasicBlock,
     ) -> EvalResult<'tcx>;
 
-    /// Called when operating on the value of pointers.
+    /// Called for all binary operations except on float types.
     ///
     /// Returns `None` if the operation should be handled by the integer
-    /// op code
+    /// op code in order to share more code between machines
     ///
-    /// Returns a (value, overflowed) pair otherwise
-    fn ptr_op<'a>(
+    /// Returns a (value, overflowed) pair if the operation succeeded
+    fn try_ptr_op<'a>(
         ecx: &EvalContext<'a, 'tcx, Self>,
         bin_op: mir::BinOp,
         left: PrimVal,

src/librustc_mir/interpret/operator.rs

@@ -153,7 +153,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> {
 
         // I: Handle operations that support pointers
         if !left_kind.is_float() && !right_kind.is_float() {
-            if let Some(handled) = M::ptr_op(self, bin_op, left, left_ty, right, right_ty)? {
+            if let Some(handled) = M::try_ptr_op(self, bin_op, left, left_ty, right, right_ty)? {
                 return Ok(handled);
             }
         }