Skip to content

Commit

Permalink
Rollup merge of rust-lang#126493 - jswrenn:fix-126460, r=compiler-errors
Browse files Browse the repository at this point in the history
safe transmute: support non-ZST, variantful, uninhabited enums

Previously, `Tree::from_enum`'s implementation branched into three disjoint cases:

 1. enums that uninhabited
 2. enums for which all but one variant is uninhabited
 3. enums with multiple variants

This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like:

    enum Uninhabited { A(!, u128) }

...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in rust-lang#126460.

In this PR, we revise the first case of `Tree::from_enum` to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with `Variants::Single { index }`, are special in their layouts otherwise resemble the `!` type and cannot be descended into like typical enums. This first case captures uninhabited enums like:

    enum Uninhabited { A(!, !), B(!) }

The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` and that do have a variant at `index`. This second case captures uninhabited enums that are not ZSTs, like:

    enum Uninhabited { A(!, u128) }

...which represent their variants with `Variants::Single`.

Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited enums like:

    enum Uninhabited { A(u8, !), B(!, u32) }

...which represent their variants with `Variants::Multiple`.

This PR also adds a comment requested by ``@RalfJung`` in his review of rust-lang#126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`.

Fixes rust-lang#126460

r? ``@compiler-errors``
  • Loading branch information
