Skip to content

Commit

Permalink
safe transmute: support non-ZST, variantful, uninhabited enums
Browse files Browse the repository at this point in the history
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 inhabited 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 #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, non-ZST 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 #126358 to
`compiler/rustc_const_eval/src/interpret/discriminant.rs`.

Fixes #126460
  • Loading branch information
jswrenn committed Jun 14, 2024
1 parent f9515fd commit aa1703d
Show file tree
Hide file tree
Showing 5 changed files with 90 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
37 changes: 15 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 Expand Up @@ -410,6 +402,7 @@ pub(crate) mod rustc {
// layouts.
let FieldsShape::Arbitrary { offsets, memory_index } = ty_and_layout.layout.fields()
else {
println!(" {:?}", ty_and_layout.layout.fields());
return Err(Err::NotYetSupported);
};

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 aa1703d

Please sign in to comment.