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

Codegen ICE/regression with 2019-06-12 nightly when using async fn<T: Fn()>(&self, T) #61793

Closed
Matthias247 opened this issue Jun 13, 2019 · 11 comments · Fixed by #62072
Closed
Assignees
Labels
A-async-await Area: Async & Await AsyncAwait-Polish Async-await issues that are part of the "polish" area 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

@Matthias247
Copy link
Contributor

Matthias247 commented Jun 13, 2019

Unfortunately the error message is super thin and I can't provide the source of the project. Therefore it's also hard to provide a minimal reproducible example.

thread 'rustc' panicked at 'assertion failed: target_offset >= offset', src/librustc_codegen_llvm/type_of.rs:130:9
stack backtrace:
   0: <unknown>
   1: <unknown>
   2: <unknown>
   3: <unknown>
  ...
  36: <unknown>
query stack during panic:
end of query stack

error: internal compiler error: unexpected panic

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.37.0-nightly (2887008e0 2019-06-12) running on x86_64-apple-darwin

note: compiler flags: -C debuginfo=2 -C incremental --crate-type bin

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

Some hints:

  • Yesterdays nightly (nightly-2019-06-11-x86_64-apple-darwin) doesn't ICE
  • The project makes heavy use of async/await. Therefore it might by related to that. I wondered whether it's about @tmandry's generator optimization from Generator optimization: Overlap locals that never have storage live at the same time #60187. However I also checked that out locally from their branch 2 days ago and built it myself, and it didn't crash. At that point I had all commits up to "Extract generator_layout as a method" included, only the last commit which was added later was missing.
@Matthias247 Matthias247 changed the title ICE/regression with todays nightly Codegen ICE/regression with todays nightly Jun 13, 2019
@jonas-schievink jonas-schievink added A-async-await Area: Async & Await C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 13, 2019
@Centril
Copy link
Contributor

Centril commented Jun 13, 2019

@Matthias247 Could you please test your code out with debuginfo-level = 2 in config.toml (you'll need to build rustc yourself) and provide the more extensive backtrace?

@Matthias247
Copy link
Contributor Author

I tried running that over the afternoon. Unfortunately something went wrong:

   Compiling rustc_macros v0.1.0 (/Users/matthias/Code/rust/rust/src/librustc_macros)
error[E0460]: found possibly newer version of crate `std` which `synstructure` depends on
 --> src/librustc_macros/src/lib.rs:6:5
  |
6 | use synstructure::decl_derive;
  |     ^^^^^^^^^^^^
  |
  = note: perhaps that crate needs to be recompiled?
  = note: the following crate versions were found:
          crate `std`: /Users/matthias/Code/rust/rust/build/x86_64-apple-darwin/stage1/lib/rustlib/x86_64-apple-darwin/lib/libstd-5ab454490186c8f5.rlib
          crate `std`: /Users/matthias/Code/rust/rust/build/x86_64-apple-darwin/stage1/lib/rustlib/x86_64-apple-darwin/lib/libstd-5ab454490186c8f5.dylib
          crate `synstructure`: /Users/matthias/Code/rust/rust/build/x86_64-apple-darwin/stage1-rustc/release/deps/libsynstructure-7d32b126cebc2b54.rlib

error: aborting due to previous error

error: Could not compile `rustc_macros`.
warning: build failed, waiting for other jobs to finish...
error: build failed

Unfortunately that happened after around 1.5 hours of compilation. This machine here is definitely too slow for doing any reasonable rustc compilation :'(

@Matthias247
Copy link
Contributor Author

Got it running by deleting enough libraries and rebuilding. Unfortunately it just prints exactly the same error message. I think the setting might already had been enabled as far as I can derive from the compiler output above:

note: compiler flags: -C debuginfo=2 -C incremental --crate-type bin

@Matthias247 Matthias247 changed the title Codegen ICE/regression with todays nightly Codegen ICE/regression with todays nightly when using async fn<T: Fn()>(&self, T) Jun 14, 2019
@Matthias247
Copy link
Contributor Author

Matthias247 commented Jun 14, 2019

I played around with my code and deleted stuff until I got a simple repro:

#![feature(async_await)]

struct Foo {
}

impl Foo {
    pub async fn do_foo<F: Fn()>(&self, _f: F) {
    }
}

async fn x() {
    let foo = Foo{};
    foo.do_foo(||{ }).await;
}

fn main() {
    let _fut = x();
}

It seems like the combination of the member function together with the closure parameter might be the culprit. A top level async fn or a a "static" member function (without &self) doesn't cause an ICE.

This also crashes if x is not an async function and thereby no await is included at all:

fn x() {
    let foo = Foo{};
    foo.do_foo(||{ });
}

@Matthias247 Matthias247 changed the title Codegen ICE/regression with todays nightly when using async fn<T: Fn()>(&self, T) Codegen ICE/regression with 2019-06-12 nightly when using async fn<T: Fn()>(&self, T) Jun 14, 2019
@Centril
Copy link
Contributor

Centril commented Jun 14, 2019

Reduction of async fn version:

#![feature(async_await)]
#![allow(unused)]

async fn foo<F>(_: &(), _: F) {}

fn main() {
    async {
        foo(&(), || {}).await;
    };
}

Reduction of non-async fn version:

#![feature(async_await)]
#![allow(unused)]

async fn foo<F>(_: &(), _: F) {}

fn main() {
    foo(&(), || {});
}

@eddyb
Copy link
Member

eddyb commented Jun 20, 2019

@Centril Note that you only need debuginfo-level = 1 to get the backtrace (2 is much more expensive). Maybe we even have builds with that enabled nowadays? Not sure though.
EDIT: also, someone needs to do this on linux, the backtraces are probably broken on macOS.

@pnkfelix
Copy link
Member

triage: leaving unprioritized; assigning to @nikomatsakis with intent that they attach priority label as well as any Async-Await specific labels. Leaving nomination tag but hoping I'll remember to skip over it at this week's meeting.

@shisoft
Copy link

shisoft commented Jun 20, 2019

Hi, any temporally way to check on this? My build for tests (just build, just tests) have been failed for days and I have no idea which part of my code caused this in a large code base.

@udoprog
Copy link
Contributor

udoprog commented Jun 20, 2019

I also see this in my project.

I built my own compiler with debug = true enabled for now, In case anyone is interested, this is what I get when building the non-async fn version from here:

thread 'rustc' panicked at 'assertion failed: target_offset >= offset', src\librustc_codegen_llvm\type_of.rs:130:9
stack backtrace:
   0: backtrace::backtrace::trace_unsynchronized<closure>
             at C:\Users\udoprog\.cargo\registry\src\github.com-1ecc6299db9ec823\backtrace-0.3.29\src\backtrace\mod.rs:66
   1: std::sys_common::backtrace::_print
             at D:\repo\rust\src\libstd\sys_common\backtrace.rs:47
   2: std::sys_common::backtrace::print
             at D:\repo\rust\src\libstd\sys_common\backtrace.rs:36
   3: std::panicking::default_hook::{{closure}}
             at D:\repo\rust\src\libstd\panicking.rs:198
   4: std::panicking::default_hook
             at D:\repo\rust\src\libstd\panicking.rs:212
   5: alloc::boxed::{{impl}}::call
             at D:\repo\rust\src\liballoc\boxed.rs:760
   6: rustc::util::common::panic_hook
             at D:\repo\rust\src\librustc\util\common.rs:40
   7: std::panicking::rust_panic_with_hook
             at D:\repo\rust\src\libstd\panicking.rs:479
   8: std::panicking::begin_panic<str*>
             at D:\repo\rust\src\libstd\panicking.rs:409
   9: rustc_codegen_llvm::type_of::struct_llfields
             at D:\repo\rust\src\librustc_codegen_llvm\type_of.rs:130
  10: rustc_codegen_llvm::type_of::{{impl}}::llvm_type
             at D:\repo\rust\src\librustc_codegen_llvm\type_of.rs:283
  11: rustc_codegen_llvm::type_of::struct_llfields
             at D:\repo\rust\src\librustc_codegen_llvm\type_of.rs:137
  12: rustc_codegen_llvm::type_of::{{impl}}::llvm_type
             at D:\repo\rust\src\librustc_codegen_llvm\type_of.rs:283
  13: rustc_codegen_llvm::type_::{{impl}}::backend_type
             at D:\repo\rust\src\librustc_codegen_llvm\type_.rs:316
  14: rustc_codegen_ssa::mir::place::PlaceRef<rustc_codegen_llvm::llvm_::ffi::::Value*>::alloca<rustc_codegen_llvm::llvm_::ffi::::Value*,rustc_codegen_llvm::builder::Builder>
             at D:\repo\rust\src\librustc_codegen_ssa\mir\place.rs:66
  15: core::iter::adapters::{{impl}}::fold::{{closure}}
             at D:\repo\rust\src\libcore\iter\adapters\mod.rs:589
  16: core::iter::adapters::{{impl}}::fold::{{closure}}
             at D:\repo\rust\src\libcore\iter\adapters\mod.rs:589
  17: core::iter::traits::iterator::Iterator::fold::{{closure}}
             at D:\repo\rust\src\libcore\iter\traits\iterator.rs:1685
  18: core::iter::traits::iterator::Iterator::try_fold
             at D:\repo\rust\src\libcore\iter\traits\iterator.rs:1573
  19: core::iter::traits::iterator::Iterator::fold
             at D:\repo\rust\src\libcore\iter\traits\iterator.rs:1685
  20: core::iter::adapters::{{impl}}::fold
             at D:\repo\rust\src\libcore\iter\adapters\mod.rs:589
  21: core::iter::adapters::{{impl}}::fold<rustc_codegen_ssa::mir::LocalRef<rustc_codegen_llvm::llvm_::ffi::::Value*>,core::iter::adapters::Map<core::ops::range::Range<usize>, fn(usize) -> rustc::mir::Local>,closure,(),mut closure*> 
             at D:\repo\rust\src\libcore\iter\adapters\mod.rs:589
  22: core::iter::adapters::chain::{{impl}}::fold<core::iter::adapters::chain::Chain<core::iter::sources::Once<rustc_codegen_ssa::mir::LocalRef<rustc_codegen_llvm::llvm_::ffi::::Value*>>, alloc::vec::IntoIter<rustc_codegen_ssa::mir::LocalRef<rustc_codegen_llvm:
             at D:\repo\rust\src\libcore\iter\adapters\chain.rs:113
  23: alloc::vec::{{impl}}::from_iter<rustc_codegen_ssa::mir::LocalRef<rustc_codegen_llvm::llvm_::ffi::::Value*>,core::iter::adapters::chain::Chain<core::iter::adapters::chain::Chain<core::iter::sources::Once<rustc_codegen_ssa::mir::LocalRef<rustc_codegen_llvm:
             at D:\repo\rust\src\liballoc\vec.rs:1909
  24: alloc::vec::{{impl}}::from_iter
             at D:\repo\rust\src\liballoc\vec.rs:1796
  25: rustc_data_structures::indexed_vec::{{impl}}::from_iter
             at D:\repo\rust\src\librustc_data_structures\indexed_vec.rs:764
  26: core::iter::traits::iterator::Iterator::collect
             at D:\repo\rust\src\libcore\iter\traits\iterator.rs:1466
  27: rustc_codegen_ssa::mir::codegen_mir<rustc_codegen_llvm::builder::Builder>
             at D:\repo\rust\src\librustc_codegen_ssa\mir\mod.rs:316
  28: rustc_codegen_ssa::base::codegen_instance<rustc_codegen_llvm::builder::Builder>
             at D:\repo\rust\src\librustc_codegen_ssa\base.rs:385
  29: rustc_codegen_ssa::mono_item::{{impl}}::define<rustc_codegen_llvm::builder::Builder>
             at D:\repo\rust\src\librustc_codegen_ssa\mono_item.rs:40
  30: rustc_codegen_llvm::base::compile_codegen_unit::module_codegen
             at D:\repo\rust\src\librustc_codegen_llvm\base.rs:143
  31: rustc::dep_graph::graph::DepGraph::with_task<rustc::ty::context::TyCtxt,syntax_pos::symbol::InternedString,rustc_codegen_ssa::ModuleCodegen<rustc_codegen_llvm::ModuleLlvm>,fn(mut rustc::ich::hcx::StableHashingContext*, rustc_codegen_ssa::ModuleCodegen<rus
             at D:\repo\rust\src\librustc\dep_graph\graph.rs:201
  32: rustc_codegen_llvm::base::compile_codegen_unit
             at D:\repo\rust\src\librustc_codegen_llvm\base.rs:110
  33: rustc_codegen_ssa::base::codegen_crate<rustc_codegen_llvm::LlvmCodegenBackend>
             at D:\repo\rust\src\librustc_codegen_ssa\base.rs:616
  34: rustc_codegen_llvm::{{impl}}::codegen_crate
             at D:\repo\rust\src\librustc_codegen_llvm\lib.rs:292
  35: rustc_interface::passes::start_codegen::{{closure}}
             at D:\repo\rust\src\librustc_interface\passes.rs:1081
  36: rustc::util::common::time_ext
             at D:\repo\rust\src\librustc\util\common.rs:151
  37: rustc::util::common::time<alloc::boxed::Box<Any>,closure>
             at D:\repo\rust\src\librustc\util\common.rs:145
  38: rustc_interface::passes::start_codegen
             at D:\repo\rust\src\librustc_interface\passes.rs:1080
  39: rustc::ty::context::tls::enter_global<closure,core::result::Result<alloc::boxed::Box<Any>, rustc::util::common::ErrorReported>>
             at D:\repo\rust\src\librustc\ty\context.rs:1962
  40: rustc_interface::passes::{{impl}}::access::{{closure}}<closure,core::result::Result<alloc::boxed::Box<Any>, rustc::util::common::ErrorReported>>
             at <::rustc_data_structures::box_region::declare_box_region_type macros>:17
  41: rustc_interface::passes::create_global_ctxt::{{closure}}
             at D:\repo\rust\src\librustc_interface\passes.rs:869
  42: rustc_interface::passes::BoxedGlobalCtxt::access
             at <::rustc_data_structures::box_region::declare_box_region_type macros>:19
  43: rustc_interface::passes::BoxedGlobalCtxt::enter<closure,core::result::Result<alloc::boxed::Box<Any>, rustc::util::common::ErrorReported>>
             at D:\repo\rust\src\librustc_interface\passes.rs:803
  44: rustc_interface::queries::{{impl}}::ongoing_codegen::{{closure}}
             at D:\repo\rust\src\librustc_interface\queries.rs:249
  45: rustc_interface::queries::Query<alloc::boxed::Box<Any>>::compute<alloc::boxed::Box<Any>,closure>
             at D:\repo\rust\src\librustc_interface\queries.rs:40
  46: rustc_interface::interface::Compiler::ongoing_codegen
             at D:\repo\rust\src\librustc_interface\queries.rs:246
  47: rustc_driver::run_compiler::{{closure}}
             at D:\repo\rust\src\librustc_driver\lib.rs:353
  48: rustc_interface::interface::run_compiler_in_existing_thread_pool<closure,core::result::Result<(), rustc::util::common::ErrorReported>>
             at D:\repo\rust\src\librustc_interface\interface.rs:123
  49: rustc_interface::interface::run_compiler::{{closure}}
             at D:\repo\rust\src\librustc_interface\interface.rs:142
  50: rustc_interface::util::spawn_thread_pool::{{closure}}::{{closure}}::{{closure}}::{{closure}}
             at D:\repo\rust\src\librustc_interface\util.rs:192
  51: rustc::ty::context::tls::with_thread_locals::{{closure}}::{{closure}}
             at D:\repo\rust\src\librustc\ty\context.rs:1917
  52: std::thread::local::LocalKey<core::cell::Cell<fn(rustc_errors::diagnostic::Diagnostic*)>>::try_with
             at D:\repo\rust\src\libstd\thread\local.rs:299
  53: std::thread::local::LocalKey<core::cell::Cell<fn(rustc_errors::diagnostic::Diagnostic*)>>::with
             at D:\repo\rust\src\libstd\thread\local.rs:245
  54: rustc::ty::context::tls::with_thread_locals::{{closure}}
             at D:\repo\rust\src\librustc\ty\context.rs:1909
  55: std::thread::local::LocalKey<core::cell::Cell<fn(syntax_pos::span_encoding::Span, mut core::fmt::Formatter*) -> core::result::Result<(), core::fmt::Error>>>::try_with
             at D:\repo\rust\src\libstd\thread\local.rs:299
  56: std::thread::local::LocalKey<core::cell::Cell<fn(syntax_pos::span_encoding::Span, mut core::fmt::Formatter*) -> core::result::Result<(), core::fmt::Error>>>::with<core::cell::Cell<fn(syntax_pos::span_encoding::Span, mut core::fmt::Formatter*) -> core::res
             at D:\repo\rust\src\libstd\thread\local.rs:245
  57: rustc::ty::context::tls::with_thread_locals
             at D:\repo\rust\src\librustc\ty\context.rs:1901
  58: rustc_interface::util::spawn_thread_pool::{{closure}}::{{closure}}::{{closure}}
             at D:\repo\rust\src\librustc_interface\util.rs:192
  59: scoped_tls::ScopedKey<rustc_data_structures::sync::Lock<usize>>::set
             at C:\Users\udoprog\.cargo\registry\src\github.com-1ecc6299db9ec823\scoped-tls-1.0.0\src\lib.rs:137
  60: rustc_interface::util::spawn_thread_pool::{{closure}}::{{closure}}
             at D:\repo\rust\src\librustc_interface\util.rs:188
  61: scoped_tls::ScopedKey<syntax_pos::Globals>::set
             at C:\Users\udoprog\.cargo\registry\src\github.com-1ecc6299db9ec823\scoped-tls-1.0.0\src\lib.rs:137
  62: syntax::with_globals::{{closure}}
             at D:\repo\rust\src\libsyntax\lib.rs:106
  63: scoped_tls::ScopedKey<syntax::Globals>::set<syntax::Globals,closure,core::result::Result<(), rustc::util::common::ErrorReported>>
             at C:\Users\udoprog\.cargo\registry\src\github.com-1ecc6299db9ec823\scoped-tls-1.0.0\src\lib.rs:137
  64: rustc_interface::util::scoped_thread::{{closure}}
             at D:\repo\rust\src\librustc_interface\util.rs:164
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
query stack during panic:
end of query stack

error: internal compiler error: unexpected panic

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.37.0-dev running on x86_64-pc-windows-msvc

note: compiler flags: -C debuginfo=2 -C incremental --crate-type bin

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

error: Could not compile `test-error`.

To learn more, run the command again with --verbose.

@tmandry
Copy link
Member

tmandry commented Jun 20, 2019

I've been investigating, and I believe this is caused by a subtle bug in #60187. (EDIT: In short, we weren't listing ZST fields at the beginning of the layout anymore.)

The reason @Matthias247 didn't reproduce in their checkout of the branch is likely that the bug didn't show up until one of the last changes I made to the branch, responding to final review feedback before merging.

Working on a fix and will have it up soon with more details.

@tmandry
Copy link
Member

tmandry commented Jun 21, 2019

Opened #62011.

@nikomatsakis nikomatsakis assigned tmandry and unassigned nikomatsakis Jun 25, 2019
bors added a commit that referenced this issue Jun 26, 2019
rustc: correctly transform memory_index mappings for generators.

Fixes #61793, closes #62011 (previous attempt at fixing #61793).

During #60187, I made the mistake of suggesting that the (re-)computation of `memory_index` in `ty::layout`, after generator-specific logic split/recombined fields, be done off of the `offsets` of those fields (which needed to be computed anyway), as opposed to the `memory_index`.

`memory_index` maps each field to its in-memory order index, which ranges over the same `0..n` values as the fields themselves, making it a bijective mapping, and more specifically a permutation (indeed, it's the permutation resulting from field reordering optimizations).

Each field has an unique "memory index", meaning a sort based on them, even an unstable one, will not put them in the wrong order. But offsets don't have that property, because of ZSTs (which do not increase the offset), so sorting based on the offset of fields alone can (and did) result in wrong orders.

Instead of going back to sorting based on (slices/subsets of) `memory_index`, or special-casing ZSTs to make sorting based on offsets produce the right results (presumably), as #62011 does, I opted to drop sorting altogether and focus on `O(n)` operations involving *permutations*:
* a permutation is easily inverted (see the `invert_mapping` `fn`)
  * an `inverse_memory_index` was already employed in other parts of the `ty::layout` code (that is, a mapping from memory order to field indices)
  * inverting twice produces the original permutation, so you can invert, modify, and invert again, if it's easier to modify the inverse mapping than the direct one
* you can modify/remove elements in a permutation, as long as the result remains dense (i.e. using every integer in `0..len`, without gaps)
  * for splitting a `0..n` permutation into disjoint `0..x` and `x..n` ranges, you can pick the elements based on a `i < x` / `i >= x` predicate, and for the latter, also subtract `x` to compact the range to `0..n-x`
  * in the general case, for taking an arbitrary subset of the permutation, you need a renumbering from that subset to a dense `0..subset.len()` - but notably, this is still `O(n)`!
* you can merge permutations, as long as the result remains disjoint (i.e. each element is unique)
  * for concatenating two `0..n` and `0..m` permutations, you can renumber the elements in the latter to `n..n+m`
* some of these operations can be combined, and an inverse mapping (be it a permutation or not) can still be used instead of a forward one by changing the "domain" of the loop performing the operation

I wish I had a nicer / more mathematical description of the recombinations involved, but my focus was to fix the bug (in a way which preserves information more directly than sorting would), so I may have missed potential changes in the surrounding generator layout code, that would make this all more straight-forward.

r? @tmandry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await AsyncAwait-Polish Async-await issues that are part of the "polish" area 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.

10 participants