internal compiler error: librustc_mir/transform/elaborate_drops.rs — drop of untracked, uninitialized value

[shepmaster]

UPDATE: Mentoring instructions can be found here

---

#![feature(nll)]

struct Node {
    elem: i32,
    next: Option<Box<Node>>,
}

fn main() {
    let mut node = Node {
        elem: 5,
        next: None,
    };

    let mut src = &mut node;
    while let Some(node) = {src}.next.as_mut().map(|node| &mut *node) {
        src = node;
    }
    src.next = None;
}

$ RUST_BACKTRACE=1 rustc +nightly a.rs
warning: field is never used: `elem`
 --> a.rs:4:5
  |
4 |     elem: i32,
  |     ^^^^^^^^^
  |
  = note: #[warn(dead_code)] on by default

error: internal compiler error: librustc_mir/transform/elaborate_drops.rs:384: drop of untracked, uninitialized value bb7, place ((*_3).1: std::option::Option<std::boxed::Box<Node>>) (Parent(Some(mp3)))
  --> a.rs:18:5
   |
18 |     src.next = None;
   |     ^^^^^^^^

thread 'rustc' panicked at 'Box<Any>', librustc_errors/lib.rs:482:9
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::_print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: core::ops::function::Fn::call
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::begin_panic
   7: rustc_errors::Handler::span_bug
   8: rustc::session::opt_span_bug_fmt::{{closure}}
   9: rustc::ty::context::tls::with_opt::{{closure}}
  10: <std::thread::local::LocalKey<T>>::try_with
  11: <std::thread::local::LocalKey<T>>::with
  12: rustc::ty::context::tls::with
  13: rustc::ty::context::tls::with_opt
  14: rustc::session::opt_span_bug_fmt
  15: rustc::session::span_bug_fmt
  16: <rustc_mir::transform::elaborate_drops::ElaborateDrops as rustc_mir::transform::MirPass>::run_pass
  17: rustc_mir::transform::optimized_mir::{{closure}}
  18: rustc_mir::transform::optimized_mir
  19: rustc::dep_graph::graph::DepGraph::with_task_impl
  20: rustc::ty::maps::<impl rustc::ty::maps::queries::optimized_mir<'tcx>>::force
  21: rustc::ty::maps::<impl rustc::ty::maps::queries::optimized_mir<'tcx>>::try_get
  22: rustc::ty::maps::TyCtxtAt::optimized_mir
  23: rustc::ty::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::instance_mir
  24: rustc_mir::monomorphize::collector::collect_items_rec
  25: rustc_mir::monomorphize::collector::collect_crate_mono_items
  26: rustc_trans::base::collect_and_partition_translation_items
  27: rustc::dep_graph::graph::DepGraph::with_task_impl
  28: rustc::ty::maps::<impl rustc::ty::maps::queries::collect_and_partition_translation_items<'tcx>>::force
  29: rustc::ty::maps::<impl rustc::ty::maps::queries::collect_and_partition_translation_items<'tcx>>::try_get
  30: rustc::ty::maps::TyCtxtAt::collect_and_partition_translation_items
  31: rustc::ty::maps::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::collect_and_partition_translation_items
  32: rustc_trans::base::trans_crate
  33: <rustc_trans::LlvmTransCrate as rustc_trans_utils::trans_crate::TransCrate>::trans_crate
  34: rustc_driver::driver::phase_4_translate_to_llvm
  35: rustc_driver::driver::compile_input::{{closure}}
  36: rustc::ty::context::TyCtxt::create_and_enter
  37: rustc_driver::driver::compile_input
  38: rustc_driver::run_compiler

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.26.0-nightly (2789b067d 2018-03-06) running on x86_64-apple-darwin

[shepmaster]

Similar error message as #41073, but no unions are obvious in this code.

[nikomatsakis]

