From 23d09aebc8c6b89ba86bce2c38a0fc31f227d722 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 ++++++++++++++------- tests/ui/layout/debug.rs | 21 +++++++ tests/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 b4597d5bc78..1bcc44237a3 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -731,36 +731,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 only_variant = &variants[FIRST_VARIANT]; for field in only_variant { - assert!(field.0.is_sized()); - align = align.max(field.align()); + assert!(!field.0.is_unsized()); - // If all non-ZST fields have the same ABI, forward this ABI - if optimize && !field.0.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 } + if optimize { + // If all non-ZST fields have the same ABI, forward this ABI + if field.0.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 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); } - 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 }; } } + 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/tests/ui/layout/debug.rs b/tests/ui/layout/debug.rs index a282e71235c..c506b98f16e 100644 --- a/tests/ui/layout/debug.rs +++ b/tests/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/tests/ui/layout/debug.stderr b/tests/ui/layout/debug.stderr index c5e1c41d130..6f6ab13eac5 100644 --- a/tests/ui/layout/debug.stderr +++ b/tests/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