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 with generators: Broken MIR: generator contains type ... in MIR, but typeck only knows about ... #44184

Closed
Eroc33 opened this issue Aug 30, 2017 · 8 comments · Fixed by #44480
Labels
A-coroutines Area: Coroutines A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html 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

@Eroc33
Copy link

Eroc33 commented Aug 30, 2017

This seems to be some sort of interaction with the String inside the Command enum in the following code as removing either variant stops the ICE from occuring.

Example:

https://play.rust-lang.org/?gist=1d2672fd04e7df3dccbaf140f99fa7f6&version=nightly

Gives an ICE internal compiler error: /checkout/src/librustc_mir/transform/generator.rs:340: Broken MIR: generator contains type std::string::String in MIR, but typeck only knows about ((), std::option::Option<Command>, Command)

Meta

rustc --version --verbose:

rustc 1.21.0-nightly (c11f689d2 2017-08-29)
binary: rustc
commit-hash: c11f689d2475dd9ab956e881238d5d7b6b485efb
commit-date: 2017-08-29
host: x86_64-pc-windows-msvc
release: 1.21.0-nightly
LLVM version: 4.0

Backtrace:

error: internal compiler error: src\librustc_mir\transform\generator.rs:340: Broken MIR: generator contains type std::string::String in MIR, but typeck only knows about ((), std::option::Option<Command>, Command)
  --> src\main.rs:15:5
   |
15 | /     move ||{
16 | |         loop{
17 | |             let val = control_interface_next();
18 | |             if let Some(command) = val{
...  |
30 | |         }
31 | |     }
   | |_____^

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

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.21.0-nightly (c11f689d2 2017-08-29) running on x86_64-pc-windows-msvc

note: run with `RUST_BACKTRACE=1` for a backtrace

thread 'rustc' panicked at 'Box<Any>', src\librustc_errors\lib.rs:443:8
stack backtrace:
   0: _rdl_shrink_in_place
   1: std::panicking::Location::column
   2: std::panicking::Location::column
   3: std::panicking::rust_panic_with_hook
   4: <unknown>
   5: <unknown>
   6: <rustc_mir::shim::DropShimElaborator<'a, 'tcx> as rustc_mir::util::elaborate_drops::DropElaborator<'a, 'tcx>>::clear_drop_flag
   7: <rustc_mir::shim::DropShimElaborator<'a, 'tcx> as rustc_mir::util::elaborate_drops::DropElaborator<'a, 'tcx>>::clear_drop_flag
   8: <rustc_mir::transform::generator::StateTransform as rustc::mir::transform::MirPass>::run_pass
   9: <rustc_mir::shim::DropShimElaborator<'a, 'tcx> as rustc_mir::util::elaborate_drops::DropElaborator<'a, 'tcx>>::get_drop_flag
  10: <rustc_mir::shim::DropShimElaborator<'a, 'tcx> as rustc_mir::util::elaborate_drops::DropElaborator<'a, 'tcx>>::get_drop_flag
  11: rustc::dep_graph::graph::DepGraph::in_task
  12: rustc::ty::maps::<impl rustc::ty::maps::queries::optimized_mir<'tcx>>::try_get
  13: rustc::ty::maps::TyCtxtAt::optimized_mir
  14: rustc::ty::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::instance_mir
  15: <rustc_trans::builder::Builder<'a, 'tcx> as core::ops::drop::Drop>::drop
  16: <rustc_trans::builder::Builder<'a, 'tcx> as core::ops::drop::Drop>::drop
  17: rustc_trans::base::trans_crate
  18: rustc_trans::base::trans_crate
  19: rustc_driver::driver::phase_4_translate_to_llvm
  20: rustc_driver::driver::compile_input
  21: <rustc_driver::derive_registrar::Finder as rustc::hir::itemlikevisit::ItemLikeVisitor<'v>>::visit_impl_item
  22: rustc_driver::driver::compile_input
  23: rustc_driver::run_compiler
  24: <unknown>
  25: _rust_maybe_catch_panic
  26: <rustc_driver::derive_registrar::Finder as rustc::hir::itemlikevisit::ItemLikeVisitor<'v>>::visit_impl_item
  27: std::sys::imp::thread::Thread::new
  28: BaseThreadInitThunk

error: Could not compile `generator_ice`.
@nagisa nagisa added A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html 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 Aug 30, 2017
@nagisa
Copy link
Member

nagisa commented Aug 30, 2017

cc @Zoxc

@arielb1
Copy link
Contributor

arielb1 commented Aug 30, 2017

So this the annoying match drop ordering having a (very) bad interaction with liveness.

The problem is that bindings in match expressions have a single "shared" destruction scope for their locals, so this:

match command {
    Command::A(_name) => {}
     Command::B => {}
}

is lowered into this (pre-drop-elaboration):

switch command {
    Command::A => {
        storagelive(_name);
        _name = (command as Command::A).0;
    }
    Command::B = {}
}
drop(_name);
storagedead(_name);

Here _name is dropped while potentially uninitialized. This has caused me quite a few annoyances before.

It does not cause unsoundness because drop elaboration inserts a drop flag, but it does cause liveness to leak across the loop and make _name live at the yield point for no good reason.

I think there's a way to solve the match problem (the reasons the regions are odd like this is because of match guards - we just have to create separate bindings for the match guards, upvar style - I hope this can't break anything real with our current borrow-check scheme).

@arielb1
Copy link
Contributor

arielb1 commented Aug 30, 2017

If we don't want to do that, another way would be to take a trick from LLVM, and use storagelive to help guide liveness - and mark a value as dead in a cfg point if its storagelive bit is not set.

@Zoxc
Copy link
Contributor

Zoxc commented Aug 31, 2017

@arielb1 By that do you mean adding storagedead(_name); before the switch or something else?

@arielb1
Copy link
Contributor

arielb1 commented Aug 31, 2017

Adding a storagedead(_name) before the switch would work (I think, LLVM semantics of storagedead aren't 100% clear).

But what I meant was that rustc_mir::util::liveness would do a "forward" dataflow pass - before its liveness pass - to find out where locals are not storagedead, and then during the "backwards" liveness pass, mark every local as dead in a cfg point if it is `storagedead.

@shepmaster shepmaster added the C-bug Category: This is a bug. label Sep 1, 2017
@rushmorem
Copy link

I'm running into this issue while playing with async/await.

@sp3d
Copy link
Contributor

sp3d commented Sep 7, 2017

I'm also running into this playing with #[async].

FWIW, because this is caused by drop logic for variables bound in match arms, it can be worked around (ownership needs of your code permitting) by adding "ref" to variable bindings in match patterns.

@Arnavion
Copy link

And if you do want to move the binding and thus can't use ref, rebinding it inside the match arm seems to help. In the original gist this can be done by changing

Command::A(_name) => {
    return;
}

to

Command::A(_name) => {
    let _name = _name;
    return;
}

bors added a commit that referenced this issue Sep 14, 2017
Analyse storage liveness and preserve it during generator transformation

This uses a dataflow analysis on `StorageLive` and `StorageDead` statements to infer where the storage of locals are live. The result of this analysis is intersected with the regular liveness analysis such that a local is can only be live when its storage is. This fixes #44184. If the storage of a local is live across a suspension point, we'll insert a `StorageLive` statement for it after the suspension point so storage liveness is preserved. This fixes #44179.

r? @arielb1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-coroutines Area: Coroutines A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html 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
Development

Successfully merging a pull request may close this issue.

9 participants