Auto merge of #52626 - brunocodutra:issue-52475, r=oli-obk

Fix issue #52475: Make loop detector only consider reachable memory

As [suggested](https://github.com/rust-lang/rust/pull/51702#discussion_r197585664) by @oli-obk `alloc_id`s should be ignored by traversing all `Allocation`s in interpreter memory at a given moment in time, beginning by `ByRef` locals in the stack.

- [x] Generalize the implementation of `Hash` for `EvalSnapshot` to traverse `Allocation`s
- [x] Generalize the implementation of `PartialEq` for `EvalSnapshot` to traverse `Allocation`s
- [x] Commit regression tests

Fixes #52626
Fixes https://github.com/rust-lang/rust/issues/52849

src/librustc/ich/hcx.rs

@@ -15,7 +15,6 @@
 use ich::{self, CachingSourceMapView, Fingerprint};
 use middle::cstore::CrateStore;
 use ty::{TyCtxt, fast_reject};
-use mir::interpret::AllocId;
 use session::Session;
 
 use std::cmp::Ord;
@@ -60,8 +59,6 @@ pub struct StableHashingContext<'a> {
     // CachingSourceMapView, so we initialize it lazily.
     raw_source_map: &'a SourceMap,
     caching_source_map: Option<CachingSourceMapView<'a>>,
-
-    pub(super) alloc_id_recursion_tracker: FxHashSet<AllocId>,
 }
 
 #[derive(PartialEq, Eq, Clone, Copy)]
@@ -105,7 +102,6 @@ pub fn new(sess: &'a Session,
             hash_spans: hash_spans_initial,
             hash_bodies: true,
             node_id_hashing_mode: NodeIdHashingMode::HashDefPath,
-            alloc_id_recursion_tracker: Default::default(),
         }
     }
 

src/librustc/ich/impls_ty.rs

@@ -412,7 +412,7 @@ fn hash_stable<W: StableHasherResult>(
         ty::tls::with_opt(|tcx| {
             trace!("hashing {:?}", *self);
             let tcx = tcx.expect("can't hash AllocIds during hir lowering");
-            let alloc_kind = tcx.alloc_map.lock().get(*self).expect("no value for AllocId");
+            let alloc_kind = tcx.alloc_map.lock().get(*self);
             alloc_kind.hash_stable(hcx, hasher);
         });
     }

src/librustc/mir/interpret/mod.rs

@@ -133,9 +133,14 @@ fn wrapping_signed_offset(self, val: u64, i: i64) -> u64 {
 impl<T: layout::HasDataLayout> PointerArithmetic for T {}
 
 
+/// Pointer is generic over the type that represents a reference to Allocations,
+/// thus making it possible for the most convenient representation to be used in
+/// each context.
+///
+/// Defaults to the index based and loosely coupled AllocId.
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, RustcEncodable, RustcDecodable, Hash)]
-pub struct Pointer {
-    pub alloc_id: AllocId,
+pub struct Pointer<Id=AllocId> {
+    pub alloc_id: Id,
     pub offset: Size,
 }
 
@@ -543,16 +548,16 @@ pub fn undef(size: Size, align: Align) -> Self {
 impl<'tcx> ::serialize::UseSpecializedDecodable for &'tcx Allocation {}
 
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, RustcEncodable, RustcDecodable)]
-pub struct Relocations(SortedMap<Size, AllocId>);
+pub struct Relocations<Id=AllocId>(SortedMap<Size, Id>);
 
