show packed alignment in mir_transform_unaligned_packed_ref

Author: 21aslade

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 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,31 @@ 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>(
+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 +65,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, 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,8 @@ 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 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::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::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 at most 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 at most 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 at most 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 at most 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 at most 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 at most 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)