GuillaumeGomez authored Jun 18, 2024
2 parents 8a21f48 + df1d616 commit 2f006d6
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 30 deletions.
11 changes: 10 additions & 1 deletion compiler/rustc_const_eval/src/interpret/discriminant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,16 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
variant_index: VariantIdx,
) -> InterpResult<'tcx, Option<(ScalarInt, usize)>> {
match self.layout_of(ty)?.variants {
abi::Variants::Single { .. } => Ok(None),
abi::Variants::Single { .. } => {
// The tag of a `Single` enum is like the tag of the niched
// variant: there's no tag as the discriminant is encoded
// entirely implicitly. If `write_discriminant` ever hits this
// case, we do a "validation read" to ensure the the right
// discriminant is encoded implicitly, so any attempt to write
// the wrong discriminant for a `Single` enum will reliably
// result in UB.
Ok(None)
}

abi::Variants::Multiple {
tag_encoding: TagEncoding::Direct,
Expand Down
36 changes: 14 additions & 22 deletions compiler/rustc_transmute/src/layout/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,37 +341,29 @@ pub(crate) mod rustc {

// We consider three kinds of enums, each demanding a different
// treatment of their layout computation:
// 1. enums that are uninhabited
// 2. enums for which all but one variant is uninhabited
// 3. enums with multiple inhabited variants
// 1. enums that are uninhabited ZSTs
// 2. enums that delegate their layout to a variant
// 3. enums with multiple variants
match layout.variants() {
_ if layout.abi.is_uninhabited() => {
// Uninhabited enums are usually (always?) zero-sized. In
// the (unlikely?) event that an uninhabited enum is
// non-zero-sized, this assert will trigger an ICE, and this
// code should be modified such that a `layout.size` amount
// of uninhabited bytes is returned instead.
//
// Uninhabited enums are currently implemented such that
// their layout is described with `Variants::Single`, even
// though they don't necessarily have a 'single' variant to
// defer to. That said, we don't bother specifically
// matching on `Variants::Single` in this arm because the
// behavioral principles here remain true even if, for
// whatever reason, the compiler describes an uninhabited
// enum with `Variants::Multiple`.
assert_eq!(layout.size, Size::ZERO);
Variants::Single { .. }
if layout.abi.is_uninhabited() && layout.size == Size::ZERO =>
{
// The layout representation of uninhabited, ZST enums is
// defined to be like that of the `!` type, as opposed of a
// typical enum. Consequently, they cannot be descended into
// as if they typical enums. We therefore special-case this
// scenario and simply return an uninhabited `Tree`.
Ok(Self::uninhabited())
}
Variants::Single { index } => {
// `Variants::Single` on non-uninhabited enums denotes that
// `Variants::Single` on enums with variants denotes that
// the enum delegates its layout to the variant at `index`.
layout_of_variant(*index)
}
Variants::Multiple { tag_field, .. } => {
// `Variants::Multiple` denotes an enum with multiple
// inhabited variants. The layout of such an enum is the
// disjunction of the layouts of its tagged variants.
// variants. The layout of such an enum is the disjunction
// of the layouts of its tagged variants.

// For enums (but not coroutines), the tag field is
// currently always the first field of the layout.
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/transmutability/enums/uninhabited_optimization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,14 @@ enum SingleUninhabited {
Y(Uninhabited),
}

enum MultipleUninhabited {
X(u8, Uninhabited),
Y(Uninhabited, u16),
}

fn main() {
assert_transmutable::<Uninhabited>();
assert_transmutable::<SingleInhabited>();
assert_transmutable::<SingleUninhabited>();
assert_transmutable::<MultipleUninhabited>();
}
24 changes: 23 additions & 1 deletion tests/ui/transmutability/uninhabited.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ fn void() {
}

// Non-ZST uninhabited types are, nonetheless, uninhabited.
fn yawning_void() {
fn yawning_void_struct() {
enum Void {}

struct YawningVoid(Void, u128);
Expand All @@ -49,6 +49,28 @@ fn yawning_void() {
assert::is_maybe_transmutable::<(), Void>(); //~ ERROR: cannot be safely transmuted
}

// Non-ZST uninhabited types are, nonetheless, uninhabited.
fn yawning_void_enum() {
enum Void {}

enum YawningVoid {
A(Void, u128),
}

const _: () = {
assert!(std::mem::size_of::<YawningVoid>() == std::mem::size_of::<u128>());
// Just to be sure the above constant actually evaluated:
assert!(false); //~ ERROR: evaluation of constant value failed
};

// This transmutation is vacuously acceptable; since one cannot construct a
// `Void`, unsoundness cannot directly arise from transmuting a void into
// anything else.
assert::is_maybe_transmutable::<YawningVoid, u128>();

assert::is_maybe_transmutable::<(), Void>(); //~ ERROR: cannot be safely transmuted
}

// References to uninhabited types are, logically, uninhabited, but for layout
// purposes are not ZSTs, and aren't treated as uninhabited when they appear in
// enum variants.
Expand Down
42 changes: 36 additions & 6 deletions tests/ui/transmutability/uninhabited.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,18 @@ LL | assert!(false);
= note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0080]: evaluation of constant value failed
--> $DIR/uninhabited.rs:65:9
--> $DIR/uninhabited.rs:63:9
|
LL | assert!(false);
| ^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: false', $DIR/uninhabited.rs:65:9
| ^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: false', $DIR/uninhabited.rs:63:9
|
= note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0080]: evaluation of constant value failed
--> $DIR/uninhabited.rs:87:9
|
LL | assert!(false);
| ^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: false', $DIR/uninhabited.rs:87:9
|
= note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)

Expand All @@ -36,11 +44,33 @@ LL | | }
LL | | }>
| |__________^ required by this bound in `is_maybe_transmutable`

error[E0277]: `()` cannot be safely transmuted into `yawning_void::Void`
error[E0277]: `()` cannot be safely transmuted into `yawning_void_struct::Void`
--> $DIR/uninhabited.rs:49:41
|
LL | assert::is_maybe_transmutable::<(), Void>();
| ^^^^ `yawning_void::Void` is uninhabited
| ^^^^ `yawning_void_struct::Void` is uninhabited
|
note: required by a bound in `is_maybe_transmutable`
--> $DIR/uninhabited.rs:10:14
|
LL | pub fn is_maybe_transmutable<Src, Dst>()
| --------------------- required by a bound in this function
LL | where
LL | Dst: BikeshedIntrinsicFrom<Src, {
| ______________^
LL | | Assume {
LL | | alignment: true,
LL | | lifetimes: true,
... |
LL | | }
LL | | }>
| |__________^ required by this bound in `is_maybe_transmutable`

error[E0277]: `()` cannot be safely transmuted into `yawning_void_enum::Void`
--> $DIR/uninhabited.rs:71:41
|
LL | assert::is_maybe_transmutable::<(), Void>();
| ^^^^ `yawning_void_enum::Void` is uninhabited
|
note: required by a bound in `is_maybe_transmutable`
--> $DIR/uninhabited.rs:10:14
Expand All @@ -59,7 +89,7 @@ LL | | }>
| |__________^ required by this bound in `is_maybe_transmutable`

error[E0277]: `u128` cannot be safely transmuted into `DistantVoid`
--> $DIR/uninhabited.rs:70:43
--> $DIR/uninhabited.rs:92:43
|
LL | assert::is_maybe_transmutable::<u128, DistantVoid>();
| ^^^^^^^^^^^ at least one value of `u128` isn't a bit-valid value of `DistantVoid`
Expand All @@ -80,7 +110,7 @@ LL | | }
LL | | }>
| |__________^ required by this bound in `is_maybe_transmutable`

error: aborting due to 5 previous errors
error: aborting due to 7 previous errors

Some errors have detailed explanations: E0080, E0277.
For more information about an error, try `rustc --explain E0080`.

0 comments on commit 2f006d6

Please sign in to comment.