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

Internal Compiler Error on stable (Broken MIR) #96512

Closed
conradludgate opened this issue Apr 28, 2022 · 15 comments · Fixed by #97325
Closed

Internal Compiler Error on stable (Broken MIR) #96512

conradludgate opened this issue Apr 28, 2022 · 15 comments · Fixed by #97325
Labels
C-bug Category: This is a bug. glacier ICE tracked in rust-lang/glacier. 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

@conradludgate
Copy link
Contributor

conradludgate commented Apr 28, 2022

Code

There's only so much that I can share. The code that seems to be erroring is similar to

enum Enum {
    Variant(Variant)
}

    tokio::spawn(async move {
        let Enum::Variant(value) = enum;
        dispatch(value).await.unwrap();
    });

Meta

rustc --version --verbose:

rustc 1.60.0 (7737e0b5c 2022-04-04)
binary: rustc
commit-hash: 7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c
commit-date: 2022-04-04
host: aarch64-apple-darwin
release: 1.60.0
LLVM version: 14.0.0
rustc 1.62.0-nightly (69a5d2481 2022-04-27)
binary: rustc
commit-hash: 69a5d2481e856a5a18885390b8cf6950b9ff8dd3
commit-date: 2022-04-27
host: aarch64-apple-darwin
release: 1.62.0-nightly
LLVM version: 14.0.1

Error output

