Internal compiler error librustc/ty/layout.rs:1627:25

[eun-ice]

I cannot isolate what exactly makes rustc crash, but I get an internal compiler error on Mac OSX and Linux.

The code compiles fine with an older nightly:

rustc 1.25.0-nightly (5313e8728 2018-02-17)
binary: rustc
commit-hash: 5313e8728f028cb7914f3c9f02804158a5732b52
commit-date: 2018-02-17
host: x86_64-apple-darwin
release: 1.25.0-nightly
LLVM version: 6.0

But it doesn't work with the current nightly:

rustc 1.27.0-nightly (79252ff4e 2018-04-29)
binary: rustc
commit-hash: 79252ff4e25d82f9fe856cb66f127b79cdace163
commit-date: 2018-04-29
host: x86_64-apple-darwin
release: 1.27.0-nightly
LLVM version: 6.0

The error is

INFO 2018-05-01T16:07:59Z: rustc_trans::base: trans_instance(std::ptr::read_volatile::<std::option::Option<std::collections::VecDeque<iobytes::IoBytes>>>)
INFO 2018-05-01T16:07:59Z: rustc_trans::base: trans_instance(std::ptr::read_volatile::<std::option::Option<usize>>)
INFO 2018-05-01T16:07:59Z: rustc_trans::base: trans_instance(std::ptr::read_volatile::<std::option::Option<metalio::mio_shared::MioEventloopMessage>>)
INFO 2018-05-01T16:07:59Z: rustc_trans::base: trans_instance(std::ptr::write_volatile::<std::option::Option<usize>>)
thread 'main' panicked at 'assertion failed: `(left == right)`
 left: `1`,
right: `0`', librustc/ty/layout.rs:1627:25
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: rustc::util::common::panic_hook
  5: std::panicking::rust_panic_with_hook
  6: std::panicking::begin_panic_fmt
  7: rustc::ty::layout::<impl rustc_target::abi::TyLayoutMethods<'tcx, C> for &'tcx rustc::ty::TyS<'tcx>>::field
  8: rustc_trans::mir::place::PlaceRef::project_field
  9: rustc_trans::intrinsic::trans_intrinsic_call
 10: rustc_trans::mir::block::<impl rustc_trans::mir::FunctionCx<'a, 'tcx>>::trans_terminator
 11: rustc_trans::mir::trans_mir
 12: rustc_trans::base::trans_instance
 13: rustc_trans::trans_item::MonoItemExt::define
 14: rustc_trans::base::compile_codegen_unit
 15: rustc::ty::maps::<impl rustc::ty::maps::config::QueryConfig<'tcx> for rustc::ty::maps::queries::compile_codegen_unit<'tcx>>::compute
 16: rustc::dep_graph::graph::DepGraph::with_task_impl
 17: rustc::ty::context::tls::with_related_context
 18: rustc::ty::maps::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
 19: rustc::ty::maps::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
 20: rustc::ty::maps::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::compile_codegen_unit
 21: rustc_trans::base::trans_crate
 22: <rustc_trans::LlvmTransCrate as rustc_trans_utils::trans_crate::TransCrate>::trans_crate
 23: rustc::util::common::time
 24: rustc_driver::driver::phase_4_translate_to_llvm
 25: rustc_driver::driver::compile_input::{{closure}}
 26: rustc::ty::context::tls::enter_context
 27: <std::thread::local::LocalKey<T>>::with
 28: rustc::ty::context::TyCtxt::create_and_enter
 29: rustc_driver::driver::compile_input
 30: rustc_driver::run_compiler_impl
 31: <scoped_tls::ScopedKey<T>>::set
 32: syntax::with_globals
 33: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
 34: __rust_maybe_catch_panic
 35: rustc_driver::run
 36: rustc_driver::main
 37: std::rt::lang_start::{{closure}}
 38: std::panicking::try::do_call
 39: __rust_maybe_catch_panic
 40: std::rt::lang_start_internal
 41: main
query stack during panic:
#0 [compile_codegen_unit] compile_codegen_unit
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.27.0-nightly (79252ff4e 2018-04-29) running on x86_64-apple-darwin

note: compiler flags: -C opt-level=3 --crate-type lib

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

[pietroalbini]

Thanks for reporting this issue! Can you post here the code that crashes the compiler?

[eun-ice]

It is a larger project I cannot share yet, I tried to isolate the relevant code but failed.

Can you tell me how to find out which files/types are causing this issue? That would help me create a reproducer.

[kennytm]

Repro I think:

use std::ptr::write_volatile;

fn main() {
    let mut a = Some(1usize);
    unsafe {
        write_volatile(&mut a, None);
    }
}

[eun-ice]

