Do not use scalar layout if there are ZSTs with alignment > 1

rust-lang · Nov 25, 2022 · 9f69049 · 9f69049
1 parent 41e0363
commit 9f69049
Show file tree

Hide file tree

Showing 3 changed files with 157 additions and 21 deletions.
diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs
@@ -901,36 +901,58 @@ pub trait LayoutCalculator {
 
  let optimize = !repr.inhibit_union_abi_opt();
  let mut size = Size::ZERO;
- let mut abi = Abi::Aggregate { sized: true };
+ let mut abi = None;
+ let mut biggest_zst_align = align;
+ let mut biggest_non_zst_align = align;
  let index = V::new(0);
  for field in &variants[index] {
- assert!(field.is_sized());
- align = align.max(field.align);
+ assert!(!field.is_unsized());
 
- // If all non-ZST fields have the same ABI, forward this ABI
- if optimize && !field.is_zst() {
- // Discard valid range information and allow undef
- let field_abi = match field.abi {
- Abi::Scalar(x) => Abi::Scalar(x.to_union()),
- Abi::ScalarPair(x, y) => Abi::ScalarPair(x.to_union(), y.to_union()),
- Abi::Vector { element: x, count } => {
- Abi::Vector { element: x.to_union(), count }
- }
- Abi::Uninhabited | Abi::Aggregate { .. } => Abi::Aggregate { sized: true },
- };
+ if optimize {
+ // If all non-ZST fields have the same ABI, forward this ABI
+ if field.is_zst() {
+ biggest_zst_align = biggest_zst_align.max(field.align);
+ } else {
+ biggest_non_zst_align = biggest_non_zst_align.max(field.align);
+ // Discard valid range information and allow undef
+ let field_abi = match field.abi {
+ Abi::Scalar(x) => Abi::Scalar(x.to_union()),
+ Abi::ScalarPair(x, y) => Abi::ScalarPair(x.to_union(), y.to_union()),
+ Abi::Vector { element: x, count } => {
+ Abi::Vector { element: x.to_union(), count }
+ }
+ Abi::Uninhabited | Abi::Aggregate { .. } => Abi::Aggregate { sized: true },
+ };
 
- if size == Size::ZERO {
- // first non ZST: initialize 'abi'
- abi = field_abi;
- } else if abi != field_abi {
- // different fields have different ABI: reset to Aggregate
- abi = Abi::Aggregate { sized: true };
+ if let Some(abi) = &mut abi {
+ if *abi != field_abi {
+ // different fields have different ABI: reset to Aggregate
+ *abi = Abi::Aggregate { sized: true };
+ }
+ } else {
+ abi = Some(field_abi);
+ }
  }
  }
 
+ align = align.max(field.align);
  size = cmp::max(size, field.size);
  }
 
+ let abi = match abi {
+ None => Abi::Aggregate { sized: true },
+ Some(non_zst_abi) => {
+ if biggest_zst_align.abi > biggest_non_zst_align.abi {
+ // If a zst has a bigger alignment than the non-zst fields,
+ // we cannot use scalar layout, because scalar(pair)s can't be
+ // more aligned than their primitive.
+ Abi::Aggregate { sized: true }
+ } else {
+ non_zst_abi
+ }
+ }
+ };
+
  if let Some(pack) = repr.pack {
  align = align.min(AbiAndPrefAlign::new(pack));
  }

diff --git a/src/test/ui/layout/debug.rs b/src/test/ui/layout/debug.rs
@@ -17,6 +17,27 @@ type Test = Result<i32, i32>; //~ ERROR: layout_of
 #[rustc_layout(debug)]
 type T = impl std::fmt::Debug; //~ ERROR: layout_of
 
+#[rustc_layout(debug)]
+pub union V { //~ ERROR: layout_of
+ a: [u16; 0],
+ b: u8,
+}
+
+#[rustc_layout(debug)]
+pub union W { //~ ERROR: layout_of
+ b: u8,
+ a: [u16; 0],
+}
+
+#[rustc_layout(debug)]
+pub union Y { //~ ERROR: layout_of
+ b: [u8; 0],
+ a: [u16; 0],
+}
+
+#[rustc_layout(debug)]
+type X = std::mem::MaybeUninit<u8>; //~ ERROR: layout_of
+
 fn f() -> T {
  0i32
 }
diff --git a/src/test/ui/layout/debug.stderr b/src/test/ui/layout/debug.stderr
@@ -307,5 +307,98 @@ error: layout_of(i32) = Layout {
 LL | type T = impl std::fmt::Debug;
  | ^^^^^^
 
-error: aborting due to 5 previous errors
+error: layout_of(V) = Layout {
+ size: Size(2 bytes),
+ align: AbiAndPrefAlign {
+ abi: Align(2 bytes),
+ pref: $PREF_ALIGN,
+ },
+ abi: Aggregate {
+ sized: true,
+ },
+ fields: Union(
+ 2,
+ ),
+ largest_niche: None,
+ variants: Single {
+ index: 0,
+ },
+ }
+ --> $DIR/debug.rs:21:1
+ |
+LL | pub union V {
+ | ^^^^^^^^^^^
+
+error: layout_of(W) = Layout {
+ size: Size(2 bytes),
+ align: AbiAndPrefAlign {
+ abi: Align(2 bytes),
+ pref: $PREF_ALIGN,
+ },
+ abi: Aggregate {
+ sized: true,
+ },
+ fields: Union(
+ 2,
+ ),
+ largest_niche: None,
+ variants: Single {
+ index: 0,
+ },
+ }
+ --> $DIR/debug.rs:27:1
+ |
+LL | pub union W {
+ | ^^^^^^^^^^^
+
+error: layout_of(Y) = Layout {
+ size: Size(0 bytes),
+ align: AbiAndPrefAlign {
+ abi: Align(2 bytes),
+ pref: $PREF_ALIGN,
+ },
+ abi: Aggregate {
+ sized: true,
+ },
+ fields: Union(
+ 2,
+ ),
+ largest_niche: None,
+ variants: Single {
+ index: 0,
+ },
+ }
+ --> $DIR/debug.rs:33:1
+ |
+LL | pub union Y {
+ | ^^^^^^^^^^^
+
+error: layout_of(std::mem::MaybeUninit<u8>) = Layout {
+ size: Size(1 bytes),
+ align: AbiAndPrefAlign {
+ abi: Align(1 bytes),
+ pref: $PREF_ALIGN,
+ },
+ abi: Scalar(
+ Union {
+ value: Int(
+ I8,
+ false,
+ ),
+ },
+ ),
+ fields: Union(
+ 2,
+ ),
+ largest_niche: None,
+ variants: Single {
+ index: 0,
+ },
+ }
+ --> $DIR/debug.rs:39:1
+ |
+LL | type X = std::mem::MaybeUninit<u8>;
+ | ^^^^^^
+
+error: aborting due to 9 previous errors