warning: Error finalizing incremental compilation session directory `.../target/debug/incremental/app-k3kpcf5j5syk/s-g9a4ygf568-1sdh6k2-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 DefId(0:2156 ~ app[24b6]::{closure#0}::{closure#0}) ((_33.0: Variant)): can't project out of PlaceTy { ty: Enum, variant_index: None }
  --> src/app/src/handler.rs:90:29
   |
90 |       tokio::spawn(async move {
   |  _____________________________^
91 | |         let Enum::Variant(value) = enum;
92 | |         dispatch(value).await.unwrap();
93 | |     });
   | |_____^
   |
   = note: delayed at compiler/rustc_borrowck/src/type_check/mod.rs:860:31

error: internal compiler error: TyKind::Error constructed but no error reported
  |
  = note: delayed at compiler/rustc_borrowck/src/type_check/mod.rs:795:20

error: internal compiler error: TyKind::Error constructed but no error reported
  |
  = note: delayed at /rustc/69a5d2481e856a5a18885390b8cf6950b9ff8dd3/compiler/rustc_middle/src/ty/relate.rs:419:59

error: internal compiler error: broken MIR in DefId(0:2159 ~ app[24b6]::{closure#0}::{closure#0}::{closure#2}) (((_1.0: Variant).0: Variant)): bad field access (uuid::Uuid: Variant): NoSolution
  --> src/app/src/handler.rs:91:26
   |
91 |         let Enum::Variant(value) = enum;
   |                           ^^^^^
   |
   = note: delayed at compiler/rustc_borrowck/src/type_check/mod.rs:770:29
Backtrace

thread 'rustc' panicked at 'Box<dyn Any>', compiler/rustc_errors/src/lib.rs:1347:13
stack backtrace:
   0:        0x104dc3e30 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h5766446765602b2f
   1:        0x104e15730 - core::fmt::write::h1454cf4bd27645af
   2:        0x104db6e88 - std::io::Write::write_fmt::h6d4bd601dc063fa4
   3:        0x104dc6b30 - std::panicking::default_hook::{{closure}}::he3278aaa6dd4ffe3
   4:        0x104dc680c - std::panicking::default_hook::h83712f1c80d5cc94
   5:        0x10c0cf59c - rustc_driver[ab005e2210e4776d]::DEFAULT_HOOK::{closure#0}::{closure#0}
   6:        0x104dc720c - std::panicking::rust_panic_with_hook::hb3ff747d28bd5b62
   7:        0x10fcb30ec - std[288a9308ea2c3692]::panicking::begin_panic::<rustc_errors[300f3d566adbb64d]::ExplicitBug>::{closure#0}
   8:        0x10fcb30a4 - std[288a9308ea2c3692]::sys_common::backtrace::__rust_end_short_backtrace::<std[288a9308ea2c3692]::panicking::begin_panic<rustc_errors[300f3d566adbb64d]::ExplicitBug>::{closure#0}, !>
   9:        0x10ff847f4 - std[288a9308ea2c3692]::panicking::begin_panic::<rustc_errors[300f3d566adbb64d]::ExplicitBug>
  10:        0x10fcbe0b8 - std[288a9308ea2c3692]::panic::panic_any::<rustc_errors[300f3d566adbb64d]::ExplicitBug>
  11:        0x10fcc21d0 - <rustc_errors[300f3d566adbb64d]::HandlerInner as core[30328a0cdeb81e9a]::ops::drop::Drop>::drop
  12:        0x10c0e364c - core[30328a0cdeb81e9a]::ptr::drop_in_place::<rustc_session[cb45daf2430bcebe]::parse::ParseSess>
  13:        0x10c0e6c50 - <alloc[ba9ec2f9f2fb29e6]::rc::Rc<rustc_session[cb45daf2430bcebe]::session::Session> as core[30328a0cdeb81e9a]::ops::drop::Drop>::drop
  14:        0x10c0d6498 - core[30328a0cdeb81e9a]::ptr::drop_in_place::<rustc_interface[1aaaf20b10225d55]::interface::Compiler>
  15:        0x10c0d454c - rustc_span[cfdc8e2acaf4a144]::with_source_map::<core[30328a0cdeb81e9a]::result::Result<(), rustc_errors[300f3d566adbb64d]::ErrorGuaranteed>, rustc_interface[1aaaf20b10225d55]::interface::create_compiler_and_run<core[30328a0cdeb81e9a]::result::Result<(), rustc_errors[300f3d566adbb64d]::ErrorGuaranteed>, rustc_driver[ab005e2210e4776d]::run_compiler::{closure#1}>::{closure#1}>
  16:        0x10c07c720 - rustc_interface[1aaaf20b10225d55]::interface::create_compiler_and_run::<core[30328a0cdeb81e9a]::result::Result<(), rustc_errors[300f3d566adbb64d]::ErrorGuaranteed>, rustc_driver[ab005e2210e4776d]::run_compiler::{closure#1}>
  17:        0x10c0758f0 - <scoped_tls[726aaecfe4f649cb]::ScopedKey<rustc_span[cfdc8e2acaf4a144]::SessionGlobals>>::set::<rustc_interface[1aaaf20b10225d55]::interface::run_compiler<core[30328a0cdeb81e9a]::result::Result<(), rustc_errors[300f3d566adbb64d]::ErrorGuaranteed>, rustc_driver[ab005e2210e4776d]::run_compiler::{closure#1}>::{closure#0}, core[30328a0cdeb81e9a]::result::Result<(), rustc_errors[300f3d566adbb64d]::ErrorGuaranteed>>
  18:        0x10c0c286c - std[288a9308ea2c3692]::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface[1aaaf20b10225d55]::util::run_in_thread_pool_with_globals<rustc_interface[1aaaf20b10225d55]::interface::run_compiler<core[30328a0cdeb81e9a]::result::Result<(), rustc_errors[300f3d566adbb64d]::ErrorGuaranteed>, rustc_driver[ab005e2210e4776d]::run_compiler::{closure#1}>::{closure#0}, core[30328a0cdeb81e9a]::result::Result<(), rustc_errors[300f3d566adbb64d]::ErrorGuaranteed>>::{closure#0}, core[30328a0cdeb81e9a]::result::Result<(), rustc_errors[300f3d566adbb64d]::ErrorGuaranteed>>
  19:        0x10c09c520 - <<std[288a9308ea2c3692]::thread::Builder>::spawn_unchecked_<rustc_interface[1aaaf20b10225d55]::util::run_in_thread_pool_with_globals<rustc_interface[1aaaf20b10225d55]::interface::run_compiler<core[30328a0cdeb81e9a]::result::Result<(), rustc_errors[300f3d566adbb64d]::ErrorGuaranteed>, rustc_driver[ab005e2210e4776d]::run_compiler::{closure#1}>::{closure#0}, core[30328a0cdeb81e9a]::result::Result<(), rustc_errors[300f3d566adbb64d]::ErrorGuaranteed>>::{closure#0}, core[30328a0cdeb81e9a]::result::Result<(), rustc_errors[300f3d566adbb64d]::ErrorGuaranteed>>::{closure#1} as core[30328a0cdeb81e9a]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
  20:        0x104dcfadc - std::sys::unix::thread::Thread::new::thread_start::ha367c009d033f6c1
  21:        0x1908e626c - __pthread_deallocate

@conradludgate conradludgate added 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. labels Apr 28, 2022
@conradludgate
Copy link
Contributor Author

I've discovered that destructuring the enum variant outside of the async block fixes the issue

@conradludgate
Copy link
Contributor Author

@hellow554
Copy link
Contributor

hellow554 commented Apr 28, 2022

@conradludgate amazing that you did reduce your testcase like that.
I removed the external dependecy to tokio and silenced the unused warning:

struct Variant;

enum Enum {
    Variant(Variant),
}

fn spawn<T>(_: T)
where
    T: std::future::Future   Send   'static,
    T::Output: Send   'static,
{
}

async fn dispatch(_: Variant) {}

fn main() {
    let eh = Enum::Variant(Variant);

    spawn(async move {
        let Enum::Variant(value) = eh;
        dispatch(value).await;
    });
}

Regressed in rollup #91555 probably in #91450


searched nightlies: from nightly-2021-12-01 to nightly-2022-04-24
regressed nightly: nightly-2021-12-06
searched commits: from efec545 to e2116ac
regressed commit: 772d51f

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --end=2022-04-24 --without-cargo --access=github --regress=ice

@tmiasko
Copy link
Contributor

tmiasko commented Apr 28, 2022

Reduced a bit more (requires an edition with precise captures, I think):

struct Struct;
enum Enum { Variant(Struct) }
fn main() {
    let _enum = Enum::Variant(Struct);
    || {
        let Enum::Variant(_value) = _enum;
    };
}

@matthiaskrgr
Copy link
Member

Looks like a duplicate of #96299 and #95271

@lqd
Copy link
Member

lqd commented Apr 28, 2022

These started ICEing in nightly-2020-12-13 (GH range), with #79553 looking very likely.

@rust-lang-glacier-bot rust-lang-glacier-bot added the glacier ICE tracked in rust-lang/glacier. label Apr 28, 2022
@conradludgate
Copy link
Contributor Author

If no one is yet investigating this, I'd like to pick it up

@conradludgate
Copy link
Contributor Author

conradludgate commented May 1, 2022

So far, I've found 2 issues that cause this behaviour.

The first is that it's having trouble in this context to get the field type of the single variant enum:

ty::Adt(adt_def, substs) if !adt_def.is_enum() => {

Fix is easy,

- ty::Adt(adt_def, substs) if !adt_def.is_enum() => {
  ty::Adt(adt_def, substs) if adt_def.variants().len() == 1 => {

The second issue is that the MIR is being badly formed. Looking at the traces, I see

│ │ │ │ ├─┐rustc_borrowck::type_check::field_ty parent=((_1.0: Struct).0: Struct), base_ty=PlaceTy { ty: Struct, variant_index: None }, field=field[0], location=bb0[1]
│ │ │ │ │ ├─0ms DEBUG rustc_borrowck::type_check kind=Adt(Struct, [])
│ │ │ │ │ ├─0ms DEBUG rustc_borrowck::type_check adt_def=Struct, substs=[], variants=[VariantDef { def_id: DefId(0:3 ~ ice_96512[22c4]::Struct), ctor_def_id: Some(DefId(0:4 ~ ice_96512[22c4]::Struct::{constructor#0})), name: "Struct", discr: Relative(0), fields: [], ctor_kind: Const, flags: NO_VARIANT_FLAGS }]
│ │ │ │ │ ├─0ms DEBUG rustc_borrowck::type_check substs=[], variant=VariantDef { def_id: DefId(0:3 ~ ice_96512[22c4]::Struct), ctor_def_id: Some(DefId(0:4 ~ ice_96512[22c4]::Struct::{constructor#0})), name: "Struct", discr: Relative(0), fields: [], ctor_kind: Const, flags: NO_VARIANT_FLAGS }
│ │ │ │ ├─┘

Which hints to me that the code is not being read as

let Enum::Variant(_value) = _enum;

but instead

let Enum::Variant(Struct(_value)) = _enum;

@tmiasko
Copy link
Contributor

tmiasko commented May 2, 2022

The second issue is that the MIR is being badly formed. Looking at the traces, I see

Yeah, it looks like constructed MIR is incorrect.

One issue seems to be in to_upvars_resolved_place_builder, towards the end it has to rebase projections on top of a capture. First it has to remove the captured prefix, and the implementation makes the assumption that the number of projections in HIR and MIR will coincide. In the HIR downcasting is part of field projection, in the MIR those are two separate elements. Thus we are left with an extra incorrect projection.

There seems to be one more issue when constructing the capture - the downcasting operation is completely missing.

While the documentation for MIR ProjectionElem::Downcast claims that

Currently, we only introduce this for ADTs with more than one variant. It may be better to just introduce it always, or always for enums.

I think that is an outdated information.

@conradludgate
Copy link
Contributor Author

conradludgate commented May 2, 2022

I also just noticed the Downcast issue. In my traces, I do see the variant projection using this Downcast for just a single variant, and the suspect PR changes does seem to ignore all Downcast projections

(there's a little bit of discussion taking place on zulip https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/reading.20MIR.20.28.2396512.29/near/280855883)

@conradludgate
Copy link
Contributor Author

Should the fix be

  1. Keeping Downcast and truncation the capture correctly
  2. Making the single variant capture not use Downcast

@tmiasko
Copy link
Contributor

tmiasko commented May 2, 2022

@arora-aman or @roxelo might be in a better position to give advice here.

I think we should keep and / or add downcast where they are currently missing (e.g., when constructing the closure), and fix how projections corresponding to captured prefix are removed. As far the latter aspect is concerned, I think the issue is in the following lines (capture.place.projections are HIR projections which combine field and downcast into one operation, while from_builder.projection are MIR projection elements where those two operations are separate):

let next_projection = capture.place.projections.len();
let mut curr_projections = from_builder.projection;
// We used some of the projections to build the capture itself,
// now we apply the remaining to the upvar resolved place.
upvar_resolved_place_builder
.projection
.extend(curr_projections.drain(next_projection..));

@arora-aman
Copy link
Member

arora-aman commented May 3, 2022

There seems to be one more issue when constructing the capture - the downcasting operation is completely missing.

While the documentation for MIR ProjectionElem::Downcast claims that

Currently, we only introduce this for ADTs with more than one variant. It may be better to just introduce it always, or always for enums.

I think that is an outdated information.

I vaguely recall we relied on this to detect if we had a multi variant enum, in which case we would completely capture the root variable.

Edit: Ignore that, scrolling into the code bit more, there is comment that Downcast can be expected for single-variant enums

@tmiasko
Copy link
Contributor

tmiasko commented May 23, 2022

@conradludgate are you still planning to work on this issue?

@conradludgate
Copy link
Contributor Author

Not currently, I spent a few full days looking but I don't want to risk burning out debugging something that I don't completely understand 😓. I hope I at least narrowed it down a bit

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jun 7, 2022
…-aman

Fix precise field capture of univariant enums

When constructing a MIR from a THIR field expression, introduce an
additional downcast projection before accessing a field of an enum.

When rebasing a place builder on top of a captured place, account for
the fact that a single HIR enum field projection corresponds to two MIR
projection elements: a downcast element and a field element.

Fixes rust-lang#95271.
Fixes rust-lang#96299.
Fixes rust-lang#96512.
Fixes rust-lang#97378.

r? `@nikomatsakis` `@arora-aman`
@bors bors closed this as completed in fd76e0e Jun 7, 2022
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. glacier ICE tracked in rust-lang/glacier. 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
Development

Successfully merging a pull request may close this issue.

7 participants