Unstable fingerprints tracking issue

[Mark-Simulacrum]

For users hitting this bug: please read this blog post which discusses what you can do. An excerpt is below:

The Internal Compiler Error asks you to report a bug, and if you can do so, we still want that information. We want to know about the cases that are failing.

But regardless of whether or not you file a bug, the problem here can be resolved by either:

1. deleting your incremental compilation cache (e.g. by running cargo clean for the whole project or cargo clean -p crate-causing-the-error), or
2. force incremental compilation to be disabled, by setting CARGO_INCREMENTAL=0 in your environment or setting build.incremental to false in your $HOME/.cargo/config.toml file (which might also be called $HOME/.cargo/config) by adding this:

build.incremental = 'false'

We recommend that users of 1.52.0 disable incremental compilation, to avoid running into this problem.

We do not recommend that users of 1.52.0 downgrade to an earlier version of Rust in response to this problem. There is at least one instance of a silent miscompilation caused by incremental compilation that was not caught until we added the fingerprint checking.

---

I believe the (new) ICEs as a result of improved error detection in the incremental code will land in 1.53 onto stable, which is in 6 weeks. We are unlikely to fix all of the fallout (i.e. all bugs, at least 25 of which are currently open, though some are duplicates) by that time and be willing to backport the fixes. I suspect we are also unlikely to want to disable the new assertions; they are catching known unsoundness. I am opening this issue to discuss possible alternatives, which are hopefully more user friendly than the current ICE.

I think one direct improvement is to adjust the panic/assert on encountering this to be a proper compile error, which can tell the user that they should (for example) delete the incremental directory and invoke the compiler again. This is already a significant improvement over the current message, I suspect. We can still ask that they file a bug report.

We should also consider if it's worth considering applying performance-hurting mitigations; it seems definitely true we can drop incremental support on beta/stable until these bugs are fixed, but perhaps a smaller hammer is also viable.

Table of ICEs:

Issue	Query name	Has MCVE	PR fixing issue	fixed in 1.5x?	PR adding regression test	Done
#83259	predicates_of	✅	#84233	1.53	#84233	✅
#83311	proc_macro_decls_static					N/A
#83126	extern_mod_stmt_cnum	mcve-83126	#83153	1.52	None yet	❌
#85197	optimized_mir	mcve-85197	#85211	1.54	#85211	✅
#84225	exported_symbols	?	#84226	1.53	None yet	❌
#85019	item_children	?	#83901	1.54	None yet	❌
#83538	evaluate_obligation (EvaluatedToOk and EvaluatedToOkModuloRegions)	✅	#85186	1.54	#85186	✅
#84963	evaluate_obligation (OverflowError)	✅	None yet	?	None yet	❌
	native_libraries	?	#85702	1.54	#85702	✅
#85360	evaluate_obligation	✅	#85868	1.56	#91065	✅

cc @Aaron1011 (driver of fixing the bugs, I believe)
cc @pnkfelix @wesleywiser (T-compiler leads)
cc @rust-lang/release for awareness

[Mark-Simulacrum]

Nominating for T-compiler as I believe we should surface this during a meeting - likely for some small discussion, but mostly for awareness, and to see if we can get some help for possible ideas. I think this is a P-critical bug - not in the sense that we must have a fix for the underlying cause, but in the sense that this issue needs to be addressed in time for the next stable release.

I think it is likely worth applying the same fix to nightly as well, but perhaps in a channel-gated way if it has (for example) simply performance implications. This is a bit unclear.

[Aaron1011]

I think emitting a compiler error would be a great idea. However, I think it should still encourage users to open an issue (emphasizing that this is an internal compiler bug), so that we can see if any new issues pop up.

[Aaron1011]

OUTDATED: See the new table in #84970 (comment)

Here's the current state of the crashes involving different queries:

• item_children - this is caused by not hashing attributes. PR Integrate attributes as part of the crate hash #83901 fixed this
• predicate_of - this was fixed by Add TRACKED_NO_CRATE_HASH and use it for --remap-path-prefix #84233. However, we could still use a regression test before closing the issue Incremental compilation ICE with predicates_of #84341
• evaluate_obligation (EvaluatedToOk/EvaluatedToOkModuloRegions) - this has an open PR Use EvaluatedToOkModuloRegions when we get a coinductive match #83913, but that PR has a large performance impact. I've been working on minimizing the syn crash, which might yield more insight into how to avoid the performance problem.
• evaluate_obligation (OverflowError) - this is issue Incremental compilation ICE with from compiler-state-dependent overflow errors  #84963. This is completely different from the other evaluate_obligation crash, and I haven't had a chance to investigate it yet. Hopefully, the fix will be much simpler.
• optimized_mir - this is issue ICE found unstable fingerprints for optimized_mir #84960 and thread 'rustc' panicked at 'found unstable fingerprints for optimized_mir #85064

As far as I know, these are all of the currently existing crashes. If anyone sees a crash involving a query other than one of these, please add it to this list.

[jyn514]

item_children - this is caused by not hashing attributes. PR #79519 is open with a fix

I think you linked to the wrong pr - 79519 is storing attributes in a side table and has already been merged.

[lqd]

item_children (which was seen in #83887 and one the reported cases of #84335) should be fixed by #83901

[jonas-schievink]

I suspect we are also unlikely to want to disable the new assertions; they are catching known unsoundness.

I think this would actually most likely be fine – these ICE-causing bugs have existed for years, and it took very long for someone to run into a miscompilation.

[Mark-Simulacrum]

Well, a known miscompilation - certainly the amount of ICEs we've seen suggests plenty of wrong results, right? Maybe in practice those don't cause problems, I don't know, but I'd rather give an error, especially if we can also e.g. poison the incremental cache automatically and make it just a matter of rerunning the compiler. In theory we could even teach Cargo to do that, though maybe that's going too far.

[Mark-Simulacrum]

We discussed this during T-compiler meeting today, and decided on these actions:

• @Aaron1011 will work on a backportable PR that switches the ICE to a nicer-looking (but still clearly 'our fault' vs. user's fault message) error, using the standard error infrastructure. This is intended to more clearly communicate that the bug is known, and that we're working on it, while also providing a clear suggestion on how to fix it (cargo clean? rm -rf /path/to/incremental/dir?).
  • There was some thoughts about possibly wanting to change all ICEs to be nicer to users, but this was left out of scope; we definitely don't want to backport that.
• We will not disable the ICE on beta/stable, since it is a pretty clear bug, and even though known problems arising from it are rare, better to be safe.