Rollup merge of rust-lang#82525 - RalfJung:unaligned-ref-warn, r=petr…

…ochenkov

make unaligned_references future-incompat lint warn-by-default

and also remove the safe_packed_borrows lint that it replaces.

`std::ptr::addr_of!` has hit beta now and will hit stable in a month, so I propose we start fixing rust-lang#27060 for real: creating a reference to a field of a packed struct needs to eventually become a hard error; this PR makes it a warn-by-default future-incompat lint. (The lint already existed, this just raises its default level.) At the same time I removed the corresponding code from unsafety checking; really there's no reason an `unsafe` block should make any difference here.

For references to packed fields outside `unsafe` blocks, this means `unaligned_refereces` replaces the previous `safe_packed_borrows` warning with a link to rust-lang#82523 (and no more talk about unsafe blocks making any difference). So behavior barely changes, the warning is just worded differently. For references to packed fields inside `unsafe` blocks, this PR shows a new future-incompat warning.

Closes rust-lang#46043 because that lint no longer exists.

compiler/rustc_lint/src/lib.rs

@@ -325,6 +325,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
     store.register_renamed("exceeding_bitshifts", "arithmetic_overflow");
     store.register_renamed("redundant_semicolon", "redundant_semicolons");
     store.register_renamed("overlapping_patterns", "overlapping_range_endpoints");
+    store.register_renamed("safe_packed_borrows", "unaligned_references");
 
     // These were moved to tool lints, but rustc still sees them when compiling normally, before
     // tool lints are registered, so `check_tool_name_for_backwards_compat` doesn't work. Use

compiler/rustc_lint_defs/src/builtin.rs

