-
Notifications
You must be signed in to change notification settings - Fork 13k
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
ICE: "assertion failed: *old == value" in optimized + incremental builds #126741
Comments
I just got a very similar ICE while compiling a proprietary codebase
Running the build again did not hit the same error. |
Ditto, similar ICE compiling a proprietary codebase. This was on CI. Rerunning on the same commit then succeeded.
Details
|
I got an ICE from the OP's code when building tests twice on x86_64-pc-windows-msvc with rustc 1.81.0-nightly (cc8da78 2024-07-04): ICE
|
Weirdly, I can't repro:
|
I did have to force it to rebuild by touching a file. |
Thanks, that does it for me too: ICE
|
I've minimized this as much as I can. It's quite small now, but still requires two crates and for some reason a path dependency doesn't work. https://github.com/saethlin/rantz_random The main crate is pub fn random() {
fastrand::bool()
}
pub fn seed() {
fastrand::seed();
} And the std::thread_local! {
static RNG: () = ();
}
#[inline]
fn with_rng(f: impl FnOnce()) {
RNG.with(|_| {
f()
})
}
#[inline]
pub fn seed() {
with_rng(|| {});
}
#[inline]
pub fn bool() {
with_rng(|| {})
} And you must have:
For me, the build ICEs about 1 in every 10 compilations, but it is of course random. |
searched nightlies: from nightly-2024-01-01 to nightly-2024-07-04 bisected with cargo-bisect-rustc v0.6.9Host triple: x86_64-unknown-linux-gnu cargo bisect-rustc --start=2024-01-01 --end=2024-07-04 --script=script So that's #125525 which is just a rewrite of the TLS implementation. All that tells me is that the compiler bug here predates that PR. |
Hunh, the ICE goes away if I add |
We are interning |
Try to fix ICE from re-interning an AllocId with different allocation contents I'm trying to fix the ICE in rust-lang#126741 and just based on println-debugging this lock scoping looks strange. r? `@ghost`
Ah! Turning off GVN makes the reproducer go away. That explains why this is being reported now. |
The root cause maybe that alloc id creation is untracked in incremental and only kept sane by stable hashing the GlobalAlloc instead of the raw index. Maybe there are some issues where we somehow leak the actual index |
I've further minimized the library crate to this: const RNG: fn() = || {};
fn with_rng() {
let _x = &RNG;
}
pub fn seed() {
with_rng();
}
pub fn bool() {
with_rng();
} |
This bisects to a handful of different commits, depending on what I set the start of the range to. I suspect that's because the reproducer relies on MIR inlining, and sometimes the critical inlining of @oli-obk I think this is more likely a race condition causing the incremental compilation decoding process to enter an invalid state. Of course I'm no expert on the code here, but the fact that triggering the crash requires multiple In a successful build, we decode In a crashing build, we decode rust/compiler/rustc_middle/src/mir/interpret/mod.rs Lines 256 to 258 in 0ca92de
... and I've been unable to find any other workload that tickles that line of code. So based on the available evidence, it looks like this whole concurrent decoding system just doesn't work. |
Yes I realize I'm making the very brave assertion that this PR from 6 years ago is buggy: #51060 I created this PR: #127442 which moves the lock scope so that instead of permitting racy decoding, the lock prevents racy decoding. This PR fixes the ICE, but I feel like it breaks the original design of the system. But also it seems to be perf-neutral. |
Try to fix ICE from re-interning an AllocId with different allocation contents As far as I can tell, based on my investigation in rust-lang#126741, the racy decoding scheme implemented here was never fully correct, but the arrangement of Allocations that's required to ICE the compiler requires some very specific MIR optimizations to create. As far as I can tell, GVN likes to create the problematic pattern, which is why we're noticing this problem now. So the solution here is to not do racy decoding. If two threads race to decoding an AllocId, one of them is going to sit on a lock until the other is done.
Try to fix ICE from re-interning an AllocId with different allocation contents As far as I can tell, based on my investigation in rust-lang/rust#126741, the racy decoding scheme implemented here was never fully correct, but the arrangement of Allocations that's required to ICE the compiler requires some very specific MIR optimizations to create. As far as I can tell, GVN likes to create the problematic pattern, which is why we're noticing this problem now. So the solution here is to not do racy decoding. If two threads race to decoding an AllocId, one of them is going to sit on a lock until the other is done.
Fixed by #127442 |
Try to fix ICE from re-interning an AllocId with different allocation contents As far as I can tell, based on my investigation in rust-lang/rust#126741, the racy decoding scheme implemented here was never fully correct, but the arrangement of Allocations that's required to ICE the compiler requires some very specific MIR optimizations to create. As far as I can tell, GVN likes to create the problematic pattern, which is why we're noticing this problem now. So the solution here is to not do racy decoding. If two threads race to decoding an AllocId, one of them is going to sit on a lock until the other is done.
Repro:
Rustc Version: rustc 1.81.0-nightly (59e2c01 2024-06-17)
Toolchain: nightly-x86_64-pc-windows-msvc
What I did:
Clone https://www.github.com/BobG1983/rantz_random.git
run cargo test
run cargo test again.
Issue:
The compiler panics with the following output:
The text was updated successfully, but these errors were encountered: