Rollup merge of #72270 - RalfJung:lint-ref-to-packed, r=oli-obk

add a lint against references to packed fields

Creating a reference to an insufficiently aligned packed field is UB and should be disallowed, both inside and outside of `unsafe` blocks. However, currently there is no stable alternative (#64490) so all we do right now is have a future incompatibility warning when doing this outside `unsafe` (#46043).

This adds an allow-by-default lint. @retep998 suggested this can help early adopters avoid issues. It also means we can then do a crater run where this is deny-by-default as suggested by @joshtriplett.

I guess the main thing to bikeshed is the lint name. I am not particularly happy with "packed_references" as it sounds like the packed field has reference type. I chose this because it is similar to "safe_packed_borrows". What about "reference_to_packed" or "unaligned_reference" or so?

Author: Dylan-DPC

src/librustc_mir/transform/check_packed_ref.rs

@@ -0,0 +1,66 @@
+use rustc_middle::mir::visit::{PlaceContext, Visitor};
+use rustc_middle::mir::*;
+use rustc_middle::ty::{self, TyCtxt};
+use rustc_session::lint::builtin::UNALIGNED_REFERENCES;
+
+use crate::transform::{MirPass, MirSource};
+use crate::util;
+
+pub struct CheckPackedRef;
+
+impl<'tcx> MirPass<'tcx> for CheckPackedRef {
+    fn run_pass(&self, tcx: TyCtxt<'tcx>, src: MirSource<'tcx>, body: &mut Body<'tcx>) {
+        let param_env = tcx.param_env(src.instance.def_id());
+        let source_info = SourceInfo::outermost(body.span);
+        let mut checker = PackedRefChecker { body, tcx, param_env, source_info };
+        checker.visit_body(&body);
+    }
+}
+
+struct PackedRefChecker<'a, 'tcx> {
+    body: &'a Body<'tcx>,
+    tcx: TyCtxt<'tcx>,
+    param_env: ty::ParamEnv<'tcx>,
+    source_info: SourceInfo,
+}
+
+impl<'a, 'tcx> Visitor<'tcx> for PackedRefChecker<'a, 'tcx> {
+    fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
+        // Make sure we know where in the MIR we are.
+        self.source_info = terminator.source_info;
+        self.super_terminator(terminator, location);
+    }
+
+    fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
+        // Make sure we know where in the MIR we are.
+        self.source_info = statement.source_info;
+        self.super_statement(statement, location);
+    }
+
+    fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _location: Location) {
+        if context.is_borrow() {
+            if util::is_disaligned(self.tcx, self.body, self.param_env, *place) {
+                let source_info = self.source_info;
+                let lint_root = self.body.source_scopes[source_info.scope]
+                    .local_data
+                    .as_ref()
+                    .assert_crate_local()
+                    .lint_root;
+                self.tcx.struct_span_lint_hir(
+                    UNALIGNED_REFERENCES,
+                    lint_root,
+                    source_info.span,
+                    |lint| {
+                        lint.build(&format!("reference to packed field is unaligned",))
+                            .note(
+                                "fields of packed structs are not properly aligned, and creating \
+                                a misaligned reference is undefined behavior (even if that \
+                                reference is never dereferenced)",
+                            )
+                            .emit()
+                    },
+                );
+            }
+        }
+    }
+}

src/librustc_mir/transform/mod.rs

@@ -17,6 +17,7 @@ pub mod add_call_guards;
 pub mod add_moves_for_packed_drops;
 pub mod add_retag;
 pub mod check_consts;
+pub mod check_packed_ref;
 pub mod check_unsafety;
 pub mod cleanup_post_borrowck;
 pub mod const_prop;
@@ -228,10 +229,11 @@ fn mir_const_qualif(tcx: TyCtxt<'_>, def_id: DefId) -> ConstQualifs {
     validator.qualifs_in_return_place()
 }
 