@kennytm Thank you, that is a reproducer.

To further narrow down, code does work properly with

rustc 1.27.0-nightly (65d201f7d 2018-04-18)
binary: rustc
commit-hash: 65d201f7d682ad921ac6e67ac07f922aa63a8ce4
commit-date: 2018-04-18
host: x86_64-apple-darwin
release: 1.27.0-nightly
LLVM version: 6.0

[pietroalbini]

@eun-ice Thanks! I'm running a bisect between the two nightlies you linked, should be ready in a few minutes.

[pietroalbini]

The bisect run points at #49420 as the cause of the regression. cc @nox @eddyb

[eun-ice]

It gets even weirder.

The reproducer by @kennytm works fine with nightly-2018-04-18 but fails with current nightly.

The following also fails with nightly-2018-04-18 and current nightly, but works fin with nightly-2018-02-17

So clearly the commit you isolated cannot be the only problem?

use std::ptr::write_volatile;

enum Some {
    A,
    B,
}

trait FooTrait<T> {
    fn get(&mut self) -> Option<T>;
}

struct FooStruct<T> {
    some: Option<T>
}

impl<T> FooTrait<T> for FooStruct<T> {
    fn get(&mut self) -> Option<T> {
        self.some.take()
    }
}

fn main() {
    let foo: Box<FooTrait<Some>> = Box::new(FooStruct { some: Some(Some::A) });
    let mut a = Some(foo);
    unsafe {
        write_volatile(&mut a, None);
    }
}

[nox]

Option<Box<FooTrait<Some>>> is a niche-filled enum, while Option<usize> is a tagged enum. I've made both use ScalarPair in two separate PRs (#49383, #49420).

This is an actual bug, in the implementation of the volatile_store intrinsic.

[nox]

Actually, I would rather say the bug is in the layout itself, should we have a field for the other part of the pair?

[eun-ice]

Sorry, I do not have a meaningful answer to this, as I am just a humble Rust user. Maybe @pietroalbini @kennytm can chime in?

On a side note: Is write_volatile/read_volatile even needed or can I just use write/read. I guard access to the memory cell using an atomic with Ordering::Acquire/Ordering::Release anyways?

[pietroalbini]

@eun-ice I've nominated this to be discussed at the next compiler team meeting (and they're way more experienced than me). Since this is a regression, we'll get this fixed before the 1.27 release ;)

[eddyb]

The implementation of volatile_write is suboptimal, it should look more like OperandValue::store (with the bx.store calls replaced with bx.volatile_store, and the memcpy_ty one made volatile).
That would also fix this issue's bug with scalar pairs being mistaken for pair-like Rust types.

@eun-ice AFAIK volatile is mostly for communicating with hardware, not synchronization.

[eun-ice]

@eddyb thanks for your response.

So if I guard pure memory access (no memmaps or anything) using an AtomicBool like this

fn write(&self) {
    if self.guard.compare_and_swap(false, true, Ordering::Acquire) == false {
        ptr:write_volatile(...)
        self.guard.store(false, Ordering::Release);
    }
}

fn read(&self) {
    if self.guard.compare_and_swap(false, true, Ordering::Acquire) == false {
        ptr:read_volatile(...)
        self.guard.store(false, Ordering::Release);
    }
}

Is ptr::read/ptr::write sufficient here or do I need ptr::read_volatile/ptr::write_volatile?

From what I heard in theory the write_volatile makes sure the compiler does not reorder this instruction. Especially if the compiler chose to move the read/write after the Release it would be a problem.

But I haven't found any documentation on how Rust does this (besides the hint that Rust does not yet have a defined memory model yet)

[eddyb]

@eun-ice I'm not well-versed in synchronization primitives, I just know volatile loads/stores aren't needed. I'd suggest asking @Amanieu (the author of parking_lot) about such patterns.

[kennytm]

@eun-ice You could use compiler_fence to prevent reordering by the compiler (or fence to also prevent runtime reordering by CPU).

[Amanieu]

@eun-ice If the guard ensures that only the current thread can read/write the protected value (which seems to be the case) then you do not need volatile accesses.

[eun-ice]

@Amanieu thank you for your comment. Do you think the compiler fence is needed to prevent Rust from ordering the read/write after the release of the guard?

[Amanieu]

@eun-ice That's not necessary, the Ordering::Acquire and Ordering::Release already enforce the ordering.

[eun-ice]

@Amanieu I am so grateful, thank you.

[eun-ice]

FYI https://crates.io/crates/atomicring https://github.com/eun-ice/atomicring

[nikomatsakis]

triage: P-high

This looks like a severe problem, @eddyb will fix.

[eddyb]

(@nox expressed interest in working on this)