Rollup merge of #131843 - workingjubilee:thaw-impossible-reprs, r=lukas-code

matthiaskrgr · web-flow · commit 6d43de643e26 · 2024-10-20T16:54:10.000+02:00
compiler: Error on layout of enums with invalid reprs

Surprising no one, the ICEs with the same message have the same root cause.

Invalid reprs can reach layout computation for various reasons. For instance, the compiler may want to use its layout computations to discern if a combination of layout-affecting attributes results in a valid type to begin with by e.g. computing its size. When the input is bad, return an error reflecting that the answer to the question is not a useful one.
diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs
@@ -54,6 +54,9 @@ pub enum LayoutCalculatorError<F> {
 
     /// A union had no fields.
     EmptyUnion,
+
+    /// The fields or variants have irreconcilable reprs
+    ReprConflict,
 }
 
 impl<F> LayoutCalculatorError<F> {
@@ -64,6 +67,7 @@ impl<F> LayoutCalculatorError<F> {
             }
             LayoutCalculatorError::SizeOverflow => LayoutCalculatorError::SizeOverflow,
             LayoutCalculatorError::EmptyUnion => LayoutCalculatorError::EmptyUnion,
+            LayoutCalculatorError::ReprConflict => LayoutCalculatorError::ReprConflict,
         }
     }
 
@@ -77,6 +81,7 @@ impl<F> LayoutCalculatorError<F> {
             }
             LayoutCalculatorError::SizeOverflow => "size overflow",
             LayoutCalculatorError::EmptyUnion => "type is a union with no fields",
+            LayoutCalculatorError::ReprConflict => "type has an invalid repr",
         })
     }
 }
@@ -514,6 +519,10 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
         }
 
         let dl = self.cx.data_layout();
+        // bail if the enum has an incoherent repr that cannot be computed
+        if repr.packed() {
+            return Err(LayoutCalculatorError::ReprConflict);
+        }
 
         let calculate_niche_filling_layout = || -> Option<TmpLayout<FieldIdx, VariantIdx>> {
             if dont_niche_optimize_enum {
diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs
@@ -30,7 +30,8 @@ use {rustc_abi as abi, rustc_hir as hir};
 use crate::errors::{
     MultipleArrayFieldsSimdType, NonPrimitiveSimdType, OversizedSimdType, ZeroLengthSimdType,
 };
-use crate::layout_sanity_check::sanity_check_layout;
+
+mod invariant;
 
 pub(crate) fn provide(providers: &mut Providers) {
     *providers = Providers { layout_of, ..*providers };
@@ -79,7 +80,7 @@ fn layout_of<'tcx>(
         record_layout_for_printing(&cx, layout);
     }
 
-    sanity_check_layout(&cx, &layout);
+    invariant::partially_check_layout(&cx, &layout);
 
     Ok(layout)
 }
@@ -115,6 +116,11 @@ fn map_error<'tcx>(
             cx.tcx().dcx().delayed_bug(format!("computed layout of empty union: {ty:?}"));
             LayoutError::Unknown(ty)
         }
+        LayoutCalculatorError::ReprConflict => {
+            // packed enums are the only known trigger of this, but others might arise
+            cx.tcx().dcx().delayed_bug(format!("computed impossible repr (packed enum?): {ty:?}"));
+            LayoutError::Unknown(ty)
+        }
     };
     error(cx, err)
 }
diff --git a/compiler/rustc_ty_utils/src/layout/invariant.rs b/compiler/rustc_ty_utils/src/layout/invariant.rs
@@ -5,7 +5,7 @@ use rustc_middle::ty::layout::{HasTyCtxt, LayoutCx, TyAndLayout};
 use rustc_target::abi::*;
 
 /// Enforce some basic invariants on layouts.
-pub(super) fn sanity_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayout<'tcx>) {
+pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayout<'tcx>) {
     let tcx = cx.tcx();
 
     // Type-level uninhabitedness should always imply ABI uninhabitedness.
diff --git a/compiler/rustc_ty_utils/src/lib.rs b/compiler/rustc_ty_utils/src/lib.rs
@@ -29,7 +29,6 @@ mod errors;
 mod implied_bounds;
 mod instance;
 mod layout;
-mod layout_sanity_check;
 mod needs_drop;
 mod opaque_types;
 mod representability;
diff --git a/tests/ui/layout/thaw-transmute-invalid-enum.rs b/tests/ui/layout/thaw-transmute-invalid-enum.rs
@@ -1,10 +1,14 @@
-//@ known-bug: rust-lang/rust#126966
+#![crate_type = "lib"]
+
 mod assert {
     use std::mem::{Assume, TransmuteFrom};
+    //~^ ERROR: use of unstable library feature 'transmutability'
+    //~| ERROR: use of unstable library feature 'transmutability'
 
     pub fn is_transmutable<Src, Dst>()
     where
         Dst: TransmuteFrom<Src>,
+        //~^ ERROR: use of unstable library feature 'transmutability'
     {
     }
 }
