rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
94.69k stars 12.2k forks source link

Error tainting in parallel compiler #126485

Closed olafes closed 23 minutes ago

olafes commented 2 weeks ago

Emitting diagnostics in one thread taints every infcx out there, but other threads don't expect their infcx to become tainted if they themselves aren't emitting anything. This leads to skipped diagnostics, early bailing out of queries that could've run to completion or painful to debug ICEs. For example, take #120601: snippet below run with -Zthreads=3 --edition=2021 works, but crashes if you uncomment the last function.

struct Tuple(i32);

async fn tuple() -> Tuple {
    Tuple(1i32)
}

async fn xyz() {
    match tuple() {
        Tuple(_) => {},
    }
}

/* uncomment this function to observe crashes
async fn fail() {
    Fail(())
}
*/

In parallel compiler, code that does different things depending on the result of InferCtxt::tainted_by_errors() can introduce spurious failures or even replace previously resolved Ty's with Ty::new_error(): https://github.com/rust-lang/rust/blob/f9515fdd5aa132e27d9b580a35b27f4b453251c1/compiler/rustc_borrowck/src/nll.rs#L202 in regioncx.infer_opaque_types: https://github.com/rust-lang/rust/blob/f9515fdd5aa132e27d9b580a35b27f4b453251c1/compiler/rustc_borrowck/src/region_infer/opaque_types.rs#L133-L134 https://github.com/rust-lang/rust/blob/f9515fdd5aa132e27d9b580a35b27f4b453251c1/compiler/rustc_borrowck/src/region_infer/opaque_types.rs#L278-L285 relevant part of InferCtxt: https://github.com/rust-lang/rust/blob/f9515fdd5aa132e27d9b580a35b27f4b453251c1/compiler/rustc_infer/src/infer/mod.rs#L1219-L1239

rustc 1.81.0-nightly (f1586001a 2024-06-13)
binary: rustc
commit-hash: f1586001ace26df7cafeb6534eaf76fb2c5513e5
commit-date: 2024-06-13
host: x86_64-unknown-linux-gnu
release: 1.81.0-nightly
LLVM version: 18.1.7
olafes commented 2 weeks ago

@rustbot label T-compiler I-ICE A-diagnostics WG-compiler-parallel

oli-obk commented 2 weeks ago

yea, the reason this exists is that it wants to detect when queries invoked by the current InferCtxt cause errors. The way to fix this is to make more queries return Result<CurrentOutput, ErrorGuaranteed> or taint their output in case the output supports that.

Next steps include some data gathering: remove this check and look at the failing ui tests with -Ztreat-err-as-bug to see which queries are reporting errors, but not tainting their result.