+/// Make MIR ready for const evaluation. This is run on all MIR, not just on consts!
 fn mir_const(tcx: TyCtxt<'_>, def_id: DefId) -> Steal<Body<'_>> {
     let def_id = def_id.expect_local();
 
-    // Unsafety check uses the raw mir, so make sure it is run
+    // Unsafety check uses the raw mir, so make sure it is run.
     let _ = tcx.unsafety_check_result(def_id);
 
     let mut body = tcx.mir_built(def_id).steal();
@@ -247,6 +249,8 @@ fn mir_const(tcx: TyCtxt<'_>, def_id: DefId) -> Steal<Body<'_>> {
         None,
         MirPhase::Const,
         &[&[
+            // MIR-level lints.
+            &check_packed_ref::CheckPackedRef,
             // What we need to do constant evaluation.
             &simplify::SimplifyCfg::new("initial"),
             &rustc_peek::SanityCheck,

src/librustc_session/lint/builtin.rs

@@ -216,10 +216,16 @@ declare_lint! {
     "lints that have been renamed or removed"
 }
 
+declare_lint! {
+    pub UNALIGNED_REFERENCES,
+    Allow,
+    "detects unaligned references to fields of packed structs",
+}
+
 declare_lint! {
     pub SAFE_PACKED_BORROWS,
     Warn,
-    "safe borrows of fields of packed structs were was erroneously allowed",
+    "safe borrows of fields of packed structs were erroneously allowed",
     @future_incompatible = FutureIncompatibleInfo {
         reference: "issue #46043 <https://github.com/rust-lang/rust/issues/46043>",
         edition: None,
@@ -545,6 +551,7 @@ declare_lint_pass! {
         INVALID_TYPE_PARAM_DEFAULT,
         CONST_ERR,
         RENAMED_AND_REMOVED_LINTS,
+        UNALIGNED_REFERENCES,
         SAFE_PACKED_BORROWS,
         PATTERNS_IN_FNS_WITHOUT_BODY,
         MISSING_FRAGMENT_SPECIFIER,

src/test/ui/issues/issue-27060-rpass.rs

@@ -7,19 +7,10 @@ pub struct Good {
     aligned: [u8; 32],
 }
 
-#[repr(packed)]
-pub struct JustArray {
-    array: [u32]
-}
-
 // kill this test when that turns to a hard error
 #[allow(safe_packed_borrows)]
 fn main() {
-    let good = Good {
-        data: &0,
-        data2: [&0, &0],
-        aligned: [0; 32]
-    };
+    let good = Good { data: &0, data2: [&0, &0], aligned: [0; 32] };
 
     unsafe {
         let _ = &good.data; // ok

src/test/ui/issues/issue-27060.rs

@@ -5,11 +5,6 @@ pub struct Good {
     aligned: [u8; 32],
 }
 
-#[repr(packed)]
-pub struct JustArray {
-    array: [u32]
-}
-
 #[deny(safe_packed_borrows)]
 fn main() {
     let good = Good {

src/test/ui/issues/issue-27060.stderr

@@ -1,11 +1,11 @@
 error: borrow of packed field is unsafe and requires unsafe function or block (error E0133)
-  --> $DIR/issue-27060.rs:26:13
+  --> $DIR/issue-27060.rs:21:13
    |
 LL |     let _ = &good.data;
    |             ^^^^^^^^^^
    |
 note: the lint level is defined here
-  --> $DIR/issue-27060.rs:13:8
+  --> $DIR/issue-27060.rs:8:8
    |
 LL | #[deny(safe_packed_borrows)]
    |        ^^^^^^^^^^^^^^^^^^^
@@ -14,7 +14,7 @@ LL | #[deny(safe_packed_borrows)]
    = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior
 
 error: borrow of packed field is unsafe and requires unsafe function or block (error E0133)
-  --> $DIR/issue-27060.rs:28:13
+  --> $DIR/issue-27060.rs:23:13
    |
 LL |     let _ = &good.data2[0];
    |             ^^^^^^^^^^^^^^

src/test/ui/lint/unaligned_references.rs

@@ -0,0 +1,22 @@
+#![deny(unaligned_references)]
+
+#[repr(packed)]
+pub struct Good {
+    data: &'static u32,
+    data2: [&'static u32; 2],
+    aligned: [u8; 32],
+}
+
+fn main() {
+    unsafe {
+        let good = Good { data: &0, data2: [&0, &0], aligned: [0; 32] };
+
+        let _ = &good.data; //~ ERROR reference to packed field
+        let _ = &good.data as *const _; //~ ERROR reference to packed field
+        let _: *const _ = &good.data; //~ ERROR reference to packed field
+        let _ = &good.data2[0]; //~ ERROR reference to packed field
+        let _ = &*good.data; // ok, behind a pointer
+        let _ = &good.aligned; // ok, has align 1
+        let _ = &good.aligned[2]; // ok, has align 1
+    }
+}

src/test/ui/lint/unaligned_references.stderr

@@ -0,0 +1,39 @@
+error: reference to packed field is unaligned
+  --> $DIR/unaligned_references.rs:14:17
+   |
+LL |         let _ = &good.data;
+   |                 ^^^^^^^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/unaligned_references.rs:1:9
+   |
+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:15:17
+   |
+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:16:27
+   |
+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:17:17
+   |
+LL |         let _ = &good.data2[0];
+   |                 ^^^^^^^^^^^^^^
+   |
+   = 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 4 previous errors
+