Using a match statement on a field of an enum results in an internal compiler error #53728

vladglv · 2018-08-27T01:09:55Z

Code

let packet = socket_sub.recv_bytes(0)?;
if let Some((info, data)) = decode_header(&packet) {
    match info.device_kind {
        _ => (),
    }
}

Where info is

#[repr(C, packed)]
pub struct DeviceInfo {
    pub uuid: [u8; 32],
    pub endianness: u8,
    pub device_io_caps: DeviceIo,
    pub device_kind: DeviceKind,
    pub device_id: u8,
}

and,

#[repr(u16)]
pub enum DeviceKind {
    Nil = 0,
    Unknown = 1,
}

Output

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`', librustc_codegen_llvm/type_.rs:293:9
stack backtrace:
   0: rust_metadata_std_3ed67ab4eddac0113ac194419553d62
   1: rust_metadata_std_3ed67ab4eddac0113ac194419553d62
   2: rust_metadata_std_3ed67ab4eddac0113ac194419553d62
   3: rust_metadata_std_3ed67ab4eddac0113ac194419553d62
   4: rust_metadata_rustc_8aa2fd7dd479240b1bd69b15bc880e38
   5: std::panicking::rust_panic_with_hook
   6: rust_metadata_std_3ed67ab4eddac0113ac194419553d62
   7: std::panicking::begin_panic_fmt
   8: rustc_codegen_llvm::type_::Type::padding_filler
   9: <unknown>
  10: <rustc_target::abi::TyLayout<'tcx, &'tcx rustc::ty::TyS<'tcx>> as rustc_codegen_llvm::type_of::LayoutLlvmExt<'tcx>>::llvm_type
  11: <rustc_target::abi::call::FnType<'tcx, &'tcx rustc::ty::TyS<'tcx>> as rustc_codegen_llvm::abi::FnTypeExt<'a, 'tcx>>::llvm_type
  12: <unknown>
  13: rustc_codegen_llvm::mono_item::predefine_fn
  14: <unknown>
  15: rust_metadata_rustc_8aa2fd7dd479240b1bd69b15bc880e38
  16: rust_metadata_rustc_8aa2fd7dd479240b1bd69b15bc880e38
  17: rust_metadata_rustc_8aa2fd7dd479240b1bd69b15bc880e38
  18: rust_metadata_rustc_8aa2fd7dd479240b1bd69b15bc880e38
  19: rustc::ty::query::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::compile_codegen_unit
  20: <unknown>
  21: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_utils::codegen_backend::CodegenBackend>::codegen_crate
  22: rustc_driver::driver::phase_4_codegen
  23: <unknown>
  24: <unknown>
  25: rustc_driver::driver::compile_input
  26: rustc_driver::run_compiler
  27: <unknown>
  28: __rust_maybe_catch_panic
  29: <unknown>
  30: rustc_driver::main
  31: <unknown>
  32: rust_metadata_std_3ed67ab4eddac0113ac194419553d62
  33: __rust_maybe_catch_panic
  34: std::rt::lang_start_internal
  35: <unknown>
  36: __libc_start_main
  37: <unknown>
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.28.0 running on x86_64-unknown-linux-gnu

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

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

Extra

I attempted to reproduce the logic in a simple example. However, the code compiled and ran properly.

The assertion failed here https://github.com/rust-lang/rust/blob/stable/src/librustc_codegen_llvm/type_.rs#L293

#[repr(u32)]
pub enum E1 {
    Nil = 0,
    Val = 1,
}

#[repr(u16)]
pub enum E2 {
    Nil = 0,
    Val = 1,
}

#[repr(C, packed)]
pub struct S {
    pub a: [u8; 32],
    pub b: u8,
    pub c: E1,
    pub d: E2,
    pub e: u8,
}

fn main() {
    let s = S {
        a: [0; 32],
        b: 0,
        c: E1::Nil,
        d: E2::Nil,
        e: 42,
    };

    match s.d {
        _ => (),
    }

    assert_eq!(std::mem::size_of::<S>(), 40);
}

The text was updated successfully, but these errors were encountered:

estebank · 2018-08-27T01:14:32Z

Can you provide access to the code causing the ICE? As you mention, the code in the ticket does not reproduce it. Also, could you try to compile the problematic code with the latest nightly?

vladglv · 2018-08-27T03:29:58Z

I can reproduce the issue with the latest nightly (rustc 1.30.0-nightly (7219130 2018-08-26)).

vladglv · 2018-08-27T03:55:14Z

Here is the code causing the ICE https://github.com/vladglv/bug-report .

KamilaBorowska · 2018-08-27T07:45:11Z

A minimal reproduction:

#[repr(u16)]
enum DeviceKind {
    Nil = 0,
}

#[repr(C, packed)]
struct DeviceInfo {
    endianness: u8,
    device_kind: DeviceKind,
}

fn main() {
    None::<(DeviceInfo, Box<u8>)>;
}

This regression was introduced in Rust 1.24.0, making it regression from stable to stable, the provided example works in Rust 1.23.0.

matthew-russo · 2018-08-28T13:15:51Z

Was messing around a bit with different combinations to narrow down where it was. Very slightly more minimal

#[repr(u16)]
enum DeviceKind {
    Nil = 0,
}

#[repr(packed)]
struct DeviceInfo {
    endianness: u8,
    device_kind: DeviceKind,
}

fn main() {
    None::<(DeviceInfo, Box<u8>)>;
}

The repr(C) part is not necessary so figured I'd remove the noise.

Might be interesting to note --
None::<(DeviceInfo, u8)>; does not throw error
None::<(DeviceInfo, u16)>; does throw error

Also perhaps interesting to note: changing DeviceKind to repr(u8) removes all errors, but any higher than that (repr(u16), repr(u32), repr(u64), and their signed counterparts) all have errors

pnkfelix · 2018-08-30T14:58:08Z

tagging with T-compiler and P-high since it seems to qualify to be in those buckets

nikomatsakis · 2018-08-30T15:03:39Z

Would be great to use https://github.com/rust-lang-nursery/cargo-bisect-rustc to try and narrow this down to at least a nightly if not a specific PR...

pnkfelix · 2018-08-30T15:05:44Z

assigning to self to ensure that bisection gets done (by me or by anyone else who feels like posting the result of bisection here)

matthew-russo · 2018-08-31T00:37:08Z

I tried running the bisection but don't know if I'm just doing it wrong or there is some other issue.

First I tried running from 1.23 to 1.24 and it says start has failed: Command I used:

./target/debug/cargo-bisect-rustc --start=2017-10-31 \
     --end 2017-12-01 \
     --test-dir /Users/matthewrusso/rust_opensource/repro_53728 \
     -- build

output:

checking nightly-2017-10-31
std for x86_64-apple-darwin: 48.27 MB / 48.27 MB [====================================================================================================================================] 100.00 % 1.07 MB/s  uninstalling nightly-2017-10-31
the --start nightly has the regression

However when I build manually with nightly-2017-10-31 it works fine

Then tried going by their relative commits and got a different error. Command I used:

./target/debug/cargo-bisect-rustc --start 8b22e70b2de5152db3b0c53cfa16eb96b0b9e40e \
     --end 23032d0afa2b0e0c60a9b2ae62709f846d90007c \
     --test-dir /Users/matthewrusso/rust_opensource/repro_53728 \
     -- build

output:

bisecting ci builds
starting at 8b22e70b2de5152db3b0c53cfa16eb96b0b9e40e, ending at 23032d0afa2b0e0c60a9b2ae62709f846d90007c
fetching commits from 8b22e70b2de5152db3b0c53cfa16eb96b0b9e40e to 23032d0afa2b0e0c60a9b2ae62709f846d90007c
opening existing repository at "rust.git"
refreshing repository
looking up first commit
looking up second commit
checking that commits are by bors and thus have ci artifacts...
finding bors merge commits
found 202 bors merge commits in the specified range
no commits between 8b22e70b2de5152db3b0c53cfa16eb96b0b9e40e and 23032d0afa2b0e0c60a9b2ae62709f846d90007c within last 167 days

matthew-russo · 2018-09-03T05:01:57Z

I've narrowed it down to be between nightly-2017-11-20 and nightly-2017-11-21. I have not locked down the exact commit but just looking at commit logs I'm assuming its this one f50fd07 as its a refactor of memory layouts. Will take a look in the next couple days.

pnkfelix · 2018-09-04T13:07:40Z

Nota bene f50fd07 is from PR #45225

pnkfelix · 2018-09-06T14:05:44Z

visited at rustc mtg. @arielb1 volunteered to do some more investigation.

eddyb · 2018-09-06T14:30:17Z

So this is caused by the fact that Option<(DeviceInfo, T)> uses a niche encoding, where the (misaligned/packed) DeviceKind at offset 1 is being used to encode None (with a tag value unused by DeviceKind itself), so its LLVM type needs to be equivalent to this:

#[repr(packed)]
struct Foo<T> {
    _filler0: [u8; 1],
    discriminant: u16, /* DeviceKind niche */
    _filler1: [u8; (round_to_multiple(3, align_of::<T>()) - 3) + size_of::<T>()],
}

But that second _filler1 tries to use an integer with the same alignment as T, which fails because the misalignment/packing needs it to be of a size that's not a multiple of that.

I'll try to open a PR soon.

EDIT: oops it's actually _filler0 (it doesn't get past that). It appears that the struct/enum alignment is considered to possibly be less than field alignment, but not individual offsets to be misaligned.

EDIT2: this assert could've fired but it doesn't here because offset is 0:

rust/src/librustc_codegen_llvm/type_of.rs

Line 137 in 35a5541

assert_eq!(offset.abi_align(padding_align) + padding, target_offset);

EDIT3: this additional assertion does get triggered, if I add it:

assert_eq!(target_offset.abi_align(field.align), target_offset);

rustc_codegen_llvm: don't assume offsets are always aligned. Fixes rust-lang#53728 by taking into account not just overall type alignment and the field's alignment when determining whether a field is aligned or not ("packed"), but also the field's offset within the type. Previously, rustc assumed that the offset was always at least as aligned as `min(struct.align, field.align)`. However, there's no real reason to have that assumption, and it obviously can't always be true after we implement `#[repr(align(N), pack(K))]`. There's also a case today where that assumption is not true, involving niche discriminants in enums: Suppose that we have the code in rust-lang#53728: ```Rust #[repr(u16)] enum DeviceKind { Nil = 0, } #[repr(packed)] struct DeviceInfo { endianness: u8, device_kind: DeviceKind, } struct Wrapper { device_info: DeviceInfo, data: u32 } ``` Observe the layout of `Option<Wrapper>`. It has an alignment of 4 because of the `u32`. `device_info.device_kind` is a good niche field to use, which means the enum ends up with this layout: ``` size = 8 align = 4 fields = [ { offset=1, type=u16 } // discriminant, .<Some>.device_info.device_kind ] ``` And here we have an discriminant with alignment 2 (`u16`) but offset 1.

