Unrolled build for #147743

Rollup merge of #147743 - 21aslade:packed-diagnostic, r=RalfJung

Show packed field alignment in mir_transform_unaligned_packed_ref

Fixes #147528

I left the expected padding for the field out of the error message so the message would be the same on all platforms. It also isn't always possible to know the expected alignment, so this makes the message simpler.

Author: rust-timer

compiler/rustc_const_eval/src/util/alignment.rs

@@ -1,24 +1,24 @@
 use rustc_abi::Align;
 use rustc_middle::mir::*;
-use rustc_middle::ty::{self, TyCtxt};
+use rustc_middle::ty::{self, AdtDef, TyCtxt};
 use tracing::debug;
 
-/// Returns `true` if this place is allowed to be less aligned
-/// than its containing struct (because it is within a packed
-/// struct).
-pub fn is_disaligned<'tcx, L>(
+/// If the place may be less aligned than its type requires
+/// (because it is in a packed type), returns the AdtDef
+/// and packed alignment of its most-unaligned projection.
+pub fn place_unalignment<'tcx, L>(
     tcx: TyCtxt<'tcx>,
     local_decls: &L,
     typing_env: ty::TypingEnv<'tcx>,
     place: Place<'tcx>,
-) -> bool
+) -> Option<(AdtDef<'tcx>, Align)>
 where
     L: HasLocalDecls<'tcx>,
 {
-    debug!("is_disaligned({:?})", place);
-    let Some(pack) = is_within_packed(tcx, local_decls, place) else {
-        debug!("is_disaligned({:?}) - not within packed", place);
-        return false;
+    debug!("unalignment({:?})", place);
+    let Some((descr, pack)) = most_packed_projection(tcx, local_decls, place) else {
+        debug!("unalignment({:?}) - not within packed", place);
+        return None;
     };
 
     let ty = place.ty(local_decls, tcx).ty;
@@ -30,31 +30,34 @@ where
                     || matches!(unsized_tail().kind(), ty::Slice(..) | ty::Str)) =>
         {
             // If the packed alignment is greater or equal to the field alignment, the type won't be
-            // further disaligned.
+            // further unaligned.
             // However we need to ensure the field is sized; for unsized fields, `layout.align` is
             // just an approximation -- except when the unsized tail is a slice, where the alignment
             // is fully determined by the type.
             debug!(
-                "is_disaligned({:?}) - align = {}, packed = {}; not disaligned",
+                "unalignment({:?}) - align = {}, packed = {}; not unaligned",
                 place,
                 layout.align.bytes(),
                 pack.bytes()
             );
-            false
+            None
         }
         _ => {
-            // We cannot figure out the layout. Conservatively assume that this is disaligned.
-            debug!("is_disaligned({:?}) - true", place);
-            true
+            // We cannot figure out the layout. Conservatively assume that this is unaligned.
+            debug!("unalignment({:?}) - unaligned", place);
+            Some((descr, pack))
         }
     }
 }
 
-pub fn is_within_packed<'tcx, L>(
+/// If the place includes a projection from a packed struct,
+/// returns the AdtDef and packed alignment of the projection
+/// with the lowest pack
+pub fn most_packed_projection<'tcx, L>(
     tcx: TyCtxt<'tcx>,
     local_decls: &L,
     place: Place<'tcx>,
