-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
unreachable!("state is never set to invalid values") is reached #127979
Comments
@janhohenheim Can you bisect this to a specific commit? |
Also, can you specify your Windows version? Just in case. |
@workingjubilee sure, will do later. First, I'm trying to narrow the example a bit down. |
For future reference, permalink at that version https://github.com/rust-lang/rust/blob/22a5267c83a3e17f2b763279eb24bb632c45dc6b/library/std/src/sys/sync/once/futex.rs You should try the nightly too, @ChrisDenton recently did some changes there that may have happened to fix things |
@tgross35 thanks for the hint, updated the description :) |
Alright, added my Windows system information and linked to a smaller example. Will run |
This comment was marked as outdated.
This comment was marked as outdated.
@tgross35 oh sorry, I have "simplified" the example too much. That's my bad. Can you try with https://github.com/TheBevyFlock/bevy_quickstart ? Just running |
Ah, yeah reproduced with the quickstart |
Are you running a bisect, or still trying to minimize? Unfortunate that this takes almost 3 minutes to compile |
Relevant bit with RUST_BACKTRACE=full
|
This does not reproduce in debug mode, interesting |
@tgross35 minimizing is proving very annoying. I've asked a friend with a pretty fast machine in the meantime to run the bisect. |
Note that there are some custom settings in the release profile, namely: [profile.release]
codegen-units = 1
lto = "thin"
opt-level = "s"
strip = "debuginfo" I do not know if any of these is relevant. |
I've not looked at this properly yet but my off the cuff guess would be this is a compiler issue around dylibs. Specifically I would try Some notes off the top of my head:
Other things you can do is to try minimizing dependencies and features (e.g. |
Indeed! Seems like LTO is the difference |
Testing some more: this happens with either |
@ChrisDenton I've got the information about this failing on As for bisect: my friend @bash reports that the nightly on
Interestingly, this is the same error that I got with an entirely different setup: rust-lang/rustc_codegen_cranelift#1508 (comment) |
Thanks for verifying that this indeed fails on stable. |
Ah, it's a very different error on stable. The one in the OP would not be possible. |
That futex code isn't used by Windows on stable. |
When reporting "other people hit this too", you must determine the exact cause of each failure. It cannot simply be "it failed". You should also NOT list a compiler version if you don't have a confirmed, clean-build on-stable repro. It is quite possible there are 5 different bugs here. |
@workingjubilee fair enough, sorry. Backtrace
I assumed that |
I was able to reduce @ChrisDenton's example a bit further: fn main() {
let mut schedule = Schedule::default();
schedule.add_systems(noop);
}
fn noop() {} |
@workingjubilee makes sense. I'll remember that next time I open a |
huh, this is... weird. which libraries are being built as dylibs? |
I can reproduce at least as far back as 1.76 using bevy 0.13, |
The whole dynamic linking business goes through bevy_dylib, which, as far as I understand, pulls in bevy_internal, which in turn pulls in the rest of the Bevy crates. |
I was able to create a minimal example that doesn't involve bevy at all! 🎉 The dylib setup is modeled after that from bevy. Edit: I'm going to run bisect on this and see what we find out :) |
Awesome repro! Interesting that the dylib doesn't even have to do anything, just be there. In case anyone doesn't want to build & dump themselves: No LTO
With lto=thin
opt-level=2 to clean things up a bit |
Running bisect unattended yields
|
Betting it's in this rollup: Betting it's this PR: |
rust/compiler/rustc_codegen_llvm/src/consts.rs Lines 331 to 337 in ff4b398
can someone try this with |
[building] edit: grumble I need to update vstools https://users.rust-lang.org/t/error-could-not-compile-rustc-driver-lib-due-to-1-previous-error/113278, feel free to beat me to this if anyone is up to date |
Just finished. The new assertion doesn't hit (stage1), repro still crashes |
@wesleywiser and @michaelwoerister were on the PR, may want to take a look. It's interesting that the PR causing this behavior was intended to fix a similar bug, #81408 |
I see. The assert is hit but passes. Does reverting the actual change fix this? |
Was already building 😆 yes, confirmed that reverting 296489c causes the repro to not crash. |
Okay, I think it's fair to say "the way we handle linkage of dylibs when combined with compiler options like LTO results in arriving in what are supposed to be unreachable sections of std's code" is |
WG-prioritization assigning priority (Zulip discussion). @rustbot label -I-prioritize +P-high |
I tried this code:
Clone https://github.com/TheBevyFlock/bevy_quickstart and run
cargo run --release
on WindowsI expected to see this happen: Literally anything else than the standard library hitting unreachable code.
Instead, this happened: We hit this line of unreachable code: https://github.com/rust-lang/rust/blob/22a5267c83a3e17f2b763279eb24bb632c45dc6b/library/std/src/sys/sync/once/futex.rs
Meta
rustc --version --verbose
:Backtrace
The text was updated successfully, but these errors were encountered: