Skip to content

Commit cb80ff1

Browse files
committed
Auto merge of rust-lang#113245 - lukas-code:unsizing-sanity-check, r=the8472
sanity check field offsets in unsizeable structs As promised in rust-lang#112062 (comment), this PR extends the layout sanity checks to ensure that structs fields don't move around when unsizing and prevent issues like rust-lang#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`
2 parents fd68a6d + 7aa5f39 commit cb80ff1

File tree

10 files changed

+107
-56
lines changed

10 files changed

+107
-56
lines changed

compiler/rustc_abi/src/layout.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ pub trait LayoutCalculator {
134134
scalar_valid_range: (Bound<u128>, Bound<u128>),
135135
discr_range_of_repr: impl Fn(i128, i128) -> (Integer, bool),
136136
discriminants: impl Iterator<Item = (VariantIdx, i128)>,
137-
niche_optimize_enum: bool,
137+
dont_niche_optimize_enum: bool,
138138
always_sized: bool,
139139
) -> Option<LayoutS> {
140140
let dl = self.current_data_layout();
@@ -183,10 +183,10 @@ pub trait LayoutCalculator {
183183
// (Typechecking will reject discriminant-sizing attrs.)
184184

185185
let v = present_first;
186-
let kind = if is_enum || variants[v].is_empty() {
186+
let kind = if is_enum || variants[v].is_empty() || always_sized {
187187
StructKind::AlwaysSized
188188
} else {
189-
if !always_sized { StructKind::MaybeUnsized } else { StructKind::AlwaysSized }
189+
StructKind::MaybeUnsized
190190
};
191191

192192
let mut st = self.univariant(dl, &variants[v], repr, kind)?;
@@ -280,7 +280,7 @@ pub trait LayoutCalculator {
280280
}
281281

282282
let calculate_niche_filling_layout = || -> Option<TmpLayout> {
283-
if niche_optimize_enum {
283+
if dont_niche_optimize_enum {
284284
return None;
285285
}
286286

compiler/rustc_hir_analysis/src/check/wfcheck.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -981,7 +981,7 @@ fn check_type_defn<'tcx>(tcx: TyCtxt<'tcx>, item: &hir::Item<'tcx>, all_sized: b
981981
// intermediate types must be sized.
982982
let needs_drop_copy = || {
983983
packed && {
984-
let ty = tcx.type_of(variant.fields.raw.last().unwrap().did).subst_identity();
984+
let ty = tcx.type_of(variant.tail().did).subst_identity();
985985
let ty = tcx.erase_regions(ty);
986986
if ty.has_infer() {
987987
tcx.sess

compiler/rustc_hir_typeck/src/cast.rs

+6-8
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
103103
Ok(match *t.kind() {
104104
ty::Slice(_) | ty::Str => Some(PointerKind::Length),
105105
ty::Dynamic(ref tty, _, ty::Dyn) => Some(PointerKind::VTable(tty.principal_def_id())),
106-
ty::Adt(def, substs) if def.is_struct() => {
107-
match def.non_enum_variant().fields.raw.last() {
108-
None => Some(PointerKind::Thin),
109-
Some(f) => {
110-
let field_ty = self.field_ty(span, f, substs);
111-
self.pointer_kind(field_ty, span)?
112-
}
106+
ty::Adt(def, substs) if def.is_struct() => match def.non_enum_variant().tail_opt() {
107+
None => Some(PointerKind::Thin),
108+
Some(f) => {
109+
let field_ty = self.field_ty(span, f, substs);
110+
self.pointer_kind(field_ty, span)?
113111
}
114-
}
112+
},
115113
ty::Tuple(fields) => match fields.last() {
116114
None => Some(PointerKind::Thin),
117115
Some(&f) => self.pointer_kind(f, span)?,

compiler/rustc_middle/src/ty/mod.rs

+16
Original file line numberDiff line numberDiff line change
@@ -2035,6 +2035,22 @@ impl VariantDef {
20352035

20362036
&self.fields[FieldIdx::from_u32(0)]
20372037
}
2038+
2039+
/// Returns the last field in this variant, if present.
2040+
#[inline]
2041+
pub fn tail_opt(&self) -> Option<&FieldDef> {
2042+
self.fields.raw.last()
2043+
}
2044+
2045+
/// Returns the last field in this variant.
2046+
///
2047+
/// # Panics
2048+
///
2049+
/// Panics, if the variant has no fields.
2050+
#[inline]
2051+
pub fn tail(&self) -> &FieldDef {
2052+
self.tail_opt().expect("expected unsized ADT to have a tail field")
2053+
}
20382054
}
20392055

20402056
impl PartialEq for VariantDef {

compiler/rustc_middle/src/ty/util.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ impl<'tcx> TyCtxt<'tcx> {
230230
if !def.is_struct() {
231231
break;
232232
}
233-
match def.non_enum_variant().fields.raw.last() {
233+
match def.non_enum_variant().tail_opt() {
234234
Some(field) => {
235235
f();
236236
ty = field.ty(self, substs);
@@ -304,7 +304,7 @@ impl<'tcx> TyCtxt<'tcx> {
304304
(&ty::Adt(a_def, a_substs), &ty::Adt(b_def, b_substs))
305305
if a_def == b_def && a_def.is_struct() =>
306306
{
307-
if let Some(f) = a_def.non_enum_variant().fields.raw.last() {
307+
if let Some(f) = a_def.non_enum_variant().tail_opt() {
308308
a = f.ty(self, a_substs);
309309
b = f.ty(self, b_substs);
310310
} else {

compiler/rustc_trait_selection/src/solve/project_goals.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
366366
}
367367

368368
ty::Adt(def, substs) if def.is_struct() => {
369-
match def.non_enum_variant().fields.raw.last() {
369+
match def.non_enum_variant().tail_opt() {
370370
None => tcx.types.unit,
371371
Some(field_def) => {
372372
let self_ty = field_def.ty(tcx, substs);

compiler/rustc_trait_selection/src/solve/trait_goals.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -425,12 +425,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
425425
return Err(NoSolution);
426426
}
427427

428-
let tail_field = a_def
429-
.non_enum_variant()
430-
.fields
431-
.raw
432-
.last()
433-
.expect("expected unsized ADT to have a tail field");
428+
let tail_field = a_def.non_enum_variant().tail();
434429
let tail_field_ty = tcx.type_of(tail_field.did);
435430

436431
let a_tail_ty = tail_field_ty.subst(tcx, a_substs);

compiler/rustc_trait_selection/src/traits/select/confirmation.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -1125,12 +1125,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
11251125
return Err(Unimplemented);
11261126
}
11271127

1128-
let tail_field = def
1129-
.non_enum_variant()
1130-
.fields
1131-
.raw
1132-
.last()
1133-
.expect("expected unsized ADT to have a tail field");
1128+
let tail_field = def.non_enum_variant().tail();
11341129
let tail_field_ty = tcx.type_of(tail_field.did);
11351130

11361131
// Extract `TailField<T>` and `TailField<U>` from `Struct<T>` and `Struct<U>`,

compiler/rustc_ty_utils/src/layout.rs

+74-27
Original file line numberDiff line numberDiff line change
@@ -463,38 +463,85 @@ fn layout_of_uncached<'tcx>(
463463
));
464464
}
465465

466-
tcx.mk_layout(
467-
cx.layout_of_struct_or_enum(
466+
let get_discriminant_type =
467+
|min, max| Integer::repr_discr(tcx, ty, &def.repr(), min, max);
468+
469+
let discriminants_iter = || {
470+
def.is_enum()
471+
.then(|| def.discriminants(tcx).map(|(v, d)| (v, d.val as i128)))
472+
.into_iter()
473+
.flatten()
474+
};
475+
476+
let dont_niche_optimize_enum = def.repr().inhibit_enum_layout_opt()
477+
|| def
478+
.variants()
479+
.iter_enumerated()
480+
.any(|(i, v)| v.discr != ty::VariantDiscr::Relative(i.as_u32()));
481+
482+
let maybe_unsized = def.is_struct()
483+
&& def.non_enum_variant().tail_opt().is_some_and(|last_field| {
484+
let param_env = tcx.param_env(def.did());
485+
!tcx.type_of(last_field.did).subst_identity().is_sized(tcx, param_env)
486+
});
487+
488+
let Some(layout) = cx.layout_of_struct_or_enum(
489+
&def.repr(),
490+
&variants,
491+
def.is_enum(),
492+
def.is_unsafe_cell(),
493+
tcx.layout_scalar_valid_range(def.did()),
494+
get_discriminant_type,
495+
discriminants_iter(),
496+
dont_niche_optimize_enum,
497+
!maybe_unsized,
498+
) else {
499+
return Err(error(cx, LayoutError::SizeOverflow(ty)));
500+
};
501+
502+
// If the struct tail is sized and can be unsized, check that unsizing doesn't move the fields around.
503+
if cfg!(debug_assertions)
504+
&& maybe_unsized
505+
&& def.non_enum_variant().tail().ty(tcx, substs).is_sized(tcx, cx.param_env)
506+
{
507+
let mut variants = variants;
508+
let tail_replacement = cx.layout_of(Ty::new_slice(tcx, tcx.types.u8)).unwrap();
509+
*variants[FIRST_VARIANT].raw.last_mut().unwrap() = tail_replacement.layout;
510+
511+
let Some(unsized_layout) = cx.layout_of_struct_or_enum(
468512
&def.repr(),
469513
&variants,
470514
def.is_enum(),
471515
def.is_unsafe_cell(),
472516
tcx.layout_scalar_valid_range(def.did()),
473-
|min, max| Integer::repr_discr(tcx, ty, &def.repr(), min, max),
474-
def.is_enum()
475-
.then(|| def.discriminants(tcx).map(|(v, d)| (v, d.val as i128)))
476-
.into_iter()
477-
.flatten(),
478-
def.repr().inhibit_enum_layout_opt()
479-
|| def
480-
.variants()
481-
.iter_enumerated()
482-
.any(|(i, v)| v.discr != ty::VariantDiscr::Relative(i.as_u32())),
483-
{
484-
let param_env = tcx.param_env(def.did());
485-
def.is_struct()
486-
&& match def.variants().iter().next().and_then(|x| x.fields.raw.last())
487-
{
488-
Some(last_field) => tcx
489-
.type_of(last_field.did)
490-
.subst_identity()
491-
.is_sized(tcx, param_env),
492-
None => false,
493-
}
494-
},
495-
)
496-
.ok_or_else(|| error(cx, LayoutError::SizeOverflow(ty)))?,
497-
)
517+
get_discriminant_type,
518+
discriminants_iter(),
519+
dont_niche_optimize_enum,
520+
!maybe_unsized,
521+
) else {
522+
bug!("failed to compute unsized layout of {ty:?}");
523+
};
524+
525+
let FieldsShape::Arbitrary { offsets: sized_offsets, .. } = &layout.fields else {
526+
bug!("unexpected FieldsShape for sized layout of {ty:?}: {:?}", layout.fields);
527+
};
528+
let FieldsShape::Arbitrary { offsets: unsized_offsets, .. } = &unsized_layout.fields else {
529+
bug!("unexpected FieldsShape for unsized layout of {ty:?}: {:?}", unsized_layout.fields);
530+
};
531+
532+
let (sized_tail, sized_fields) = sized_offsets.raw.split_last().unwrap();
533+
let (unsized_tail, unsized_fields) = unsized_offsets.raw.split_last().unwrap();
534+
535+
if sized_fields != unsized_fields {
536+
bug!("unsizing {ty:?} changed field order!\n{layout:?}\n{unsized_layout:?}");
537+
}
538+
539+
if sized_tail < unsized_tail {
540+
bug!("unsizing {ty:?} moved tail backwards!\n{layout:?}\n{unsized_layout:?}");
541+
}
542+
}
543+
544+
tcx.mk_layout(layout)
498545
}
499546

500547
// Types with no meaningful known layout.

compiler/rustc_ty_utils/src/ty.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ fn adt_sized_constraint(tcx: TyCtxt<'_>, def_id: DefId) -> &[Ty<'_>] {
103103
let result = tcx.mk_type_list_from_iter(
104104
def.variants()
105105
.iter()
106-
.filter_map(|v| v.fields.raw.last())
106+
.filter_map(|v| v.tail_opt())
107107
.flat_map(|f| sized_constraint_for_ty(tcx, def, tcx.type_of(f.did).subst_identity())),
108108
);
109109

0 commit comments

Comments
 (0)