@@ -15,15 +19,18 @@ enum Ox00 {
 }
 
 #[repr(C, packed(2))]
+//~^ ERROR: attribute should be applied to a struct
 enum OxFF {
     V = 0xFF,
 }
 
 fn test() {
     union Superset {
         a: Ox00,
+        //~^ ERROR: field must implement `Copy`
         b: OxFF,
     }
 
     assert::is_transmutable::<Superset, Subset>();
+    //~^ ERROR: cannot find type `Subset`
 }
diff --git a/tests/ui/layout/thaw-transmute-invalid-enum.stderr b/tests/ui/layout/thaw-transmute-invalid-enum.stderr
@@ -0,0 +1,68 @@
+error[E0412]: cannot find type `Subset` in this scope
+  --> $DIR/thaw-transmute-invalid-enum.rs:34:41
+   |
+LL |     assert::is_transmutable::<Superset, Subset>();
+   |                                         ^^^^^^ not found in this scope
+   |
+help: you might be missing a type parameter
+   |
+LL | fn test<Subset>() {
+   |        ++++++++
+
+error[E0517]: attribute should be applied to a struct or union
+  --> $DIR/thaw-transmute-invalid-enum.rs:21:11
+   |
+LL |   #[repr(C, packed(2))]
+   |             ^^^^^^^^^
+LL |
+LL | / enum OxFF {
+LL | |     V = 0xFF,
+LL | | }
+   | |_- not a struct or union
+
+error[E0658]: use of unstable library feature 'transmutability'
+  --> $DIR/thaw-transmute-invalid-enum.rs:4:20
+   |
+LL |     use std::mem::{Assume, TransmuteFrom};
+   |                    ^^^^^^
+   |
+   = note: see issue #99571 <https://github.com/rust-lang/rust/issues/99571> for more information
+   = help: add `#![feature(transmutability)]` to the crate attributes to enable
+   = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
+
+error[E0658]: use of unstable library feature 'transmutability'
+  --> $DIR/thaw-transmute-invalid-enum.rs:4:28
+   |
+LL |     use std::mem::{Assume, TransmuteFrom};
+   |                            ^^^^^^^^^^^^^
+   |
+   = note: see issue #99571 <https://github.com/rust-lang/rust/issues/99571> for more information
+   = help: add `#![feature(transmutability)]` to the crate attributes to enable
+   = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
+
+error[E0658]: use of unstable library feature 'transmutability'
+  --> $DIR/thaw-transmute-invalid-enum.rs:10:14
+   |
+LL |         Dst: TransmuteFrom<Src>,
+   |              ^^^^^^^^^^^^^^^^^^
+   |
+   = note: see issue #99571 <https://github.com/rust-lang/rust/issues/99571> for more information
+   = help: add `#![feature(transmutability)]` to the crate attributes to enable
+   = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
+
+error[E0740]: field must implement `Copy` or be wrapped in `ManuallyDrop<...>` to be used in a union
+  --> $DIR/thaw-transmute-invalid-enum.rs:29:9
+   |
+LL |         a: Ox00,
+   |         ^^^^^^^
+   |
+   = note: union fields must not have drop side-effects, which is currently enforced via either `Copy` or `ManuallyDrop<...>`
+help: wrap the field type in `ManuallyDrop<...>`
+   |
+LL |         a: std::mem::ManuallyDrop<Ox00>,
+   |            +++++++++++++++++++++++    +
+
+error: aborting due to 6 previous errors
+
+Some errors have detailed explanations: E0412, E0517, E0658, E0740.
+For more information about an error, try `rustc --explain E0412`.
diff --git a/tests/ui/layout/thaw-validate-invalid-enum.rs b/tests/ui/layout/thaw-validate-invalid-enum.rs
@@ -1,7 +1,6 @@
-//@ known-bug: rust-lang/rust#128870
 //@ compile-flags: -Zvalidate-mir
 
-#[repr(packed)]
+#[repr(packed)] //~ ERROR: attribute should be applied to a struct
 #[repr(u32)]
 enum E {
     A,
@@ -12,7 +11,7 @@ enum E {
 fn main() {
     union InvalidTag {
         int: u32,
-        e: E,
+        e: E, //~ ERROR: field must implement `Copy`
     }
     let _invalid_tag = InvalidTag { int: 4 };
 }
diff --git a/tests/ui/layout/thaw-validate-invalid-enum.stderr b/tests/ui/layout/thaw-validate-invalid-enum.stderr
@@ -0,0 +1,29 @@
+error[E0517]: attribute should be applied to a struct or union
+  --> $DIR/thaw-validate-invalid-enum.rs:3:8
+   |
+LL |   #[repr(packed)]
+   |          ^^^^^^
+LL |   #[repr(u32)]
+LL | / enum E {
+LL | |     A,
+LL | |     B,
+LL | |     C,
+LL | | }
+   | |_- not a struct or union
+
+error[E0740]: field must implement `Copy` or be wrapped in `ManuallyDrop<...>` to be used in a union
+  --> $DIR/thaw-validate-invalid-enum.rs:14:9
+   |
+LL |         e: E,
+   |         ^^^^
+   |
+   = note: union fields must not have drop side-effects, which is currently enforced via either `Copy` or `ManuallyDrop<...>`
+help: wrap the field type in `ManuallyDrop<...>`
+   |
+LL |         e: std::mem::ManuallyDrop<E>,
+   |            +++++++++++++++++++++++ +
+
+error: aborting due to 2 previous errors
+
+Some errors have detailed explanations: E0517, E0740.
+For more information about an error, try `rustc --explain E0517`.

Original file line number	Diff line number	Diff line change
`@@ -54,6 +54,9 @@ pub enum LayoutCalculatorError<F> {`
`54`	`54`
`55`	`55`	`/// A union had no fields.`
`56`	`56`	`EmptyUnion,`
	`57`	`+`
	`58`	`+ /// The fields or variants have irreconcilable reprs`
	`59`	`+ ReprConflict,`
`57`	`60`	`}`
`58`	`61`
`59`	`62`	`impl<F> LayoutCalculatorError<F> {`
`@@ -64,6 +67,7 @@ impl<F> LayoutCalculatorError<F> {`
`64`	`67`	`}`
`65`	`68`	`LayoutCalculatorError::SizeOverflow => LayoutCalculatorError::SizeOverflow,`
`66`	`69`	`LayoutCalculatorError::EmptyUnion => LayoutCalculatorError::EmptyUnion,`
	`70`	`+ LayoutCalculatorError::ReprConflict => LayoutCalculatorError::ReprConflict,`
`67`	`71`	`}`
`68`	`72`	`}`
`69`	`73`
`@@ -77,6 +81,7 @@ impl<F> LayoutCalculatorError<F> {`
`77`	`81`	`}`
`78`	`82`	`LayoutCalculatorError::SizeOverflow => "size overflow",`
`79`	`83`	`LayoutCalculatorError::EmptyUnion => "type is a union with no fields",`
	`84`	`+ LayoutCalculatorError::ReprConflict => "type has an invalid repr",`
`80`	`85`	`})`
`81`	`86`	`}`
`82`	`87`	`}`
`@@ -514,6 +519,10 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {`
`514`	`519`	`}`
`515`	`520`
`516`	`521`	`let dl = self.cx.data_layout();`
	`522`	`+ // bail if the enum has an incoherent repr that cannot be computed`
	`523`	`+ if repr.packed() {`
	`524`	`+ return Err(LayoutCalculatorError::ReprConflict);`
	`525`	`+ }`
`517`	`526`
`518`	`527`	`let calculate_niche_filling_layout = \|\| -> Option<TmpLayout<FieldIdx, VariantIdx>> {`
`519`	`528`	`if dont_niche_optimize_enum {`
Original file line number	Diff line number	Diff line change
`@@ -1,10 +1,14 @@`
`1`		`-//@ known-bug: rust-lang/rust#126966`
	`1`	`+#![crate_type = "lib"]`
	`2`	`+`
`2`	`3`	`mod assert {`
`3`	`4`	`use std::mem::{Assume, TransmuteFrom};`
	`5`	`+ //~^ ERROR: use of unstable library feature 'transmutability'`
	`6`	`+ //~\| ERROR: use of unstable library feature 'transmutability'`
`4`	`7`
`5`	`8`	`pub fn is_transmutable<Src, Dst>()`
`6`	`9`	`where`
`7`	`10`	`Dst: TransmuteFrom<Src>,`
	`11`	`+ //~^ ERROR: use of unstable library feature 'transmutability'`
`8`	`12`	`{`
`9`	`13`	`}`
`10`	`14`	`}`
`@@ -15,15 +19,18 @@ enum Ox00 {`
`15`	`19`	`}`
`16`	`20`
`17`	`21`	`#[repr(C, packed(2))]`
	`22`	`+//~^ ERROR: attribute should be applied to a struct`
`18`	`23`	`enum OxFF {`
`19`	`24`	`V = 0xFF,`
`20`	`25`	`}`
`21`	`26`
`22`	`27`	`fn test() {`
`23`	`28`	`union Superset {`
`24`	`29`	`a: Ox00,`
	`30`	+ //~^ ERROR: field must implement `Copy`
`25`	`31`	`b: OxFF,`
`26`	`32`	`}`
`27`	`33`
`28`	`34`	`assert::is_transmutable::<Superset, Subset>();`
	`35`	+ //~^ ERROR: cannot find type `Subset`
`29`	`36`	`}`