diff --git a/compiler/rustc_codegen_ssa/src/glue.rs b/compiler/rustc_codegen_ssa/src/glue.rs index c34f1dbf8569d..0b82a5e343518 100644 --- a/compiler/rustc_codegen_ssa/src/glue.rs +++ b/compiler/rustc_codegen_ssa/src/glue.rs @@ -59,10 +59,13 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( debug!("DST {} layout: {:?}", t, layout); let i = layout.fields.count() - 1; - let sized_size = layout.fields.offset(i).bytes(); + let unsized_offset_unadjusted = layout.fields.offset(i).bytes(); let sized_align = layout.align.abi.bytes(); - debug!("DST {} statically sized prefix size: {} align: {}", t, sized_size, sized_align); - let sized_size = bx.const_usize(sized_size); + debug!( + "DST {} offset of dyn field: {}, statically sized align: {}", + t, unsized_offset_unadjusted, sized_align + ); + let unsized_offset_unadjusted = bx.const_usize(unsized_offset_unadjusted); let sized_align = bx.const_usize(sized_align); // Recurse to get the size of the dynamically sized field (must be @@ -70,26 +73,27 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( let field_ty = layout.field(bx, i).ty; let (unsized_size, mut unsized_align) = size_and_align_of_dst(bx, field_ty, info); - // FIXME (#26403, #27023): We should be adding padding - // to `sized_size` (to accommodate the `unsized_align` - // required of the unsized field that follows) before - // summing it with `sized_size`. (Note that since #26403 - // is unfixed, we do not yet add the necessary padding - // here. But this is where the add would go.) + // # First compute the dynamic alignment - // Return the sum of sizes and max of aligns. - let size = bx.add(sized_size, unsized_size); - - // Packed types ignore the alignment of their fields. + // Packed type alignment would have to be capped, but their tails always have known alignment. + // Therefore, their alignment has already been taken into account when computing `sized_align` + // and `unsized_offset_unadjusted`, so no further adjustment is needed. if let ty::Adt(def, _) = t.kind() { if def.repr().packed() { + let unsized_tail = + bx.tcx().struct_tail_with_normalize(field_ty, |ty| ty, || {}); + assert!(matches!(unsized_tail.kind(), ty::Slice(..) | ty::Str)); + // Therefore we do not need to adjust anything. + // It's not actually correct to say that the unsized field has this alignment, but the + // code below works correctly if we set this and it avoids having to compute + // the actual alignment (which is `unsized_align.min(packed)`). unsized_align = sized_align; } } // Choose max of two known alignments (combined value must // be aligned according to more restrictive of the two). - let align = match ( + let full_align = match ( bx.const_to_opt_u128(sized_align, false), bx.const_to_opt_u128(unsized_align, false), ) { @@ -104,6 +108,21 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( } }; + // # Then compute the dynamic size + + // The full formula for the size would be: + // let unsized_offset_adjusted = unsized_offset_unadjusted.align_to(unsized_align); + // let full_size = (unsized_offset_adjusted + unsized_size).align_to(full_align); + // However, `unsized_size` is a multiple of `unsized_align`. + // Therefore, we can equivalently do the `align_to(unsized_align)` *after* adding `unsized_size`: + // let full_size = (offset + unsized_size).align_to(unsized_align).align_to(full_align); + // Furthermore, `align >= unsized_align`, and therefore we only need to do: + // let full_size = (offset + unsized_size).align_to(full_align); + // This formula also has the advantage of working correctly when the type is packed + // and we set `unsized_align = sized_align`. + + let full_size = bx.add(unsized_offset_unadjusted, unsized_size); + // Issue #27023: must add any necessary padding to `size` // (to make it a multiple of `align`) before returning it. // @@ -115,12 +134,12 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( // // `(size + (align-1)) & -align` let one = bx.const_usize(1); - let addend = bx.sub(align, one); - let add = bx.add(size, addend); - let neg = bx.neg(align); - let size = bx.and(add, neg); + let addend = bx.sub(full_align, one); + let add = bx.add(full_size, addend); + let neg = bx.neg(full_align); + let full_size = bx.and(add, neg); - (size, align) + (full_size, full_align) } } } diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 04e5b550d6d9b..d79c9a8d5153c 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -684,14 +684,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { assert!(layout.fields.count() > 0); trace!("DST layout: {:?}", layout); - let sized_size = layout.fields.offset(layout.fields.count() - 1); + let unsized_offset_unadjusted = layout.fields.offset(layout.fields.count() - 1); let sized_align = layout.align.abi; - trace!( - "DST {} statically sized prefix size: {:?} align: {:?}", - layout.ty, - sized_size, - sized_align - ); // Recurse to get the size of the dynamically sized field (must be // the last field). Can't have foreign types here, how would we @@ -705,36 +699,35 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { return Ok(None); }; - // FIXME (#26403, #27023): We should be adding padding - // to `sized_size` (to accommodate the `unsized_align` - // required of the unsized field that follows) before - // summing it with `sized_size`. (Note that since #26403 - // is unfixed, we do not yet add the necessary padding - // here. But this is where the add would go.) - - // Return the sum of sizes and max of aligns. - let size = sized_size + unsized_size; // `Size` addition + // # First compute the dynamic alignment - // Packed types ignore the alignment of their fields. + // Packed type alignment needs to be capped. if let ty::Adt(def, _) = layout.ty.kind() { - if def.repr().packed() { - unsized_align = sized_align; + if let Some(packed) = def.repr().pack { + unsized_align = unsized_align.min(packed); } } // Choose max of two known alignments (combined value must // be aligned according to more restrictive of the two). - let align = sized_align.max(unsized_align); + let full_align = sized_align.max(unsized_align); + + // # Then compute the dynamic size - // Issue #27023: must add any necessary padding to `size` - // (to make it a multiple of `align`) before returning it. - let size = size.align_to(align); + let unsized_offset_adjusted = unsized_offset_unadjusted.align_to(unsized_align); + let full_size = (unsized_offset_adjusted + unsized_size).align_to(full_align); + + // Just for our sanitiy's sake, assert that this is equal to what codegen would compute. + assert_eq!( + full_size, + (unsized_offset_unadjusted + unsized_size).align_to(full_align) + ); // Check if this brought us over the size limit. - if size > self.max_size_of_val() { + if full_size > self.max_size_of_val() { throw_ub!(InvalidMeta(InvalidMetaKind::TooBig)); } - Ok(Some((size, align))) + Ok(Some((full_size, full_align))) } ty::Dynamic(_, _, ty::Dyn) => { let vtable = metadata.unwrap_meta().to_pointer(self)?;