rustc_codegen_llvm: don't assume offsets are always aligned. Fixes #53728 by taking into account not just overall type alignment and the field's alignment when determining whether a field is aligned or not ("packed"), but also the field's offset within the type. Previously, rustc assumed that the offset was always at least as aligned as `min(struct.align, field.align)`. However, there's no real reason to have that assumption, and it obviously can't always be true after we implement `#[repr(align(N), pack(K))]`. There's also a case today where that assumption is not true, involving niche discriminants in enums: Suppose that we have the code in #53728: ```Rust #[repr(u16)] enum DeviceKind { Nil = 0, } #[repr(packed)] struct DeviceInfo { endianness: u8, device_kind: DeviceKind, } struct Wrapper { device_info: DeviceInfo, data: u32 } ``` Observe the layout of `Option<Wrapper>`. It has an alignment of 4 because of the `u32`. `device_info.device_kind` is a good niche field to use, which means the enum ends up with this layout: ``` size = 8 align = 4 fields = [ { offset=1, type=u16 } // discriminant, .<Some>.device_info.device_kind ] ``` And here we have an discriminant with alignment 2 (`u16`) but offset 1.

estebank added the I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ label Aug 27, 2018

estebank added the A-codegen Area: Code generation label Aug 27, 2018

estebank added the regression-from-stable-to-stable Performance or correctness regression from one stable version to another. label Aug 27, 2018

pnkfelix added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 30, 2018

pnkfelix added the P-high High priority label Aug 30, 2018

pnkfelix self-assigned this Aug 30, 2018

pnkfelix assigned arielb1 Sep 6, 2018

eddyb mentioned this issue Sep 6, 2018

rustc_codegen_llvm: don't assume offsets are always aligned. #53998

Merged

bors closed this as completed in #53998 Sep 9, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Using a match statement on a field of an enum results in an internal compiler error #53728

Using a match statement on a field of an enum results in an internal compiler error #53728

vladglv commented Aug 27, 2018

estebank commented Aug 27, 2018

vladglv commented Aug 27, 2018

vladglv commented Aug 27, 2018

KamilaBorowska commented Aug 27, 2018 •

edited by estebank

Loading

matthew-russo commented Aug 28, 2018 •

edited

Loading

pnkfelix commented Aug 30, 2018

nikomatsakis commented Aug 30, 2018

pnkfelix commented Aug 30, 2018

matthew-russo commented Aug 31, 2018 •

edited

Loading

matthew-russo commented Sep 3, 2018

pnkfelix commented Sep 4, 2018

pnkfelix commented Sep 6, 2018

eddyb commented Sep 6, 2018 •

edited

Loading

Using a match statement on a field of an enum results in an internal compiler error #53728

Using a match statement on a field of an enum results in an internal compiler error #53728

Comments

vladglv commented Aug 27, 2018

Code

Output

Extra

estebank commented Aug 27, 2018

vladglv commented Aug 27, 2018

vladglv commented Aug 27, 2018

KamilaBorowska commented Aug 27, 2018 • edited by estebank Loading

matthew-russo commented Aug 28, 2018 • edited Loading

pnkfelix commented Aug 30, 2018

nikomatsakis commented Aug 30, 2018

pnkfelix commented Aug 30, 2018

matthew-russo commented Aug 31, 2018 • edited Loading

matthew-russo commented Sep 3, 2018

pnkfelix commented Sep 4, 2018

pnkfelix commented Sep 6, 2018

eddyb commented Sep 6, 2018 • edited Loading

KamilaBorowska commented Aug 27, 2018 •

edited by estebank

Loading

matthew-russo commented Aug 28, 2018 •

edited

Loading

matthew-russo commented Aug 31, 2018 •

edited

Loading

eddyb commented Sep 6, 2018 •

edited

Loading