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: assertion failed: self.stashed_diagnostics.is_empty() #12364

Closed
jessebraham opened this issue Feb 27, 2024 · 14 comments
Closed

ICE: assertion failed: self.stashed_diagnostics.is_empty() #12364

jessebraham opened this issue Feb 27, 2024 · 14 comments
Labels
C-bug Category: Clippy is not doing the correct thing I-ICE Issue: Clippy panicked, giving an Internal Compilation Error (ICE) ❄️

Comments

@jessebraham
Copy link

jessebraham commented Feb 27, 2024

Summary

We run clippy checks as part of our CI workflow in the esp-hal project. We have been using the nightly release channel for various reasons, and this has largely not been a problem over the last couple of years.

As of yesterday (2024-02-26) this ICE has reared its head. I had expected this to be resolved with a new nightly release, as has usually been the case in the rare cases we encounter problems like this, however we are still experiencing this ICE.

The most recent workflow run (as of time of writing) can be seen failing here:
https://github.com/esp-rs/esp-hal/actions/runs/8066109592/job/22033416529

I apologize, I am unable to provide a minimal verifiable example at this time; if my schedule clears up this week perhaps I can look into it, but can't make any promises.

Version

rustc 1.78.0-nightly (fc3800f65 2024-02-26)
binary: rustc
commit-hash: fc3800f65777a365b5125706d60f97e4d0675efe
commit-date: 2024-02-26
host: x86_64-apple-darwin
release: 1.78.0-nightly
LLVM version: 18.1.0

Error output

Backtrace

