Skip to content

ICE: broken MIR in CoroutineKindShim #129074

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
matthiaskrgr opened this issue Aug 14, 2024 · 2 comments · Fixed by #129101
Closed

ICE: broken MIR in CoroutineKindShim #129074

matthiaskrgr opened this issue Aug 14, 2024 · 2 comments · Fixed by #129101
Assignees
Labels
A-async-closures `async || {}` C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@matthiaskrgr
Copy link
Member

auto-reduced (treereduce-rust):

#![feature(async_closure)]

fn main() {}

fn needs_fn_mut<T>(mut x: impl FnMut() -> T) {
    x();
}

fn hello(x: &Ty) {
    needs_fn_mut(async || {
        x.hello();
    });
}

struct Ty;
impl Ty {
    fn hello(self) {}
}

original:

//@ build-pass
//@ edition: 2021

// Regression test for https://github.com/rust-lang/rust/issues/26043
// if it has no self-borrows. In this case, `&Ty` is not borrowed from the closure env,
// since it's fine to reborrow it with its original lifetime. See the doc comment on
// `should_reborrow_from_env_of_parent_coroutine_closure` for more detail for when we
// must borrow from the closure env.

#![feature(async_closure)]

fn main() {
    hello(&Ty);
}

fn needs_fn_mut<T>(mut x: impl FnMut() -> T) {
    x();
}

fn hello(x: &Ty) {
    needs_fn_mut(async || { x.hello(); });
}

struct Ty;
impl Ty {
    fn hello(self) {}
}

Version information

rustc 1.82.0-nightly (e5b3e68ab 2024-08-13)
binary: rustc
commit-hash: e5b3e68abf170556b9d56c6f9028318e53c9f06b
commit-date: 2024-08-13
host: x86_64-unknown-linux-gnu
release: 1.82.0-nightly
LLVM version: 19.1.0

Command:
/home/matthias/.rustup/toolchains/master/bin/rustc -Zvalidate-mir --edition=2018

Program output