-impl Relocations {
-    pub fn new() -> Relocations {
+impl<Id> Relocations<Id> {
+    pub fn new() -> Self {
         Relocations(SortedMap::new())
     }
 
     // The caller must guarantee that the given relocations are already sorted
     // by address and contain no duplicates.
-    pub fn from_presorted(r: Vec<(Size, AllocId)>) -> Relocations {
+    pub fn from_presorted(r: Vec<(Size, Id)>) -> Self {
         Relocations(SortedMap::from_presorted_elements(r))
     }
 }

src/librustc/mir/interpret/value.rs

@@ -326,7 +326,7 @@ fn from(ptr: Pointer) -> Self {
 /// size. Like a range of bytes in an `Allocation`, a `Scalar` can either represent the raw bytes
 /// of a simple value or a pointer into another `Allocation`
 #[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, RustcEncodable, RustcDecodable, Hash)]
-pub enum Scalar {
+pub enum Scalar<Id=AllocId> {
     /// The raw bytes of a simple value.
     Bits {
         /// The first `size` bytes are the value.
@@ -338,12 +338,12 @@ pub enum Scalar {
     /// A pointer into an `Allocation`. An `Allocation` in the `memory` module has a list of
     /// relocations, but a `Scalar` is only large enough to contain one, so we just represent the
     /// relocation and its associated offset together as a `Pointer` here.
-    Ptr(Pointer),
+    Ptr(Pointer<Id>),
 }
 
 #[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, RustcEncodable, RustcDecodable, Hash)]
-pub enum ScalarMaybeUndef {
-    Scalar(Scalar),
+pub enum ScalarMaybeUndef<Id=AllocId> {
+    Scalar(Scalar<Id>),
     Undef,
 }
 

src/librustc_data_structures/stable_hasher.rs

@@ -281,6 +281,23 @@ fn hash_stable<W: StableHasherResult>(&self,
     }
 }
 
+impl<T1, T2, T3, T4, CTX> HashStable<CTX> for (T1, T2, T3, T4)
+     where T1: HashStable<CTX>,
+           T2: HashStable<CTX>,
+           T3: HashStable<CTX>,
+           T4: HashStable<CTX>,
+{
+    fn hash_stable<W: StableHasherResult>(&self,
+                                          ctx: &mut CTX,
+                                          hasher: &mut StableHasher<W>) {
+        let (ref _0, ref _1, ref _2, ref _3) = *self;
+        _0.hash_stable(ctx, hasher);
+        _1.hash_stable(ctx, hasher);
+        _2.hash_stable(ctx, hasher);
+        _3.hash_stable(ctx, hasher);
+    }
+}
+
 impl<T: HashStable<CTX>, CTX> HashStable<CTX> for [T] {
     default fn hash_stable<W: StableHasherResult>(&self,
                                                   ctx: &mut CTX,

src/librustc_mir/const_eval.rs

@@ -196,6 +196,8 @@ fn into(self) -> EvalError<'tcx> {
     }
 }
 
+impl_stable_hash_for!(struct CompileTimeEvaluator {});
+
 #[derive(Clone, Debug)]
 enum ConstEvalError {
     NeedsRfc(String),

src/librustc_mir/interpret/eval_context.rs

@@ -9,12 +9,12 @@
 // except according to those terms.
 
 use std::fmt::Write;
-use std::hash::{Hash, Hasher};
 use std::mem;
 
 use rustc::hir::def_id::DefId;
 use rustc::hir::def::Def;
 use rustc::hir::map::definitions::DefPathData;
+use rustc::ich::StableHashingContext;
 use rustc::mir;
 use rustc::ty::layout::{
     self, Size, Align, HasDataLayout, LayoutOf, TyLayout
@@ -22,10 +22,10 @@
 use rustc::ty::subst::{Subst, Substs};
 use rustc::ty::{self, Ty, TyCtxt, TypeFoldable};
 use rustc::ty::query::TyCtxtAt;
-use rustc_data_structures::fx::{FxHashSet, FxHasher};
 use rustc_data_structures::indexed_vec::IndexVec;
+use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableHasherResult};
 use rustc::mir::interpret::{
-    GlobalId, Scalar, FrameInfo,
+    GlobalId, Scalar, FrameInfo, AllocId,
     EvalResult, EvalErrorKind,
     ScalarMaybeUndef,
     truncate, sign_extend,
@@ -38,6 +38,8 @@
     Memory, Machine
 };
 
+use super::snapshot::InfiniteLoopDetector;
+
 pub struct EvalContext<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'mir, 'tcx>> {
     /// Stores the `Machine` instance.
     pub machine: M,
@@ -95,7 +97,7 @@ pub struct Frame<'mir, 'tcx: 'mir> {
     /// The locals are stored as `Option<Value>`s.
     /// `None` represents a local that is currently dead, while a live local
     /// can either directly contain `Scalar` or refer to some part of an `Allocation`.
-    pub locals: IndexVec<mir::Local, LocalValue>,
+    pub locals: IndexVec<mir::Local, LocalValue<AllocId>>,
 
     ////////////////////////////////////////////////////////////////////////////////
     // Current position within the function
@@ -108,38 +110,16 @@ pub struct Frame<'mir, 'tcx: 'mir> {
     pub stmt: usize,
 }
 
-impl<'mir, 'tcx: 'mir> Eq for Frame<'mir, 'tcx> {}
-
-impl<'mir, 'tcx: 'mir> PartialEq for Frame<'mir, 'tcx> {
-    fn eq(&self, other: &Self) -> bool {
-        let Frame {
-            mir: _,
-            instance,
-            span: _,
-            return_to_block,
-            return_place,
-            locals,
-            block,
-            stmt,
-        } = self;
-
-        // Some of these are constant during evaluation, but are included
-        // anyways for correctness.
-        *instance == other.instance
-            && *return_to_block == other.return_to_block
-            && *return_place == other.return_place
-            && *locals == other.locals
-            && *block == other.block
-            && *stmt == other.stmt
-    }
-}
+impl<'a, 'mir, 'tcx: 'mir> HashStable<StableHashingContext<'a>> for Frame<'mir, 'tcx> {
+    fn hash_stable<W: StableHasherResult>(
+        &self,
+        hcx: &mut StableHashingContext<'a>,
+        hasher: &mut StableHasher<W>) {
 
-impl<'mir, 'tcx: 'mir> Hash for Frame<'mir, 'tcx> {
-    fn hash<H: Hasher>(&self, state: &mut H) {
         let Frame {
-            mir: _,
+            mir,
             instance,
-            span: _,
+            span,
             return_to_block,
             return_place,
             locals,
@@ -147,12 +127,8 @@ fn hash<H: Hasher>(&self, state: &mut H) {
             stmt,
         } = self;
 
-        instance.hash(state);
-        return_to_block.hash(state);
-        return_place.hash(state);
-        locals.hash(state);
-        block.hash(state);
-        stmt.hash(state);
+        (mir, instance, span, return_to_block).hash_stable(hcx, hasher);
+        (return_place, locals, block, stmt).hash_stable(hcx, hasher);
     }
 }
 
@@ -168,15 +144,27 @@ pub enum StackPopCleanup {
     None { cleanup: bool },
 }
 
+impl<'a> HashStable<StableHashingContext<'a>> for StackPopCleanup {
+    fn hash_stable<W: StableHasherResult>(
+        &self,
+        hcx: &mut StableHashingContext<'a>,
+        hasher: &mut StableHasher<W>) {
+        match self {
+            StackPopCleanup::Goto(ref block) => block.hash_stable(hcx, hasher),
+            StackPopCleanup::None { cleanup } => cleanup.hash_stable(hcx, hasher),
+        }
+    }
+}
+
 // State of a local variable
 #[derive(Copy, Clone, PartialEq, Eq, Hash)]
-pub enum LocalValue {
+pub enum LocalValue<Id=AllocId> {
     Dead,
     // Mostly for convenience, we re-use the `Operand` type here.
     // This is an optimization over just always having a pointer here;
     // we can thus avoid doing an allocation when the local just stores
     // immediate values *and* never has its address taken.
-    Live(Operand),
+    Live(Operand<Id>),
 }
 
 impl<'tcx> LocalValue {
@@ -195,72 +183,10 @@ pub fn access_mut(&mut self) -> EvalResult<'tcx, &mut Operand> {
     }
 }
 
-/// The virtual machine state during const-evaluation at a given point in time.
-type EvalSnapshot<'a, 'mir, 'tcx, M>
-    = (M, Vec<Frame<'mir, 'tcx>>, Memory<'a, 'mir, 'tcx, M>);
-
-pub(super) struct InfiniteLoopDetector<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'mir, 'tcx>> {
-    /// The set of all `EvalSnapshot` *hashes* observed by this detector.
-    ///
-    /// When a collision occurs in this table, we store the full snapshot in
-    /// `snapshots`.
-    hashes: FxHashSet<u64>,
-
-    /// The set of all `EvalSnapshot`s observed by this detector.
-    ///
-    /// An `EvalSnapshot` will only be fully cloned once it has caused a
-    /// collision in `hashes`. As a result, the detector must observe at least
-    /// *two* full cycles of an infinite loop before it triggers.
-    snapshots: FxHashSet<EvalSnapshot<'a, 'mir, 'tcx, M>>,
-}
-
-impl<'a, 'mir, 'tcx, M> Default for InfiniteLoopDetector<'a, 'mir, 'tcx, M>
-    where M: Machine<'mir, 'tcx>,
-          'tcx: 'a + 'mir,
-{
-    fn default() -> Self {
-        InfiniteLoopDetector {
-            hashes: FxHashSet::default(),
-            snapshots: FxHashSet::default(),
-        }
-    }
-}
-
-impl<'a, 'mir, 'tcx, M> InfiniteLoopDetector<'a, 'mir, 'tcx, M>
-    where M: Machine<'mir, 'tcx>,
-          'tcx: 'a + 'mir,
-{
-    /// Returns `true` if the loop detector has not yet observed a snapshot.
-    pub fn is_empty(&self) -> bool {
-        self.hashes.is_empty()
-    }
-
-    pub fn observe_and_analyze(
-        &mut self,
-        machine: &M,
-        stack: &Vec<Frame<'mir, 'tcx>>,
-        memory: &Memory<'a, 'mir, 'tcx, M>,
-    ) -> EvalResult<'tcx, ()> {
-        let snapshot = (machine, stack, memory);
-
-        let mut fx = FxHasher::default();
-        snapshot.hash(&mut fx);
-        let hash = fx.finish();
-
-        if self.hashes.insert(hash) {
-            // No collision
-            return Ok(())
-        }
-
-        if self.snapshots.insert((machine.clone(), stack.clone(), memory.clone())) {
-            // Spurious collision or first cycle
-            return Ok(())
-        }
-
-        // Second cycle
-        Err(EvalErrorKind::InfiniteLoop.into())
-    }
-}
+impl_stable_hash_for!(enum self::LocalValue {
+    Dead,
+    Live(x),
+});
 
 impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout for &'a EvalContext<'a, 'mir, 'tcx, M> {
     #[inline]

src/librustc_mir/interpret/machine.rs

@@ -15,17 +15,19 @@
 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 {
+pub trait Machine<'mir, 'tcx>: Clone + Eq + Hash + for<'a> HashStable<StableHashingContext<'a>> {
     /// Additional data that can be accessed via the Memory
-    type MemoryData: Clone + Eq + Hash;
+    type MemoryData: Clone + Eq + Hash + for<'a> HashStable<StableHashingContext<'a>>;
 
     /// Additional memory kinds a machine wishes to distinguish from the builtin ones
     type MemoryKinds: ::std::fmt::Debug + Copy + Clone + Eq + Hash;

src/librustc_mir/interpret/memory.rs

@@ -17,7 +17,6 @@
 //! short-circuiting the empty case!
 
 use std::collections::VecDeque;
-use std::hash::{Hash, Hasher};
 use std::ptr;
 
 use rustc::ty::{self, Instance, query::TyCtxtAt};
@@ -26,7 +25,7 @@
                             EvalResult, Scalar, EvalErrorKind, AllocType, PointerArithmetic,
                             truncate};
 pub use rustc::mir::interpret::{write_target_uint, read_target_uint};
-use rustc_data_structures::fx::{FxHashSet, FxHashMap, FxHasher};
+use rustc_data_structures::fx::{FxHashSet, FxHashMap};
 
 use syntax::ast::Mutability;
 
@@ -70,58 +69,6 @@ fn data_layout(&self) -> &TargetDataLayout {
     }
 }
 
-impl<'a, 'mir, 'tcx, M> Eq for Memory<'a, 'mir, 'tcx, M>
-    where M: Machine<'mir, 'tcx>,
-          'tcx: 'a + 'mir,
-{}
-
-impl<'a, 'mir, 'tcx, M> PartialEq for Memory<'a, 'mir, 'tcx, M>
-    where M: Machine<'mir, 'tcx>,
-          'tcx: 'a + 'mir,
-{
-    fn eq(&self, other: &Self) -> bool {
-        let Memory {
-            data,
-            alloc_map,
-            tcx: _,
-        } = self;
-
-        *data == other.data
-            && *alloc_map == other.alloc_map
-    }
-}
-
-impl<'a, 'mir, 'tcx, M> Hash for Memory<'a, 'mir, 'tcx, M>
-    where M: Machine<'mir, 'tcx>,
-          'tcx: 'a + 'mir,
-{
-    fn hash<H: Hasher>(&self, state: &mut H) {
-        let Memory {
-            data,
-            alloc_map: _,
-            tcx: _,
-        } = self;
-
-        data.hash(state);
-
-        // We ignore some fields which don't change between evaluation steps.
-
-        // Since HashMaps which contain the same items may have different
-        // iteration orders, we use a commutative operation (in this case
-        // addition, but XOR would also work), to combine the hash of each
-        // `Allocation`.
-        self.alloc_map.iter()
-            .map(|(&id, alloc)| {
-                let mut h = FxHasher::default();
-                id.hash(&mut h);
-                alloc.hash(&mut h);
-                h.finish()
-            })
-            .fold(0u64, |hash, x| hash.wrapping_add(x))
-            .hash(state);
-    }
-}
-
 impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
     pub fn new(tcx: TyCtxtAt<'a, 'tcx, 'tcx>, data: M::MemoryData) -> Self {
         Memory {

src/librustc_mir/interpret/mod.rs

@@ -17,6 +17,7 @@
 mod machine;
 mod memory;
 mod operator;
+mod snapshot;
 mod step;
 mod terminator;
 mod traits;

src/librustc_mir/interpret/operand.rs

@@ -17,9 +17,10 @@
 use rustc::{mir, ty};
 use rustc::ty::layout::{self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt};
 use rustc_data_structures::indexed_vec::Idx;
-
 use rustc::mir::interpret::{
-    GlobalId, ConstValue, Scalar, EvalResult, Pointer, ScalarMaybeUndef, EvalErrorKind
+    GlobalId, AllocId,
+    ConstValue, Pointer, Scalar, ScalarMaybeUndef,
+    EvalResult, EvalErrorKind
 };
 use super::{EvalContext, Machine, MemPlace, MPlaceTy, MemoryKind};
 
@@ -31,9 +32,9 @@
 /// In particular, thanks to `ScalarPair`, arithmetic operations and casts can be entirely
 /// defined on `Value`, and do not have to work with a `Place`.
 #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
-pub enum Value {
-    Scalar(ScalarMaybeUndef),
-    ScalarPair(ScalarMaybeUndef, ScalarMaybeUndef),
+pub enum Value<Id=AllocId> {
+    Scalar(ScalarMaybeUndef<Id>),
+    ScalarPair(ScalarMaybeUndef<Id>, ScalarMaybeUndef<Id>),
 }
 
 impl<'tcx> Value {
@@ -81,6 +82,11 @@ pub fn to_scalar_ptr(self) -> EvalResult<'tcx, Scalar> {
     }
 }
 
+impl_stable_hash_for!(enum ::interpret::Value {
+    Scalar(x),
+    ScalarPair(x, y),
+});
+
 // ScalarPair needs a type to interpret, so we often have a value and a type together
 // as input for binary and cast operations.
 #[derive(Copy, Clone, Debug)]
@@ -101,9 +107,9 @@ fn deref(&self) -> &Value {
 /// or still in memory.  The latter is an optimization, to delay reading that chunk of
 /// memory and to avoid having to store arbitrary-sized data here.
 #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
-pub enum Operand {
-    Immediate(Value),
-    Indirect(MemPlace),
+pub enum Operand<Id=AllocId> {
+    Immediate(Value<Id>),
+    Indirect(MemPlace<Id>),
 }
 
 impl Operand {
@@ -126,6 +132,11 @@ pub fn to_immediate(self) -> Value {
     }
 }
 
+impl_stable_hash_for!(enum ::interpret::Operand {
+    Immediate(x),
+    Indirect(x),
+});
+
 #[derive(Copy, Clone, Debug)]
 pub struct OpTy<'tcx> {
     crate op: Operand, // ideally we'd make this private, but const_prop needs this

src/librustc_mir/interpret/place.rs

@@ -14,33 +14,41 @@
 
 use std::convert::TryFrom;
 
+use rustc::ich::StableHashingContext;
 use rustc::mir;
 use rustc::ty::{self, Ty};
 use rustc::ty::layout::{self, Size, Align, LayoutOf, TyLayout, HasDataLayout};
 use rustc_data_structures::indexed_vec::Idx;
+use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableHasherResult};
 
 use rustc::mir::interpret::{
-    GlobalId, Scalar, EvalResult, Pointer, ScalarMaybeUndef, PointerArithmetic
+    GlobalId, AllocId, Scalar, EvalResult, Pointer, ScalarMaybeUndef, PointerArithmetic
 };
 use super::{EvalContext, Machine, Value, ValTy, Operand, OpTy, MemoryKind};
 
 #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
-pub struct MemPlace {
+pub struct MemPlace<Id=AllocId> {
     /// A place may have an integral pointer for ZSTs, and since it might
     /// be turned back into a reference before ever being dereferenced.
     /// However, it may never be undef.
-    pub ptr: Scalar,
+    pub ptr: Scalar<Id>,
     pub align: Align,
     /// Metadata for unsized places.  Interpretation is up to the type.
     /// Must not be present for sized types, but can be missing for unsized types
     /// (e.g. `extern type`).
-    pub extra: Option<Scalar>,
+    pub extra: Option<Scalar<Id>>,
 }
 
+impl_stable_hash_for!(struct ::interpret::MemPlace {
+    ptr,
+    align,
+    extra,
+});
+
 #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
-pub enum Place {
+pub enum Place<Id=AllocId> {
     /// A place referring to a value allocated in the `Memory` system.
-    Ptr(MemPlace),
+    Ptr(MemPlace<Id>),
 
     /// To support alloc-free locals, we are able to write directly to a local.
     /// (Without that optimization, we'd just always be a `MemPlace`.)
@@ -50,6 +58,21 @@ pub enum Place {
     },
 }
 
+impl<'a> HashStable<StableHashingContext<'a>> for Place {
+    fn hash_stable<W: StableHasherResult>(
+        &self, hcx: &mut StableHashingContext<'a>,
+        hasher: &mut StableHasher<W>) {
+
+        match self {
+            Place::Ptr(mem_place) => mem_place.hash_stable(hcx, hasher),
+
+            Place::Local { frame, local } => {
+                frame.hash_stable(hcx, hasher);
+                local.hash_stable(hcx, hasher);
+            },
+        }
+    }
+}
 #[derive(Copy, Clone, Debug)]
 pub struct PlaceTy<'tcx> {
     place: Place,

src/librustc_mir/interpret/snapshot.rs

+//! This module contains the machinery necessary to detect infinite loops
+//! during const-evaluation by taking snapshots of the state of the interpreter
+//! at regular intervals.
+
+use std::hash::{Hash, Hasher};
+
+use rustc::ich::{StableHashingContext, StableHashingContextProvider};
+use rustc::mir;
+use rustc::mir::interpret::{
+    AllocId, Pointer, Scalar, ScalarMaybeUndef,
+    Relocations, Allocation, UndefMask,
+    EvalResult, EvalErrorKind,
+};
+
+use rustc::ty::{self, TyCtxt};
+use rustc::ty::layout::Align;
+use rustc_data_structures::fx::FxHashSet;
+use rustc_data_structures::indexed_vec::IndexVec;
+use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableHasherResult};
+use syntax::ast::Mutability;
+use syntax::source_map::Span;
+
+use super::eval_context::{LocalValue, StackPopCleanup};
+use super::{Frame, Memory, Machine, Operand, MemPlace, Place, Value};
+
+pub(super) struct InfiniteLoopDetector<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'mir, 'tcx>> {
+    /// The set of all `EvalSnapshot` *hashes* observed by this detector.
+    ///
+    /// When a collision occurs in this table, we store the full snapshot in
+    /// `snapshots`.
+    hashes: FxHashSet<u64>,
+
+    /// The set of all `EvalSnapshot`s observed by this detector.
+    ///
+    /// An `EvalSnapshot` will only be fully cloned once it has caused a
+    /// collision in `hashes`. As a result, the detector must observe at least
+    /// *two* full cycles of an infinite loop before it triggers.
+    snapshots: FxHashSet<EvalSnapshot<'a, 'mir, 'tcx, M>>,
+}
+
+impl<'a, 'mir, 'tcx, M> Default for InfiniteLoopDetector<'a, 'mir, 'tcx, M>
+    where M: Machine<'mir, 'tcx>,
+          'tcx: 'a + 'mir,
+{
+    fn default() -> Self {
+        InfiniteLoopDetector {
+            hashes: FxHashSet::default(),
+            snapshots: FxHashSet::default(),
+        }
+    }
+}
+
+impl<'a, 'mir, 'tcx, M> InfiniteLoopDetector<'a, 'mir, 'tcx, M>
+    where M: Machine<'mir, 'tcx>,
+          'tcx: 'a + 'mir,
+{
+    /// Returns `true` if the loop detector has not yet observed a snapshot.
+    pub fn is_empty(&self) -> bool {
+        self.hashes.is_empty()
+    }
+
+    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);
+        let hash = hasher.finish();
+
+        if self.hashes.insert(hash) {
+            // No collision
+            return Ok(())
+        }
+
+        info!("snapshotting the state of the interpreter");
+
+        if self.snapshots.insert(EvalSnapshot::new(machine, memory, stack)) {
+            // Spurious collision or first cycle
+            return Ok(())
+        }
+
+        // Second cycle
+        Err(EvalErrorKind::InfiniteLoop.into())
+    }
+}
+
+trait SnapshotContext<'a> {
+    fn resolve(&'a self, id: &AllocId) -> Option<&'a Allocation>;
+}
+
+/// Taking a snapshot of the evaluation context produces a view of
+/// the state of the interpreter that is invariant to `AllocId`s.
+trait Snapshot<'a, Ctx: SnapshotContext<'a>> {
+    type Item;
+    fn snapshot(&self, ctx: &'a Ctx) -> Self::Item;
+}
+
+macro_rules! __impl_snapshot_field {
+    ($field:ident, $ctx:expr) => ($field.snapshot($ctx));
+    ($field:ident, $ctx:expr, $delegate:expr) => ($delegate);
+}
+
+macro_rules! impl_snapshot_for {
+    // FIXME(mark-i-m): Some of these should be `?` rather than `*`.
+    (enum $enum_name:ident {
+        $( $variant:ident $( ( $($field:ident $(-> $delegate:expr)*),* ) )* ),* $(,)*
+    }) => {
+
+        impl<'a, Ctx> self::Snapshot<'a, Ctx> for $enum_name
+            where Ctx: self::SnapshotContext<'a>,
+        {
+            type Item = $enum_name<AllocIdSnapshot<'a>>;
+
+            #[inline]
+            fn snapshot(&self, __ctx: &'a Ctx) -> Self::Item {
+                match *self {
+                    $(
+                        $enum_name::$variant $( ( $(ref $field),* ) )* =>
+                            $enum_name::$variant $(
+                                ( $( __impl_snapshot_field!($field, __ctx $(, $delegate)*) ),* ),
+                            )*
+                    )*
+                }
+            }
+        }
+    };
+
+    // FIXME(mark-i-m): same here.
+    (struct $struct_name:ident { $($field:ident $(-> $delegate:expr)*),*  $(,)* }) => {
+        impl<'a, Ctx> self::Snapshot<'a, Ctx> for $struct_name
+            where Ctx: self::SnapshotContext<'a>,
+        {
+            type Item = $struct_name<AllocIdSnapshot<'a>>;
+
+            #[inline]
+            fn snapshot(&self, __ctx: &'a Ctx) -> Self::Item {
+                let $struct_name {
+                    $(ref $field),*
+                } = *self;
+
+                $struct_name {
+                    $( $field: __impl_snapshot_field!($field, __ctx $(, $delegate)*) ),*
+                }
+            }
+        }
+    };
+}
+
+impl<'a, Ctx, T> Snapshot<'a, Ctx> for Option<T>
+    where Ctx: SnapshotContext<'a>,
+          T: Snapshot<'a, Ctx>
+{
+    type Item = Option<<T as Snapshot<'a, Ctx>>::Item>;
+
+    fn snapshot(&self, ctx: &'a Ctx) -> Self::Item {
+        match self {
+            Some(x) => Some(x.snapshot(ctx)),
+            None => None,
+        }
+    }
+}
+
+#[derive(Eq, PartialEq)]
+struct AllocIdSnapshot<'a>(Option<AllocationSnapshot<'a>>);
+
+impl<'a, Ctx> Snapshot<'a, Ctx> for AllocId
+    where Ctx: SnapshotContext<'a>,
+{
+    type Item = AllocIdSnapshot<'a>;
+
+    fn snapshot(&self, ctx: &'a Ctx) -> Self::Item {
+        AllocIdSnapshot(ctx.resolve(self).map(|alloc| alloc.snapshot(ctx)))
+    }
+}
+
+impl_snapshot_for!(struct Pointer {
+    alloc_id,
+    offset -> *offset,
+});
+
+impl<'a, Ctx> Snapshot<'a, Ctx> for Scalar
+    where Ctx: SnapshotContext<'a>,
+{
+    type Item = Scalar<AllocIdSnapshot<'a>>;
+
+    fn snapshot(&self, ctx: &'a Ctx) -> Self::Item {
+        match self {
+            Scalar::Ptr(p) => Scalar::Ptr(p.snapshot(ctx)),
+            Scalar::Bits{ size, bits } => Scalar::Bits {
+                size: *size,
+                bits: *bits,
+            },
+        }
+    }
+}
+
+impl_snapshot_for!(enum ScalarMaybeUndef {
+    Scalar(s),
+    Undef,
+});
+
+impl_snapshot_for!(struct MemPlace {
+    ptr,
+    extra,
+    align -> *align,
+});
+
+impl<'a, Ctx> Snapshot<'a, Ctx> for Place
+    where Ctx: SnapshotContext<'a>,
+{
+    type Item = Place<AllocIdSnapshot<'a>>;
+
+    fn snapshot(&self, ctx: &'a Ctx) -> Self::Item {
+        match self {
+            Place::Ptr(p) => Place::Ptr(p.snapshot(ctx)),
+
+            Place::Local{ frame, local } => Place::Local{
+                frame: *frame,
+                local: *local,
+            },
+        }
+    }
+}
+
+impl_snapshot_for!(enum Value {
+    Scalar(s),
+    ScalarPair(s, t),
+});
+
+impl_snapshot_for!(enum Operand {
+    Immediate(v),
+    Indirect(m),
+});
+
+impl_snapshot_for!(enum LocalValue {
+    Live(v),
+    Dead,
+});
+
+impl<'a, Ctx> Snapshot<'a, Ctx> for Relocations
+    where Ctx: SnapshotContext<'a>,
+{
+    type Item = Relocations<AllocIdSnapshot<'a>>;
+
+    fn snapshot(&self, ctx: &'a Ctx) -> Self::Item {
+        Relocations::from_presorted(self.iter()
+            .map(|(size, id)| (*size, id.snapshot(ctx)))
+            .collect())
+    }
+}
+
+#[derive(Eq, PartialEq)]
+struct AllocationSnapshot<'a> {
+    bytes: &'a [u8],
+    relocations: Relocations<AllocIdSnapshot<'a>>,
+    undef_mask: &'a UndefMask,
+    align: &'a Align,
+    mutability: &'a Mutability,
+}
+
+impl<'a, Ctx> Snapshot<'a, Ctx> for &'a Allocation
+    where Ctx: SnapshotContext<'a>,
+{
+    type Item = AllocationSnapshot<'a>;
+
+    fn snapshot(&self, ctx: &'a Ctx) -> Self::Item {
+        let Allocation { bytes, relocations, undef_mask, align, mutability } = self;
+
+        AllocationSnapshot {
+            bytes,
+            undef_mask,
+            align,
+            mutability,
+            relocations: relocations.snapshot(ctx),
+        }
+    }
+}
+
+#[derive(Eq, PartialEq)]
+struct FrameSnapshot<'a, 'tcx: 'a> {
+    instance: &'a ty::Instance<'tcx>,
+    span: &'a Span,
+    return_to_block: &'a StackPopCleanup,
+    return_place: Place<AllocIdSnapshot<'a>>,
+    locals: IndexVec<mir::Local, LocalValue<AllocIdSnapshot<'a>>>,
+    block: &'a mir::BasicBlock,
+    stmt: usize,
+}
+
+impl<'a, 'mir, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a Frame<'mir, 'tcx>
+    where Ctx: SnapshotContext<'a>,
+{
+    type Item = FrameSnapshot<'a, 'tcx>;
+
+    fn snapshot(&self, ctx: &'a Ctx) -> Self::Item {
+        let Frame {
+            mir: _,
+            instance,
+            span,
+            return_to_block,
+            return_place,
+            locals,
+            block,
+            stmt,
+        } = self;
+
+        FrameSnapshot {
+            instance,
+            span,
+            return_to_block,
+            block,
+            stmt: *stmt,
+            return_place: return_place.snapshot(ctx),
+            locals: locals.iter().map(|local| local.snapshot(ctx)).collect(),
+        }
+    }
+}
+
+#[derive(Eq, PartialEq)]
+struct MemorySnapshot<'a, 'mir: 'a, 'tcx: 'a + 'mir, M: Machine<'mir, 'tcx> + 'a> {
+    data: &'a M::MemoryData,
+}
+
+impl<'a, 'mir, 'tcx, M> Memory<'a, 'mir, 'tcx, M>
+    where M: Machine<'mir, 'tcx>,
+{
+    fn snapshot<'b: 'a>(&'b self) -> MemorySnapshot<'b, 'mir, 'tcx, M> {
+        let Memory { data, .. } = self;
+        MemorySnapshot { data }
+    }
+}
+
+impl<'a, 'b, 'mir, 'tcx, M> SnapshotContext<'b> for Memory<'a, 'mir, 'tcx, M>
+    where M: Machine<'mir, 'tcx>,
+{
+    fn resolve(&'b self, id: &AllocId) -> Option<&'b Allocation> {
+        self.get(*id).ok()
+    }
+}
+
+/// 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>>,
+}
+
+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 {
+
+        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())
+    }
+}
+
+impl<'a, 'mir, 'tcx, M> Hash for EvalSnapshot<'a, 'mir, 'tcx, M>
+    where M: Machine<'mir, 'tcx>,
+{
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        // Implement in terms of hash stable, so that k1 == k2 -> hash(k1) == hash(k2)
+        let mut hcx = self.memory.tcx.get_stable_hashing_context();
+        let mut hasher = StableHasher::<u64>::new();
+        self.hash_stable(&mut hcx, &mut hasher);
+        hasher.finish().hash(state)
+    }
+}
+
+impl<'a, 'b, 'mir, 'tcx, M> HashStable<StableHashingContext<'b>>
+    for EvalSnapshot<'a, 'mir, 'tcx, M>
+    where M: Machine<'mir, 'tcx>,
+{
+    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);
+    }
+}
+
+impl<'a, 'mir, 'tcx, M> Eq for EvalSnapshot<'a, 'mir, 'tcx, M>
+    where M: Machine<'mir, 'tcx>,
+{}
+
+impl<'a, 'mir, 'tcx, M> PartialEq for EvalSnapshot<'a, 'mir, 'tcx, M>
+    where M: Machine<'mir, 'tcx>,
+{
+    fn eq(&self, other: &Self) -> bool {
+        self.snapshot() == other.snapshot()
+    }
+}

src/librustc_mir/interpret/step.rs

@@ -73,7 +73,12 @@ pub fn inc_step_counter_and_detect_loops(&mut self) -> EvalResult<'tcx, ()> {
                 "Constant evaluating a complex constant, this might take some time");
         }
 
-        self.loop_detector.observe_and_analyze(&self.machine, &self.stack, &self.memory)
+        self.loop_detector.observe_and_analyze(
+            &self.tcx,
+            &self.machine,
+            &self.memory,
+            &self.stack[..],
+        )
     }
 
     pub fn run(&mut self) -> EvalResult<'tcx> {

src/test/ui/consts/const-eval/issue-52475.rs

+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(const_let)]
+
+fn main() {
+    let _ = [(); {
+        //~^ WARNING Constant evaluating a complex constant, this might take some time
+        //~| ERROR could not evaluate repeat length
+        let mut x = &0;
+        let mut n = 0;
+        while n < 5 { //~ ERROR constant contains unimplemented expression type
+            n = (n + 1) % 5;
+            x = &0; // Materialize a new AllocId
+        }
+        0
+    }];
+}

src/test/ui/consts/const-eval/issue-52475.stderr

+error[E0019]: constant contains unimplemented expression type
+  --> $DIR/issue-52475.rs:19:9
+   |
+LL | /         while n < 5 { //~ ERROR constant contains unimplemented expression type
+LL | |             n = (n + 1) % 5;
+LL | |             x = &0; // Materialize a new AllocId
+LL | |         }
+   | |_________^
+
+warning: Constant evaluating a complex constant, this might take some time
+  --> $DIR/issue-52475.rs:14:18
+   |
+LL |       let _ = [(); {
+   |  __________________^
+LL | |         //~^ WARNING Constant evaluating a complex constant, this might take some time
+LL | |         //~| ERROR could not evaluate repeat length
+LL | |         let mut x = &0;
+...  |
+LL | |         0
+LL | |     }];
+   | |_____^
+
+error[E0080]: could not evaluate repeat length
+  --> $DIR/issue-52475.rs:14:18
+   |
+LL |       let _ = [(); {
+   |  __________________^
+LL | |         //~^ WARNING Constant evaluating a complex constant, this might take some time
+LL | |         //~| ERROR could not evaluate repeat length
+LL | |         let mut x = &0;
+...  |
+LL | |             n = (n + 1) % 5;
+   | |                 ----------- duplicate interpreter state observed here, const evaluation will never terminate
+...  |
+LL | |         0
+LL | |     }];
+   | |_____^
+
+error: aborting due to 2 previous errors
+
+Some errors occurred: E0019, E0080.
+For more information about an error, try `rustc --explain E0019`.