Variant with value 1 of binary field-less enum with repr(i8) is transmuted to -1

[boxdot]

This code reproduces the bug:

#[repr(i8)]
pub enum Enum {
    VariantA = 0,
    VariantB = 1,
}

pub fn main() {
    assert_eq!(1, unsafe { std::mem::transmute::<i8, Enum>(1) } as i8);
}

Output:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `-1`', src/main.rs:8:5

in debug and release modes starting with compiler:

rustc 1.24.0 (4d90ac38c 2018-02-12)
binary: rustc
commit-hash: 4d90ac38c0b61bb69470b61ea2cccea0df48d9e5
commit-date: 2018-02-12
host: x86_64-apple-darwin
release: 1.24.0
LLVM version: 4.0

The test passes for:

rustc 1.23.0 (766bd11c8 2018-01-01)
binary: rustc
commit-hash: 766bd11c8a3c019ca53febdcd77b2215379dd67d
commit-date: 2018-01-01
host: x86_64-apple-darwin
release: 1.23.0
LLVM version: 4.0

Not affected are enums with more than 2 values, and enums with repr({i16, i32, i64}).

[shepmaster]

Why is this a bug? I didn't think that either #[repr(i8)] or assigning a discriminant actually affected the bitwise representation of the enum. It seems more like your previous code was working accidentally / via undefined behavior.

[boxdot]

Because the binary representation of Enum::VariantB is ambiguous if we store it in i8. Indeed, let's extend the test:

let b = unsafe { std::mem::transmute::<i8, Enum>(std::mem::transmute::<Enum, i8>(Enum::VariantB)) };
assert_eq!(1, Enum::VariantB as i8);
assert_eq!(1, b as i8);

The first assertion passes, the second assertion fails with:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `-1`'

Further, if we derive Debug and PartialEq for Enum, then in the following code

assert_eq!(Enum::VariantB, b);
println!("{:?}", b);

the first line passes, the second fails with

5 Illegal instruction     timeout --signal=KILL ${timeout} "$@"

(Btw. I don't have previous code. I just try to write serialization for such enums. And logically, since this is not working, we (on IRC) checked for which compilers it worked before.)

[oli-obk]

This is a pre-miri compiler. Can you try the current stable and nightly?

Also I agree with @shepmaster , the memory representation of the enum could just as well be 42 and 99 for the variants, even if casting results in the discriminant values 0 and 1. Maybe repr(I8) has a defined layout, you should check the RFC.

[boxdot]

The failing checks and illegal instruction for the current stable 1.26.2 (594fb25 2018-06-01) and latest nightly 1.28.0-nightly (967c1f3 2018-06-15) are still present.

In the extended test, I do not assume anything about the memory representation except that it should be unique. This does not hold. More severely, println terminates the program with illegal hardware instruction. I am not sure what transmute guarantees exactly (I guess this is implementation defined), but the assumption that "transmute from a type and back" is identity seems very reasonable.

In the assembly code (https://godbolt.org/g/WuCRLG) generated for Enum as i8, it seems that there is a conversion from i8 to i1. The instruction and 1 discards the upper bits. So, it looks to me that sometimes this is done, and sometimes not, and that's why we end up with two different binary representations of Enum::VariantB.

About the RFC: I cannot find in which RFC repr(i8, ...) was introduced.

[eddyb]

@shepmaster @oli-obk What you're saying holds for an enum with explicit discriminant values, but without #[repr].

Looking at this formulation (on playground):

pub fn roundtrip(x: i8) -> i8 {
    (unsafe { std::mem::transmute::<i8, Enum>(x) }) as i8
}

  %3 = trunc i8 %2 to i1
  ; ...
  %5 = sext i1 %3 to i8
  ret i8 %5

So that seems wrong. Because the enum only uses values 0 and 1, we pass it around as i1, which is fine, but the enum casting code assumes it started with the full memory type.
All we have to do, I think, is use from_immediate before doing the actual cast.
EDIT: seems better not to treat any bool-like as signed in the first place.

What I don't understand is why this would work in 1.23.0, if the bug was introduced in f62e43d.

[eddyb]

Note that you don't even need transmute, it's the as cast that's broken:

#[repr(i8)]
pub enum Enum {
    VariantA,
    VariantB,
}

fn make_b() -> Enum { Enum::VariantB }

fn main() {
    assert_eq!(1, make_b() as i8);
}

Also, it doesn't matter what type is being cast to, neither the size nor the signedness, it's always !0.

[eddyb]

@rust-lang/compiler Do we want to backport the fix for this?

[nagisa]

The fix was simple enough and fixes a fairly serious problem so I'd prefer to backport. S.

…

On Sun, Jun 17, 2018, 09:14 Eduard-Mihai Burtescu ***@***.***> wrote: @rust-lang/compiler <https://github.com/orgs/rust-lang/teams/compiler> Do we want to backport the fix for this? — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub <#51582 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AApc0j8j4yF6C-1ev4-ntSspEyqoTOJwks5t9fPigaJpZM4UqOar> .