Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sanity check field offsets in unsizeable structs #113245

Merged
merged 3 commits into from
Jul 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions compiler/rustc_abi/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ pub trait LayoutCalculator {
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();
Expand Down Expand Up @@ -183,10 +183,10 @@ pub trait LayoutCalculator {
// (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)?;
Expand Down Expand Up @@ -280,7 +280,7 @@ pub trait LayoutCalculator {
}

let calculate_niche_filling_layout = || -> Option<TmpLayout> {
if niche_optimize_enum {
if dont_niche_optimize_enum {
return None;
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -981,7 +981,7 @@ fn check_type_defn<'tcx>(tcx: TyCtxt<'tcx>, item: &hir::Item<'tcx>, all_sized: b
// 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
Expand Down
14 changes: 6 additions & 8 deletions compiler/rustc_hir_typeck/src/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
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)?,
Expand Down
16 changes: 16 additions & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2028,6 +2028,22 @@ impl VariantDef {

&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 {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ impl<'tcx> TyCtxt<'tcx> {
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);
Expand Down Expand Up @@ -304,7 +304,7 @@ impl<'tcx> TyCtxt<'tcx> {
(&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 {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/solve/project_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
}

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);
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_trait_selection/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,12 +398,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1125,12 +1125,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
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>`,
Expand Down
101 changes: 74 additions & 27 deletions compiler/rustc_ty_utils/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,38 +463,85 @@ fn layout_of_uncached<'tcx>(
));
}

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.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ty_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ fn adt_sized_constraint(tcx: TyCtxt<'_>, def_id: DefId) -> &[Ty<'_>] {
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())),
);

Expand Down
Loading