-
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
Missing ICE info after some compiler panics #105637
Comments
Requesting prioritization because affects the quality of bug reports and may interfere with bisection. @rustbot prioritize |
@jruderman do you test on an x86_64 mac? otherwise, I'm curious how you bisected this to #103647. |
Yes, this is all on x86_64 mac. |
I'm not sure why this would affect ICEs so I've posted the #105646 revert to allow time for some investigation. |
Revert "enable ThinLTO for rustc on x86_64-apple-darwin dist builds" Apparently ThinLTO on x64 mac can regress some of the ICEs' output. This reverts rust-lang#103647 to allow for investigation, and fixes rust-lang#105637 in the meantime.
This issue isn't platform-specific, and happens on all nightlies built with ThinLTO:
Examples:
#![feature(associated_const_equality)]
#![allow(dead_code)]
trait TraitWAssocConst {
const A: usize;
}
fn foo<T: TraitWAssocConst<A = 1>>() {}
fn bar<T: TraitWAssocConst<A = 0>>() {
foo::<T>();
}
fn main() {} And we can see on the playground that the usual ICE post-trace output is missing. Note that this ICE was not present on
struct Apple((Apple, Option(Banana ? Citron)));
fn main() {} We can see the ICE is missing the post trace output on the current beta (1.66.beta.2) on the playground, and the output changed in Since this is useful output for reporting issues, debugging and bisection, should we do something about ThinLTO landing on stable on linux in 2 days (e.g. disabling |
WG-prioritization assigning priority (Zulip discussion). @rustbot label -I-prioritize +P-critical |
Revert "enable ThinLTO for rustc on x86_64-apple-darwin dist builds" Apparently ThinLTO on x64 mac can regress some of the ICEs' output. This reverts rust-lang#103647 to allow for investigation, and helps with rust-lang#105637 in the meantime.
#105800 should fix the |
Don't copy symbols from dylibs with `-Zdylib-lto` When `rustc_driver` started being built with `-Zdylib-lto -Clto=thin`, some libstd symbols were copied by the LTO process into the dylib. That causes duplicate local symbols that are not present otherwise. Depending on the situation (lib loading order apparently), the duplicated symbols could cause issues: `rustc_driver` overrode the panic hook, but it didn't apply to rustc main's hook (the default from libstd). This is the cause of rust-lang#105637, in some situations the panic hook installed by `rustc_driver` isn't executed, and only libstd's backtrace is shown (and a double panic). The query stack, as well as the various notes to open a GH about the ICE, don't appear. It's not clear exactly what is needed to trigger the issue, but I have simulated a reproducer [here](https://github.com/lqd/issue-105637) with cargo involved, the incorrect panic hook is executed on my machine. It is hard to reproduce in a unit test: `cargo run` + `rustup` involves LD_LIBRARY_PATH, which is not the case for `compiletest`. cargo also adds unconditional flags that are then overridden in [`bootstrap` when building rustc with `rust.lto = thin`](https://github.com/rust-lang/rust/blob/9c07efe84f28a44f3044237696acc295aa407ee5/src/bootstrap/compile.rs#L702-L714) as done on CI). All this to say the compilation and execution environment in `bootstrap` leading to the bug building `rustc_driver` is different from our UI tests, and I believe one of the reasons it's hard to make an exact reproducer test. Thankfully there's _still_ a difference in the behavior though: although in the unit test the correct panic hook seems to be executed compared to my repro and the current nightly, only the fix removes the double panic here. The `7e8277aefa12f1469fb1df01418ff5846a7854a9` `try` build: - fixes the reproducer repo linked above - restores the ICE messages from rust-lang#105321 back to the state in its OP compared to the description in rust-lang#105637 - restores the ICE message and the query stack from rust-lang#105777 compared to nightly While I believe this technically fixes the P-critical issue rust-lang#105637, I would not want to close it yet as we may want to backport to beta/stable (if a point release happens, it would fix the ICEs reported on 1.66.0, which is built with ThinLTO on linux). Once this PR lands, I'll also open another PR to re-enable ThinLTO on x64 darwin's dist builder.
…mulacrum Re-enable ThinLTO for rustc on `x86_64-apple-darwin` ThinLTO was disabled on x64 mac in rust-lang#105646 because of the rust-lang#105637 regression. It was later discovered that the issue was present on other targets as well, as the mac revert was already landing. The linux/win reverts, however, did not land before the root cause was identified. rust-lang#105800 fixed the underlying issue in `-Zdylib-lto` handling, and the x64 msvc and linux targets are now fixed, ICEs are using the correct `rustc_driver` panic hook. This PR re-enables ThinLTO on mac for improved perf now that the issue should be fixed everywhere.
Can this be closed now? |
Probably. The fix has landed on nightly and beta, but the PR is still stable-nominated. It's not really critical to do such a backport unless someone experiences issues on 1.66 or the triage WG sees too many bug reports they don't like. |
As @lqd says - the stable backport was discussed but ultimately kept "on hold" to see how the patch behaves in the beta channel. A stable backport sometimes means a point release and that's up to Ok if you prefer to close this issue, we can always re-open if a stable backport will be approved (that will be discussed probably next week) |
This is several months old at this point, I think it's safe to say there won't be a stable backport 😄 |
Several panic messages changed recently:
unwrap
onNone
infind_cycle_in_stack
#105321, the output now includes "panic in a function that cannot unwind"Missing output example
As an example, here's the post-stack-trace output when testing #105632 with an older build:
Regression
Seems to be a regression from bdb07a8 (#103647) (@lqd)
Version
The text was updated successfully, but these errors were encountered: