From 77fce67733d2a1403f17b382dfbd3802003172a6 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 20 Dec 2020 18:11:11 +0000 Subject: [PATCH 1/2] Make recursion limit fatal in project This avoid the hang/oom from #79714 --- compiler/rustc_trait_selection/src/traits/project.rs | 9 ++++++++- src/test/ui/issues/issue-23122-2.stderr | 3 +-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index a11499e43209f..7fa070caa2758 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -734,7 +734,14 @@ fn project_type<'cx, 'tcx>( if !selcx.tcx().sess.recursion_limit().value_within_limit(obligation.recursion_depth) { debug!("project: overflow!"); - return Err(ProjectionTyError::TraitSelectionError(SelectionError::Overflow)); + match selcx.query_mode() { + super::TraitQueryMode::Standard => { + selcx.infcx().report_overflow_error(&obligation, true); + } + super::TraitQueryMode::Canonical => { + return Err(ProjectionTyError::TraitSelectionError(SelectionError::Overflow)); + } + } } let obligation_trait_ref = &obligation.predicate.trait_ref(selcx.tcx()); diff --git a/src/test/ui/issues/issue-23122-2.stderr b/src/test/ui/issues/issue-23122-2.stderr index ce3bffe602ca0..ff7e884ea6f83 100644 --- a/src/test/ui/issues/issue-23122-2.stderr +++ b/src/test/ui/issues/issue-23122-2.stderr @@ -1,11 +1,10 @@ -error[E0275]: overflow evaluating the requirement `<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next: Sized` +error[E0275]: overflow evaluating the requirement `<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next` --> $DIR/issue-23122-2.rs:9:5 | LL | type Next = as Next>::Next; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: consider adding a `#![recursion_limit="256"]` attribute to your crate (`issue_23122_2`) - = note: required because of the requirements on the impl of `Next` for `GetNext<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next>` error: aborting due to previous error From 2e92b13a606ba2f073c789dfd6c33f889f04c8cf Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 20 Dec 2020 18:13:05 +0000 Subject: [PATCH 2/2] Prevent caching projections in the case of cycles When normalizing a projection which results in a cycle, we would cache the result of `project_type` without the nested obligations (because they're not needed for inference). This would result in the nested obligations only being handled once in fulfill, which would avoid the cycle error. Fixes #79714, a regresion from #79305 caused by the removal of `get_paranoid_cache_value_obligation`. --- compiler/rustc_infer/src/traits/project.rs | 16 +++++++- .../src/traits/project.rs | 14 ++++--- .../src/traits/select/mod.rs | 4 ++ .../defaults-cyclic-fail-1.rs | 4 +- .../defaults-cyclic-fail-1.stderr | 10 ++--- .../defaults-cyclic-fail-2.rs | 4 +- .../defaults-cyclic-fail-2.stderr | 10 ++--- .../ui/associated-types/impl-wf-cycle-1.rs | 29 ++++++++++++++ .../associated-types/impl-wf-cycle-1.stderr | 39 +++++++++++++++++++ .../ui/associated-types/impl-wf-cycle-2.rs | 16 ++++++++ .../associated-types/impl-wf-cycle-2.stderr | 25 ++++++++++++ 11 files changed, 150 insertions(+), 21 deletions(-) create mode 100644 src/test/ui/associated-types/impl-wf-cycle-1.rs create mode 100644 src/test/ui/associated-types/impl-wf-cycle-1.stderr create mode 100644 src/test/ui/associated-types/impl-wf-cycle-2.rs create mode 100644 src/test/ui/associated-types/impl-wf-cycle-2.stderr diff --git a/compiler/rustc_infer/src/traits/project.rs b/compiler/rustc_infer/src/traits/project.rs index 65284bcee912c..33bddf1dedc1b 100644 --- a/compiler/rustc_infer/src/traits/project.rs +++ b/compiler/rustc_infer/src/traits/project.rs @@ -90,6 +90,7 @@ impl ProjectionCacheKey<'tcx> { pub enum ProjectionCacheEntry<'tcx> { InProgress, Ambiguous, + Recur, Error, NormalizedTy(NormalizedTy<'tcx>), } @@ -143,7 +144,12 @@ impl<'tcx> ProjectionCache<'_, 'tcx> { "ProjectionCacheEntry::insert_ty: adding cache entry: key={:?}, value={:?}", key, value ); - let fresh_key = self.map().insert(key, ProjectionCacheEntry::NormalizedTy(value)); + let mut map = self.map(); + if let Some(ProjectionCacheEntry::Recur) = map.get(&key) { + debug!("Not overwriting Recur"); + return; + } + let fresh_key = map.insert(key, ProjectionCacheEntry::NormalizedTy(value)); assert!(!fresh_key, "never started projecting `{:?}`", key); } @@ -197,6 +203,14 @@ impl<'tcx> ProjectionCache<'_, 'tcx> { assert!(!fresh, "never started projecting `{:?}`", key); } + /// Indicates that while trying to normalize `key`, `key` was required to + /// be normalized again. Selection or evaluation should eventually report + /// an error here. + pub fn recur(&mut self, key: ProjectionCacheKey<'tcx>) { + let fresh = self.map().insert(key, ProjectionCacheEntry::Recur); + assert!(!fresh, "never started projecting `{:?}`", key); + } + /// Indicates that trying to normalize `key` resulted in /// error. pub fn error(&mut self, key: ProjectionCacheKey<'tcx>) { diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index 7fa070caa2758..fa0526445c194 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -496,12 +496,6 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( return Ok(None); } Err(ProjectionCacheEntry::InProgress) => { - // If while normalized A::B, we are asked to normalize - // A::B, just return A::B itself. This is a conservative - // answer, in the sense that A::B *is* clearly equivalent - // to A::B, though there may be a better value we can - // find. - // Under lazy normalization, this can arise when // bootstrapping. That is, imagine an environment with a // where-clause like `A::B == u32`. Now, if we are asked @@ -512,6 +506,14 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( debug!("found cache entry: in-progress"); + // Cache that normalizing this projection resulted in a cycle. This + // should ensure that, unless this happens within a snapshot that's + // rolled back, fulfillment or evaluation will notice the cycle. + + infcx.inner.borrow_mut().projection_cache().recur(cache_key); + return Err(InProgress); + } + Err(ProjectionCacheEntry::Recur) => { return Err(InProgress); } Err(ProjectionCacheEntry::NormalizedTy(ty)) => { diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index f1c86eab0956e..a8f81445b0397 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -291,6 +291,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { self.infcx.tcx } + pub(super) fn query_mode(&self) -> TraitQueryMode { + self.query_mode + } + /////////////////////////////////////////////////////////////////////////// // Selection // diff --git a/src/test/ui/associated-types/defaults-cyclic-fail-1.rs b/src/test/ui/associated-types/defaults-cyclic-fail-1.rs index afb2b3df716eb..61ef013236e8d 100644 --- a/src/test/ui/associated-types/defaults-cyclic-fail-1.rs +++ b/src/test/ui/associated-types/defaults-cyclic-fail-1.rs @@ -24,13 +24,13 @@ impl Tr for u32 { // ...but not in an impl that redefines one of the types. impl Tr for bool { type A = Box; - //~^ ERROR type mismatch resolving `::B == _` + //~^ ERROR overflow evaluating the requirement `::B == _` } // (the error is shown twice for some reason) impl Tr for usize { type B = &'static Self::A; - //~^ ERROR type mismatch resolving `::A == _` + //~^ ERROR overflow evaluating the requirement `::A == _` } fn main() { diff --git a/src/test/ui/associated-types/defaults-cyclic-fail-1.stderr b/src/test/ui/associated-types/defaults-cyclic-fail-1.stderr index ae7150d47ca99..5e98520b41187 100644 --- a/src/test/ui/associated-types/defaults-cyclic-fail-1.stderr +++ b/src/test/ui/associated-types/defaults-cyclic-fail-1.stderr @@ -1,15 +1,15 @@ -error[E0271]: type mismatch resolving `::B == _` +error[E0275]: overflow evaluating the requirement `::B == _` --> $DIR/defaults-cyclic-fail-1.rs:26:5 | LL | type A = Box; - | ^^^^^^^^^^^^^^^^^^^^^^ cyclic type of infinite size + | ^^^^^^^^^^^^^^^^^^^^^^ -error[E0271]: type mismatch resolving `::A == _` +error[E0275]: overflow evaluating the requirement `::A == _` --> $DIR/defaults-cyclic-fail-1.rs:32:5 | LL | type B = &'static Self::A; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ cyclic type of infinite size + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0271`. +For more information about this error, try `rustc --explain E0275`. diff --git a/src/test/ui/associated-types/defaults-cyclic-fail-2.rs b/src/test/ui/associated-types/defaults-cyclic-fail-2.rs index ba4bb0d5a296b..e91c9f2d29a82 100644 --- a/src/test/ui/associated-types/defaults-cyclic-fail-2.rs +++ b/src/test/ui/associated-types/defaults-cyclic-fail-2.rs @@ -25,13 +25,13 @@ impl Tr for u32 { impl Tr for bool { type A = Box; - //~^ ERROR type mismatch resolving `::B == _` + //~^ ERROR overflow evaluating the requirement `::B == _` } // (the error is shown twice for some reason) impl Tr for usize { type B = &'static Self::A; - //~^ ERROR type mismatch resolving `::A == _` + //~^ ERROR overflow evaluating the requirement `::A == _` } fn main() { diff --git a/src/test/ui/associated-types/defaults-cyclic-fail-2.stderr b/src/test/ui/associated-types/defaults-cyclic-fail-2.stderr index 0dfbac2dec5d5..c538805f85821 100644 --- a/src/test/ui/associated-types/defaults-cyclic-fail-2.stderr +++ b/src/test/ui/associated-types/defaults-cyclic-fail-2.stderr @@ -1,15 +1,15 @@ -error[E0271]: type mismatch resolving `::B == _` +error[E0275]: overflow evaluating the requirement `::B == _` --> $DIR/defaults-cyclic-fail-2.rs:27:5 | LL | type A = Box; - | ^^^^^^^^^^^^^^^^^^^^^^ cyclic type of infinite size + | ^^^^^^^^^^^^^^^^^^^^^^ -error[E0271]: type mismatch resolving `::A == _` +error[E0275]: overflow evaluating the requirement `::A == _` --> $DIR/defaults-cyclic-fail-2.rs:33:5 | LL | type B = &'static Self::A; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ cyclic type of infinite size + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0271`. +For more information about this error, try `rustc --explain E0275`. diff --git a/src/test/ui/associated-types/impl-wf-cycle-1.rs b/src/test/ui/associated-types/impl-wf-cycle-1.rs new file mode 100644 index 0000000000000..ba074210a2b5f --- /dev/null +++ b/src/test/ui/associated-types/impl-wf-cycle-1.rs @@ -0,0 +1,29 @@ +// Regression test for #79714 + +trait Baz {} +impl Baz for () {} +impl Baz for (T,) {} + +trait Fiz {} +impl Fiz for bool {} + +trait Grault { + type A; + type B; +} + +impl Grault for (T,) +where + Self::A: Baz, + Self::B: Fiz, +{ + type A = (); + //~^ ERROR overflow evaluating the requirement `<(T,) as Grault>::A == _` + type B = bool; + //~^ ERROR overflow evaluating the requirement `<(T,) as Grault>::A == _` +} +//~^^^^^^^^^^ ERROR overflow evaluating the requirement `<(T,) as Grault>::A == _` + +fn main() { + let x: <(_,) as Grault>::A = (); +} diff --git a/src/test/ui/associated-types/impl-wf-cycle-1.stderr b/src/test/ui/associated-types/impl-wf-cycle-1.stderr new file mode 100644 index 0000000000000..82328048c99a3 --- /dev/null +++ b/src/test/ui/associated-types/impl-wf-cycle-1.stderr @@ -0,0 +1,39 @@ +error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _` + --> $DIR/impl-wf-cycle-1.rs:15:1 + | +LL | / impl Grault for (T,) +LL | | where +LL | | Self::A: Baz, +LL | | Self::B: Fiz, +... | +LL | | +LL | | } + | |_^ + | + = note: required because of the requirements on the impl of `Grault` for `(T,)` + = note: 1 redundant requirements hidden + = note: required because of the requirements on the impl of `Grault` for `(T,)` + +error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _` + --> $DIR/impl-wf-cycle-1.rs:20:5 + | +LL | type A = (); + | ^^^^^^^^^^^^ + | + = note: required because of the requirements on the impl of `Grault` for `(T,)` + = note: 1 redundant requirements hidden + = note: required because of the requirements on the impl of `Grault` for `(T,)` + +error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _` + --> $DIR/impl-wf-cycle-1.rs:22:5 + | +LL | type B = bool; + | ^^^^^^^^^^^^^^ + | + = note: required because of the requirements on the impl of `Grault` for `(T,)` + = note: 1 redundant requirements hidden + = note: required because of the requirements on the impl of `Grault` for `(T,)` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0275`. diff --git a/src/test/ui/associated-types/impl-wf-cycle-2.rs b/src/test/ui/associated-types/impl-wf-cycle-2.rs new file mode 100644 index 0000000000000..6fccc54f229ae --- /dev/null +++ b/src/test/ui/associated-types/impl-wf-cycle-2.rs @@ -0,0 +1,16 @@ +// Regression test for #79714 + +trait Grault { + type A; +} + +impl Grault for (T,) +where + Self::A: Copy, +{ + type A = (); + //~^ ERROR overflow evaluating the requirement `<(T,) as Grault>::A == _` +} +//~^^^^^^^ ERROR overflow evaluating the requirement `<(T,) as Grault>::A == _` + +fn main() {} diff --git a/src/test/ui/associated-types/impl-wf-cycle-2.stderr b/src/test/ui/associated-types/impl-wf-cycle-2.stderr new file mode 100644 index 0000000000000..5cd18a33adf37 --- /dev/null +++ b/src/test/ui/associated-types/impl-wf-cycle-2.stderr @@ -0,0 +1,25 @@ +error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _` + --> $DIR/impl-wf-cycle-2.rs:7:1 + | +LL | / impl Grault for (T,) +LL | | where +LL | | Self::A: Copy, +LL | | { +LL | | type A = (); +LL | | +LL | | } + | |_^ + | + = note: required because of the requirements on the impl of `Grault` for `(T,)` + +error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _` + --> $DIR/impl-wf-cycle-2.rs:11:5 + | +LL | type A = (); + | ^^^^^^^^^^^^ + | + = note: required because of the requirements on the impl of `Grault` for `(T,)` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0275`.