ICE: using operand local as place

[gereeter]

The following code, extracted from #46845, causes an ICE on the 11-27 nightly ("rust 1.23.0-nightly (827cb0d 2017-11-26)") but does not error on the 11-26 nightly ("rust 1.23.0-nightly (e97ba83 2017-11-25)"):

#![allow(unused)]
use std::mem;

#[derive(Copy, Clone)]
enum Never {}

union Foo {
    a: u64,
    b: Never
}

fn main() {
    println!("{}", mem::size_of::<Foo>());
    
    let f = [Foo { a: 42 }, Foo { a: 10 }];
    println!("{:?}", unsafe { f[0].a });
}

This only triggers when mir-opt-level is at least 1. Since it is the only commit in between the nightlies that seems to modify the MIR or have anything to do with MIR optimization, #46264 would seem to be the culprit, but I have no idea why it would cause this and it seems completely valid - it may have just triggered a bug in a different optimization.

The error produced (on the latest nightly, which seems consistent with older nightlies) is:

error: internal compiler error: /checkout/src/librustc_trans/mir/place.rs:418: using operand local _19 as place

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.24.0-nightly (dc39c3169 2017-12-17) running on x86_64-unknown-linux-gnu

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

thread 'rustc' panicked at 'Box<Any>', /checkout/src/librustc_errors/lib.rs:502:8
stack backtrace:
   0:     0x681393ad62cb - std::sys::unix::backtrace::tracing::imp::unwind_backtrace::h1926165e1b36cfb3
                               at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1:     0x681393ace1fe - std::sys_common::backtrace::print::ha9b03f8537574de7
                               at /checkout/src/libstd/sys_common/backtrace.rs:68
                               at /checkout/src/libstd/sys_common/backtrace.rs:57
   2:     0x681393adf590 - std::panicking::default_hook::{{closure}}::ha761f65ed43616d1
                               at /checkout/src/libstd/panicking.rs:381
   3:     0x681393adf28b - std::panicking::default_hook::hbf975d28f2fde606
                               at /checkout/src/libstd/panicking.rs:391
   4:     0x681393adfa6b - std::panicking::rust_panic_with_hook::hed519d1046029849
                               at /checkout/src/libstd/panicking.rs:577
   5:     0x68138e477087 - std::panicking::begin_panic::h4a07dfb616d2b6c2
   6:     0x68138e4950f1 - rustc_errors::Handler::bug::hd3ec924cfe1490af
   7:     0x68138f87d7bf - <std::thread::local::LocalKey<T>>::with::h009f9d74e45758a5
   8:     0x68138fc4c51e - rustc::ty::context::tls::with_opt::h920ceb57d31432a0
   9:     0x68138f8f3ca7 - rustc::session::opt_span_bug_fmt::h6bd5ea6e8f364a9c
  10:     0x68138f8f3bb6 - rustc::session::bug_fmt::h6c9d3c21ae190481
  11:     0x6813930c4df1 - rustc_trans::mir::place::<impl rustc_trans::mir::MirContext<'a, 'tcx>>::trans_place::h1baceb614fc69b1d
  12:     0x6813930c44ed - rustc_trans::mir::place::<impl rustc_trans::mir::MirContext<'a, 'tcx>>::trans_place::h1baceb614fc69b1d
  13:     0x6813930c44ed - rustc_trans::mir::place::<impl rustc_trans::mir::MirContext<'a, 'tcx>>::trans_place::h1baceb614fc69b1d
  14:     0x6813930c51e0 - rustc_trans::mir::operand::<impl rustc_trans::mir::MirContext<'a, 'tcx>>::trans_consume::hb950635664c219b3
  15:     0x6813930c5247 - rustc_trans::mir::operand::<impl rustc_trans::mir::MirContext<'a, 'tcx>>::trans_operand::h7ca0c0416b6561bf
  16:     0x6813930c6244 - rustc_trans::mir::rvalue::<impl rustc_trans::mir::MirContext<'a, 'tcx>>::trans_rvalue_operand::h1f024ba9ed74ffc0
  17:     0x6813930bd327 - rustc_trans::mir::trans_mir::h7314495f603c165e
  18:     0x6813930dc12e - rustc_trans::base::trans_instance::he9686fb8a1d51ecf
  19:     0x68139310e9e4 - rustc_trans::trans_item::TransItemExt::define::hdf7266789d12e58f
  20:     0x6813930e03a3 - rustc_trans::base::compile_codegen_unit::hca6950c887a49133
  21:     0x68138fb6ece1 - rustc::ty::maps::<impl rustc::ty::maps::queries::compile_codegen_unit<'tcx>>::compute_result::h938a8535660b3359
  22:     0x68138fa6100b - rustc::dep_graph::graph::DepGraph::with_task_impl::ha66325b12aaf0b82
  23:     0x68138f758501 - rustc_errors::Handler::track_diagnostics::hc8e1eae80fd70636
  24:     0x68138f6224c0 - rustc::ty::maps::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::cycle_check::h0ff68d26efe3fb80
  25:     0x68138fb6ed8f - rustc::ty::maps::<impl rustc::ty::maps::queries::compile_codegen_unit<'tcx>>::force::hf02f109f2a85248a
  26:     0x68138fb6f778 - rustc::ty::maps::<impl rustc::ty::maps::queries::compile_codegen_unit<'tcx>>::try_get::hae32d3ca6a777bab
  27:     0x68138f91365e - rustc::ty::maps::TyCtxtAt::compile_codegen_unit::h42b98ce49b758926
  28:     0x68138f6830b9 - rustc::ty::maps::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::compile_codegen_unit::he36118865284d843
  29:     0x6813930de539 - rustc_trans::base::trans_crate::h3e62c0b92edbfaa1
  30:     0x68139317b696 - <rustc_trans::LlvmTransCrate as rustc_trans_utils::trans_crate::TransCrate>::trans_crate::hddcf3b4bd77449a5
  31:     0x681393ee1d8d - rustc_driver::driver::phase_4_translate_to_llvm::h8f75cfac6818aba2
  32:     0x681393f31512 - rustc_driver::driver::compile_input::{{closure}}::hb499ea42753f8393
  33:     0x681393f2a241 - <std::thread::local::LocalKey<T>>::with::hb92f749835d8a4c9
  34:     0x681393f2c926 - <std::thread::local::LocalKey<T>>::with::hed06138de0c2dcea
  35:     0x681393f862fc - rustc::ty::context::TyCtxt::create_and_enter::he394799bb6339b54
  36:     0x681393ed6aad - rustc_driver::driver::compile_input::h5409c0e3df4029d1
  37:     0x681393f5df88 - rustc_driver::run_compiler::hf7ab608cab285f60
  38:     0x681393e8b3e1 - std::sys_common::backtrace::__rust_begin_short_backtrace::hb46c15b4e473f11a
  39:     0x681393b14a5e - __rust_maybe_catch_panic
                               at /checkout/src/libpanic_unwind/lib.rs:101
  40:     0x681393e645f2 - <F as alloc::boxed::FnBox<A>>::call_box::h16e0ae6fb43d31a3
  41:     0x681393ad0b77 - std::sys_common::thread::start_thread::h8f325f936df43fbb
                               at /checkout/src/liballoc/boxed.rs:827
  42:     0x681393aeb288 - std::sys::unix::thread::Thread::new::thread_start::hec2e96890ab26caf
                               at /checkout/src/libstd/sys/unix/thread.rs:90
  43:     0x68138d7a7089 - start_thread
  44:     0x6813937be42e - __clone
  45:                0x0 - <unknown>