@@ -1057,6 +1057,7 @@ declare_lint! {
     ///     unsafe {
     ///         let foo = Foo { field1: 0, field2: 0 };
     ///         let _ = &foo.field1;
+    ///         println!("{}", foo.field1); // An implicit `&` is added here, triggering the lint.
     ///     }
     /// }
     /// ```
@@ -1065,20 +1066,20 @@ declare_lint! {
     ///
     /// ### Explanation
     ///
-    /// Creating a reference to an insufficiently aligned packed field is
-    /// [undefined behavior] and should be disallowed.
-    ///
-    /// This lint is "allow" by default because there is no stable
-    /// alternative, and it is not yet certain how widespread existing code
-    /// will trigger this lint.
-    ///
-    /// See [issue #27060] for more discussion.
+    /// Creating a reference to an insufficiently aligned packed field is [undefined behavior] and
+    /// should be disallowed. Using an `unsafe` block does not change anything about this. Instead,
+    /// the code should do a copy of the data in the packed field or use raw pointers and unaligned
+    /// accesses. See [issue #82523] for more information.
     ///
     /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
-    /// [issue #27060]: https://github.com/rust-lang/rust/issues/27060
+    /// [issue #82523]: https://github.com/rust-lang/rust/issues/82523
     pub UNALIGNED_REFERENCES,
-    Allow,
+    Warn,
     "detects unaligned references to fields of packed structs",
+    @future_incompatible = FutureIncompatibleInfo {
+        reference: "issue #82523 <https://github.com/rust-lang/rust/issues/82523>",
+        edition: None,
+    };
     report_in_external_macro
 }
 
@@ -1150,49 +1151,6 @@ declare_lint! {
     "detects attempts to mutate a `const` item",
 }
 
-declare_lint! {
-    /// The `safe_packed_borrows` lint detects borrowing a field in the
-    /// interior of a packed structure with alignment other than 1.
-    ///
-    /// ### Example
-    ///
-    /// ```rust
-    /// #[repr(packed)]
-    /// pub struct Unaligned<T>(pub T);
-    ///
-    /// pub struct Foo {
-    ///     start: u8,
-    ///     data: Unaligned<u32>,
-    /// }
-    ///
-    /// fn main() {
-    ///     let x = Foo { start: 0, data: Unaligned(1) };
-    ///     let y = &x.data.0;
-    /// }
-    /// ```
-    ///
-    /// {{produces}}
-    ///
-    /// ### Explanation
-    ///
-    /// This type of borrow is unsafe and can cause errors on some platforms
-    /// and violates some assumptions made by the compiler. This was
-    /// previously allowed unintentionally. This is a [future-incompatible]
-    /// lint to transition this to a hard error in the future. See [issue
-    /// #46043] for more details, including guidance on how to solve the
-    /// problem.
-    ///
-    /// [issue #46043]: https://github.com/rust-lang/rust/issues/46043
-    /// [future-incompatible]: ../index.md#future-incompatible-lints
-    pub SAFE_PACKED_BORROWS,
-    Warn,
-    "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,
-    };
-}
-
 declare_lint! {
     /// The `patterns_in_fns_without_body` lint detects `mut` identifier
     /// patterns as a parameter in functions without a body.
@@ -2953,7 +2911,6 @@ declare_lint_pass! {
         RENAMED_AND_REMOVED_LINTS,
         UNALIGNED_REFERENCES,
         CONST_ITEM_MUTATION,
-        SAFE_PACKED_BORROWS,
         PATTERNS_IN_FNS_WITHOUT_BODY,
         MISSING_FRAGMENT_SPECIFIER,
         LATE_BOUND_LIFETIME_ARGUMENTS,

compiler/rustc_middle/src/mir/query.rs

@@ -23,15 +23,9 @@ pub enum UnsafetyViolationKind {
     General,
     /// Permitted both in `const fn`s and regular `fn`s.
     GeneralAndConstFn,
-    /// Borrow of packed field.
-    /// Has to be handled as a lint for backwards compatibility.
-    BorrowPacked,
     /// Unsafe operation in an `unsafe fn` but outside an `unsafe` block.
     /// Has to be handled as a lint for backwards compatibility.
     UnsafeFn,
-    /// Borrow of packed field in an `unsafe fn` but outside an `unsafe` block.
-    /// Has to be handled as a lint for backwards compatibility.
-    UnsafeFnBorrowPacked,
 }
 
 #[derive(Copy, Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)]

compiler/rustc_mir/src/transform/check_packed_ref.rs

@@ -1,11 +1,18 @@
+use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_middle::mir::visit::{PlaceContext, Visitor};
 use rustc_middle::mir::*;
+use rustc_middle::ty::query::Providers;
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_session::lint::builtin::UNALIGNED_REFERENCES;
+use rustc_span::symbol::sym;
 
 use crate::transform::MirPass;
 use crate::util;
 
+pub(crate) fn provide(providers: &mut Providers) {
+    *providers = Providers { unsafe_derive_on_repr_packed, ..*providers };
+}
+
 pub struct CheckPackedRef;
 
 impl<'tcx> MirPass<'tcx> for CheckPackedRef {
@@ -24,6 +31,41 @@ struct PackedRefChecker<'a, 'tcx> {
     source_info: SourceInfo,
 }
 
+fn unsafe_derive_on_repr_packed(tcx: TyCtxt<'_>, def_id: LocalDefId) {
+    let lint_hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
+
+    tcx.struct_span_lint_hir(UNALIGNED_REFERENCES, lint_hir_id, tcx.def_span(def_id), |lint| {
+        // FIXME: when we make this a hard error, this should have its
+        // own error code.
+        let message = if tcx.generics_of(def_id).own_requires_monomorphization() {
+            "`#[derive]` can't be used on a `#[repr(packed)]` struct with \
+             type or const parameters (error E0133)"
+                .to_string()
+        } else {
+            "`#[derive]` can't be used on a `#[repr(packed)]` struct that \
+             does not derive Copy (error E0133)"
+                .to_string()
+        };
+        lint.build(&message).emit()
+    });
+}
+
+fn builtin_derive_def_id(tcx: TyCtxt<'_>, def_id: DefId) -> Option<DefId> {
+    debug!("builtin_derive_def_id({:?})", def_id);
+    if let Some(impl_def_id) = tcx.impl_of_method(def_id) {
+        if tcx.has_attr(impl_def_id, sym::automatically_derived) {
+            debug!("builtin_derive_def_id({:?}) - is {:?}", def_id, impl_def_id);
+            Some(impl_def_id)
+        } else {
+            debug!("builtin_derive_def_id({:?}) - not automatically derived", def_id);
+            None
+        }
+    } else {
+        debug!("builtin_derive_def_id({:?}) - not a method", def_id);
+        None
+    }
+}
+
 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.
@@ -40,26 +82,33 @@ impl<'a, 'tcx> Visitor<'tcx> for PackedRefChecker<'a, 'tcx> {
     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("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()
-                    },
-                );
+                let def_id = self.body.source.instance.def_id();
+                if let Some(impl_def_id) = builtin_derive_def_id(self.tcx, def_id) {
+                    // If a method is defined in the local crate,
+                    // the impl containing that method should also be.
+                    self.tcx.ensure().unsafe_derive_on_repr_packed(impl_def_id.expect_local());
+                } else {
+                    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("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()
+                        },
+                    );
+                }
             }
         }
     }