thread '<unnamed>' panicked at compiler/rustc_errors/src/lib.rs:565:9:
assertion failed: self.stashed_diagnostics.is_empty()
stack backtrace:
 0: _rust_begin_unwind
 1: core::panicking::panic_fmt
 2: core::panicking::panic
 3: <rustc_errors::DiagCtxtInner as core::ops::drop::Drop>::drop
 4: core::ptr::drop_in_place::<rustc_errors::DiagCtxt>
 5: core::ptr::drop_in_place::<rustc_session::parse::ParseSess>
 6: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::set::<clippy_lints::doc::needless_doctest_main::check::check_code_sample::{closure#0}::{closure#0}, (bool, alloc::vec::Vec<core::ops::range::Range<usize>>)>
 7: rustc_span::create_session_globals_then::<(bool, alloc::vec::Vec<core::ops::range::Range<usize>>), clippy_lints::doc::needless_doctest_main::check::check_code_sample::{closure#0}::{closure#0}>
 8: rustc_driver_impl::catch_fatal_errors::<clippy_lints::doc::needless_doctest_main::check::check_code_sample::{closure#0}, (bool, alloc::vec::Vec<core::ops::range::Range<usize>>)>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

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

note: we would appreciate a bug report: https://github.com/rust-lang/rust-clippy/issues/new?template=ice.yml

note: please attach the file at `/Users/jesse/Work/esp-hal/esp-hal/rustc-ice-2024-02-27T14_29_42-23311.txt` to your bug report

note: compiler flags: --crate-type lib -C embed-bitcode=no -C debuginfo=2 -C incremental=[REDACTED]

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
end of query stack
note: Clippy version: clippy 0.1.78 (fc3800f6 2024-02-26)

thread 'rustc' panicked at src/tools/clippy/clippy_lints/src/doc/needless_doctest_main.rs:125:10:
thread::spawn failed: Any { .. }
stack backtrace:
 0: _rust_begin_unwind
 1: core::panicking::panic_fmt
 2: core::result::unwrap_failed
 3: clippy_lints::doc::check_attrs
 4: <clippy_lints::doc::Documentation as rustc_lint::passes::LateLintPass>::check_item
 5: <rustc_lint::late::LateContextAndPass<rustc_lint::late::RuntimeCombinedLateLintPass> as rustc_hir::intravisit::Visitor>::visit_nested_item
 6: rustc_lint::late::late_lint_crate
 7: std::panicking::try::<(), core::panic::unwind_safe::AssertUnwindSafe<rustc_lint::late::check_crate::{closure#0}>>
 8: rustc_data_structures::sync::parallel::disabled::join::<rustc_lint::late::check_crate::{closure#0}, rustc_lint::late::check_crate::{closure#1}, (), ()>
 9: rustc_lint::late::check_crate
10: std::panicking::try::<(), core::panic::unwind_safe::AssertUnwindSafe<rustc_interface::passes::analysis::{closure#6}::{closure#1}::{closure#0}::{closure#1}::{closure#0}::{closure#3}::{closure#0}>>
11: <rustc_data_structures::sync::parallel::ParallelGuard>::run::<(), rustc_interface::passes::analysis::{closure#6}::{closure#1}::{closure#0}::{closure#0}::{closure#2}>
12: std::panicking::try::<(), core::panic::unwind_safe::AssertUnwindSafe<rustc_interface::passes::analysis::{closure#6}::{closure#1}::{closure#0}>>
13: <rustc_data_structures::sync::parallel::ParallelGuard>::run::<(), rustc_interface::passes::analysis::{closure#6}::{closure#1}::{closure#0}>
14: <rustc_session::session::Session>::time::<(), rustc_interface::passes::analysis::{closure#6}>
15: rustc_interface::passes::analysis
    [... omitted 2 frames ...]
16: <rustc_middle::ty::context::GlobalCtxt>::enter::<rustc_driver_impl::run_compiler::{closure#0}::{closure#1}::{closure#3}, core::result::Result<(), rustc_span::ErrorGuaranteed>>
17: <rustc_interface::interface::Compiler>::enter::<rustc_driver_impl::run_compiler::{closure#0}::{closure#1}, core::result::Result<core::option::Option<rustc_interface::queries::Linker>, rustc_span::ErrorGuaranteed>>
18: rustc_span::set_source_map::<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_interface::interface::run_compiler<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#0}>::{closure#0}::{closure#0}>
19: rustc_span::create_session_globals_then::<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#0}>::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#0}>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

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

note: we would appreciate a bug report: https://github.com/rust-lang/rust-clippy/issues/new?template=ice.yml

note: please attach the file at `/Users/jesse/Work/esp-hal/esp-hal/rustc-ice-2024-02-27T14_29_42-23311.txt` to your bug report

note: compiler flags: --crate-type lib -C embed-bitcode=no -C debuginfo=2 -C incremental=[REDACTED]

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
#0 [analysis] running analysis passes on this crate
end of query stack
note: Clippy version: clippy 0.1.78 (fc3800f6 2024-02-26)

error: could not compile `esp-hal` (lib)

@jessebraham jessebraham added C-bug Category: Clippy is not doing the correct thing I-ICE Issue: Clippy panicked, giving an Internal Compilation Error (ICE) ❄️ labels Feb 27, 2024
@y21
Copy link
Member

y21 commented Feb 27, 2024

I'm guessing the needless_doc_test_main lint needs to do what rust-lang/rust#121487 did for rustfmt, since it tries to parse doc test code in a new compiler session without using run_compiler. Cc @nnethercote does that sound right?

@y21
Copy link
Member

y21 commented Feb 27, 2024

For reference, MCVE: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=d488b7ddbb3ad540da9be8267c0e10f7

@nnethercote
Copy link
Contributor

nnethercote commented Feb 27, 2024

@y21 That's right, it'll probably need an emit_stashed_diagnostics just before both exits from the loop. So that's the quick fix.

Having said that, I'm thinking about changing the whole approach from rust-lang/rust#121206 because my changes are causing problems I didn't anticipate, which would make that quick fix unnecessary.

If you need this fixed ASAP I can do the quick fix (or you could do it if you like). Otherwise, waiting for a more comprehensive fix might be easier. What do you think?

@nnethercote
Copy link
Contributor

I will probably get to the comprehensive fix today, so it's probably best to wait for that.

@y21
Copy link
Member

y21 commented Feb 27, 2024

Nice, it's probably better to wait for the proper, more comprehensive fix then, I agree. Thanks!

@nnethercote
Copy link
Contributor

Some questions:

  • Why doesn't this show up in normal rust-lang/rust testing?
  • Why does it show up in esp-hal CI?
  • How can I run the needless_doc_test_main test myself?

Thanks.

@y21
Copy link
Member

y21 commented Feb 27, 2024

Not sure I understand. needless_doctest_main is a normal Clippy lint that looks at doc tests (such as the one example I linked), but it is not a test itself.

It was caught by a user of clippy and not by clippy itself or in rust's CI because we presumably had no such test or code that excercised that code path, but esp-hal did.

@nnethercote
Copy link
Contributor

Ah, that makes sense. I misinterpreted the name needless_doc_test_main as being the name of a test, rather than a lint. Thanks!

@nnethercote
Copy link
Contributor

Any suggestions on how I should write a test for this to add to clippy would be welcome :)

nnethercote added a commit to nnethercote/rust that referenced this issue Feb 28, 2024
I removed it in rust-lang#121206 because I thought thought it wasn't necessary.
But then I had to add an `emit_stashed_diagnostics` call elsewhere in
rustfmt to avoid the assertion failure (which took two attempts to get
right, rust-lang#121487 and rust-lang#121615), and now there's an assertion failure in
clippy as well (rust-lang/rust-clippy#12364).

So this commit just reinstates the call in `DiagCtxtInner::drop`. It
also reverts the rustfmt changes from rust-lang#121487 and rust-lang#121615, though it
keeps the tests added for those PRs.
@flip1995
Copy link
Member

The best way is to create a test file in src/tools/clippy/tests/ui/crashes/ice-12364.rs (assuming you want to do this in the rustc repo) and copy the reproducer y21 posted above: #12364 (comment)

You then might have to run ./x.py test src/tools/clippy --bless to generate a possible stderr file.

@nnethercote
Copy link
Contributor

This should be fixed by rust-lang/rust#121669.

@jessebraham
Copy link
Author

Awesome, thank you @nnethercote for the fix!

nnethercote added a commit to nnethercote/rust that referenced this issue Feb 29, 2024
I removed it in rust-lang#121206 because I thought thought it wasn't necessary.
But then I had to add an `emit_stashed_diagnostics` call elsewhere in
rustfmt to avoid the assertion failure (which took two attempts to get
right, rust-lang#121487 and rust-lang#121615), and now there's an assertion failure in
clippy as well (rust-lang/rust-clippy#12364).

So this commit just reinstates the call in `DiagCtxtInner::drop`. It
also reverts the rustfmt changes from rust-lang#121487 and rust-lang#121615, though it
keeps the tests added for those PRs.
@nnethercote
Copy link
Contributor

rust-lang/rust#121669 just merged, and will make it into the next Nightly, in the next 24 hours or so. Please close this PR once you have confirmed the problem is fixed, or let us know if it's not fixed.

nnethercote added a commit to nnethercote/rust that referenced this issue Mar 1, 2024
I removed it in rust-lang#121206 because I thought thought it wasn't necessary.
But then I had to add an `emit_stashed_diagnostics` call elsewhere in
rustfmt to avoid the assertion failure (which took two attempts to get
right, rust-lang#121487 and rust-lang#121615), and now there's an assertion failure in
clippy as well (rust-lang/rust-clippy#12364).

So this commit just reinstates the call in `DiagCtxtInner::drop`. It
also reverts the rustfmt changes from rust-lang#121487 and rust-lang#121615, though it
keeps the tests added for those PRs.
@jessebraham
Copy link
Author

Looks like our clippy CI job is operational again, so things appear to be fixed. Thank you again for resolving this!

github-actions bot pushed a commit to ytmimi/rustfmt that referenced this issue Apr 8, 2024
I removed it in #121206 because I thought thought it wasn't necessary.
But then I had to add an `emit_stashed_diagnostics` call elsewhere in
rustfmt to avoid the assertion failure (which took two attempts to get
right, #121487 and #121615), and now there's an assertion failure in
clippy as well (rust-lang/rust-clippy#12364).

So this commit just reinstates the call in `DiagCtxtInner::drop`. It
also reverts the rustfmt changes from #121487 and #121615, though it
keeps the tests added for those PRs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-ICE Issue: Clippy panicked, giving an Internal Compilation Error (ICE) ❄️
Projects
None yet
Development

No branches or pull requests

4 participants