cc @scottmcm, the author of #46264.

[scottmcm]

Definitely seems to be exposed by this MIR optimization:

diff --git a/rustc.main.002-014.InstCombine.before.mir b/rustc.main.002-014.InstCombine.after.mir
index 81357e1..0f5c50c 100644
--- a/rustc.main.002-014.InstCombine.before.mir
+++ b/rustc.main.002-014.InstCombine.after.mir
@@ -1,7 +1,7 @@
 // MIR for `main`
 // source = MirSource { def_id: DefId(0/0:5 ~ x[317d]::main[0]), promoted: None }
 // pass_name = InstCombine
-// disambiguator = before
+// disambiguator = after

 fn main() -> () {
     let mut _0: ();                      // return place
@@ -238,7 +238,14 @@ fn main() -> () {
                                          // <E2><94><94> span: x.rs:16:33: 16:34
                                          // <E2><94><94> ty: usize
                                          // <E2><94><94> literal: const 0usize
-        _54 = Len(_32);                  // bb3[32]: scope 4 at x.rs:16:31: 16:35
+        _54 = const 2usize;              // bb3[32]: scope 4 at x.rs:16:31: 16:35
+                                         // ty::Const
+                                         // <E2><94><94> ty: usize
+                                         // <E2><94><94> val: Integral(Usize(Us64(2)))
+                                         // mir::Constant
+                                         // <E2><94><94> span: x.rs:16:31: 16:35
+                                         // <E2><94><94> ty: usize
+                                         // <E2><94><94> literal: const 2usize
         _55 = Lt(_53, _54);              // bb3[33]: scope 4 at x.rs:16:31: 16:35
         assert(move _55, "index out of bounds: the len is {} but the index is {}", move _54, _53) -> bb4; // bb3[34]: scope 4 at x.rs:16:31: 16:35
     }

The two is correct:

        let _32: [Foo; 2];               // "f" in scope 2 at x.rs:15:9: 15:10

And it's not affecting downstream MIR optimizations either. Here's the PreTrans MIR diff:

diff --git a/working.mir b/broken.mir
index b6ba84c..5d8b34a 100644
--- a/working.mir
+++ b/broken.mir
@@ -213,7 +213,14 @@ fn main() -> () {
                                          // <E2><94><94> span: x.rs:16:33: 16:34
                                          // <E2><94><94> ty: usize
                                          // <E2><94><94> literal: const 0usize
-        _36 = Len(_19);                  // bb3[32]: scope 4 at x.rs:16:31: 16:35
+        _36 = const 2usize;              // bb3[32]: scope 4 at x.rs:16:31: 16:35
         _37 = Lt(_35, _36);              // bb3[33]: scope 4 at x.rs:16:31: 16:35
         assert(move _37, "index out of bounds: the len is {} but the index is {}", move _36, _35) -> bb4; // bb3[34]: scope 4 at x.rs:16:31: 16:35
     }

Attached the tail.txt of the trans log. I don't know how this is breaking anything.

[eddyb]

Len forces the array to be in memory, but there's nothing handling non-memory indexing. [(u64, !); N] should also reproduce, maybe?

Ideally this would be moved to the group above itsef (that does nothing):

rust/src/librustc_trans/mir/analyze.rs

Line 194 in edbd7d2

PlaceContext::Inspect |

That way Len(local) doesn't require the local to be in memory and assuming Len(place) is updated to check the type first to avoid evaluating the place at all for fixed-length arrays, the primary issue here should be reproducible with no MIR optimizations.

This isn't triggered because the final type is u64, even if it comes from a ZST:

rust/src/librustc_trans/mir/operand.rs

Lines 287 to 293 in edbd7d2

	let ty = self.monomorphized_place_ty(place);
	let layout = bcx.ccx.layout_of(ty);
	// ZSTs don't require any actual memory access.
	if layout.is_zst() {
	return OperandRef::new_zst(bcx.ccx, layout);
	}

It could maybe be moved in the recursive function (assuming this doesn't cost us much).
Or we could do that for ProjectionElem other than Field, specifically.

[arielb1]

@eddyb

Could you try to get this fixed before beta?

[arielb1]

I don't think -Z mir-opt-level can be used on beta, so this does not affect beta

[eddyb]

Minimal repro with -Z mir-opt-level=1 (independent of #46845):

pub enum Void {}
pub fn foo(xs: [(Void, u32); 1]) -> u32 { xs[0].1 }

EDIT: and without -Z mir-opt-level:

#![feature(slice_patterns)]
pub enum Void {}
pub fn foo([(_, x)]: [(Void, u32); 1]) -> u32 { x }

This still requires nightly (because of the feature-gated patterns).