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`
This commit is contained in:
bors 2023-07-07 15:42:29 +00:00
commit cb80ff132a
10 changed files with 107 additions and 56 deletions

View file

@ -134,7 +134,7 @@ pub trait LayoutCalculator {
scalar_valid_range: (Bound<u128>, Bound<u128>), scalar_valid_range: (Bound<u128>, Bound<u128>),
discr_range_of_repr: impl Fn(i128, i128) -> (Integer, bool), discr_range_of_repr: impl Fn(i128, i128) -> (Integer, bool),
discriminants: impl Iterator<Item = (VariantIdx, i128)>, discriminants: impl Iterator<Item = (VariantIdx, i128)>,
niche_optimize_enum: bool, dont_niche_optimize_enum: bool,
always_sized: bool, always_sized: bool,
) -> Option<LayoutS> { ) -> Option<LayoutS> {
let dl = self.current_data_layout(); let dl = self.current_data_layout();
@ -183,10 +183,10 @@ pub trait LayoutCalculator {
// (Typechecking will reject discriminant-sizing attrs.) // (Typechecking will reject discriminant-sizing attrs.)
let v = present_first; 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 StructKind::AlwaysSized
} else { } else {
if !always_sized { StructKind::MaybeUnsized } else { StructKind::AlwaysSized } StructKind::MaybeUnsized
}; };
let mut st = self.univariant(dl, &variants[v], repr, kind)?; let mut st = self.univariant(dl, &variants[v], repr, kind)?;
@ -280,7 +280,7 @@ pub trait LayoutCalculator {
} }
let calculate_niche_filling_layout = || -> Option<TmpLayout> { let calculate_niche_filling_layout = || -> Option<TmpLayout> {
if niche_optimize_enum { if dont_niche_optimize_enum {
return None; return None;
} }

View file

@ -981,7 +981,7 @@ fn check_type_defn<'tcx>(tcx: TyCtxt<'tcx>, item: &hir::Item<'tcx>, all_sized: b
// intermediate types must be sized. // intermediate types must be sized.
let needs_drop_copy = || { let needs_drop_copy = || {
packed && { 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); let ty = tcx.erase_regions(ty);
if ty.has_infer() { if ty.has_infer() {
tcx.sess tcx.sess

View file

@ -103,15 +103,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Ok(match *t.kind() { Ok(match *t.kind() {
ty::Slice(_) | ty::Str => Some(PointerKind::Length), ty::Slice(_) | ty::Str => Some(PointerKind::Length),
ty::Dynamic(ref tty, _, ty::Dyn) => Some(PointerKind::VTable(tty.principal_def_id())), ty::Dynamic(ref tty, _, ty::Dyn) => Some(PointerKind::VTable(tty.principal_def_id())),
ty::Adt(def, substs) if def.is_struct() => { ty::Adt(def, substs) if def.is_struct() => match def.non_enum_variant().tail_opt() {
match def.non_enum_variant().fields.raw.last() {
None => Some(PointerKind::Thin), None => Some(PointerKind::Thin),
Some(f) => { Some(f) => {
let field_ty = self.field_ty(span, f, substs); let field_ty = self.field_ty(span, f, substs);
self.pointer_kind(field_ty, span)? self.pointer_kind(field_ty, span)?
} }
} },
}
ty::Tuple(fields) => match fields.last() { ty::Tuple(fields) => match fields.last() {
None => Some(PointerKind::Thin), None => Some(PointerKind::Thin),
Some(&f) => self.pointer_kind(f, span)?, Some(&f) => self.pointer_kind(f, span)?,

View file

@ -2035,6 +2035,22 @@ impl VariantDef {
&self.fields[FieldIdx::from_u32(0)] &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 { impl PartialEq for VariantDef {

View file

@ -230,7 +230,7 @@ impl<'tcx> TyCtxt<'tcx> {
if !def.is_struct() { if !def.is_struct() {
break; break;
} }
match def.non_enum_variant().fields.raw.last() { match def.non_enum_variant().tail_opt() {
Some(field) => { Some(field) => {
f(); f();
ty = field.ty(self, substs); ty = field.ty(self, substs);
@ -304,7 +304,7 @@ impl<'tcx> TyCtxt<'tcx> {
(&ty::Adt(a_def, a_substs), &ty::Adt(b_def, b_substs)) (&ty::Adt(a_def, a_substs), &ty::Adt(b_def, b_substs))
if a_def == b_def && a_def.is_struct() => 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); a = f.ty(self, a_substs);
b = f.ty(self, b_substs); b = f.ty(self, b_substs);
} else { } else {

View file

@ -366,7 +366,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
} }
ty::Adt(def, substs) if def.is_struct() => { 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, None => tcx.types.unit,
Some(field_def) => { Some(field_def) => {
let self_ty = field_def.ty(tcx, substs); let self_ty = field_def.ty(tcx, substs);

View file

@ -425,12 +425,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
return Err(NoSolution); return Err(NoSolution);
} }
let tail_field = a_def let tail_field = a_def.non_enum_variant().tail();
.non_enum_variant()
.fields
.raw
.last()
.expect("expected unsized ADT to have a tail field");
let tail_field_ty = tcx.type_of(tail_field.did); let tail_field_ty = tcx.type_of(tail_field.did);
let a_tail_ty = tail_field_ty.subst(tcx, a_substs); let a_tail_ty = tail_field_ty.subst(tcx, a_substs);

View file

@ -1125,12 +1125,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
return Err(Unimplemented); return Err(Unimplemented);
} }
let tail_field = def let tail_field = def.non_enum_variant().tail();
.non_enum_variant()
.fields
.raw
.last()
.expect("expected unsized ADT to have a tail field");
let tail_field_ty = tcx.type_of(tail_field.did); let tail_field_ty = tcx.type_of(tail_field.did);
// Extract `TailField<T>` and `TailField<U>` from `Struct<T>` and `Struct<U>`, // Extract `TailField<T>` and `TailField<U>` from `Struct<T>` and `Struct<U>`,

View file

@ -463,38 +463,85 @@ fn layout_of_uncached<'tcx>(
)); ));
} }
tcx.mk_layout( let get_discriminant_type =
cx.layout_of_struct_or_enum( |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(), &def.repr(),
&variants, &variants,
def.is_enum(), def.is_enum(),
def.is_unsafe_cell(), def.is_unsafe_cell(),
tcx.layout_scalar_valid_range(def.did()), tcx.layout_scalar_valid_range(def.did()),
|min, max| Integer::repr_discr(tcx, ty, &def.repr(), min, max), get_discriminant_type,
def.is_enum() discriminants_iter(),
.then(|| def.discriminants(tcx).map(|(v, d)| (v, d.val as i128))) dont_niche_optimize_enum,
.into_iter() !maybe_unsized,
.flatten(), ) else {
def.repr().inhibit_enum_layout_opt() return Err(error(cx, LayoutError::SizeOverflow(ty)));
|| def };
.variants()
.iter_enumerated() // If the struct tail is sized and can be unsized, check that unsizing doesn't move the fields around.
.any(|(i, v)| v.discr != ty::VariantDiscr::Relative(i.as_u32())), if cfg!(debug_assertions)
&& maybe_unsized
&& def.non_enum_variant().tail().ty(tcx, substs).is_sized(tcx, cx.param_env)
{ {
let param_env = tcx.param_env(def.did()); let mut variants = variants;
def.is_struct() let tail_replacement = cx.layout_of(Ty::new_slice(tcx, tcx.types.u8)).unwrap();
&& match def.variants().iter().next().and_then(|x| x.fields.raw.last()) *variants[FIRST_VARIANT].raw.last_mut().unwrap() = tail_replacement.layout;
{
Some(last_field) => tcx let Some(unsized_layout) = cx.layout_of_struct_or_enum(
.type_of(last_field.did) &def.repr(),
.subst_identity() &variants,
.is_sized(tcx, param_env), def.is_enum(),
None => false, def.is_unsafe_cell(),
tcx.layout_scalar_valid_range(def.did()),
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 {
.ok_or_else(|| error(cx, LayoutError::SizeOverflow(ty)))?, bug!("unsizing {ty:?} moved tail backwards!\n{layout:?}\n{unsized_layout:?}");
) }
}
tcx.mk_layout(layout)
} }
// Types with no meaningful known layout. // Types with no meaningful known layout.

View file

@ -103,7 +103,7 @@ fn adt_sized_constraint(tcx: TyCtxt<'_>, def_id: DefId) -> &[Ty<'_>] {
let result = tcx.mk_type_list_from_iter( let result = tcx.mk_type_list_from_iter(
def.variants() def.variants()
.iter() .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())), .flat_map(|f| sized_constraint_for_ty(tcx, def, tcx.type_of(f.did).subst_identity())),
); );