Interpreter calls discriminant_for_variant on zero variant enum

[tmiasko]

Interpreter calls discriminant_for_variant on zero variant enum, when asked unsafely to do so :-).

Code

#![feature(const_discriminant)]
#![feature(const_raw_ptr_deref)]

pub enum Void { }

pub const C: () = {
    unsafe { std::mem::discriminant(&*(&() as *const () as *const Void)); };
};

Error

error: internal compiler error: /rustc/41dfaaa3c66759395835b3af59b22f4f22175dc8/compiler/rustc_middle/src/ty/sty.rs:2009:17: discriminant_for_variant called on zero variant enum

thread 'rustc' panicked at 'Box<dyn Any>', compiler/rustc_errors/src/lib.rs:1146:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.57.0-nightly (41dfaaa3c 2021-10-10) running on x86_64-unknown-linux-gnu

note: compiler flags: --crate-type lib

query stack during panic:
#0 [eval_to_allocation_raw] const-evaluating + checking `C`
#1 [eval_to_const_value_raw] simplifying constant for the type system `C`
end of query stack
error: aborting due to previous error

@rustbot label: +requires-nightly +A-const-eval

[oli-obk]

cc @RalfJung

[RalfJung]

Ah, nasty unsafe code is nasty. ;) This reminds me of discussions I had with @eddyb a while ago about the Layout for 0-variant enums.

Basically the bad code is like

match op.layout.variants {
            Variants::Single { index } => {
                let discr = match op.layout.ty.discriminant_for_variant(*self.tcx, index) {

but is this really the only part of the compiler that assumes that for Variants::Single, we can use discriminant_for_variant?

[RalfJung]

FTR this is the layout of the empty enum:

Layout {
    fields: Primitive,
    variants: Single {
        index: 0,
    },
    abi: Uninhabited,
    largest_niche: None,
    align: AbiAndPrefAlign {
        abi: Align {
            pow2: 0,
        },
        pref: Align {
            pow2: 0,
        },
    },
    size: Size {
        raw: 0,
    },
}

The fix could be

• to make discriminant_for_variant return None for 0-variant enums (treating them basically like any other type that does not actually have a discriminant)
• to add some logic in InterpCx::read_discriminant that avoids calling discriminant_for_variant for 0-variant enums

I haven't made up my mind yet about which one seems better... but I am leaning towards the first option.

[RalfJung]

As pointed out at #89764 (comment), our codegen backend actually says that read_discriminant on any uninhabited type is UB. If we want to stick to that, this limits our options, at least for Miri (CTFE does not have to do all the same checks): pretty much the only clean solution I can think of is to require the place to contain data that satisfies the validity invariant.

CTFE does not check validity invariants, so this does not clearly say what we should do there; we could still treat all types without discriminants the same (by returning 0).

[oli-obk]

In release mode https://play.rust-lang.org/?version=stable&mode=release&edition=2018&gist=b4e16b630ff26b1c56b45647a8330f9a returns an undef (look at llvm IR) even though from all docs it should be sound. At least as long as &Void is sound, which I don't know if it has been decided yet.

[oli-obk]

I have not been able to weaponize it, but we should probably just make the codegen backend return 0

[RalfJung]

In my book &Void is not sound, but indeed it has not been decided yet.

[RalfJung]

In #89764, things look like we should properly check full validity of a value when reading its discriminant. However, that on its own cannot fix this ICE since the first thing validity checking will do is of course read the discriminant...