mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #115277 - RalfJung:is_1zst, r=davidtwco

Browse Source 
fix some issues around ZST handling

This fixes two bugs:
- We used to entirely skip enum variants like `B([u16; 0], !)`, even failing to properly align the enum!  Honoring the alignment of uninhabited variants is important for the same reason that we must reserve space for their fields -- see [here](https://github.com/rust-lang/rust/issues/49298#issuecomment-380615281) for why.
- ~~We uses to reject `repr(transparent)` on `struct MyType([u16; 0])`, which is weird because a one-field struct should always be allowed to be transparent around that field.~~ (moved to separate PR)

I also found two places in the layout code that did something special for ZST without explaining why, and removing those special cases doesn't seem to have any effect except for reordering some zero-sized fields which shouldn't be an issue... maybe PR CI will explain why those cases were needed, or maybe they became obsolete at some point.
This commit is contained in:
bors 2023-08-29 10:22:35 +00:00
commit 0b84f18b24
20 changed files with 130 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
compiler/rustc_abi/src/layout.rs
View File

@ -157,8 +157,10 @@ fn layout_of_struct_or_enum(
// for non-ZST uninhabited data (mostly partial initialization).
let absent = |fields: &IndexSlice<FieldIdx, Layout<'_>>| {
let uninhabited = fields.iter().any(|f| f.abi().is_uninhabited());
let is_zst = fields.iter().all(|f| f.0.is_zst());
uninhabited && is_zst
// We cannot ignore alignment; that might lead us to entirely discard a variant and
// produce an enum that is less aligned than it should be!
let is_1zst = fields.iter().all(|f| f.0.is_1zst());
uninhabited && is_1zst
};
let (present_first, present_second) = {
let mut present_variants = variants
@ -357,10 +359,8 @@ struct TmpLayout {
// It'll fit, but we need to make some adjustments.
match layout.fields {
FieldsShape::Arbitrary { ref mut offsets, .. } => {
for (j, offset) in offsets.iter_enumerated_mut() {
if !variants[i][j].0.is_zst() {
*offset += this_offset;
}
for offset in offsets.iter_mut() {
*offset += this_offset;
}
}
_ => {
@ -504,7 +504,7 @@ struct TmpLayout {
// to make room for a larger discriminant.
for field_idx in st.fields.index_by_increasing_offset() {
let field = &field_layouts[FieldIdx::from_usize(field_idx)];
if !field.0.is_zst() || field.align().abi.bytes() != 1 {
if !field.0.is_1zst() {
start_align = start_align.min(field.align().abi);
break;
}
@ -603,12 +603,15 @@ struct TmpLayout {
abi = Abi::Scalar(tag);
} else {
// Try to use a ScalarPair for all tagged enums.
// That's possible only if we can find a common primitive type for all variants.
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 {
panic!();
};
// We skip *all* ZST here and later check if we are good in terms of alignment.
// This lets us handle some cases involving aligned ZST.
let mut fields = iter::zip(field_layouts, offsets).filter(|p| !p.0.0.is_zst());
let (field, offset) = match (fields.next(), fields.next()) {
(None, None) => {
@ -954,9 +957,6 @@ fn univariant(
};
(
// Place ZSTs first to avoid "interesting offsets", especially with only one
// or two non-ZST fields. This helps Scalar/ScalarPair layouts.
!f.0.is_zst(),
// Then place largest alignments first.
cmp::Reverse(alignment_group_key(f)),
// Then prioritize niche placement within alignment group according to
@ -1073,9 +1073,10 @@ fn univariant(
let size = min_size.align_to(align.abi);
let mut layout_of_single_non_zst_field = None;
let mut abi = Abi::Aggregate { sized };
// Unpack newtype ABIs and find scalar pairs.
// Try to make this a Scalar/ScalarPair.
if sized && size.bytes() > 0 {
// All other fields must be ZSTs.
// We skip *all* ZST here and later check if we are good in terms of alignment.
// This lets us handle some cases involving aligned ZST.
let mut non_zst_fields = fields.iter_enumerated().filter(|&(_, f)| !f.0.is_zst());
match (non_zst_fields.next(), non_zst_fields.next(), non_zst_fields.next()) {

10
compiler/rustc_abi/src/lib.rs
View File

@ -1660,15 +1660,25 @@ pub struct PointeeInfo {
impl LayoutS {
/// Returns `true` if the layout corresponds to an unsized type.
#[inline]
pub fn is_unsized(&self) -> bool {
self.abi.is_unsized()
}
#[inline]
pub fn is_sized(&self) -> bool {
self.abi.is_sized()
}
/// Returns `true` if the type is sized and a 1-ZST (meaning it has size 0 and alignment 1).
pub fn is_1zst(&self) -> bool {
self.is_sized() && self.size.bytes() == 0 && self.align.abi.bytes() == 1
}
/// Returns `true` if the type is a ZST and not unsized.
///
/// Note that this does *not* imply that the type is irrelevant for layout! It can still have
/// non-trivial alignment constraints. You probably want to use `is_1zst` instead.
pub fn is_zst(&self) -> bool {
match self.abi {
Abi::Scalar(_) | Abi::ScalarPair(..) | Abi::Vector { .. } => false,

4
compiler/rustc_codegen_cranelift/src/unsize.rs
View File

@ -88,7 +88,8 @@ fn unsize_ptr<'tcx>(
let src_f = src_layout.field(fx, i);
assert_eq!(src_layout.fields.offset(i).bytes(), 0);
assert_eq!(dst_layout.fields.offset(i).bytes(), 0);
if src_f.is_zst() {
if src_f.is_1zst() {
// We are looking for the one non-1-ZST field; this is not it.
continue;
}
assert_eq!(src_layout.size, src_f.size);
@ -151,6 +152,7 @@ pub(crate) fn coerce_unsized_into<'tcx>(
let dst_f = dst.place_field(fx, FieldIdx::new(i));
if dst_f.layout().is_zst() {
// No data here, nothing to copy/coerce.
continue;
}

4
compiler/rustc_codegen_cranelift/src/vtable.rs
View File

@ -51,8 +51,8 @@ pub(crate) fn get_ptr_and_method_ref<'tcx>(
'descend_newtypes: while !arg.layout().ty.is_unsafe_ptr() && !arg.layout().ty.is_ref() {
for i in 0..arg.layout().fields.count() {
let field = arg.value_field(fx, FieldIdx::new(i));
if !field.layout().is_zst() {
// we found the one non-zero-sized field that is allowed
if !field.layout().is_1zst() {
// we found the one non-1-ZST field that is allowed
// now find *its* non-zero-sized field, or stop if it's a
// pointer
arg = field;

4
compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
View File

@ -445,9 +445,9 @@ pub fn type_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll D
ty::RawPtr(ty::TypeAndMut { ty: pointee_type, .. }) | ty::Ref(_, pointee_type, _) => {
build_pointer_or_reference_di_node(cx, t, pointee_type, unique_type_id)
}
// Box<T, A> may have a non-ZST allocator A. In that case, we
// Box<T, A> may have a non-1-ZST allocator A. In that case, we
// cannot treat Box<T, A> as just an owned alias of `*mut T`.
ty::Adt(def, args) if def.is_box() && cx.layout_of(args.type_at(1)).is_zst() => {
ty::Adt(def, args) if def.is_box() && cx.layout_of(args.type_at(1)).is_1zst() => {
build_pointer_or_reference_di_node(cx, t, t.boxed_ty(), unique_type_id)
}
ty::FnDef(..) | ty::FnPtr(_) => build_subroutine_type_di_node(cx, unique_type_id),

8
compiler/rustc_codegen_ssa/src/base.rs
View File

@ -202,7 +202,7 @@ pub fn unsize_ptr<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
(src, unsized_info(bx, a, b, old_info))
}
(&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => {
assert_eq!(def_a, def_b);
assert_eq!(def_a, def_b); // implies same number of fields
let src_layout = bx.cx().layout_of(src_ty);
let dst_layout = bx.cx().layout_of(dst_ty);
if src_ty == dst_ty {
@ -211,7 +211,8 @@ pub fn unsize_ptr<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
let mut result = None;
for i in 0..src_layout.fields.count() {
let src_f = src_layout.field(bx.cx(), i);
if src_f.is_zst() {
if src_f.is_1zst() {
// We are looking for the one non-1-ZST field; this is not it.
continue;
}
@ -272,13 +273,14 @@ pub fn coerce_unsized_into<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
}
(&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => {
assert_eq!(def_a, def_b);
assert_eq!(def_a, def_b); // implies same number of fields
for i in def_a.variant(FIRST_VARIANT).fields.indices() {
let src_f = src.project_field(bx, i.as_usize());
let dst_f = dst.project_field(bx, i.as_usize());
if dst_f.layout.is_zst() {
// No data here, nothing to copy/coerce.
continue;
}

13
compiler/rustc_codegen_ssa/src/mir/block.rs
View File

@ -933,8 +933,8 @@ fn codegen_call_terminator(
{
for i in 0..op.layout.fields.count() {
let field = op.extract_field(bx, i);
if !field.layout.is_zst() {
// we found the one non-zero-sized field that is allowed
if !field.layout.is_1zst() {
// we found the one non-1-ZST field that is allowed
// now find *its* non-zero-sized field, or stop if it's a
// pointer
op = field;
@ -975,10 +975,11 @@ fn codegen_call_terminator(
{
for i in 0..op.layout.fields.count() {
let field = op.extract_field(bx, i);
if !field.layout.is_zst() {
// we found the one non-zero-sized field that is allowed
// now find *its* non-zero-sized field, or stop if it's a
// pointer
if !field.layout.is_1zst() {
// We found the one non-1-ZST field that is allowed. (The rules
// for `DispatchFromDyn` ensure there's exactly one such field.)
// Now find *its* non-zero-sized field, or stop if it's a
// pointer.
op = field;
continue 'descend_newtypes;
}

2
compiler/rustc_codegen_ssa/src/mir/mod.rs
View File

@ -145,7 +145,7 @@ fn new_operand(layout: TyAndLayout<'tcx>) -> LocalRef<'tcx, V> {
if layout.is_zst() {
// Zero-size temporaries aren't always initialized, which
// doesn't matter because they don't contain data, but
// we need something in the operand.
// we need something sufficiently aligned in the operand.
LocalRef::Operand(OperandRef::zero_sized(layout))
} else {
LocalRef::PendingOperand

3
compiler/rustc_codegen_ssa/src/mir/operand.rs
View File

@ -50,7 +50,8 @@ pub enum OperandValue<V> {
/// from [`ConstMethods::const_poison`].
///
/// An `OperandValue` *must* be this variant for any type for which
/// `is_zst` on its `Layout` returns `true`.
/// `is_zst` on its `Layout` returns `true`. Note however that
/// these values can still require alignment.
ZeroSized,
}

3
compiler/rustc_codegen_ssa/src/mir/place.rs
View File

@ -114,7 +114,8 @@ pub fn project_field<Bx: BuilderMethods<'a, 'tcx, Value = V>>(
bx.struct_gep(ty, self.llval, 1)
}
Abi::Scalar(_) | Abi::ScalarPair(..) | Abi::Vector { .. } if field.is_zst() => {
// ZST fields are not included in Scalar, ScalarPair, and Vector layouts, so manually offset the pointer.
// ZST fields (even some that require alignment) are not included in Scalar,
// ScalarPair, and Vector layouts, so manually offset the pointer.
bx.gep(bx.cx().type_i8(), self.llval, &[bx.const_usize(offset.bytes())])
}
Abi::Scalar(_) | Abi::ScalarPair(..) => {

1
compiler/rustc_codegen_ssa/src/mir/rvalue.rs
View File

@ -1004,6 +1004,7 @@ pub fn rvalue_creates_operand(&self, rvalue: &mir::Rvalue<'tcx>, span: Span) ->
mir::Rvalue::Aggregate(..) => {
let ty = rvalue.ty(self.mir, self.cx.tcx());
let ty = self.monomorphize(ty);
// For ZST this can be `OperandValueKind::ZeroSized`.
self.cx.spanned_layout_of(ty, span).is_zst()
}
}

20
compiler/rustc_const_eval/src/interpret/cast.rs
View File

@ -410,21 +410,25 @@ fn unsize_into(
self.unsize_into_ptr(src, dest, *s, *c)
}
(&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => {
assert_eq!(def_a, def_b);
assert_eq!(def_a, def_b); // implies same number of fields
// unsizing of generic struct with pointer fields
// Example: `Arc<T>` -> `Arc<Trait>`
// here we need to increase the size of every &T thin ptr field to a fat ptr
// Unsizing of generic struct with pointer fields, like `Arc<T>` -> `Arc<Trait>`.
// There can be extra fields as long as they don't change their type or are 1-ZST.
// There might also be no field that actually needs unsizing.
let mut found_cast_field = false;
for i in 0..src.layout.fields.count() {
let cast_ty_field = cast_ty.field(self, i);
if cast_ty_field.is_zst() {
continue;
}
let src_field = self.project_field(src, i)?;
let dst_field = self.project_field(dest, i)?;
if src_field.layout.ty == cast_ty_field.ty {
if src_field.layout.is_1zst() && cast_ty_field.is_1zst() {
// Skip 1-ZST fields.
} else if src_field.layout.ty == cast_ty_field.ty {
self.copy_op(&src_field, &dst_field, /*allow_transmute*/ false)?;
} else {
if found_cast_field {
span_bug!(self.cur_span(), "unsize_into: more than one field to cast");
}
found_cast_field = true;
self.unsize_into(&src_field, cast_ty_field, &dst_field)?;
}
}

1
compiler/rustc_const_eval/src/interpret/operand.rs
View File

@ -239,6 +239,7 @@ fn offset_(&self, offset: Size, layout: TyAndLayout<'tcx>, cx: &impl HasDataLayo
// if the entire value is uninit, then so is the field (can happen in ConstProp)
(Immediate::Uninit, _) => Immediate::Uninit,
// the field contains no information, can be left uninit
// (Scalar/ScalarPair can contain even aligned ZST, not just 1-ZST)
_ if layout.is_zst() => Immediate::Uninit,
// some fieldless enum variants can have non-zero size but still `Aggregate` ABI... try
// to detect those here and also give them no data

8
compiler/rustc_const_eval/src/interpret/terminator.rs
View File

@ -684,15 +684,15 @@ pub(crate) fn eval_fn_call(
}
_ => {
// Not there yet, search for the only non-ZST field.
// (The rules for `DispatchFromDyn` ensure there's exactly one such field.)
let mut non_zst_field = None;
for i in 0..receiver.layout.fields.count() {
let field = self.project_field(&receiver, i)?;
let zst =
field.layout.is_zst() && field.layout.align.abi.bytes() == 1;
let zst = field.layout.is_1zst();
if !zst {
assert!(
non_zst_field.is_none(),
"multiple non-ZST fields in dyn receiver type {}",
"multiple non-1-ZST fields in dyn receiver type {}",
receiver.layout.ty
);
non_zst_field = Some(field);
@ -700,7 +700,7 @@ pub(crate) fn eval_fn_call(
}
receiver = non_zst_field.unwrap_or_else(|| {
panic!(
"no non-ZST fields in dyn receiver type {}",
"no non-1-ZST fields in dyn receiver type {}",
receiver.layout.ty
)
});

4
tests/ui/layout/debug.stderr
View File

@ -117,13 +117,13 @@ error: layout_of(S) = Layout {
fields: Arbitrary {
offsets: [
Size(0 bytes),
Size(0 bytes),
Size(8 bytes),
Size(4 bytes),
],
memory_index: [
1,
0,
2,
1,
],
},
largest_niche: None,

18
tests/ui/layout/enum.rs Normal file
View File

@ -0,0 +1,18 @@
// normalize-stderr-test "pref: Align\([1-8] bytes\)" -> "pref: $$PREF_ALIGN"
//! Various enum layout tests.
#![feature(rustc_attrs)]
#![feature(never_type)]
#![crate_type = "lib"]
#[rustc_layout(align)]
enum UninhabitedVariantAlign { //~ERROR: abi: Align(2 bytes)
A([u8; 32]),
B([u16; 0], !), // make sure alignment in uninhabited fields is respected
}
#[rustc_layout(size)]
enum UninhabitedVariantSpace { //~ERROR: size: Size(16 bytes)
A,
B([u8; 15], !), // make sure there is space being reserved for this field.
}

14
tests/ui/layout/enum.stderr Normal file
View File

@ -0,0 +1,14 @@
error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: $PREF_ALIGN }
--> $DIR/enum.rs:9:1
|
LL | enum UninhabitedVariantAlign {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: size: Size(16 bytes)
--> $DIR/enum.rs:15:1
|
LL | enum UninhabitedVariantSpace {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 2 previous errors

12
tests/ui/layout/struct.rs Normal file
View File

@ -0,0 +1,12 @@
// normalize-stderr-test "pref: Align\([1-8] bytes\)" -> "pref: $$PREF_ALIGN"
//! Various struct layout tests.
#![feature(rustc_attrs)]
#![feature(never_type)]
#![crate_type = "lib"]
#[rustc_layout(abi)]
struct AlignedZstPreventsScalar(i16, [i32; 0]); //~ERROR: abi: Aggregate
#[rustc_layout(abi)]
struct AlignedZstButStillScalar(i32, [i16; 0]); //~ERROR: abi: Scalar

14
tests/ui/layout/struct.stderr Normal file
View File

@ -0,0 +1,14 @@
error: abi: Aggregate { sized: true }
--> $DIR/struct.rs:9:1
|
LL | struct AlignedZstPreventsScalar(i16, [i32; 0]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: abi: Scalar(Initialized { value: Int(I32, true), valid_range: 0..=4294967295 })
--> $DIR/struct.rs:12:1
|
LL | struct AlignedZstButStillScalar(i32, [i16; 0]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 2 previous errors

10
tests/ui/print_type_sizes/zero-sized-fields.stdout
View File

@ -1,16 +1,16 @@
print-type-size type: `S5<(), Empty>`: 16 bytes, alignment: 4 bytes
print-type-size field `.w`: 4 bytes
print-type-size field `.x`: 4 bytes
print-type-size field `.y`: 4 bytes
print-type-size field `.z`: 4 bytes
print-type-size field `.tagw`: 0 bytes
print-type-size field `.unit`: 0 bytes
print-type-size field `.void`: 0 bytes
print-type-size field `.empty`: 0 bytes
print-type-size field `.tagz`: 0 bytes
print-type-size field `.w`: 4 bytes
print-type-size field `.x`: 4 bytes
print-type-size field `.y`: 4 bytes
print-type-size field `.z`: 4 bytes
print-type-size type: `S1`: 8 bytes, alignment: 4 bytes
print-type-size field `.tag`: 0 bytes
print-type-size field `.x`: 4 bytes
print-type-size field `.y`: 4 bytes
print-type-size field `.tag`: 0 bytes
print-type-size type: `Empty`: 0 bytes, alignment: 1 bytes
print-type-size type: `Void`: 0 bytes, alignment: 1 bytes