From 1bbae507d4aeb4e1733aaf875246b95e8b08f8a8 Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Mon, 18 Apr 2022 21:28:53 -0400 Subject: [PATCH 1/2] mark payload fields of ScalarPair enums as Scalar::Union when they're not always initialized --- compiler/rustc_middle/src/ty/layout.rs | 37 +- ...6158-scalarpair-payload-might-be-uninit.rs | 53 ++ ...-scalarpair-payload-might-be-uninit.stderr | 754 ++++++++++++++++++ 3 files changed, 828 insertions(+), 16 deletions(-) create mode 100644 src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.rs create mode 100644 src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index f6d139fe59d3c..7cf2984a63f90 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -1120,21 +1120,12 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { match st[i].abi() { Abi::Scalar(_) => Abi::Scalar(niche_scalar), Abi::ScalarPair(first, second) => { - // We need to use scalar_unit to reset the - // valid range to the maximal one for that - // primitive, because only the niche is - // guaranteed to be initialised, not the - // other primitive. + // Only the niche is guaranteed to be initialised, + // so use union layout for the other primitive. if offset.bytes() == 0 { - Abi::ScalarPair( - niche_scalar, - scalar_unit(second.primitive()), - ) + Abi::ScalarPair(niche_scalar, second.to_union()) } else { - Abi::ScalarPair( - scalar_unit(first.primitive()), - niche_scalar, - ) + Abi::ScalarPair(first.to_union(), niche_scalar) } } _ => Abi::Aggregate { sized: true }, @@ -1329,6 +1320,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { } else { // Try to use a ScalarPair for all tagged enums. 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 { bug!(); @@ -1336,7 +1328,10 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let mut fields = iter::zip(field_layouts, offsets).filter(|p| !p.0.is_zst()); let (field, offset) = match (fields.next(), fields.next()) { - (None, None) => continue, + (None, None) => { + common_prim_initialized_in_all_variants = false; + continue; + } (Some(pair), None) => pair, _ => { common_prim = None; @@ -1344,7 +1339,11 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { } }; let prim = match field.abi { - Abi::Scalar(scalar) => scalar.primitive(), + Abi::Scalar(scalar) => { + common_prim_initialized_in_all_variants &= + matches!(scalar, Scalar::Initialized { .. }); + scalar.primitive() + } _ => { common_prim = None; break; @@ -1364,7 +1363,13 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { } } if let Some((prim, offset)) = common_prim { - let pair = self.scalar_pair(tag, scalar_unit(prim)); + let prim_scalar = if common_prim_initialized_in_all_variants { + scalar_unit(prim) + } else { + // Common prim might be uninit. + Scalar::Union { value: prim } + }; + let pair = self.scalar_pair(tag, prim_scalar); let pair_offsets = match pair.fields { FieldsShape::Arbitrary { ref offsets, ref memory_index } => { assert_eq!(memory_index, &[0, 1]); diff --git a/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.rs b/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.rs new file mode 100644 index 0000000000000..bc3667c5f809f --- /dev/null +++ b/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.rs @@ -0,0 +1,53 @@ +#![crate_type = "lib"] +#![feature(rustc_attrs)] + +use std::mem::MaybeUninit; + +enum HasNiche { + A, + B, + C, +} + +// This should result in ScalarPair(Initialized, Union), +// since the u8 payload will be uninit for `None`. +#[rustc_layout(debug)] +pub enum MissingPayloadField { //~ ERROR: layout_of + Some(u8), + None +} + +// This should result in ScalarPair(Initialized, Initialized), +// since the u8 field is present in all variants, +// and hence will always be initialized. +#[rustc_layout(debug)] +pub enum CommonPayloadField { //~ ERROR: layout_of + A(u8), + B(u8), +} + +// This should result in ScalarPair(Initialized, Union), +// since, though a u8-sized field is present in all variants, it might be uninit. +#[rustc_layout(debug)] +pub enum CommonPayloadFieldIsMaybeUninit { //~ ERROR: layout_of + A(u8), + B(MaybeUninit), +} + +// This should result in ScalarPair(Initialized, Union), +// since only the niche field (used for the tag) is guaranteed to be initialized. +#[rustc_layout(debug)] +pub enum NicheFirst { //~ ERROR: layout_of + A(HasNiche, u8), + B, + C +} + +// This should result in ScalarPair(Union, Initialized), +// since only the niche field (used for the tag) is guaranteed to be initialized. +#[rustc_layout(debug)] +pub enum NicheSecond { //~ ERROR: layout_of + A(u8, HasNiche), + B, + C, +} diff --git a/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr b/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr new file mode 100644 index 0000000000000..af51f813076ed --- /dev/null +++ b/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr @@ -0,0 +1,754 @@ +error: layout_of(MissingPayloadField) = Layout { + fields: Arbitrary { + offsets: [ + Size { + raw: 0, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Multiple { + tag: Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + tag_encoding: Direct, + tag_field: 0, + variants: [ + Layout { + fields: Arbitrary { + offsets: [ + Size { + raw: 1, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Single { + index: 0, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 2, + }, + }, + Layout { + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + variants: Single { + index: 1, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 1, + }, + }, + ], + }, + abi: ScalarPair( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + Union { + value: Int( + I8, + false, + ), + }, + ), + largest_niche: Some( + Niche { + offset: Size { + raw: 0, + }, + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + ), + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 2, + }, + } + --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:15:1 + | +LL | / pub enum MissingPayloadField { +LL | | Some(u8), +LL | | None +LL | | } + | |_^ + +error: layout_of(CommonPayloadField) = Layout { + fields: Arbitrary { + offsets: [ + Size { + raw: 0, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Multiple { + tag: Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + tag_encoding: Direct, + tag_field: 0, + variants: [ + Layout { + fields: Arbitrary { + offsets: [ + Size { + raw: 1, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Single { + index: 0, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 2, + }, + }, + Layout { + fields: Arbitrary { + offsets: [ + Size { + raw: 1, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Single { + index: 1, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 2, + }, + }, + ], + }, + abi: ScalarPair( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=255, + }, + ), + largest_niche: Some( + Niche { + offset: Size { + raw: 0, + }, + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + ), + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 2, + }, + } + --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:24:1 + | +LL | / pub enum CommonPayloadField { +LL | | A(u8), +LL | | B(u8), +LL | | } + | |_^ + +error: layout_of(CommonPayloadFieldIsMaybeUninit) = Layout { + fields: Arbitrary { + offsets: [ + Size { + raw: 0, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Multiple { + tag: Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + tag_encoding: Direct, + tag_field: 0, + variants: [ + Layout { + fields: Arbitrary { + offsets: [ + Size { + raw: 1, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Single { + index: 0, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 2, + }, + }, + Layout { + fields: Arbitrary { + offsets: [ + Size { + raw: 1, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Single { + index: 1, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 2, + }, + }, + ], + }, + abi: ScalarPair( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + Union { + value: Int( + I8, + false, + ), + }, + ), + largest_niche: Some( + Niche { + offset: Size { + raw: 0, + }, + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + ), + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 2, + }, + } + --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:32:1 + | +LL | / pub enum CommonPayloadFieldIsMaybeUninit { +LL | | A(u8), +LL | | B(MaybeUninit), +LL | | } + | |_^ + +error: layout_of(NicheFirst) = Layout { + fields: Arbitrary { + offsets: [ + Size { + raw: 0, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Multiple { + tag: Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=4, + }, + tag_encoding: Niche { + dataful_variant: 0, + niche_variants: 1..=2, + niche_start: 3, + }, + tag_field: 0, + variants: [ + Layout { + fields: Arbitrary { + offsets: [ + Size { + raw: 0, + }, + Size { + raw: 1, + }, + ], + memory_index: [ + 0, + 1, + ], + }, + variants: Single { + index: 0, + }, + abi: ScalarPair( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=2, + }, + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=255, + }, + ), + largest_niche: Some( + Niche { + offset: Size { + raw: 0, + }, + value: Int( + I8, + false, + ), + valid_range: 0..=2, + }, + ), + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 2, + }, + }, + Layout { + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + variants: Single { + index: 1, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 0, + }, + }, + Layout { + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + variants: Single { + index: 2, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 0, + }, + }, + ], + }, + abi: ScalarPair( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=4, + }, + Union { + value: Int( + I8, + false, + ), + }, + ), + largest_niche: Some( + Niche { + offset: Size { + raw: 0, + }, + value: Int( + I8, + false, + ), + valid_range: 0..=4, + }, + ), + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 2, + }, + } + --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:40:1 + | +LL | / pub enum NicheFirst { +LL | | A(HasNiche, u8), +LL | | B, +LL | | C +LL | | } + | |_^ + +error: layout_of(NicheSecond) = Layout { + fields: Arbitrary { + offsets: [ + Size { + raw: 1, + }, + ], + memory_index: [ + 0, + ], + }, + variants: Multiple { + tag: Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=4, + }, + tag_encoding: Niche { + dataful_variant: 0, + niche_variants: 1..=2, + niche_start: 3, + }, + tag_field: 0, + variants: [ + Layout { + fields: Arbitrary { + offsets: [ + Size { + raw: 0, + }, + Size { + raw: 1, + }, + ], + memory_index: [ + 0, + 1, + ], + }, + variants: Single { + index: 0, + }, + abi: ScalarPair( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=255, + }, + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=2, + }, + ), + largest_niche: Some( + Niche { + offset: Size { + raw: 1, + }, + value: Int( + I8, + false, + ), + valid_range: 0..=2, + }, + ), + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 2, + }, + }, + Layout { + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + variants: Single { + index: 1, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 0, + }, + }, + Layout { + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + variants: Single { + index: 2, + }, + abi: Aggregate { + sized: true, + }, + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 0, + }, + }, + ], + }, + abi: ScalarPair( + Union { + value: Int( + I8, + false, + ), + }, + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=4, + }, + ), + largest_niche: Some( + Niche { + offset: Size { + raw: 1, + }, + value: Int( + I8, + false, + ), + valid_range: 0..=4, + }, + ), + align: AbiAndPrefAlign { + abi: Align { + pow2: 0, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 2, + }, + } + --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:49:1 + | +LL | / pub enum NicheSecond { +LL | | A(u8, HasNiche), +LL | | B, +LL | | C, +LL | | } + | |_^ + +error: aborting due to 5 previous errors + From 4dcc1aae0a9d2b9eeee3f6404b8b7873a80aa6de Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Fri, 22 Apr 2022 01:46:24 -0400 Subject: [PATCH 2/2] normalize out pref_align (copied from another test) --- ...6158-scalarpair-payload-might-be-uninit.rs | 1 + ...-scalarpair-payload-might-be-uninit.stderr | 78 ++++++------------- 2 files changed, 23 insertions(+), 56 deletions(-) diff --git a/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.rs b/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.rs index bc3667c5f809f..89387e01ba572 100644 --- a/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.rs +++ b/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.rs @@ -1,3 +1,4 @@ +// normalize-stderr-test "pref: Align \{\n *pow2: [1-3],\n *\}" -> "pref: $$PREF_ALIGN" #![crate_type = "lib"] #![feature(rustc_attrs)] diff --git a/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr b/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr index af51f813076ed..46187aae30445 100644 --- a/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr +++ b/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr @@ -42,9 +42,7 @@ error: layout_of(MissingPayloadField) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 2, @@ -66,9 +64,7 @@ error: layout_of(MissingPayloadField) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 1, @@ -107,15 +103,13 @@ error: layout_of(MissingPayloadField) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 2, }, } - --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:15:1 + --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:16:1 | LL | / pub enum MissingPayloadField { LL | | Some(u8), @@ -167,9 +161,7 @@ error: layout_of(CommonPayloadField) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 2, @@ -197,9 +189,7 @@ error: layout_of(CommonPayloadField) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 2, @@ -239,15 +229,13 @@ error: layout_of(CommonPayloadField) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 2, }, } - --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:24:1 + --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:25:1 | LL | / pub enum CommonPayloadField { LL | | A(u8), @@ -299,9 +287,7 @@ error: layout_of(CommonPayloadFieldIsMaybeUninit) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 2, @@ -329,9 +315,7 @@ error: layout_of(CommonPayloadFieldIsMaybeUninit) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 2, @@ -370,15 +354,13 @@ error: layout_of(CommonPayloadFieldIsMaybeUninit) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 2, }, } - --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:32:1 + --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:33:1 | LL | / pub enum CommonPayloadFieldIsMaybeUninit { LL | | A(u8), @@ -462,9 +444,7 @@ error: layout_of(NicheFirst) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 2, @@ -486,9 +466,7 @@ error: layout_of(NicheFirst) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 0, @@ -510,9 +488,7 @@ error: layout_of(NicheFirst) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 0, @@ -551,15 +527,13 @@ error: layout_of(NicheFirst) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 2, }, } - --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:40:1 + --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:41:1 | LL | / pub enum NicheFirst { LL | | A(HasNiche, u8), @@ -644,9 +618,7 @@ error: layout_of(NicheSecond) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 2, @@ -668,9 +640,7 @@ error: layout_of(NicheSecond) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 0, @@ -692,9 +662,7 @@ error: layout_of(NicheSecond) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 0, @@ -733,15 +701,13 @@ error: layout_of(NicheSecond) = Layout { abi: Align { pow2: 0, }, - pref: Align { - pow2: 3, - }, + pref: $PREF_ALIGN, }, size: Size { raw: 2, }, } - --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:49:1 + --> $DIR/issue-96158-scalarpair-payload-might-be-uninit.rs:50:1 | LL | / pub enum NicheSecond { LL | | A(u8, HasNiche),