safe transmute: support non-ZST, variantful, uninhabited enums

jswrenn · jswrenn · commit df1d6168f4de · 2024-06-14T21:11:08.000Z
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
diff --git a/compiler/rustc_const_eval/src/interpret/discriminant.rs b/compiler/rustc_const_eval/src/interpret/discriminant.rs
@@ -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,
diff --git a/compiler/rustc_transmute/src/layout/tree.rs b/compiler/rustc_transmute/src/layout/tree.rs
@@ -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.
diff --git a/tests/ui/transmutability/enums/uninhabited_optimization.rs b/tests/ui/transmutability/enums/uninhabited_optimization.rs
@@ -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>();
 }
diff --git a/tests/ui/transmutability/uninhabited.rs b/tests/ui/transmutability/uninhabited.rs
@@ -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);
@@ -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.
diff --git a/tests/ui/transmutability/uninhabited.stderr b/tests/ui/transmutability/uninhabited.stderr
@@ -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)
 
@@ -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
@@ -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`
@@ -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`.

Original file line number	Diff line number	Diff line change
`@@ -19,8 +19,14 @@ enum SingleUninhabited {`
`19`	`19`	`Y(Uninhabited),`
`20`	`20`	`}`
`21`	`21`
	`22`	`+enum MultipleUninhabited {`
	`23`	`+ X(u8, Uninhabited),`
	`24`	`+ Y(Uninhabited, u16),`
	`25`	`+}`
	`26`	`+`
`22`	`27`	`fn main() {`
`23`	`28`	`assert_transmutable::<Uninhabited>();`
`24`	`29`	`assert_transmutable::<SingleInhabited>();`
`25`	`30`	`assert_transmutable::<SingleUninhabited>();`
	`31`	`+ assert_transmutable::<MultipleUninhabited>();`
`26`	`32`	`}`