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 rust-lang#126460.

In this PR, we revise the first case of `Tree::from_enum` to consider only the
narrower category of "enums that are variantless". These enums, whose layouts
are described with `Variants::Single { index }`, are special in that they do not
have a variant at `index`.

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 }` but that do have a variant at `index`.

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
  • Loading branch information
jswrenn committed Jun 14, 2024
1 parent f9515fd commit 3f653f5
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 15 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
25 changes: 12 additions & 13 deletions compiler/rustc_transmute/src/layout/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,30 +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
// 1. enums that variantless
// 2. enums for which all but one variant is uninhabited
// 3. enums with multiple inhabited variants
match layout.variants() {
_ if layout.abi.is_uninhabited() => {
// Uninhabited enums are usually (always?) zero-sized. In
Variants::Single { .. } if def.variants().is_empty() => {
// Uninhabited enums in the form `enum Void {}` are
// currently implemented such that their layout is
// described with `Variants::Single`, even though they do
// not have a 'single' variant to defer to. Consequently,
// we cannot use the regular code path for these enums,
// because that path assumes that a variant exists at the
// `Single`'s `index`.
//
// Variantless 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);
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)
}
Expand Down
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

0 comments on commit 3f653f5

Please sign in to comment.