-) -> Option<Align>
+) -> Option<(AdtDef<'tcx>, Align)>
 where
     L: HasLocalDecls<'tcx>,
 {
@@ -65,9 +68,11 @@ where
         .take_while(|(_base, elem)| !matches!(elem, ProjectionElem::Deref))
         // Consider the packed alignments at play here...
         .filter_map(|(base, _elem)| {
-            base.ty(local_decls, tcx).ty.ty_adt_def().and_then(|adt| adt.repr().pack)
+            let adt = base.ty(local_decls, tcx).ty.ty_adt_def()?;
+            let pack = adt.repr().pack?;
+            Some((adt, pack))
         })
         // ... and compute their minimum.
         // The overall smallest alignment is what matters.
-        .min()
+        .min_by_key(|(_, align)| *align)
 }

compiler/rustc_const_eval/src/util/mod.rs

@@ -6,7 +6,7 @@ mod check_validity_requirement;
 mod compare_types;
 mod type_name;
 
-pub use self::alignment::{is_disaligned, is_within_packed};
+pub use self::alignment::{most_packed_projection, place_unalignment};
 pub use self::check_validity_requirement::check_validity_requirement;
 pub(crate) use self::check_validity_requirement::validate_scalar_in_layout;
 pub use self::compare_types::{relate_types, sub_types};

compiler/rustc_mir_transform/messages.ftl

@@ -69,8 +69,11 @@ mir_transform_tail_expr_local = {$is_generated_name ->
         *[false] `{$name}` calls a custom destructor
     }
 
-mir_transform_unaligned_packed_ref = reference to packed field is unaligned
-    .note = packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
+mir_transform_unaligned_packed_ref = reference to field of packed {$ty_descr} is unaligned
+    .note = this {$ty_descr} is {$align ->
+        [one] {""}
+        *[other] {"at most "}
+    }{$align}-byte aligned, but the type of this field may require higher alignment
     .note_ub = creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
     .help = copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
 

compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs

@@ -51,7 +51,7 @@ impl<'tcx> crate::MirPass<'tcx> for AddMovesForPackedDrops {
 
             match terminator.kind {
                 TerminatorKind::Drop { place, .. }
-                    if util::is_disaligned(tcx, body, typing_env, place) =>
+                    if util::place_unalignment(tcx, body, typing_env, place).is_some() =>
                 {
                     add_move_for_packed_drop(
                         tcx,

compiler/rustc_mir_transform/src/check_packed_ref.rs

@@ -37,7 +37,9 @@ impl<'tcx> Visitor<'tcx> for PackedRefChecker<'_, 'tcx> {
     }
 
     fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _location: Location) {
-        if context.is_borrow() && util::is_disaligned(self.tcx, self.body, self.typing_env, *place)
+        if context.is_borrow()
+            && let Some((adt, pack)) =
+                util::place_unalignment(self.tcx, self.body, self.typing_env, *place)
         {
             let def_id = self.body.source.instance.def_id();
             if let Some(impl_def_id) = self.tcx.trait_impl_of_assoc(def_id)
@@ -48,7 +50,11 @@ impl<'tcx> Visitor<'tcx> for PackedRefChecker<'_, 'tcx> {
                 // shouldn't do.
                 span_bug!(self.source_info.span, "builtin derive created an unaligned reference");
             } else {
-                self.tcx.dcx().emit_err(errors::UnalignedPackedRef { span: self.source_info.span });
+                self.tcx.dcx().emit_err(errors::UnalignedPackedRef {
+                    span: self.source_info.span,
+                    ty_descr: adt.descr(),
+                    align: pack.bytes(),
+                });
             }
         }
     }

compiler/rustc_mir_transform/src/dead_store_elimination.rs

@@ -23,7 +23,7 @@ use rustc_mir_dataflow::impls::{
 };
 
 use crate::simplify::UsedInStmtLocals;
-use crate::util::is_within_packed;
+use crate::util::most_packed_projection;
 
 /// Performs the optimization on the body
 ///
@@ -65,7 +65,7 @@ fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {
                     // the move may be codegened as a pointer to that field.
                     // Using that disaligned pointer may trigger UB in the callee,
                     // so do nothing.
-                    && is_within_packed(tcx, body, place).is_none()
+                    && most_packed_projection(tcx, body, place).is_none()
                 {
                     call_operands_to_move.push((bb, index));
                 }

compiler/rustc_mir_transform/src/errors.rs

@@ -99,6 +99,8 @@ pub(crate) enum ConstMutate {
 pub(crate) struct UnalignedPackedRef {
     #[primary_span]
     pub span: Span,
+    pub ty_descr: &'static str,
+    pub align: u64,
 }
 
 #[derive(Diagnostic)]

compiler/rustc_mir_transform/src/validate.rs

@@ -20,7 +20,7 @@ use rustc_middle::{bug, span_bug};
 use rustc_mir_dataflow::debuginfo::debuginfo_locals;
 use rustc_trait_selection::traits::ObligationCtxt;
 
-use crate::util::{self, is_within_packed};
+use crate::util::{self, most_packed_projection};
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 enum EdgeKind {
@@ -409,7 +409,9 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
 
                     // The call destination place and Operand::Move place used as an argument might
                     // be passed by a reference to the callee. Consequently they cannot be packed.
-                    if is_within_packed(self.tcx, &self.body.local_decls, destination).is_some() {
+                    if most_packed_projection(self.tcx, &self.body.local_decls, destination)
+                        .is_some()
+                    {
                         // This is bad! The callee will expect the memory to be aligned.
                         self.fail(
                             location,
@@ -423,7 +425,9 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
 
                 for arg in args {
                     if let Operand::Move(place) = &arg.node {
-                        if is_within_packed(self.tcx, &self.body.local_decls, *place).is_some() {
+                        if most_packed_projection(self.tcx, &self.body.local_decls, *place)
+                            .is_some()
+                        {
                             // This is bad! The callee will expect the memory to be aligned.
                             self.fail(
                                 location,

tests/ui/binding/issue-53114-safety-checks.rs

@@ -20,12 +20,12 @@ fn let_wild_gets_unsafe_field() {
     let u1 = U { a: I(0) };
     let u2 = U { a: I(1) };
     let p = P { a: &2, b: &3 };
-    let _ = &p.b;  //~ ERROR    reference to packed field
+    let _ = &p.b;  //~ ERROR    reference to field of packed struct
     let _ = u1.a;  //~ ERROR  [E0133]
     let _ = &u2.a; //~ ERROR  [E0133]
 
     // variation on above with `_` in substructure
-    let (_,) = (&p.b,);  //~ ERROR     reference to packed field
+    let (_,) = (&p.b,);  //~ ERROR     reference to field of packed struct
     let (_,) = (u1.a,);  //~ ERROR   [E0133]
     let (_,) = (&u2.a,); //~ ERROR   [E0133]
 }
@@ -34,12 +34,12 @@ fn let_ascribe_gets_unsafe_field() {
     let u1 = U { a: I(0) };
     let u2 = U { a: I(1) };
     let p = P { a: &2, b: &3 };
-    let _: _ = &p.b;  //~ ERROR    reference to packed field
+    let _: _ = &p.b;  //~ ERROR    reference to field of packed struct
     let _: _ = u1.a;  //~ ERROR  [E0133]
     let _: _ = &u2.a; //~ ERROR  [E0133]
 
     // variation on above with `_` in substructure
-    let (_,): _ = (&p.b,);  //~ ERROR     reference to packed field
+    let (_,): _ = (&p.b,);  //~ ERROR     reference to field of packed struct
     let (_,): _ = (u1.a,);  //~ ERROR   [E0133]
     let (_,): _ = (&u2.a,); //~ ERROR   [E0133]
 }
@@ -48,12 +48,12 @@ fn match_unsafe_field_to_wild() {
     let u1 = U { a: I(0) };
     let u2 = U { a: I(1) };
     let p = P { a: &2, b: &3 };
-    match &p.b  { _ => { } } //~ ERROR     reference to packed field
+    match &p.b  { _ => { } } //~ ERROR     reference to field of packed struct
     match u1.a  { _ => { } } //~ ERROR   [E0133]
     match &u2.a { _ => { } } //~ ERROR   [E0133]
 
     // variation on above with `_` in substructure
-    match (&p.b,)  { (_,) => { } } //~ ERROR     reference to packed field
+    match (&p.b,)  { (_,) => { } } //~ ERROR     reference to field of packed struct
     match (u1.a,)  { (_,) => { } } //~ ERROR   [E0133]
     match (&u2.a,) { (_,) => { } } //~ ERROR   [E0133]
 }

tests/ui/binding/issue-53114-safety-checks.stderr

@@ -1,20 +1,20 @@
-error[E0793]: reference to packed field is unaligned
+error[E0793]: reference to field of packed struct is unaligned
   --> $DIR/issue-53114-safety-checks.rs:23:13
    |
 LL |     let _ = &p.b;
    |             ^^^^
    |
-   = note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
+   = note: this struct is 1-byte aligned, but the type of this field may require higher alignment
    = note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
    = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
 
-error[E0793]: reference to packed field is unaligned
+error[E0793]: reference to field of packed struct is unaligned
   --> $DIR/issue-53114-safety-checks.rs:28:17
    |
 LL |     let (_,) = (&p.b,);
    |                 ^^^^
    |
-   = note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
+   = note: this struct is 1-byte aligned, but the type of this field may require higher alignment
    = note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
    = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
 
@@ -50,23 +50,23 @@ LL |     let (_,) = (&u2.a,);
    |
    = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
 
-error[E0793]: reference to packed field is unaligned
+error[E0793]: reference to field of packed struct is unaligned
   --> $DIR/issue-53114-safety-checks.rs:37:16
    |
 LL |     let _: _ = &p.b;
    |                ^^^^
    |
-   = note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
+   = note: this struct is 1-byte aligned, but the type of this field may require higher alignment
    = note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
    = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
 
-error[E0793]: reference to packed field is unaligned
+error[E0793]: reference to field of packed struct is unaligned
   --> $DIR/issue-53114-safety-checks.rs:42:20
    |
 LL |     let (_,): _ = (&p.b,);
    |                    ^^^^
    |
-   = note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
+   = note: this struct is 1-byte aligned, but the type of this field may require higher alignment
    = note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
    = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
 
@@ -102,23 +102,23 @@ LL |     let (_,): _ = (&u2.a,);
    |
    = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
 
-error[E0793]: reference to packed field is unaligned
+error[E0793]: reference to field of packed struct is unaligned
   --> $DIR/issue-53114-safety-checks.rs:51:11
    |
 LL |     match &p.b  { _ => { } }
    |           ^^^^
    |
-   = note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
+   = note: this struct is 1-byte aligned, but the type of this field may require higher alignment
    = note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
    = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
 
-error[E0793]: reference to packed field is unaligned
+error[E0793]: reference to field of packed struct is unaligned
   --> $DIR/issue-53114-safety-checks.rs:56:12
    |
 LL |     match (&p.b,)  { (_,) => { } }
    |            ^^^^
    |
-   = note: packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
+   = note: this struct is 1-byte aligned, but the type of this field may require higher alignment
    = note: creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
    = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)