ICE: Broken MIR in generators

[Nemo157]

#![feature(async_await)]

async fn bar() {
}

pub async fn foo() {
    std::thread::spawn(move || ());
    bar().await;
}

(playground), error:

error: internal compiler error: src/librustc_mir/transform/generator.rs:532: Broken MIR: generator contains type std::thread::JoinHandle<()> in MIR, but typeck only knows about {impl std::future::Future, ()}
 --> src/lib.rs:6:20
  |
6 |   pub async fn foo() {
  |  ____________________^
7 | |     std::thread::spawn(move || ());
8 | |     bar().await;
9 | | }
  | |_^

thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:573:9
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:47
   3: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:36
   4: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:197
   5: std::panicking::default_hook
             at src/libstd/panicking.rs:211
   6: rustc::util::common::panic_hook
   7: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:478
   8: std::panicking::begin_panic
   9: rustc_errors::Handler::span_bug
  10: rustc::util::bug::opt_span_bug_fmt::{{closure}}
  11: rustc::ty::context::tls::with_opt::{{closure}}
  12: rustc::ty::context::tls::with_context_opt
  13: rustc::ty::context::tls::with_opt
  14: rustc::util::bug::opt_span_bug_fmt
  15: rustc::util::bug::span_bug_fmt
  16: <rustc_mir::transform::generator::StateTransform as rustc_mir::transform::MirPass>::run_pass
  17: rustc_mir::transform::run_passes::{{closure}}
  18: rustc_mir::transform::run_passes
  19: rustc_mir::transform::optimized_mir
  20: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::optimized_mir>::compute
  21: rustc::dep_graph::graph::DepGraph::with_task_impl
  22: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  23: rustc::ty::layout::LayoutCx<rustc::ty::context::TyCtxt>::layout_raw_uncached
  24: rustc::ty::layout::layout_raw
  25: rustc::ty::query::__query_compute::layout_raw
  26: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::layout_raw>::compute
  27: rustc::dep_graph::graph::DepGraph::with_task_impl
  28: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  29: <rustc::ty::layout::LayoutCx<rustc::ty::context::TyCtxt> as rustc_target::abi::LayoutOf>::layout_of
  30: <rustc_mir::transform::const_prop::ConstPropagator as rustc::mir::visit::MutVisitor>::visit_statement
  31: <rustc_mir::transform::const_prop::ConstProp as rustc_mir::transform::MirPass>::run_pass
  32: rustc_mir::transform::run_passes::{{closure}}
  33: rustc_mir::transform::run_passes
  34: rustc_mir::transform::optimized_mir
  35: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::optimized_mir>::compute
  36: rustc::dep_graph::graph::DepGraph::with_task_impl
  37: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  38: rustc_mir::monomorphize::collector::collect_items_rec
  39: rustc_mir::monomorphize::collector::collect_crate_mono_items::{{closure}}
  40: rustc::util::common::time
  41: rustc_mir::monomorphize::collector::collect_crate_mono_items
  42: rustc::util::common::time
  43: rustc_mir::monomorphize::partitioning::collect_and_partition_mono_items
  44: rustc::ty::query::__query_compute::collect_and_partition_mono_items
  45: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::collect_and_partition_mono_items>::compute
  46: rustc::dep_graph::graph::DepGraph::with_task_impl
  47: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  48: rustc_codegen_ssa::back::symbol_export::exported_symbols_provider_local
  49: rustc::ty::query::__query_compute::exported_symbols
  50: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::exported_symbols>::compute
  51: rustc::dep_graph::graph::DepGraph::with_task_impl
  52: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  53: rustc_metadata::encoder::EncodeContext::encode_crate_root
  54: rustc::dep_graph::graph::DepGraph::with_ignore
  55: rustc_metadata::encoder::encode_metadata
  56: rustc_metadata::cstore_impl::<impl rustc::middle::cstore::CrateStore for rustc_metadata::cstore::CStore>::encode_metadata
  57: rustc::ty::context::TyCtxt::encode_metadata
  58: rustc_interface::passes::encode_and_write_metadata
  59: rustc::util::common::time
  60: rustc_interface::passes::start_codegen
  61: rustc::ty::context::tls::enter_global
  62: rustc_interface::passes::BoxedGlobalCtxt::access::{{closure}}
  63: rustc_interface::passes::create_global_ctxt::{{closure}}
  64: rustc_interface::passes::BoxedGlobalCtxt::enter
  65: rustc_interface::queries::Query<T>::compute
  66: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::ongoing_codegen
  67: rustc_interface::interface::run_compiler_in_existing_thread_pool
  68: std::thread::local::LocalKey<T>::with
  69: scoped_tls::ScopedKey<T>::set
  70: syntax::with_globals
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
query stack during panic:
#0 [optimized_mir] processing `foo::{{closure}}#0`
#1 [layout_raw] computing layout of `[static generator@src/lib.rs:6:20: 9:2 {std::future::GenFuture<[static generator@src/lib.rs:3:16: 4:2 {}]>, ()}]`
#2 [optimized_mir] processing `foo`
#3 [collect_and_partition_mono_items] collect_and_partition_mono_items
#4 [exported_symbols] exported_symbols
end of query stack
error: aborting due to previous error
note: rustc 1.37.0-nightly (7cdaffd79 2019-06-05) running on x86_64-unknown-linux-gnu

This is minimized from failing doc-tests in futures (rust-lang/futures-rs#1657), from the CI builds there it appears the ICE was introduced between rustc 1.37.0-nightly (5d8f59f4b 2019-06-04) and rustc 1.37.0-nightly (7cdaffd79 2019-06-05).

[Nemo157]

cc @Zoxc, skimming the commits between the above nightlies #61069 seems a likely candidate.

[Zoxc]

We don't generate StorageDead for the JoinHandle MIR local here, so it appears live during the await.

[DoumanAsh]

In case if it will be useful, I caught similar ICE with a different callstack https://gist.github.com/DoumanAsh/2ce9e8aa6c360f5d74bdd169188fec8d

But I imagine it is the same issue.

Code in question: https://gitlab.com/Douman/yukikaze/blob/1.0/src/client/mod.rs#L216-317

[Zoxc]

@tmandry Did you see any reason for why StorageDead is not generated when poking around in scope.rs?

[jbg]

For anyone hitting this issue looking (like me) for a workaround, assigning the value to a local variable works:

pub async fn foo() {
    let _ = std::thread::spawn(move || ());
    bar().await;
}

[nikomatsakis]

A variant that also ICEs (playground):

#![feature(async_await)]

#[derive(Default)]
struct Foo {
    
}

impl Drop for Foo {
    fn drop(&mut self) { }
}

async fn bar() {
}

pub async fn foo() {
    Foo::default();
    bar().await;
}

seems like the problem is "dropping" a value that needs drop from a call

[Zoxc]

Looking over our MIR construction code there seems to be lots of locals which do not generate a StorageLive / StorageDead pair. I'd suggest just reverting #61069 until we can address that. That would make it UB to use a pointer leaked from a drop impl in a generator. I would like that to be UB anyway, but @RalfJung's stacked borrows can't express it.

[tmandry]

Sorry I missed your earlier comment @Zoxc, there are locals that we never generate any StorageLive/Dead statements for, and we have to handle these explicitly in the generator code as "storage ignored" locals.

I don't know why we omit these, and for some reason thought it was only for "simple" types (I'm surprised to learn that we also do this for locals which are Drop.)

[nikomatsakis]

This does not appear to be a regression, but maybe there's some other problem afflicting 2019-05-03:

> cargo-bisect-rustc --test-dir=issue-61579 --start=2019-05-03 --end=2019-06-12
checking nightly-2019-05-03
std for x86_64-unknown-linux-gnu: 59.85 MB / 59.85 MB [=========================] 100.00 % 3.36 MB/s  uninstalling nightly-2019-05-03
the --start nightly has the regression

[tmandry]

@nikomatsakis I'm seeing a regression between 2019-06-05 and 2019-06-06, so I think there was indeed another problem with your start point.

This is probably due to the @Zoxc mentioned above; I also describe it here: #61731 (comment)

[RalfJung]

That would make it UB to use a pointer leaked from a drop impl in a generator. I would like that to be UB anyway, but @RalfJung's stacked borrows can't express it.

I'm all ears for a model that makes this happen. However, I don't think we should have UB without having a clear idea of how to operationally check for it. And even then we still have to weigh the complexity of the model (that every unsafe code author will have to basically know by heart!) against the alternatives (such as emitting Storage* for more locals).