rustc_abi: audit uses of is_zst; fix a case of giving an enum insufficient alignment

Author: RalfJung

compiler/rustc_abi/src/layout.rs

@@ -157,8 +157,10 @@ pub trait LayoutCalculator {
         // for non-ZST uninhabited data (mostly partial initialization).
         let absent = |fields: &IndexSlice<FieldIdx, Layout<'_>>| {
             let uninhabited = fields.iter().any(|f| f.abi().is_uninhabited());
-            let is_zst = fields.iter().all(|f| f.0.is_zst());
-            uninhabited && is_zst
+            // We cannot ignore alignment; that might lead us to entirely discard a variant and
+            // produce an enum that is less aligned than it should be!
+            let is_1zst = fields.iter().all(|f| f.0.is_1zst());
+            uninhabited && is_1zst
         };
         let (present_first, present_second) = {
             let mut present_variants = variants
@@ -358,8 +360,11 @@ pub trait LayoutCalculator {
                 match layout.fields {
                     FieldsShape::Arbitrary { ref mut offsets, .. } => {
                         for (j, offset) in offsets.iter_enumerated_mut() {
+                            // keep ZST at offset 0 to simplify Scalar/ScalarPair layout
                             if !variants[i][j].0.is_zst() {
                                 *offset += this_offset;
+                            } else {
+                                debug_assert_eq!(offset.bytes(), 0);
                             }
                         }
                     }
@@ -504,7 +509,7 @@ pub trait LayoutCalculator {
                 // to make room for a larger discriminant.
                 for field_idx in st.fields.index_by_increasing_offset() {
                     let field = &field_layouts[FieldIdx::from_usize(field_idx)];
-                    if !field.0.is_zst() || field.align().abi.bytes() != 1 {
+                    if !field.0.is_1zst() {
                         start_align = start_align.min(field.align().abi);
                         break;
                     }
@@ -603,12 +608,15 @@ pub trait LayoutCalculator {
             abi = Abi::Scalar(tag);
         } else {
             // Try to use a ScalarPair for all tagged enums.
+            // That's possible only if we can find a common primitive type for all variants.
             let mut common_prim = None;
             let mut common_prim_initialized_in_all_variants = true;
             for (field_layouts, layout_variant) in iter::zip(variants, &layout_variants) {
                 let FieldsShape::Arbitrary { ref offsets, .. } = layout_variant.fields else {
                     panic!();
                 };
+                // We skip *all* ZST here and later check if we are good in terms of alignment.
+                // This lets us handle some cases involving aligned ZST.
                 let mut fields = iter::zip(field_layouts, offsets).filter(|p| !p.0.0.is_zst());
                 let (field, offset) = match (fields.next(), fields.next()) {
                     (None, None) => {
@@ -954,8 +962,10 @@ fn univariant(
                         };
 
                         (
-                            // Place ZSTs first to avoid "interesting offsets", especially with only one
-                            // or two non-ZST fields. This helps Scalar/ScalarPair layouts.
+                            // Place ZSTs first to avoid "interesting offsets", especially with only
+                            // one or two non-ZST fields. This helps Scalar/ScalarPair layouts. Note
+                            // that these can ignore even some aligned ZST as long as the alignment
+                            // is less than that of the scalar, hence we treat *all* ZST like that.
                             !f.0.is_zst(),
                             // Then place largest alignments first.
                             cmp::Reverse(alignment_group_key(f)),
@@ -1073,9 +1083,10 @@ fn univariant(
     let size = min_size.align_to(align.abi);
     let mut layout_of_single_non_zst_field = None;
     let mut abi = Abi::Aggregate { sized };
-    // Unpack newtype ABIs and find scalar pairs.
+    // Try to make this a Scalar/ScalarPair.
     if sized && size.bytes() > 0 {
-        // All other fields must be ZSTs.
+        // We skip *all* ZST here and later check if we are good in terms of alignment.
+        // This lets us handle some cases involving aligned ZST.
         let mut non_zst_fields = fields.iter_enumerated().filter(|&(_, f)| !f.0.is_zst());
 
         match (non_zst_fields.next(), non_zst_fields.next(), non_zst_fields.next()) {

tests/ui/layout/enum.rs

@@ -0,0 +1,18 @@
+// normalize-stderr-test "pref: Align\([1-8] bytes\)" -> "pref: $$PREF_ALIGN"
+//! Various enum layout tests.
+
+#![feature(rustc_attrs)]
+#![feature(never_type)]
+#![crate_type = "lib"]
+
+#[rustc_layout(align)]
+enum UninhabitedVariantAlign { //~ERROR: abi: Align(2 bytes)
+    A([u8; 32]),
+    B([u16; 0], !), // make sure alignment in uninhabited fields is respected
+}
+
+#[rustc_layout(size)]
+enum UninhabitedVariantSpace { //~ERROR: size: Size(16 bytes)
+    A,
+    B([u8; 15], !), // make sure there is space being reserved for this field.
+}

tests/ui/layout/enum.stderr

@@ -0,0 +1,14 @@
+error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: $PREF_ALIGN }
+  --> $DIR/enum.rs:9:1
+   |
+LL | enum UninhabitedVariantAlign {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: size: Size(16 bytes)
+  --> $DIR/enum.rs:15:1
+   |
+LL | enum UninhabitedVariantSpace {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+

tests/ui/layout/struct.rs

@@ -0,0 +1,12 @@
+// normalize-stderr-test "pref: Align\([1-8] bytes\)" -> "pref: $$PREF_ALIGN"
+//! Various struct layout tests.
+
+#![feature(rustc_attrs)]
+#![feature(never_type)]
+#![crate_type = "lib"]
+
+#[rustc_layout(abi)]
+struct AlignedZstPreventsScalar(i16, [i32; 0]); //~ERROR: abi: Aggregate
+
+#[rustc_layout(abi)]
+struct AlignedZstButStillScalar(i32, [i16; 0]); //~ERROR: abi: Scalar

tests/ui/layout/struct.stderr

@@ -0,0 +1,14 @@
+error: abi: Aggregate { sized: true }
+  --> $DIR/struct.rs:9:1
+   |
+LL | struct AlignedZstPreventsScalar(i16, [i32; 0]);
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: abi: Scalar(Initialized { value: Int(I32, true), valid_range: 0..=4294967295 })
+  --> $DIR/struct.rs:12:1
+   |
+LL | struct AlignedZstButStillScalar(i32, [i16; 0]);
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+