Auto merge of #83605 - RalfJung:unaligned, r=petrochenkov

unaligned_references: align(N) fields in packed(N) structs are fine This removes some false positives from the unaligned_references lint: in a `repr(packed(2))` struct, fields of alignment 2 (and less) are guaranteed to be properly aligned, so we do not have to consider them "disaligned".
rust-lang · Mar 29, 2021 · cc41030 · cc41030
2 parents cb0e0db + 1ab05c1
commit cc41030
Show file tree

Hide file tree

Showing 7 changed files with 87 additions and 34 deletions.
diff --git a/compiler/rustc_mir/src/util/alignment.rs b/compiler/rustc_mir/src/util/alignment.rs
@@ -1,5 +1,6 @@
 use rustc_middle::mir::*;
 use rustc_middle::ty::{self, TyCtxt};
+use rustc_target::abi::Align;
 
 /// Returns `true` if this place is allowed to be less aligned
 /// than its containing struct (because it is within a packed
@@ -14,17 +15,25 @@ where
     L: HasLocalDecls<'tcx>,
 {
     debug!("is_disaligned({:?})", place);
-    if !is_within_packed(tcx, local_decls, place) {
-        debug!("is_disaligned({:?}) - not within packed", place);
-        return false;
-    }
+    let pack = match is_within_packed(tcx, local_decls, place) {
+        None => {
+            debug!("is_disaligned({:?}) - not within packed", place);
+            return false;
+        }
+        Some(pack) => pack,
+    };
 
     let ty = place.ty(local_decls, tcx).ty;
     match tcx.layout_raw(param_env.and(ty)) {
-        Ok(layout) if layout.align.abi.bytes() == 1 => {
-            // if the alignment is 1, the type can't be further
-            // disaligned.
-            debug!("is_disaligned({:?}) - align = 1", place);
+        Ok(layout) if layout.align.abi <= pack => {
+            // If the packed alignment is greater or equal to the field alignment, the type won't be
+            // further disaligned.
+            debug!(
+                "is_disaligned({:?}) - align = {}, packed = {}; not disaligned",
+                place,
+                layout.align.abi.bytes(),
+                pack.bytes()
+            );
             false
         }
         _ => {
@@ -34,7 +43,11 @@ where
     }
 }
 
-fn is_within_packed<'tcx, L>(tcx: TyCtxt<'tcx>, local_decls: &L, place: Place<'tcx>) -> bool
+fn is_within_packed<'tcx, L>(
+    tcx: TyCtxt<'tcx>,
+    local_decls: &L,
+    place: Place<'tcx>,
+) -> Option<Align>
 where
     L: HasLocalDecls<'tcx>,
 {
@@ -45,13 +58,13 @@ where
             ProjectionElem::Field(..) => {
                 let ty = place_base.ty(local_decls, tcx).ty;
                 match ty.kind() {
-                    ty::Adt(def, _) if def.repr.packed() => return true,
+                    ty::Adt(def, _) => return def.repr.pack,
                     _ => {}
                 }
             }
             _ => {}
         }
     }
 
-    false
+    None
 }
diff --git a/src/test/ui/lint/unaligned_references.rs b/src/test/ui/lint/unaligned_references.rs
@@ -8,6 +8,13 @@ pub struct Good {
     aligned: [u8; 32],
 }
 
+#[repr(packed(2))]
+pub struct Packed2 {
+    x: u32,
+    y: u16,
+    z: u8,
+}
+
 fn main() {
     unsafe {
         let good = Good { data: 0, ptr: &0, data2: [0, 0], aligned: [0; 32] };
@@ -32,4 +39,12 @@ fn main() {
         let _ = &good.aligned; // ok, has align 1
         let _ = &good.aligned[2]; // ok, has align 1
     }
+
+    unsafe {
+        let packed2 = Packed2 { x: 0, y: 0, z: 0 };
+        let _ = &packed2.x; //~ ERROR reference to packed field
+        //~^ previously accepted
+        let _ = &packed2.y; // ok, has align 2 in packed(2) struct
+        let _ = &packed2.z; // ok, has align 1
+    }
 }
diff --git a/src/test/ui/lint/unaligned_references.stderr b/src/test/ui/lint/unaligned_references.stderr
@@ -1,5 +1,5 @@
 error: reference to packed field is unaligned
-  --> $DIR/unaligned_references.rs:15:17
+  --> $DIR/unaligned_references.rs:22:17
    |
 LL |         let _ = &good.ptr;
    |                 ^^^^^^^^^
@@ -14,7 +14,7 @@ LL | #![deny(unaligned_references)]
    = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
 
 error: reference to packed field is unaligned
-  --> $DIR/unaligned_references.rs:17:17
+  --> $DIR/unaligned_references.rs:24:17
    |
 LL |         let _ = &good.data;
    |                 ^^^^^^^^^^
@@ -24,7 +24,7 @@ LL |         let _ = &good.data;
    = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
 
 error: reference to packed field is unaligned
-  --> $DIR/unaligned_references.rs:20:17
+  --> $DIR/unaligned_references.rs:27:17
    |
 LL |         let _ = &good.data as *const _;
    |                 ^^^^^^^^^^
@@ -34,7 +34,7 @@ LL |         let _ = &good.data as *const _;
    = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
 
 error: reference to packed field is unaligned
-  --> $DIR/unaligned_references.rs:22:27
+  --> $DIR/unaligned_references.rs:29:27
    |
 LL |         let _: *const _ = &good.data;
    |                           ^^^^^^^^^^
@@ -44,7 +44,7 @@ LL |         let _: *const _ = &good.data;
    = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
 
 error: reference to packed field is unaligned
-  --> $DIR/unaligned_references.rs:25:17
+  --> $DIR/unaligned_references.rs:32:17
    |
 LL |         let _ = good.data.clone();
    |                 ^^^^^^^^^
@@ -54,7 +54,7 @@ LL |         let _ = good.data.clone();
    = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
 
 error: reference to packed field is unaligned
-  --> $DIR/unaligned_references.rs:28:17
+  --> $DIR/unaligned_references.rs:35:17
    |
 LL |         let _ = &good.data2[0];
    |                 ^^^^^^^^^^^^^^
@@ -63,5 +63,15 @@ LL |         let _ = &good.data2[0];
    = note: for more information, see issue #82523 <https://github.com/rust-lang/rust/issues/82523>
    = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
 
-error: aborting due to 6 previous errors
+error: reference to packed field is unaligned
+  --> $DIR/unaligned_references.rs:45:17
+   |
+LL |         let _ = &packed2.x;
+   |                 ^^^^^^^^^^
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #82523 <https://github.com/rust-lang/rust/issues/82523>
+   = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
+
+error: aborting due to 7 previous errors
 
diff --git a/src/test/ui/packed/packed-struct-borrow-element-64bit.rs b/src/test/ui/packed/packed-struct-borrow-element-64bit.rs
@@ -0,0 +1,17 @@
+// run-pass (note: this is spec-UB, but it works for now)
+// ignore-32bit (needs `usize` to be 8-aligned to reproduce all the errors below)
+#![allow(dead_code)]
+// ignore-emscripten weird assertion?
+
+#[repr(C, packed(4))]
+struct Foo4C {
+    bar: u8,
+    baz: usize
+}
+
+pub fn main() {
+    let foo = Foo4C { bar: 1, baz: 2 };
+    let brw = &foo.baz; //~WARN reference to packed field is unaligned
+    //~^ previously accepted
+    assert_eq!(*brw, 2);
+}
diff --git a/src/test/ui/packed/packed-struct-borrow-element-64bit.stderr b/src/test/ui/packed/packed-struct-borrow-element-64bit.stderr
@@ -0,0 +1,13 @@
+warning: reference to packed field is unaligned
+  --> $DIR/packed-struct-borrow-element-64bit.rs:14:15
+   |
+LL |     let brw = &foo.baz;
+   |               ^^^^^^^^
+   |
+   = note: `#[warn(unaligned_references)]` on by default
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #82523 <https://github.com/rust-lang/rust/issues/82523>
+   = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
+
+warning: 1 warning emitted
+
diff --git a/src/test/ui/packed/packed-struct-borrow-element.rs b/src/test/ui/packed/packed-struct-borrow-element.rs
@@ -30,9 +30,4 @@ pub fn main() {
     let brw = &foo.baz; //~WARN reference to packed field is unaligned
     //~^ previously accepted
     assert_eq!(*brw, 2);
-
-    let foo = Foo4C { bar: 1, baz: 2 };
-    let brw = &foo.baz; //~WARN reference to packed field is unaligned
-    //~^ previously accepted
-    assert_eq!(*brw, 2);
 }
diff --git a/src/test/ui/packed/packed-struct-borrow-element.stderr b/src/test/ui/packed/packed-struct-borrow-element.stderr
@@ -19,15 +19,5 @@ LL |     let brw = &foo.baz;
    = note: for more information, see issue #82523 <https://github.com/rust-lang/rust/issues/82523>
    = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
 
-warning: reference to packed field is unaligned
-  --> $DIR/packed-struct-borrow-element.rs:35:15
-   |
-LL |     let brw = &foo.baz;
-   |               ^^^^^^^^
-   |
-   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-   = note: for more information, see issue #82523 <https://github.com/rust-lang/rust/issues/82523>
-   = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
-
-warning: 3 warnings emitted
+warning: 2 warnings emitted