Skip to content
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: Broken MIR in Item(...) with let_else usage (use of local which has no storage here) #102317

Closed
fasterthanlime opened this issue Sep 26, 2022 · 19 comments · Fixed by #102394
Closed
Assignees
Labels
C-bug Category: This is a bug. F-let_else Issues related to let-else statements (RFC 3137) I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@fasterthanlime
Copy link
Contributor

fasterthanlime commented Sep 26, 2022

Reminiscent of #99975, which is now closed, but I'm able to reproduce it easily on today's nightly:

struct SegmentJob {
    spec: String,
}

pub async fn run() -> Result<(), ()> {
    let jobs = get_data();
    let Some(job) = jobs.into_iter().next() else {
        println!("no jobs!");
        return Ok(())
    };

    println!("should run job {}", job.spec);

    Ok(())
}

fn get_data() -> Vec<SegmentJob> {
    Default::default()
}

I expected to see this happen: typechecks fine.

Instead, this happened:

cargo clippy
    Checking broken-mir v0.1.0 (/home/amos/bearcove/broken-mir)
warning: Error finalizing incremental compilation session directory `/home/amos/bearcove/broken-mir/target/debug/incremental/broken_mir-3jg493t7jkrit/s-gdwtcmorgw-1apjoc8-working`: No such file or directory (os error 2)

error: internal compiler error: no errors encountered even though `delay_span_bug` issued