Worth noting: without the feature(nll) flag, we get this error:

   Compiling playground v0.0.1 (file:///playground)
error[E0382]: use of moved value: `src`
  --> src/main.rs:16:5
   |
13 |     while let Some(node) = {src}.next.as_mut().map(|node| &mut *node) {
   |                             --- value moved here
...
16 |     src.next = None;
   |     ^^^^^^^^^^^^^^^ value used here after move
   |
   = note: move occurs because `src` has type `&mut Node`, which does not implement the `Copy` trait

I think this results not from NLL per se but rather MIR borrowck.

[nikomatsakis]

It seems like the "move" analysis is wrong here -- src is moved as part of while let, then re-initialized in the loop body, but it may still be uninitializd at the point where we do src.next = None. Maybe the problem is that we are not checking the "assigned path" to see if it is referencing moved data?

[nikomatsakis]

Yep. For example, this code is accepted (but should not be):

#![feature(nll)]

fn main() {
    let mut src = &mut (22, 44);
    {src};
    src.0 = 66;
}

Similarly, this code will ICE:

#![feature(nll)]

struct Node {
    elem: i32,
    next: Option<Box<Node>>,
}

fn main() {
    let mut node = Node {
        elem: 5,
        next: None,
    };

    let mut src = &mut node;
    {src};
    src.next = None;
}

The difference is that the latter code encounters drop elaboration.

[nikomatsakis]

Mentoring instructions

The problem is that we have a mir assignment like so:

place = rvalue

we have to check the place to make sure it is not reading from uninitialized data. In particular, if the place includes a deref (e.g., a place like *p), then the base path p must be initialized. We are actually trying to do that check by invoking check_if_assigned_path_is_moved here:

rust/src/librustc_mir/borrow_check/mod.rs

Lines 963 to 976 in a04b88d

	// Write of P[i] or *P, or WriteAndRead of any P, requires P init'd.
	match mode {
	MutateMode::WriteAndRead => {
	self.check_if_path_is_moved(
	context,
	InitializationRequiringAction::Update,
	place_span,
	flow_state,
	);
	}
	MutateMode::JustWrite => {
	self.check_if_assigned_path_is_moved(context, place_span, flow_state);
	}
	}

But if you read the source for that function, it has a crucial bug:

rust/src/librustc_mir/borrow_check/mod.rs

Lines 1440 to 1445 in a04b88d

	fn check_if_assigned_path_is_moved(
	&mut self,
	context: Context,
	(place, span): (&Place<'tcx>, Span),
	flow_state: &Flows<'cx, 'gcx, 'tcx>,
	) {

In particular, trace through what will happen with a place like *p. That is a deref projection, so we'll go down this arm:

rust/src/librustc_mir/borrow_check/mod.rs

Line 1454 in a04b88d

Place::Projection(ref proj) => {

and then into this match arm

rust/src/librustc_mir/borrow_check/mod.rs

Line 1466 in a04b88d

{ }

at which point we fall through to here:

rust/src/librustc_mir/borrow_check/mod.rs

Line 1498 in a04b88d

place = base;

so then if place was *p, we update place to p. Now we go back around the loop. This time, p is a Local, so we decide it is ok:

rust/src/librustc_mir/borrow_check/mod.rs

Line 1450 in a04b88d

Place::Local(_) | Place::Static(_) => {

What we need to do is to change how Deref is handled in this match. Instead of doing nothing, it should check that the base path is initialized; this is actually the same as this existing call:

rust/src/librustc_mir/borrow_check/mod.rs

Lines 1485 to 1487 in a04b88d

	self.check_if_path_is_moved(
	context, InitializationRequiringAction::Assignment,
	(base, span), flow_state);

Basically, we would do almost the same thing as the arm for fields, except that we don't care about the type of base -- it must always be initialized.

[nikomatsakis]

cc @rust-lang/wg-compiler-nll -- haven't fixed an NLL bug in a while? Here's a fresh one, complete with mentoring instructions! Come and get it while it's hot!