From 9f690491393b3c48048986d4fb931cd4f85b2839 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Nov 2022 14:45:42 +0000 Subject: [PATCH] Do not use scalar layout if there are ZSTs with alignment > 1 --- compiler/rustc_abi/src/layout.rs | 62 ++++++++++++++------- src/test/ui/layout/debug.rs | 21 +++++++ src/test/ui/layout/debug.stderr | 95 +++++++++++++++++++++++++++++++- 3 files changed, 157 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index 39ea7a85be652..42af9ea861205 100644 --- 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 index a282e71235c31..c506b98f16e0f 100644 --- a/src/test/ui/layout/debug.rs +++ b/src/test/ui/layout/debug.rs @@ -17,6 +17,27 @@ type Test = Result; //~ 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; //~ ERROR: layout_of + fn f() -> T { 0i32 } diff --git a/src/test/ui/layout/debug.stderr b/src/test/ui/layout/debug.stderr index c5e1c41d1304c..6f6ab13eac583 100644 --- 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) = 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; + | ^^^^^^ + +error: aborting due to 9 previous errors