Auto merge of #113245 - lukas-code:unsizing-sanity-check, r=the8472
sanity check field offsets in unsizeable structs
As promised in https://github.com/rust-lang/rust/pull/112062#issuecomment-1567494994, this PR extends the layout sanity checks to ensure that structs fields don't move around when unsizing and prevent issues like https://github.com/rust-lang/rust/issues/112048 in the future. Like most other layout sanity checks, this only runs on compilers with debug assertions enabled.
Here is how it looks when it fails:
```text
error: internal compiler error: compiler/rustc_ty_utils/src/layout.rs:533:21: unsizing GcNode<std::boxed::Box<i32>> changed field order!
Layout { size: Size(32 bytes), align: AbiAndPrefAlign { abi: Align(8 bytes), pref: Align(8 bytes) }, abi: Aggregate { sized: true }, fields: Arbitrary { offsets: [Size(0 bytes), Size(8 bytes), Size(24 bytes)], memory_index: [0, 1, 2] }, largest_niche: Some(Niche { offset: Size(24 bytes), value: Pointer(AddressSpace(0)), valid_range: 1..=18446744073709551615 }), variants: Single { index: 0 } }
Layout { size: Size(24 bytes), align: AbiAndPrefAlign { abi: Align(8 bytes), pref: Align(8 bytes) }, abi: Aggregate { sized: false }, fields: Arbitrary { offsets: [Size(16 bytes), Size(0 bytes), Size(24 bytes)], memory_index: [1, 0, 2] }, largest_niche: None, variants: Single { index: 0 } }
```
r? `@the8472`
diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs
index 73f9deb..f6875d8 100644
--- a/compiler/rustc_abi/src/layout.rs
+++ b/compiler/rustc_abi/src/layout.rs
@@ -134,7 +134,7 @@
scalar_valid_range: (Bound<u128>, Bound<u128>),
discr_range_of_repr: impl Fn(i128, i128) -> (Integer, bool),
discriminants: impl Iterator<Item = (VariantIdx, i128)>,
- niche_optimize_enum: bool,
+ dont_niche_optimize_enum: bool,
always_sized: bool,
) -> Option<LayoutS> {
let dl = self.current_data_layout();
@@ -183,10 +183,10 @@
// (Typechecking will reject discriminant-sizing attrs.)
let v = present_first;
- let kind = if is_enum || variants[v].is_empty() {
+ let kind = if is_enum || variants[v].is_empty() || always_sized {
StructKind::AlwaysSized
} else {
- if !always_sized { StructKind::MaybeUnsized } else { StructKind::AlwaysSized }
+ StructKind::MaybeUnsized
};
let mut st = self.univariant(dl, &variants[v], repr, kind)?;
@@ -280,7 +280,7 @@
}
let calculate_niche_filling_layout = || -> Option<TmpLayout> {
- if niche_optimize_enum {
+ if dont_niche_optimize_enum {
return None;
}
diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs
index 7a625dd..d4748b7 100644
--- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs
+++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs
@@ -981,7 +981,7 @@
// intermediate types must be sized.
let needs_drop_copy = || {
packed && {
- let ty = tcx.type_of(variant.fields.raw.last().unwrap().did).subst_identity();
+ let ty = tcx.type_of(variant.tail().did).subst_identity();
let ty = tcx.erase_regions(ty);
if ty.has_infer() {
tcx.sess
diff --git a/compiler/rustc_hir_typeck/src/cast.rs b/compiler/rustc_hir_typeck/src/cast.rs
index 7179938..6339333 100644
--- a/compiler/rustc_hir_typeck/src/cast.rs
+++ b/compiler/rustc_hir_typeck/src/cast.rs
@@ -103,15 +103,13 @@
Ok(match *t.kind() {
ty::Slice(_) | ty::Str => Some(PointerKind::Length),
ty::Dynamic(ref tty, _, ty::Dyn) => Some(PointerKind::VTable(tty.principal_def_id())),
- ty::Adt(def, substs) if def.is_struct() => {
- match def.non_enum_variant().fields.raw.last() {
- None => Some(PointerKind::Thin),
- Some(f) => {
- let field_ty = self.field_ty(span, f, substs);
- self.pointer_kind(field_ty, span)?
- }
+ ty::Adt(def, substs) if def.is_struct() => match def.non_enum_variant().tail_opt() {
+ None => Some(PointerKind::Thin),
+ Some(f) => {
+ let field_ty = self.field_ty(span, f, substs);
+ self.pointer_kind(field_ty, span)?
}
- }
+ },
ty::Tuple(fields) => match fields.last() {
None => Some(PointerKind::Thin),
Some(&f) => self.pointer_kind(f, span)?,
diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs
index 938b901..aa8bfd3 100644
--- a/compiler/rustc_middle/src/ty/mod.rs
+++ b/compiler/rustc_middle/src/ty/mod.rs
@@ -2035,6 +2035,22 @@
&self.fields[FieldIdx::from_u32(0)]
}
+
+ /// Returns the last field in this variant, if present.
+ #[inline]
+ pub fn tail_opt(&self) -> Option<&FieldDef> {
+ self.fields.raw.last()
+ }
+
+ /// Returns the last field in this variant.
+ ///
+ /// # Panics
+ ///
+ /// Panics, if the variant has no fields.
+ #[inline]
+ pub fn tail(&self) -> &FieldDef {
+ self.tail_opt().expect("expected unsized ADT to have a tail field")
+ }
}
impl PartialEq for VariantDef {
diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs
index 720d770..e2e4a2d 100644
--- a/compiler/rustc_middle/src/ty/util.rs
+++ b/compiler/rustc_middle/src/ty/util.rs
@@ -230,7 +230,7 @@
if !def.is_struct() {
break;
}
- match def.non_enum_variant().fields.raw.last() {
+ match def.non_enum_variant().tail_opt() {
Some(field) => {
f();
ty = field.ty(self, substs);
@@ -304,7 +304,7 @@
(&ty::Adt(a_def, a_substs), &ty::Adt(b_def, b_substs))
if a_def == b_def && a_def.is_struct() =>
{
- if let Some(f) = a_def.non_enum_variant().fields.raw.last() {
+ if let Some(f) = a_def.non_enum_variant().tail_opt() {
a = f.ty(self, a_substs);
b = f.ty(self, b_substs);
} else {
diff --git a/compiler/rustc_trait_selection/src/solve/project_goals.rs b/compiler/rustc_trait_selection/src/solve/project_goals.rs
index aa69071..e53b784 100644
--- a/compiler/rustc_trait_selection/src/solve/project_goals.rs
+++ b/compiler/rustc_trait_selection/src/solve/project_goals.rs
@@ -366,7 +366,7 @@
}
ty::Adt(def, substs) if def.is_struct() => {
- match def.non_enum_variant().fields.raw.last() {
+ match def.non_enum_variant().tail_opt() {
None => tcx.types.unit,
Some(field_def) => {
let self_ty = field_def.ty(tcx, substs);
diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals.rs b/compiler/rustc_trait_selection/src/solve/trait_goals.rs
index cd68626..ef5f25b 100644
--- a/compiler/rustc_trait_selection/src/solve/trait_goals.rs
+++ b/compiler/rustc_trait_selection/src/solve/trait_goals.rs
@@ -425,12 +425,7 @@
return Err(NoSolution);
}
- let tail_field = a_def
- .non_enum_variant()
- .fields
- .raw
- .last()
- .expect("expected unsized ADT to have a tail field");
+ let tail_field = a_def.non_enum_variant().tail();
let tail_field_ty = tcx.type_of(tail_field.did);
let a_tail_ty = tail_field_ty.subst(tcx, a_substs);
diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs
index 9deacd7..7adc29b 100644
--- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs
+++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs
@@ -1125,12 +1125,7 @@
return Err(Unimplemented);
}
- let tail_field = def
- .non_enum_variant()
- .fields
- .raw
- .last()
- .expect("expected unsized ADT to have a tail field");
+ let tail_field = def.non_enum_variant().tail();
let tail_field_ty = tcx.type_of(tail_field.did);
// Extract `TailField<T>` and `TailField<U>` from `Struct<T>` and `Struct<U>`,
diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs
index 9ef9120..b67cd96 100644
--- a/compiler/rustc_ty_utils/src/layout.rs
+++ b/compiler/rustc_ty_utils/src/layout.rs
@@ -463,38 +463,85 @@
));
}
- tcx.mk_layout(
- cx.layout_of_struct_or_enum(
+ let get_discriminant_type =
+ |min, max| Integer::repr_discr(tcx, ty, &def.repr(), min, max);
+
+ let discriminants_iter = || {
+ def.is_enum()
+ .then(|| def.discriminants(tcx).map(|(v, d)| (v, d.val as i128)))
+ .into_iter()
+ .flatten()
+ };
+
+ let dont_niche_optimize_enum = def.repr().inhibit_enum_layout_opt()
+ || def
+ .variants()
+ .iter_enumerated()
+ .any(|(i, v)| v.discr != ty::VariantDiscr::Relative(i.as_u32()));
+
+ let maybe_unsized = def.is_struct()
+ && def.non_enum_variant().tail_opt().is_some_and(|last_field| {
+ let param_env = tcx.param_env(def.did());
+ !tcx.type_of(last_field.did).subst_identity().is_sized(tcx, param_env)
+ });
+
+ let Some(layout) = cx.layout_of_struct_or_enum(
+ &def.repr(),
+ &variants,
+ def.is_enum(),
+ def.is_unsafe_cell(),
+ tcx.layout_scalar_valid_range(def.did()),
+ get_discriminant_type,
+ discriminants_iter(),
+ dont_niche_optimize_enum,
+ !maybe_unsized,
+ ) else {
+ return Err(error(cx, LayoutError::SizeOverflow(ty)));
+ };
+
+ // If the struct tail is sized and can be unsized, check that unsizing doesn't move the fields around.
+ if cfg!(debug_assertions)
+ && maybe_unsized
+ && def.non_enum_variant().tail().ty(tcx, substs).is_sized(tcx, cx.param_env)
+ {
+ let mut variants = variants;
+ let tail_replacement = cx.layout_of(Ty::new_slice(tcx, tcx.types.u8)).unwrap();
+ *variants[FIRST_VARIANT].raw.last_mut().unwrap() = tail_replacement.layout;
+
+ let Some(unsized_layout) = cx.layout_of_struct_or_enum(
&def.repr(),
&variants,
def.is_enum(),
def.is_unsafe_cell(),
tcx.layout_scalar_valid_range(def.did()),
- |min, max| Integer::repr_discr(tcx, ty, &def.repr(), min, max),
- def.is_enum()
- .then(|| def.discriminants(tcx).map(|(v, d)| (v, d.val as i128)))
- .into_iter()
- .flatten(),
- def.repr().inhibit_enum_layout_opt()
- || def
- .variants()
- .iter_enumerated()
- .any(|(i, v)| v.discr != ty::VariantDiscr::Relative(i.as_u32())),
- {
- let param_env = tcx.param_env(def.did());
- def.is_struct()
- && match def.variants().iter().next().and_then(|x| x.fields.raw.last())
- {
- Some(last_field) => tcx
- .type_of(last_field.did)
- .subst_identity()
- .is_sized(tcx, param_env),
- None => false,
- }
- },
- )
- .ok_or_else(|| error(cx, LayoutError::SizeOverflow(ty)))?,
- )
+ get_discriminant_type,
+ discriminants_iter(),
+ dont_niche_optimize_enum,
+ !maybe_unsized,
+ ) else {
+ bug!("failed to compute unsized layout of {ty:?}");
+ };
+
+ let FieldsShape::Arbitrary { offsets: sized_offsets, .. } = &layout.fields else {
+ bug!("unexpected FieldsShape for sized layout of {ty:?}: {:?}", layout.fields);
+ };
+ let FieldsShape::Arbitrary { offsets: unsized_offsets, .. } = &unsized_layout.fields else {
+ bug!("unexpected FieldsShape for unsized layout of {ty:?}: {:?}", unsized_layout.fields);
+ };
+
+ let (sized_tail, sized_fields) = sized_offsets.raw.split_last().unwrap();
+ let (unsized_tail, unsized_fields) = unsized_offsets.raw.split_last().unwrap();
+
+ if sized_fields != unsized_fields {
+ bug!("unsizing {ty:?} changed field order!\n{layout:?}\n{unsized_layout:?}");
+ }
+
+ if sized_tail < unsized_tail {
+ bug!("unsizing {ty:?} moved tail backwards!\n{layout:?}\n{unsized_layout:?}");
+ }
+ }
+
+ tcx.mk_layout(layout)
}
// Types with no meaningful known layout.
diff --git a/compiler/rustc_ty_utils/src/ty.rs b/compiler/rustc_ty_utils/src/ty.rs
index 466616c..6e5c504 100644
--- a/compiler/rustc_ty_utils/src/ty.rs
+++ b/compiler/rustc_ty_utils/src/ty.rs
@@ -103,7 +103,7 @@
let result = tcx.mk_type_list_from_iter(
def.variants()
.iter()
- .filter_map(|v| v.fields.raw.last())
+ .filter_map(|v| v.tail_opt())
.flat_map(|f| sized_constraint_for_ty(tcx, def, tcx.type_of(f.did).subst_identity())),
);