error: internal compiler error: broken MIR in Item(WithOptConstParam { did: DefId(0:7 ~ broken_mir[e8cc]::run::{closure#0}), const_param_did: None }) (after pass PhaseChange-Runtime(Optimized)) at bb29[0]:
                                use of local _21, which has no storage here
  --> src/lib.rs:10:6
   |
10 |     };
   |      ^
   |
   = note: delayed at compiler/rustc_const_eval/src/transform/validate.rs:128:36

thread 'rustc' panicked at 'Box<dyn Any>', compiler/rustc_errors/src/lib.rs:1530:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust-clippy/issues/new

note: Clippy version: clippy 0.1.65 (f5193a9 2022-09-25)

query stack during panic:
end of query stack
warning: `broken-mir` (lib) generated 1 warning
error: could not compile `broken-mir`; 1 warning emitted

Meta

rustc --version --verbose:

rustc 1.66.0-nightly (f5193a9fc 2022-09-25)
binary: rustc
commit-hash: f5193a9fcc73dc09e41a90c5a2c97fc9acc16032
commit-date: 2022-09-25
host: x86_64-unknown-linux-gnu
release: 1.66.0-nightly
LLVM version: 15.0.0
Backtrace

thread 'rustc' panicked at 'Box<dyn Any>', compiler/rustc_errors/src/lib.rs:1530:13
stack backtrace:
   0: std::panicking::begin_panic::<rustc_errors::ExplicitBug>
   1: std::panic::panic_any::<rustc_errors::ExplicitBug>
   2: <rustc_errors::HandlerInner as core::ops::drop::Drop>::drop
   3: core::ptr::drop_in_place::<rustc_session::parse::ParseSess>
   4: <alloc::rc::Rc<rustc_session::session::Session> as core::ops::drop::Drop>::drop
   5: core::ptr::drop_in_place::<rustc_interface::interface::Compiler>
   6: rustc_interface::interface::create_compiler_and_run::<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver::run_compiler::{closure#1}>
   7: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::set::<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>

@fasterthanlime fasterthanlime added the C-bug Category: This is a bug. label Sep 26, 2022
@fasterthanlime
Copy link
Contributor Author

(I forgot to mention it in the original issue, but the let = if let Some(..) version of the code checks just fine).

@Noratrieb
Copy link
Member

@rustbot label +E-needs-mcve

@rustbot rustbot added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Sep 26, 2022
@Noratrieb
Copy link
Member

Oh nvm this has an MCVE already lol
@rustbot label -E-needs-mcve

@rustbot rustbot removed the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Sep 26, 2022
@fasterthanlime
Copy link
Contributor Author

Oh nvm this has an MCVE already lol

Oh yeah this is the complete code - here's my repo for it, pinned toolchain and all: https://github.com/fasterthanlime/broken-mir

@est31
Copy link
Member

est31 commented Sep 26, 2022

The issue seems to not affect cargo build, but requires clippy instead... interesting.

@est31
Copy link
Member

est31 commented Sep 26, 2022

Given that it is both a scoping problem, and lint related, this might be fixed by #102257. If that PR is the fix, we should maybe backport it to beta.

cc @dingxiangfei2009

@Rageking8
Copy link
Contributor

Rageking8 commented Sep 26, 2022

Tested it with the rust playground and was only able to get the ICE to trigger for release mode and not debug mode builds (current latest nightly).

@rustbot label +T-compiler +I-ICE +F-let-else +requires-nightly

@rustbot rustbot added F-let_else Issues related to let-else statements (RFC 3137) I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 26, 2022
@fasterthanlime
Copy link
Contributor Author

Tested it with the rust playground and was only able to get the ICE to trigger for release mode and not debug mode builds (current latest nightly).

That's true for cargo build. cargo clippy always fails though (I'm not sure if --release does anything here).

@Rageking8
Copy link
Contributor

On a side note why is the let else label not delimited by a _ in the center, like the other feature labels?

@Rageking8
Copy link
Contributor

The issue seems to not affect cargo build, but requires clippy instead... interesting.

The ICE can also be reproduced with cargo build as mentioned above, although only in release mode (for latest nightly, on playground). Not sure whether it is lint related or not.

@est31
Copy link
Member

est31 commented Sep 26, 2022

The ICE can also be reproduced with cargo build as mentioned above, although only in release mode (for latest nightly, on playground). Not sure whether it is lint related or not.

Oh it seems to issue the ICE if you are both in release mode and you use the "Build" feature of the playground. I've tried using the "Run" feature in release mode but it didn't issue the ICE. The playground changes automatically from "Run" to "Build" if you remove the main function, but in this instance I'd added a fn main() {} so it didn't change to "Build" for me. Interesting...

@dingxiangfei2009
Copy link
Contributor

dingxiangfei2009 commented Sep 26, 2022

Interestingly dropping the async from fn run produces a correct MIR with -C opt-level=3. I am having a look.

@rustbot claim

@est31
Copy link
Member

est31 commented Sep 26, 2022

Okay I have locally applied #102257 and and the PR does not fix this issue, still there.

@est31
Copy link
Member

est31 commented Sep 26, 2022

FTR this is the test file I've used (might be helpful for @dingxiangfei2009 ):

// issue #102317
// run-pass
// compile-flags: --edition 2021 -C opt-level=3 -Zvalidate-mir

struct SegmentJob {
    spec: String,
}

pub async fn run() -> Result<(), ()> {
    let jobs = Vec::<SegmentJob>::new();
    let Some(job) = jobs.into_iter().next() else {
        return Ok(())
    };

    Ok(())
}

fn main() {}

@estebank estebank added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed requires-nightly This issue requires a nightly compiler in some way. labels Sep 27, 2022
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Sep 27, 2022
@estebank
Copy link
Contributor

Removing requires-nightly because it is reproducible in beta too, marking as beta regression so this doesn't accidentally slip into stable.

@dingxiangfei2009
Copy link
Contributor

@est31 investigating unwind drops introduced by build_exit_tree. It looks terribly wrong. 🤔

@Noratrieb
Copy link
Member

This might be the same problem as #99852. I have looked into this before with @JakobDegen, which lead us to discover #100513. I have no idea what exactly is wrong in the code though, it's very complicated.

@Rageking8
Copy link
Contributor

Removing requires-nightly because it is reproducible in beta too, marking as beta regression so this doesn't accidentally slip into stable.

Oops, accidentally added that. My bad.

@apiraino
Copy link
Contributor

apiraino commented Oct 4, 2022

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Oct 4, 2022
@bors bors closed this as completed in c97d02c Oct 5, 2022
Aaron1011 pushed a commit to Aaron1011/rust that referenced this issue Jan 6, 2023
…-obk

Fix unwind drop glue for if-then scopes

cc `@est31`

Fix rust-lang#102317
Fix rust-lang#99852

This PR fixes the drop glue for unwinding from a panic originated in a drop while breaking out for the else block in an `if-then` scope.
MIR validation does not fail for the synchronous versions of the test program, because `StorageDead` statements are skipped over in the unwinding process. It is only becoming a problem when it is inside a generator where `StorageDead` must be kept around.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. F-let_else Issues related to let-else statements (RFC 3137) I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants