Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nightly 1.71-2023-04-17 overflows the recursion limit #110475

Open
benthecarman opened this issue Apr 18, 2023 · 21 comments
Open

nightly 1.71-2023-04-17 overflows the recursion limit #110475

benthecarman opened this issue Apr 18, 2023 · 21 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-monomorphization Area: Monomorphization C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@benthecarman
Copy link

benthecarman commented Apr 18, 2023

I now get this error from one of my dependent crates (rust-miniscript) when compiling with the --release flag

error[E0275]: overflow evaluating the requirement `F: FnMut<(&Pk,)>`
  |
  = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`miniscript`)
  = note: required for `&mut F` to implement `FnMut<(&Pk,)>`
  = note: 128 redundant requirements hidden
  = note: required for `&mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut ...` to implement `FnMut<(&Pk,)>`
  = note: the full type name has been written to '/home/ben/projects/mutiny-node/target/wasm32-unknown-unknown/release/deps/miniscript-e33708ffc0e16b91.long-type-8328810264563709237.txt'

@benthecarman benthecarman added the C-bug Category: This is a bug. label Apr 18, 2023
@jyn514
Copy link
Member

jyn514 commented Apr 18, 2023

Have you tried raising the recursion limit like it suggests? We don't have stability guarantees for the recursion limit.

If that doesn't help, please link to an exact version of your crate so it's possible to reproduce the error.

@jyn514 jyn514 changed the title Rust Nightly (1.71.0-nightly (7908a1d65 2023-04-17)) breaks my builds nightly 1.71-2023-04-17 overflows the recursion limit Apr 18, 2023
@jyn514 jyn514 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 18, 2023
@compiler-errors
Copy link
Member

The lack of a span on this error suggests that this is due to an overflow during codegen. Given the &mut &mut ..., I doubt raising the recursion limit will help here.

I'll bisect this I guess. Seems to reproduce on miniscript with cargo build --release --all-features.

@compiler-errors
Copy link
Member

Well bisection came back with garbage:

searched nightlies: from nightly-2023-04-01 to nightly-2023-04-18
regressed nightly: nightly-2023-04-18
searched commit range: d0f204e...7908a1d
regressed commit: 53ac4f8

bisected with cargo-bisect-rustc v0.6.5

Host triple: aarch64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start 2023-04-01 --end 2023-04-18 -- build --release --all-features 

Though given the regression happened a few days ago, I'm suspicious of #109247. The overflow is actually happening in the MIR inliner. Here's a snippet of the call stack of the error:

  24:     0xffffa865f308 - <rustc_trait_selection[59d5fff7e50533b4]::traits::select::SelectionContext>::candidate_from_obligation
  25:     0xffffa8653904 - <rustc_trait_selection[59d5fff7e50533b4]::traits::select::SelectionContext>::select_from_obligation
  26:     0xffffa865e974 - <rustc_trait_selection[59d5fff7e50533b4]::traits::select::SelectionContext>::select
  27:     0xffffa8521564 - <rustc_trait_selection[59d5fff7e50533b4]::traits::fulfill::FulfillProcessor>::process_trait_obligation
  28:     0xffffa8520820 - <rustc_trait_selection[59d5fff7e50533b4]::traits::fulfill::FulfillProcessor as rustc_data_structures[7f1a4c6933e5419]::obligation_forest::ObligationProcessor>::process_obligation
  29:     0xffffa84db470 - <rustc_data_structures[7f1a4c6933e5419]::obligation_forest::ObligationForest<rustc_trait_selection[59d5fff7e50533b4]::traits::fulfill::PendingPredicateObligation>>::process_obligations::<rustc_trait_selection[59d5fff7e50533b4]::traits::fulfill::FulfillProcessor>
  30:     0xffffa851e104 - <rustc_trait_selection[59d5fff7e50533b4]::traits::fulfill::FulfillmentContext as rustc_infer[8facaf8255ab7cac]::traits::engine::TraitEngine>::select_where_possible
  31:     0xffffa77519f0 - <dyn rustc_infer[8facaf8255ab7cac]::traits::engine::TraitEngine as rustc_infer[8facaf8255ab7cac]::traits::engine::TraitEngineExt>::select_all_or_error
  32:     0xffffa77eeb58 - rustc_traits[88a431509884da59]::codegen::codegen_select_candidate
  33:     0xffffa7cbbb64 - rustc_query_system[7175af1eaca55fc6]::query::plumbing::try_execute_query::<rustc_query_impl[9bf706fa0936a2ef]::queries::codegen_select_candidate, rustc_query_impl[9bf706fa0936a2ef]::plumbing::QueryCtxt>
  34:     0xffffa7b789b8 - <rustc_query_impl[9bf706fa0936a2ef]::Queries as rustc_middle[c197f7dd5ada7ea7]::ty::query::QueryEngine>::codegen_select_candidate
  35:     0xffffa69d81b8 - rustc_ty_utils[e19ef8fcd720bdb9]::instance::inner_resolve_instance
  36:     0xffffa69d7758 - rustc_ty_utils[e19ef8fcd720bdb9]::instance::resolve_instance
  37:     0xffffa7c86000 - rustc_query_system[7175af1eaca55fc6]::query::plumbing::try_execute_query::<rustc_query_impl[9bf706fa0936a2ef]::queries::resolve_instance, rustc_query_impl[9bf706fa0936a2ef]::plumbing::QueryCtxt>
  38:     0xffffa7b859fc - <rustc_query_impl[9bf706fa0936a2ef]::Queries as rustc_middle[c197f7dd5ada7ea7]::ty::query::QueryEngine>::resolve_instance
  39:     0xffffa8885cd4 - <rustc_middle[c197f7dd5ada7ea7]::ty::instance::Instance>::resolve_opt_const_arg
  40:     0xffffa8885808 - <rustc_middle[c197f7dd5ada7ea7]::ty::instance::Instance>::resolve
  41:     0xffffa6f5e6b0 - rustc_mir_transform[d2e7dbab7faafc68]::inline::cycle::mir_callgraph_reachable::process
[ like 400 identical calls ]
 425:     0xffffa6f5e934 - rustc_mir_transform[d2e7dbab7faafc68]::inline::cycle::mir_callgraph_reachable::process
 426:     0xffffa6f5e2c4 - rustc_mir_transform[d2e7dbab7faafc68]::inline::cycle::mir_callgraph_reachable
 427:     0xffffa7cb6054 - rustc_query_system[7175af1eaca55fc6]::query::plumbing::try_execute_query::<rustc_query_impl[9bf706fa0936a2ef]::queries::mir_callgraph_reachable, rustc_query_impl[9bf706fa0936a2ef]::plumbing::QueryCtxt>
 428:     0xffffa7b73ee8 - <rustc_query_impl[9bf706fa0936a2ef]::Queries as rustc_middle[c197f7dd5ada7ea7]::ty::query::QueryEngine>::mir_callgraph_reachable
 429:     0xffffa6efde84 - <rustc_mir_transform[d2e7dbab7faafc68]::inline::Inliner>::try_inlining
 430:     0xffffa6efcf60 - <rustc_mir_transform[d2e7dbab7faafc68]::inline::Inliner>::process_blocks
 431:     0xffffa6efca28 - <rustc_mir_transform[d2e7dbab7faafc68]::inline::Inline as rustc_middle[c197f7dd5ada7ea7]::mir::MirPass>::run_pass
 432:     0xffffa6f396ec - rustc_mir_transform[d2e7dbab7faafc68]::pass_manager::run_passes_inner
 433:     0xffffa6e51ee8 - rustc_mir_transform[d2e7dbab7faafc68]::optimized_mir

cc @cjgillot @saethlin

@cjgillot
Copy link
Contributor

From preliminary investigation, for_each_key is is polymorphically recursive: for_each_key<'_, F> calls for_each_key<'_, &mut F>. Downstream crates calling it should emit an error when building an executable. Do they?

@apoelstra
Copy link
Contributor

@cjgillot they do not, until some recent nightly compilers, in which they now start emitting errors.

@apoelstra
Copy link
Contributor

@cjgillot sorry, I am incorrect. Callers do emit errors. Previously this would happen later in the compile stage, and provide line number information, and only occur if they actually try to use the function. Now it occurs earlier, with no line number information, and regardless of whether the function is actually used.

@lnicola
Copy link
Member

lnicola commented May 4, 2023

I think we're hitting this on georust/geo#1010, but I can't actually reproduce it. Could it only fire intermittently or for some specific platforms?

The problematic code is https://github.com/georust/geo/blob/0.24.1/geo/src/algorithm/map_coords.rs#L850.

@apoelstra
Copy link
Contributor

Looks like this bug has hit stable now :(.

@jyn514 jyn514 added this to the 1.71.0 milestone Jul 13, 2023
apoelstra added a commit to BlockstreamResearch/rust-simplicity that referenced this issue Jul 13, 2023
Newly-released rustc 1.71 no longer compiles our rust-miniscript
dependency. Patch rust-miniscript, then elements-miniscript, then this,
to hack around it.
@cjgillot cjgillot added A-diagnostics Area: Messages for errors, warnings, and lints A-monomorphization Area: Monomorphization labels Jul 14, 2023
@cjgillot cjgillot removed their assignment Jul 14, 2023
uncomputable pushed a commit to uncomputable/rust-simplicity that referenced this issue Jul 15, 2023
Newly-released rustc 1.71 no longer compiles our rust-miniscript
dependency. Patch rust-miniscript, then elements-miniscript, then this,
to hack around it.
apoelstra added a commit to apoelstra/elements-miniscript that referenced this issue Jul 18, 2023
apoelstra added a commit to BlockstreamResearch/rust-simplicity that referenced this issue Jul 18, 2023
b608ec1 Cargo.toml: patch for rust-lang/rust#110475 (Andrew Poelstra)
ab184db human_encoding: use ErrorSet when finalizing NamedConstructNode (Andrew Poelstra)
70b989a human_encoding: introduce Error and ErrorSet (Andrew Poelstra)
9d379ce human_encoding: introduce NamedConstructNode (Andrew Poelstra)
64c6ea4 human_encoding: change assertl/assertr syntax (Andrew Poelstra)
2252354 types: replace 1 + A with A? in display (Andrew Poelstra)
8193fd2 types: don't put parentheses around display of outermost type (Andrew Poelstra)
2adb0a8 node: some more utility methods on Inner (Andrew Poelstra)
a516651 clippy: fix some new lints (Andrew Poelstra)
07ac274 analysis: feature-gate an import (Andrew Poelstra)

Pull request description:

  After/during review of #152 we decided to change the syntax in a couple ways:

  * We introduced the `?` syntax in types where `A?` is shorthand for `1 + A`
  * We changed the CMR syntax from `#expr` to `#{expr}`
  * We added a new `#<64 hex chars>` format for directly specifying CMRs without giving an expression

  This PR also includes some cleanups and new types and utility methods that will be used by the parser. Let me know if you want me to move any to the next PR, or if you'd like me to pull the parser into this one.

  Fixes #157

ACKs for top commit:
  uncomputable:
    ACK b608ec1 Feel free to address the nit.

Tree-SHA512: 91488059ab119d724cda84065256fd7941f09f39ee91b6f7b7453bc65a8b201164059a0cfc456a091916d920f2a1757caf9e2d613ec11cf5c607ebb6a620cac7
@NotGovernor
Copy link

I'm new to rust, having this error, but also am worried my amatuerity? will probably mean anything I could add here would be more noise than signal. :D So forgive me if that is the case here.

I wanted to contribute my path to duplicating this. My project uses SurrealDB which depends on the geo library mentioned above. When I compile locally on my windows machine there is no issue. However, when I use the official rust docker image https://hub.docker.com/_/rust which builds using the RUN cargo install --path . command I get the recursion overflow error.

When I modify the docker to do a normal RUN cargo build I no longer get the error. I'm new enough to not know why, I just wanted to drop this here in case it helps. <3

@lnicola
Copy link
Member

lnicola commented Jul 23, 2023

@NotGovernor I'm not familiar with SurrealDB, but the problematic code was removed in geo 0.25.0, and SurrealDB bumped that dependency from 0.25.0 to 0.25.1 about three weeks ago, so you shouldn't be seeing this.

I suggest checking which version you're using. cargo tree might help.

darosior added a commit to darosior/liana that referenced this issue Jul 31, 2023
See rust-lang/rust#110475.

I can't update my upstream rust-miniscript fork until
rust-bitcoin/rust-miniscript#566 is merged.
@sergei-pq
Copy link

I'm hitting this bug too with rust-miniscript. It's really strange error because it only happens if you have too many dependencies in your cargo crate. I can only reproduce this in a large repo with many dependencies / a lot of code, in a smaller isolated project with just miniscript and a few other dependencies, the issue doesn't happen, but it does in a multi-module crate which is really frustrating.

@apoelstra
Copy link
Contributor

@sergei-pq which version of rust-miniscript are you using? We have a patch but we haven't backported it to every version.

@sergei-pq
Copy link

sergei-pq commented Aug 3, 2023

@apoelstra I observed this behavior on 9.0.1 being pulled in from bdk crate -- the problem is I can't change bdk directly. Instead I tried to use this patch operation to fix it.

bdk = {version = "0.28.0", features = ["default", "keys-bip39", "bip39"]}
miniscript = "10.0.0"

[patch.crates-io]
miniscript = { version = "10.0.0" }

This actually works for hotfixing BDK (or it at least fixes the issue since bdk is miniscript ^9 and seems to not break the code yet. This fix worked on my most recent issue with miniscript -- although I'm not certain it's entirely resolved. If I can't get it working I'm likely going to have to fork bdk and upgrade that directly -- or use the backported patch strategy.

nbuffon added a commit to nbuffon/its-client that referenced this issue Aug 4, 2023
Previously used version 0.23 was carrying a [recursive declaration issue][1]

[1]: rust-lang/rust#110475

Signed-off-by: Nicolas Buffon <[email protected]>
sanket1729 added a commit to ElementsProject/elements-miniscript that referenced this issue Aug 4, 2023
6c9e8ab ci: pin some more deps for 1.48.0 (Andrew Poelstra)
f551774 patch elements-miniscript for rustc recursion bug (Andrew Poelstra)

Pull request description:

  cc rust-lang/rust#110475

Top commit has no ACKs.

Tree-SHA512: 499db6a81ca0b48e921aac458b1d6e55022ac7c161e48a0b3e4ffeb97abb1983a7b25e7bb1cce459b568c0b9ee97b3b62321ce8e1aa2ac6e4edc56ad83302d99
@benthecarman
Copy link
Author

@apoelstra would be great if it was backported for v9.x

@apoelstra
Copy link
Contributor

I have an open PR rust-bitcoin/rust-miniscript#566

nbuffon added a commit to nbuffon/its-client that referenced this issue Aug 8, 2023
Previously used version 0.23 was carrying a [recursive declaration issue][1]
Fixed since geo 0.25 as mentioned in [related issue][2]

[1]: rust-lang/rust#110475
[2]: georust/geo#1010

Signed-off-by: Nicolas Buffon <[email protected]>
@apoelstra
Copy link
Contributor

Done.

@ActuallyHappening
Copy link

ActuallyHappening commented Aug 16, 2023

Still getting this issue in rustlang/rust:nightly docker image
Cargo.toml contains geo="0.26.0", building with cargo +nightly chef cook --release ... and cargo leptos build --release
image

@Mark-Simulacrum Mark-Simulacrum added the regression-from-stable-to-stable Performance or correctness regression from one stable version to another. label Aug 18, 2023
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Aug 18, 2023
@apiraino
Copy link
Contributor

@ActuallyHappening I'm trying to reproduce your error. Using the latest rustc 1.73.0-nightly (0768872 2023-08-17) I can compile the project leptos with cargo build --release. I think in your screenshot I'm reading geo version 0.24.1 instead of 0.26.0 (as per your comment)?

@ActuallyHappening
Copy link

Your right, after some investigation my git dependancy on surrealdb was using the outdated version of geo:

[dependencies.surrealdb]
git = "https://github.com/surrealdb/surrealdb.git"
tag = "v1.0.0-beta.9"
optional = true

which lead to this dependancy being included (from surreal/lib/Cargo.toml in commit c3773b2e):

geo = { version = "0.24.1", features = ["use-serde"] }

My full project failed to compile (using cargo leptos build --release) on the latest nightly nightly-x86_64-unknown-linux-gnu unchanged - rustc 1.74.0-nightly (ef85656a1 2023-08-21), but DOES compile on stable stable-x86_64-unknown-linux-gnu unchanged - rustc 1.71.1 (eb26296b5 2023-08-03).
Using cargo tree shows that geo 0.24.1 is a dep of surrealdb.

I haven't completely solved this issue for my specific use case, but changing my Cargo.toml section to this seems to help:

# upgrading surrealdb so that it uses newer geo version
[dependencies.surrealdb]
git = "https://github.com/surrealdb/surrealdb.git"
# tag = "v1.0.0-beta.9"
branch = "main"
optional = true

# [patch.crates-io.geo]
# git = "https://github.com/georust/geo.git"
# branch = "main"

@apiraino
Copy link
Contributor

Can this issue can be closed since the geo crate was updated?

thanks for a feedback cc @benthecarman @ActuallyHappening

@rustbot label -I-prioritize

@rustbot rustbot removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Dec 14, 2023
@apoelstra
Copy link
Contributor

Probably everybody hitting this bug (the miniscript crate also did) has updated to fix or remove the affected code (which could not have been used, since this bug only affects unused code). Since multiple actual rustc releases went out with this bug, even though it was reported with multiple months' lead time and at least two instances of deployed affeced code.

It theoretically represents a regression though, in that you can write Rust code which compiles before 1.71 and does not compile after.

For my part (as a maintainer of miniscript) I wouldn't mind closing it.

jonathanmetzman pushed a commit to google/oss-fuzz that referenced this issue Apr 5, 2024
The rust-lang/rust#110475 issue referenced is no longer a problem since
the `geo` crate has been updated in SurrealDB. However, the change in
default Rust version in the image builder implemented in
#11681 breaks fuzzing due to SurrealDB now requiring Rust
1.77 after surrealdb/surrealdb#3591, leading to
a bump in the MSRV in surrealdb/surrealdb#3778.

I have replaced the obsolete `--cfg uuid_unstable` for the generic
`--cfg surrealdb_unstable`, which will allow fuzzing any features still
deemed unstable in SurrealDB including the new experimental parser.
apoelstra added a commit to apoelstra/elements-miniscript that referenced this issue Aug 8, 2024
The Concrete impl used a weird complier gitch `let pred = |x| pred(x)`
to avoid an infinite recursion bug (which in turn triggered a compiler
bug that they didn't fix even though I filed it a DAY AFTER IT WAS
INTRODUCED ON NIGHTLY and instead they waited 3 months and released it
on stable forcing me to backport a workaround to a gazillion versions of
rust-miniscript).

The bug was rust-lang/rust#110475 BTW, just to
make sure this commit triggers another notification on that issue.

Clippy doesn't like this. Clippy is obviously wrong but rather than
having this stupid fight again just switch to a different idiom.

Meanwhile, the Semantic impl of ForEachKey panics iff any keys are
present, regardless of the closure passed to it. This is funny but
completely useless and we should just remove it.
apoelstra added a commit to apoelstra/elements-miniscript that referenced this issue Aug 8, 2024
The Concrete impl used a weird complier gitch `let pred = |x| pred(x)`
to avoid an infinite recursion bug (which in turn triggered a compiler
bug that they didn't fix even though I filed it a DAY AFTER IT WAS
INTRODUCED ON NIGHTLY and instead they waited 3 months and released it
on stable forcing me to backport a workaround to a gazillion versions of
rust-miniscript).

The bug was rust-lang/rust#110475 BTW, just to
make sure this commit triggers another notification on that issue.

Clippy doesn't like this. Clippy is obviously wrong but rather than
having this stupid fight again just switch to a different idiom.

Meanwhile, the Semantic impl of ForEachKey panics iff any keys are
present, regardless of the closure passed to it. This is funny but
completely useless and we should just remove it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-monomorphization Area: Monomorphization C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests