Enums with only one variant and without #[repr(_)] have size 0 in 1.27 beta

[pietroalbini]

Some crates are defining enums with only one variant without using #[repr(_)], and in 1.27 beta those enums have size 0. This breaks those crates when they use transmute though.

An easy fix for those crates is adding the appropriate #[repr()] attribute to the enums, but we might want to revert the change to avoid breaking compatibility.

// Taken from libvirt-rpc in src/request.rs

#[derive(Debug)]
pub enum EventShutdownDetailType 
    Finished = 0,
}

• libvirt-rpc regressed from stable to beta (build log) cc @polachok
• utp regressed from stable to beta (build log) cc @meqif
• temp_utp regressed from stable to beta (build log) cc @vinipsmaker

[hanna-kruppe]

cc @nox

[nox]

That was expected, I just don't think those transmute should be done in the first place. There is no guarantee that the enum has the size you expect it to have.

[pietroalbini]

That transmute was allowed on stable though. Would it be possible to revert the 0-size if a number is attached to the variant? At the moment, A can't be transmuted, while B can:

pub enum A 
    Foo = 0,
}

pub enum B
    Foo = 0,
    Bar = 1,
}

[nox]

That’s true, but all 3 of these crates using transmute this way are unsound and allow those transmute calls to be reached from safe code with values which aren’t the actual single discriminant anyway. Even if we revert that patch, those are bugs that should be fixed. AFAIK transmute being allowed on things with no explicit representation has always been icky.

[eddyb]

One such unsound transmute was the cause for one of the regressions from #45225, where an integer was transmuted to an enum even when it didn't match any of the enum variants, and that resulted in a crash elsewhere because the value was used as a niche.
I agree with @nox that catching these is good, but @rust-lang/compiler should take a look.

[eddyb]

Also, all such problem can be fixed "locally", by adding a match or if-else chain and panicking if an incorrect value is given, without even changing the definition of the enum, at all.
So my suggestion would be to take each x.* and 0.y.* group of releases that contains the bad transmutes, yank them, and release one patch version for each, without the transmutes.

[nikomatsakis]

Some discussion from IRC:

https://botbot.me/mozilla/rustc/2018-05-17/?msg=100159968&page=1

[nikomatsakis]

Discussed in the @rust-lang/compiler meeting at or around this point in the IRC log.

The current consensus is:

• These transmutes were undefined behavior and hence we will not change the layout rules in response. The layout of enums is basically undefined.
• However, it's clear that the layout rules and expectations are underdocumented. We should do two things to fix that:
  • clearer documentation in e.g. the rust reference (this is probably a matter for the Unsafe Code Guidelines team)
  • a more aggressive lint to identify transmutes that are not defined

Because these crates have no reverse dependencies, patching is relatively easy -- only new versions are needed. @polachok, @meqif, and @vinipsmaker: we would be very happy to help you rewrite the relevant code if needed! Let us know what help you may need!

[pietroalbini]

Closing this as won't fix then.

[meqif]

I fully agree with the prevailing viewpoint that transmuting was undefined behavior, and that it really shouldn't be used.

In fact, this issue came up before and I fixed it in a later version of utp — I'm not sure why an older version of the crate was targeted here.

[pietroalbini]

In fact, this issue came up before and I fixed it in a later version of utp — I'm not sure why an older version of the crate was targeted here.

Crater tests always the latest version of every crate, and also an older version for some of them.

[scottmcm]

Note that previous changes have also "broken" UB transmutes, such as transmute::<[u8; 6], (u8,u16,u8)>, so this is a previously-decided category of allowed change.