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

Compiler assertion failed: tcx.migrate_borrowck() when failing to move #[thread_local] static vars. #54797

Closed
MaulingMonkey opened this issue Oct 3, 2018 · 12 comments
Assignees
Labels
A-NLL Area: Non Lexical Lifetimes (NLL) I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ NLL-sound Working towards the "invalid code does not compile" goal P-high High priority

Comments

@MaulingMonkey
Copy link
Contributor

MaulingMonkey commented Oct 3, 2018

Hello! I have a compiler assertion on rustc 1.31.0-nightly (2bd5993 2018-10-02) when combining #[thread_local] and bad code that tries to move from the thread local global. This also occurred on an earlier nightly I tried upgrading from (months old? I didn't think to save that compiler version, sorry!)

#![feature(thread_local)]

pub struct CommandWriter {}

#[thread_local] static mut TL_COMMAND_BUFFER_WRITER : Option<Box<CommandWriter>> = Option::None;
static mut G_COMMAND_BUFFER_WRITER : Option<Box<CommandWriter>> = Option::None;

impl CommandWriter {
    pub fn with_this_thread<F: FnOnce(&CommandWriter)> (f: F) {
        unsafe {
            f(&TL_COMMAND_BUFFER_WRITER.unwrap()); // [1] Compiler panic: thread 'main' panicked at 'assertion failed: tcx.migrate_borrowck()', librustc_mir\transform\elaborate_drops.rs:52:17
            //f(&G_COMMAND_BUFFER_WRITER.unwrap()); // [2] Proper compiler error:  "cannot move out of static item"
            //f(&TL_COMMAND_BUFFER_WRITER.as_ref().unwrap()); // Builds fine
        }
    }
}

(Playground)

Compiler output with the code as posted and RUST_BACKTRACE=1:

warning: static item is never used: `G_COMMAND_BUFFER_WRITER`
 --> src\lib.rs:6:1
  |
6 | static mut G_COMMAND_BUFFER_WRITER : Option<Box<CommandWriter>> = Option::None;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: #[warn(dead_code)] on by default

thread 'main' panicked at 'assertion failed: tcx.migrate_borrowck()', librustc_mir\transform\elaborate_drops.rs:52:17
stack backtrace:
   0: <std::sync::mpsc::RecvTimeoutError as core::fmt::Debug>::fmt
   1: <std::path::Iter<'a> as core::convert::AsRef<std::path::Path>>::as_ref
   2: std::panicking::take_hook
   3: std::panicking::take_hook
   4: <rustc::ty::sty::Binder<rustc::ty::ProjectionPredicate<'tcx>> as rustc::ty::ToPredicate<'tcx>>::to_predicate
   5: std::panicking::rust_panic_with_hook
   6: <rustc_mir::borrow_check::nll::constraints::graph::Reverse as rustc_mir::borrow_check::nll::constraints::graph::ConstraintGraphDirecton>::end_region
   7: <rustc_mir::transform::elaborate_drops::ElaborateDrops as rustc_mir::transform::MirPass>::run_pass
   8: <rustc_mir::transform::MirSource as core::fmt::Debug>::fmt
   9: rustc_mir::transform::optimized_mir
  10: <rustc::ty::query::job::QueryInfo<'tcx> as core::fmt::Debug>::fmt
  11: <rustc_target::spec::RelroLevel as rustc::session::config::dep_tracking::DepTrackingHash>::hash
  12: rustc::ty::context::tls::track_diagnostic
  13: rustc::dep_graph::graph::DepGraph::assert_ignored
  14: rustc::ty::context::tls::track_diagnostic
  15: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::try_print_query_stack
  16: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::try_print_query_stack
  17: rustc::ty::query::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::optimized_mir
  18: <rustc_metadata::schema::LazyState as core::fmt::Debug>::fmt
  19: <rustc_metadata::schema::LazyState as core::fmt::Debug>::fmt
  20: rustc_metadata::decoder::__ty_decoder_impl::<impl serialize::serialize::Decoder for rustc_metadata::decoder::DecodeContext<'a, 'tcx>>::read_u64
  21: rustc_metadata::index::<impl rustc_metadata::schema::LazySeq<rustc_metadata::index::Index>>::lookup
  22: rustc_metadata::dynamic_lib::dl::symbol
  23: <rustc_metadata::encoder::ImplVisitor<'a, 'tcx> as rustc::hir::itemlikevisit::ItemLikeVisitor<'v>>::visit_item
  24: rustc_metadata::cstore_impl::<impl rustc::middle::cstore::CrateStore for rustc_metadata::cstore::CStore>::encode_metadata
  25: rustc::ty::context::TyCtxt::encode_metadata
  26: rustc_codegen_llvm::base::codegen_instance
  27: rustc_codegen_llvm::type_::<impl rustc_codegen_llvm::llvm::ffi::::Type>::uint_from_ty
  28: <rustc_codegen_llvm::base::ValueIter<'ll> as core::iter::iterator::Iterator>::next
  29: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_utils::codegen_backend::CodegenBackend>::codegen_crate
  30: rustc_driver::target_features::add_configuration
  31: rustc_driver::driver::phase_4_codegen
  32: rustc_driver::profile::trace::write_style
  33: <humantime::duration::Error as std::error::Error>::cause
  34: <rustc_driver::pretty::NoAnn<'hir> as rustc_driver::pretty::HirPrinterSupport<'hir>>::sess
  35: <rustc_driver::CompilationFailure as core::fmt::Debug>::fmt
  36: rustc_driver::driver::compile_input
  37: rustc_driver::run_compiler
  38: <env_logger::Logger as log::Log>::flush
  39: rustc_driver::run_compiler
  40: rustc_driver::target_features::add_configuration
  41: _rust_maybe_catch_panic
  42: rustc_driver::profile::dump
  43: rustc_driver::main
  44: <unknown>
  45: std::panicking::update_panic_count
  46: _rust_maybe_catch_panic
  47: std::rt::lang_start_internal
  48: <unknown>
  49: <unknown>
  50: BaseThreadInitThunk
  51: RtlUserThreadStart
query stack during panic:
#0 [optimized_mir] processing `CommandWriter::with_this_thread`
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.31.0-nightly (2bd5993ca 2018-10-02) running on x86_64-pc-windows-msvc

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

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

Reasonable/expected errors when the line marked [1] is commented out and line marked [2] is uncommented:

error[E0507]: cannot move out of static item
  --> src\lib.rs:12:16
   |
12 |             f(&G_COMMAND_BUFFER_WRITER.unwrap()); // [2] Proper compiler error:  "cannot move out of static item"
   |                ^^^^^^^^^^^^^^^^^^^^^^^ cannot move out of static item

error: aborting due to previous error

For more information about this error, try `rustc --explain E0507`.

Meta

rustc --version --verbose:
rustc 1.31.0-nightly (2bd5993 2018-10-02)
binary: rustc
commit-hash: 2bd5993
commit-date: 2018-10-02
host: x86_64-pc-windows-msvc
release: 1.31.0-nightly
LLVM version: 8.0

@Havvy Havvy added the I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ label Oct 3, 2018
@estebank estebank added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Oct 4, 2018
@pnkfelix pnkfelix added A-NLL Area: Non Lexical Lifetimes (NLL) NLL-complete Working towards the "valid code works" goal labels Oct 11, 2018
@pnkfelix pnkfelix self-assigned this Oct 11, 2018
@pnkfelix pnkfelix added P-high High priority NLL-sound Working towards the "invalid code does not compile" goal and removed NLL-complete Working towards the "valid code works" goal labels Oct 11, 2018
@pnkfelix
Copy link
Member

assigning to self. will investigate.

@pnkfelix
Copy link
Member

If you turn on the 2018 edition, the ICE goes away (because that makes the migrate_borrowck function start returning true:

https://play.rust-lang.org/?gist=c141f8d0adaa3c4e945bdbfa052e2fee&version=nightly&mode=debug&edition=2018

@pnkfelix
Copy link
Member

pnkfelix commented Oct 11, 2018

((interestingly, our fallback to the AST-borrowck here signals no error on the code in question, so the migrate mode downgrades the error to a warning. will investigate...))


Update: in case people care, the error that is downgraded to a warning here is the following:

warning[E0507]: cannot move out of static item
  --> src/lib.rs:11:16
   |
11 |             f(&TL_COMMAND_BUFFER_WRITER.unwrap());
   |                ^^^^^^^^^^^^^^^^^^^^^^^^ cannot move out of static item
   |
   = warning: This error has been downgraded to a warning for backwards compatibility with previous releases.
           It represents potential unsoundness in your code.
           This warning will become a hard error in the future.

@pnkfelix
Copy link
Member

Curiouser and curiouser: the failing assertion is preceded by an interesting comment:

let move_data = match MoveData::gather_moves(mir, tcx) {
Ok(move_data) => move_data,
Err((move_data, _move_errors)) => {
// The only way we should be allowing any move_errors
// in here is if we are in the migration path for the
// NLL-based MIR-borrowck.
//
// If we are in the migration path, we have already
// reported these errors as warnings to the user. So
// we will just ignore them here.
assert!(tcx.migrate_borrowck());
move_data

@pnkfelix
Copy link
Member

discussed at NLL triage meeting. Not an RC2 blocker. Putting on the Release milestone.

@pnkfelix pnkfelix added this to the Rust 2018 Release milestone Oct 16, 2018
@pnkfelix
Copy link
Member

hey @estebank, why is this tagged as a stable-to-nightly regression? This can't be on stable, since its using a #![feature(..)], right?

@estebank
Copy link
Contributor

@pnkfelix you're absolutely right. Missed that. Removing.

@estebank estebank removed the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Oct 16, 2018
@pnkfelix
Copy link
Member

Aha!

I was flummoxed by my failures to reproduce the problem locally...

turns out you have to pass --crate-type lib to reproduce. It won't happen if you e.g. add an fn main() { } and make a binary.

@pnkfelix
Copy link
Member

Okay, so now I know how to reproduce the problem.

I guess the "right" thing would be to report the errors nicely from elaborate_drops when we are in borrowck=ast mode. I'm not sure whether they should be errors or warnings though.

@pnkfelix
Copy link
Member

pnkfelix commented Oct 16, 2018

(Another option might be to plug in a fix to AST-borrowck...)

Update: this is what I went ahead with, see PR #55150

@pnkfelix
Copy link
Member

This is actually a dupe of #47215.

We just hit it a bit differently here because instead of unwrapping the result, I started asserting that we must be in borrowck=migrate for this to occur. (Which I now know is not the case.)

@pnkfelix
Copy link
Member

closing as duplicate of #47215. (And assigning the latter to myself.)

pietroalbini added a commit to pietroalbini/rust that referenced this issue Oct 23, 2018
…-from-moving-out-of-thread-local-under-ast-borrowck, r=nikomatsakis

Do not allow moving out of thread local under ast borrowck

AST borrowck failed to prevent moving out of a thread-local static.

This was broken. And it also (sometimes?) caused an ICE during drop elaboration.

Fix rust-lang#47215
Fix rust-lang#54797
pietroalbini added a commit to pietroalbini/rust that referenced this issue Oct 25, 2018
…-from-moving-out-of-thread-local-under-ast-borrowck, r=nikomatsakis

Do not allow moving out of thread local under ast borrowck

AST borrowck failed to prevent moving out of a thread-local static.

This was broken. And it also (sometimes?) caused an ICE during drop elaboration.

Fix rust-lang#47215
Fix rust-lang#54797
bors added a commit that referenced this issue Oct 27, 2018
…g-out-of-thread-local-under-ast-borrowck, r=nikomatsakis

Do not allow moving out of thread local under ast borrowck

AST borrowck failed to prevent moving out of a thread-local static.

This was broken. And it also (sometimes?) caused an ICE during drop elaboration.

Fix #47215
Fix #54797
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non Lexical Lifetimes (NLL) I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ NLL-sound Working towards the "invalid code does not compile" goal P-high High priority
Projects
None yet
Development

No branches or pull requests

4 participants