thread 'rustc' panicked at compiler/rustc_mir_transform/src/validate.rs:92:25:
broken MIR in CoroutineKindShim { coroutine_def_id: DefId(0:8 ~ mvce[40fc]::hello::{closure#0}::{closure#0}) } (after pass CheckPackedRef) at bb0[2]:
encountered `Assign((_4, move (*(_1.0: &&Ty))))` with incompatible types:
left-hand side has type: Ty
right-hand side has type: &Ty
stack backtrace:
   0:     0x7d48fabbe6ad - <std::sys::backtrace::BacktraceLock::print::DisplayBacktrace as core::fmt::Display>::fmt::hd8cddadc2f1356ba
   1:     0x7d48fb4049ef - core::fmt::write::hf4a4be5c7aa55e78
   2:     0x7d48fc3c5511 - std::io::Write::write_fmt::h937572d76c9c7a9b
   3:     0x7d48fabc0d8b - std::panicking::default_hook::{{closure}}::h663ce340916a5919
   4:     0x7d48fabc09fe - std::panicking::default_hook::h5cb270171c609139
   5:     0x7d48f9d4b6c9 - std[c898dee7a900a719]::panicking::update_hook::<alloc[f79bb38400b26b08]::boxed::Box<rustc_driver_impl[7bc6674bbf855457]::install_ice_hook::{closure#0}>>::{closure#0}
   6:     0x7d48fabc16a7 - std::panicking::rust_panic_with_hook::h527dcb0b253a1e21
   7:     0x7d48fabc1367 - std::panicking::begin_panic_handler::{{closure}}::hbc0a646a011dd19f
   8:     0x7d48fabbeb69 - std::sys::backtrace::__rust_end_short_backtrace::h44802a1bb26a785a
   9:     0x7d48fabc1034 - rust_begin_unwind
  10:     0x7d48f8150563 - core::panicking::panic_fmt::hb546fe36be3a0f24
  11:     0x7d48f86e7783 - <rustc_mir_transform[9616e0e93931520b]::validate::CfgChecker>::fail::<alloc[f79bb38400b26b08]::string::String>
  12:     0x7d48f86e5a17 - <rustc_mir_transform[9616e0e93931520b]::validate::Validator as rustc_middle[2e745f839ecdcb12]::mir::MirPass>::run_pass
  13:     0x7d48f9671540 - rustc_mir_transform[9616e0e93931520b]::pass_manager::validate_body
  14:     0x7d48fb402213 - rustc_mir_transform[9616e0e93931520b]::pass_manager::run_passes_inner
  15:     0x7d48fb40295f - rustc_mir_transform[9616e0e93931520b]::pass_manager::run_passes_inner
  16:     0x7d48fbbd8d2a - rustc_query_impl[52474172086763ba]::plumbing::__rust_begin_short_backtrace::<rustc_query_impl[52474172086763ba]::query_impl::mir_built::dynamic_query::{closure#2}::{closure#0}, rustc_middle[2e745f839ecdcb12]::query::erase::Erased<[u8; 8usize]>>
  17:     0x7d48fb6d3df9 - rustc_query_system[1f2263bc77e8d28d]::query::plumbing::try_execute_query::<rustc_query_impl[52474172086763ba]::DynamicConfig<rustc_query_system[1f2263bc77e8d28d]::query::caches::VecCache<rustc_span[6e02c34e35948369]::def_id::LocalDefId, rustc_middle[2e745f839ecdcb12]::query::erase::Erased<[u8; 8usize]>>, false, false, false>, rustc_query_impl[52474172086763ba]::plumbing::QueryCtxt, false>
  18:     0x7d48fb6d390d - rustc_query_impl[52474172086763ba]::query_impl::mir_built::get_query_non_incr::__rust_end_short_backtrace
  19:     0x7d48fb821700 - <rustc_mir_build[54542b08fd9c63c9]::check_unsafety::UnsafetyVisitor>::visit_inner_body
  20:     0x7d48f7ad5c04 - <rustc_mir_build[54542b08fd9c63c9]::check_unsafety::UnsafetyVisitor as rustc_middle[2e745f839ecdcb12]::thir::visit::Visitor>::visit_expr
  21:     0x7d48fb821814 - <rustc_mir_build[54542b08fd9c63c9]::check_unsafety::UnsafetyVisitor>::visit_inner_body
  22:     0x7d48f7ad5c04 - <rustc_mir_build[54542b08fd9c63c9]::check_unsafety::UnsafetyVisitor as rustc_middle[2e745f839ecdcb12]::thir::visit::Visitor>::visit_expr
  23:     0x7d48f7ad7027 - <rustc_mir_build[54542b08fd9c63c9]::check_unsafety::UnsafetyVisitor as rustc_middle[2e745f839ecdcb12]::thir::visit::Visitor>::visit_expr
  24:     0x7d48f7ad5c04 - <rustc_mir_build[54542b08fd9c63c9]::check_unsafety::UnsafetyVisitor as rustc_middle[2e745f839ecdcb12]::thir::visit::Visitor>::visit_expr
  25:     0x7d48fbc57619 - <rustc_mir_build[54542b08fd9c63c9]::check_unsafety::UnsafetyVisitor as rustc_middle[2e745f839ecdcb12]::thir::visit::Visitor>::visit_block
  26:     0x7d48f7ad7060 - <rustc_mir_build[54542b08fd9c63c9]::check_unsafety::UnsafetyVisitor as rustc_middle[2e745f839ecdcb12]::thir::visit::Visitor>::visit_expr
  27:     0x7d48f7ad5c04 - <rustc_mir_build[54542b08fd9c63c9]::check_unsafety::UnsafetyVisitor as rustc_middle[2e745f839ecdcb12]::thir::visit::Visitor>::visit_expr
  28:     0x7d48f7acf407 - rustc_mir_build[54542b08fd9c63c9]::check_unsafety::check_unsafety
  29:     0x7d48fbba063d - rustc_query_impl[52474172086763ba]::plumbing::__rust_begin_short_backtrace::<rustc_query_impl[52474172086763ba]::query_impl::check_unsafety::dynamic_query::{closure#2}::{closure#0}, rustc_middle[2e745f839ecdcb12]::query::erase::Erased<[u8; 0usize]>>
  30:     0x7d48fbba08d8 - rustc_query_system[1f2263bc77e8d28d]::query::plumbing::try_execute_query::<rustc_query_impl[52474172086763ba]::DynamicConfig<rustc_query_system[1f2263bc77e8d28d]::query::caches::VecCache<rustc_span[6e02c34e35948369]::def_id::LocalDefId, rustc_middle[2e745f839ecdcb12]::query::erase::Erased<[u8; 0usize]>>, false, false, false>, rustc_query_impl[52474172086763ba]::plumbing::QueryCtxt, false>
  31:     0x7d48fbba0581 - rustc_query_impl[52474172086763ba]::query_impl::check_unsafety::get_query_non_incr::__rust_end_short_backtrace
  32:     0x7d48fbbadffa - rustc_interface[922598e417aca69f]::passes::run_required_analyses
  33:     0x7d48fbf3611e - rustc_interface[922598e417aca69f]::passes::analysis
  34:     0x7d48fbf360f1 - rustc_query_impl[52474172086763ba]::plumbing::__rust_begin_short_backtrace::<rustc_query_impl[52474172086763ba]::query_impl::analysis::dynamic_query::{closure#2}::{closure#0}, rustc_middle[2e745f839ecdcb12]::query::erase::Erased<[u8; 1usize]>>
  35:     0x7d48fc38efae - rustc_query_system[1f2263bc77e8d28d]::query::plumbing::try_execute_query::<rustc_query_impl[52474172086763ba]::DynamicConfig<rustc_query_system[1f2263bc77e8d28d]::query::caches::SingleCache<rustc_middle[2e745f839ecdcb12]::query::erase::Erased<[u8; 1usize]>>, false, false, false>, rustc_query_impl[52474172086763ba]::plumbing::QueryCtxt, false>
  36:     0x7d48fc38ed0f - rustc_query_impl[52474172086763ba]::query_impl::analysis::get_query_non_incr::__rust_end_short_backtrace
  37:     0x7d48fc1ced29 - rustc_interface[922598e417aca69f]::interface::run_compiler::<core[57139b9ad86af3c3]::result::Result<(), rustc_span[6e02c34e35948369]::ErrorGuaranteed>, rustc_driver_impl[7bc6674bbf855457]::run_compiler::{closure#0}>::{closure#1}
  38:     0x7d48fc1b6690 - std[c898dee7a900a719]::sys::backtrace::__rust_begin_short_backtrace::<rustc_interface[922598e417aca69f]::util::run_in_thread_with_globals<rustc_interface[922598e417aca69f]::util::run_in_thread_pool_with_globals<rustc_interface[922598e417aca69f]::interface::run_compiler<core[57139b9ad86af3c3]::result::Result<(), rustc_span[6e02c34e35948369]::ErrorGuaranteed>, rustc_driver_impl[7bc6674bbf855457]::run_compiler::{closure#0}>::{closure#1}, core[57139b9ad86af3c3]::result::Result<(), rustc_span[6e02c34e35948369]::ErrorGuaranteed>>::{closure#0}, core[57139b9ad86af3c3]::result::Result<(), rustc_span[6e02c34e35948369]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[57139b9ad86af3c3]::result::Result<(), rustc_span[6e02c34e35948369]::ErrorGuaranteed>>
  39:     0x7d48fc1b6cfa - <<std[c898dee7a900a719]::thread::Builder>::spawn_unchecked_<rustc_interface[922598e417aca69f]::util::run_in_thread_with_globals<rustc_interface[922598e417aca69f]::util::run_in_thread_pool_with_globals<rustc_interface[922598e417aca69f]::interface::run_compiler<core[57139b9ad86af3c3]::result::Result<(), rustc_span[6e02c34e35948369]::ErrorGuaranteed>, rustc_driver_impl[7bc6674bbf855457]::run_compiler::{closure#0}>::{closure#1}, core[57139b9ad86af3c3]::result::Result<(), rustc_span[6e02c34e35948369]::ErrorGuaranteed>>::{closure#0}, core[57139b9ad86af3c3]::result::Result<(), rustc_span[6e02c34e35948369]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[57139b9ad86af3c3]::result::Result<(), rustc_span[6e02c34e35948369]::ErrorGuaranteed>>::{closure#1} as core[57139b9ad86af3c3]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
  40:     0x7d48fc1b706b - std::sys::pal::unix::thread::Thread::new::thread_start::h2b4f313cc68cf720
  41:     0x7d48f66a339d - <unknown>
  42:     0x7d48f672849c - <unknown>
  43:                0x0 - <unknown>

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

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: please make sure that you have updated to the latest nightly

note: rustc 1.82.0-nightly (e5b3e68ab 2024-08-13) running on x86_64-unknown-linux-gnu

note: compiler flags: -Z validate-mir -Z dump-mir-dir=dir

query stack during panic:
#0 [mir_built] building MIR for `hello::{closure#0}::{closure#0}`
#1 [check_unsafety] unsafety-checking `hello`
end of query stack

@rustbot label +F-async_closure

@matthiaskrgr matthiaskrgr added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Aug 14, 2024
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. A-async-closures `async || {}` labels Aug 14, 2024
@matthiaskrgr
Copy link
Member Author

nightly-2024-05-23
93e7cb8
=> #125259

@compiler-errors compiler-errors self-assigned this Aug 14, 2024
@compiler-errors
Copy link
Member

compiler-errors commented Aug 14, 2024

With this full minimization:

// flags: -Zvalidate-mir

#![feature(async_closure)]

fn hello(x: &Ty) {
    let c = async || {
        x.hello();
    };
}

struct Ty;
impl Ty {
    fn hello(self) {}
}

fn main() {}

The real blame commit is: #123349

@compiler-errors compiler-errors removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 14, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Aug 15, 2024
…-ref, r=lcnr

Fix projections when parent capture is by-ref but child capture is by-value in the `ByMoveBody` pass

This fixes a somewhat strange bug where we build the incorrect MIR in rust-lang#129074. This one is weird, but I don't expect it to actually matter in practice since it almost certainly results in a move error in borrowck. However, let's not ICE.

Given the code:

```
#![feature(async_closure)]

// NOT copy.
struct Ty;

fn hello(x: &Ty) {
    let c = async || {
        *x;
        //~^ ERROR cannot move out of `*x` which is behind a shared reference
    };
}

fn main() {}
```

The parent coroutine-closure captures `x: &Ty` by-ref, resulting in an upvar of `&&Ty`. The child coroutine captures `x` by-value, resulting in an upvar of `&Ty`. When constructing the by-move body for the coroutine-closure, we weren't applying an additional deref projection to convert the parent capture into the child capture, resulting in an type error in assignment, which is a validation ICE.

As I said above, this only occurs (AFAICT) in code that eventually results in an error, because it is only triggered by HIR that attempts to move a non-copy value out of a ref. This doesn't occur if `Ty` is `Copy`, since we'd instead capture `x` by-ref in the child coroutine.

Fixes rust-lang#129074
@bors bors closed this as completed in 6c898d2 Aug 15, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Aug 15, 2024
Rollup merge of rust-lang#129101 - compiler-errors:deref-on-parent-by-ref, r=lcnr

Fix projections when parent capture is by-ref but child capture is by-value in the `ByMoveBody` pass

This fixes a somewhat strange bug where we build the incorrect MIR in rust-lang#129074. This one is weird, but I don't expect it to actually matter in practice since it almost certainly results in a move error in borrowck. However, let's not ICE.

Given the code:

```
#![feature(async_closure)]

// NOT copy.
struct Ty;

fn hello(x: &Ty) {
    let c = async || {
        *x;
        //~^ ERROR cannot move out of `*x` which is behind a shared reference
    };
}

fn main() {}
```

The parent coroutine-closure captures `x: &Ty` by-ref, resulting in an upvar of `&&Ty`. The child coroutine captures `x` by-value, resulting in an upvar of `&Ty`. When constructing the by-move body for the coroutine-closure, we weren't applying an additional deref projection to convert the parent capture into the child capture, resulting in an type error in assignment, which is a validation ICE.

As I said above, this only occurs (AFAICT) in code that eventually results in an error, because it is only triggered by HIR that attempts to move a non-copy value out of a ref. This doesn't occur if `Ty` is `Copy`, since we'd instead capture `x` by-ref in the child coroutine.

Fixes rust-lang#129074
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-async-closures `async || {}` C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
3 participants