Rollup merge of #54076 - RalfJung:miri-snapshot, r=oli-obk

miri loop detector hashing

* fix enum hashing to also consider discriminant
* do not hash extra machine state
* standalone miri is not interested in loop detection, so let it opt-out

In the future I think we want to move the hashing logic out of the miri engine, this is CTFE-only.

r? @oli-obk

src/librustc_mir/const_eval.rs

@@ -239,6 +239,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeEvaluator {
     type MemoryKinds = !;
 
     const MUT_STATIC_KIND: Option<!> = None; // no mutating of statics allowed
+    const DETECT_LOOPS: bool = true;
 
     fn find_fn<'a>(
         ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,

src/librustc_mir/interpret/eval_context.rs

@@ -65,6 +65,8 @@ pub struct EvalContext<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'mir, 'tcx>> {
     /// detector period.
     pub(super) steps_since_detector_enabled: isize,
 
+    /// Extra state to detect loops.
+    /// FIXME: Move this to the CTFE machine's state, out of the general miri engine.
     pub(super) loop_detector: InfiniteLoopDetector<'a, 'mir, 'tcx, M>,
 }
 
@@ -110,6 +112,7 @@ pub struct Frame<'mir, 'tcx: 'mir> {
     pub stmt: usize,
 }
 
+// Not using the macro because that does not support types depending on 'tcx
 impl<'a, 'mir, 'tcx: 'mir> HashStable<StableHashingContext<'a>> for Frame<'mir, 'tcx> {
     fn hash_stable<W: StableHasherResult>(
         &self,
@@ -144,11 +147,14 @@ pub enum StackPopCleanup {
     None { cleanup: bool },
 }
 
+// Can't use the macro here because that does not support named enum fields.
 impl<'a> HashStable<StableHashingContext<'a>> for StackPopCleanup {
     fn hash_stable<W: StableHasherResult>(
         &self,
         hcx: &mut StableHashingContext<'a>,
-        hasher: &mut StableHasher<W>) {
+        hasher: &mut StableHasher<W>)
+    {
+        mem::discriminant(self).hash_stable(hcx, hasher);
         match self {
             StackPopCleanup::Goto(ref block) => block.hash_stable(hcx, hasher),
             StackPopCleanup::None { cleanup } => cleanup.hash_stable(hcx, hasher),

src/librustc_mir/interpret/machine.rs

@@ -12,29 +12,29 @@
 //! This separation exists to ensure that no fancy miri features like
 //! interpreting common C functions leak into CTFE.
 
-use std::hash::Hash;
-
 use rustc::hir::def_id::DefId;
-use rustc::ich::StableHashingContext;
 use rustc::mir::interpret::{Allocation, EvalResult, Scalar};
 use rustc::mir;
 use rustc::ty::{self, layout::TyLayout, query::TyCtxtAt};
-use rustc_data_structures::stable_hasher::HashStable;
 
 use super::{EvalContext, PlaceTy, OpTy};
 
 /// Methods of this trait signifies a point where CTFE evaluation would fail
 /// and some use case dependent behaviour can instead be applied
-pub trait Machine<'mir, 'tcx>: Clone + Eq + Hash + for<'a> HashStable<StableHashingContext<'a>> {
+pub trait Machine<'mir, 'tcx>: Clone + Eq {
     /// Additional data that can be accessed via the Memory
-    type MemoryData: Clone + Eq + Hash + for<'a> HashStable<StableHashingContext<'a>>;
+    type MemoryData: Clone + Eq;
 
     /// Additional memory kinds a machine wishes to distinguish from the builtin ones
-    type MemoryKinds: ::std::fmt::Debug + Copy + Clone + Eq + Hash;
+    type MemoryKinds: ::std::fmt::Debug + Copy + Clone + Eq;
 
     /// The memory kind to use for mutated statics -- or None if those are not supported.
     const MUT_STATIC_KIND: Option<Self::MemoryKinds>;
 
+    /// Whether to attempt to detect infinite loops (any kind of infinite
+    /// execution, really).
+    const DETECT_LOOPS: bool;
+
     /// Entry point to all function calls.
     ///
     /// Returns either the mir to use for the call, or `None` if execution should

src/librustc_mir/interpret/place.rs

@@ -13,6 +13,7 @@
 //! All high-level functions to write to memory work on places as destinations.
 
 use std::convert::TryFrom;
+use std::mem;
 
 use rustc::ich::StableHashingContext;
 use rustc::mir;
@@ -57,11 +58,13 @@ pub enum Place<Id=AllocId> {
     },
 }
 
+// Can't use the macro here because that does not support named enum fields.
 impl<'a> HashStable<StableHashingContext<'a>> for Place {
     fn hash_stable<W: StableHasherResult>(
         &self, hcx: &mut StableHashingContext<'a>,
-        hasher: &mut StableHasher<W>) {
-
+        hasher: &mut StableHasher<W>)
+    {
+        mem::discriminant(self).hash_stable(hcx, hasher);
         match self {
             Place::Ptr(mem_place) => mem_place.hash_stable(hcx, hasher),
 

src/librustc_mir/interpret/snapshot.rs

@@ -62,14 +62,13 @@ impl<'a, 'mir, 'tcx, M> InfiniteLoopDetector<'a, 'mir, 'tcx, M>
     pub fn observe_and_analyze(
         &mut self,
         tcx: &TyCtxt<'b, 'tcx, 'tcx>,
-        machine: &M,
         memory: &Memory<'a, 'mir, 'tcx, M>,
         stack: &[Frame<'mir, 'tcx>],
     ) -> EvalResult<'tcx, ()> {
 
         let mut hcx = tcx.get_stable_hashing_context();
         let mut hasher = StableHasher::<u64>::new();
-        (machine, stack).hash_stable(&mut hcx, &mut hasher);
+        stack.hash_stable(&mut hcx, &mut hasher);
         let hash = hasher.finish();
 
         if self.hashes.insert(hash) {
@@ -79,7 +78,7 @@ impl<'a, 'mir, 'tcx, M> InfiniteLoopDetector<'a, 'mir, 'tcx, M>
 
         info!("snapshotting the state of the interpreter");
 
-        if self.snapshots.insert(EvalSnapshot::new(machine, memory, stack)) {
+        if self.snapshots.insert(EvalSnapshot::new(memory, stack)) {
             // Spurious collision or first cycle
             return Ok(())
         }
@@ -345,7 +344,6 @@ impl<'a, 'b, 'mir, 'tcx, M> SnapshotContext<'b> for Memory<'a, 'mir, 'tcx, M>
 
 /// The virtual machine state during const-evaluation at a given point in time.
 struct EvalSnapshot<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'mir, 'tcx>> {
-    machine: M,
     memory: Memory<'a, 'mir, 'tcx, M>,
     stack: Vec<Frame<'mir, 'tcx>>,
 }
@@ -354,21 +352,20 @@ impl<'a, 'mir, 'tcx, M> EvalSnapshot<'a, 'mir, 'tcx, M>
     where M: Machine<'mir, 'tcx>,
 {
     fn new(
-        machine: &M,
         memory: &Memory<'a, 'mir, 'tcx, M>,
-        stack: &[Frame<'mir, 'tcx>]) -> Self {
-
+        stack: &[Frame<'mir, 'tcx>]
+    ) -> Self {
         EvalSnapshot {
-            machine: machine.clone(),
             memory: memory.clone(),
             stack: stack.into(),
         }
     }
 
     fn snapshot<'b: 'a>(&'b self)
-        -> (&'b M, MemorySnapshot<'b, 'mir, 'tcx, M>, Vec<FrameSnapshot<'a, 'tcx>>) {
-        let EvalSnapshot{ machine, memory, stack } = self;
-        (&machine, memory.snapshot(), stack.iter().map(|frame| frame.snapshot(memory)).collect())
+        -> (MemorySnapshot<'b, 'mir, 'tcx, M>, Vec<FrameSnapshot<'a, 'tcx>>)
+    {
+        let EvalSnapshot{ memory, stack } = self;
+        (memory.snapshot(), stack.iter().map(|frame| frame.snapshot(memory)).collect())
     }
 }
 
@@ -384,6 +381,8 @@ impl<'a, 'mir, 'tcx, M> Hash for EvalSnapshot<'a, 'mir, 'tcx, M>
     }
 }
 
+// Not using the macro because we need special handling for `memory`, which the macro
+// does not support at the same time as the extra bounds on the type.
 impl<'a, 'b, 'mir, 'tcx, M> HashStable<StableHashingContext<'b>>
     for EvalSnapshot<'a, 'mir, 'tcx, M>
     where M: Machine<'mir, 'tcx>,
@@ -391,10 +390,10 @@ impl<'a, 'b, 'mir, 'tcx, M> HashStable<StableHashingContext<'b>>
     fn hash_stable<W: StableHasherResult>(
         &self,
         hcx: &mut StableHashingContext<'b>,
-        hasher: &mut StableHasher<W>) {
-
-        let EvalSnapshot{ machine, memory, stack } = self;
-        (machine, &memory.data, stack).hash_stable(hcx, hasher);
+        hasher: &mut StableHasher<W>)
+    {
+        let EvalSnapshot{ memory: _, stack } = self;
+        stack.hash_stable(hcx, hasher);
     }
 }
 

src/librustc_mir/interpret/step.rs

@@ -65,6 +65,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
             }
         }
 
+        if !M::DETECT_LOOPS {
+            return Ok(());
+        }
+
         if self.loop_detector.is_empty() {
             // First run of the loop detector
 
@@ -75,7 +79,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
 
         self.loop_detector.observe_and_analyze(
             &self.tcx,
-            &self.machine,
             &self.memory,
             &self.stack[..],
         )