Rollup merge of rust-lang#102513 - RalfJung:no-more-unaligned-reference, r=cjgillot

make unaligned_reference a hard error

The `unaligned_references` lint has been warn-by-default since Rust 1.53 (rust-lang#82525) and deny-by-default with mention in cargo future-incompat reports since Rust 1.62 (rust-lang#95372). Current nightly will become Rust 1.66, so (unless major surprises show up with crater) I think it is time we make this a hard error, and close this old soundness gap in the language.

Fixes rust-lang#82523.

Author: Dylan-DPC

compiler/rustc_error_codes/src/error_codes.rs

@@ -5,7 +5,7 @@
 // /!\ IMPORTANT /!\
 //
 // Error messages' format must follow the RFC 1567 available here:
-// https://github.com/rust-lang/rfcs/pull/1567
+// https://rust-lang.github.io/rfcs/1567-long-error-codes-explanation-normalization.html
 
 register_diagnostics! {
 E0001: include_str!("./error_codes/E0001.md"),
@@ -494,6 +494,7 @@ E0786: include_str!("./error_codes/E0786.md"),
 E0787: include_str!("./error_codes/E0787.md"),
 E0788: include_str!("./error_codes/E0788.md"),
 E0790: include_str!("./error_codes/E0790.md"),
+E0791: include_str!("./error_codes/E0791.md"),
 ;
 //  E0006, // merged with E0005
 //  E0008, // cannot bind by-move into a pattern guard

compiler/rustc_error_codes/src/error_codes/E0791.md

@@ -0,0 +1,64 @@
+An unaligned reference to a field of a [packed] struct got created.
+
+Erroneous code example:
+
+```compile_fail,E0791
+#[repr(packed)]
+pub struct Foo {
+    field1: u64,
+    field2: u8,
+}
+
+unsafe {
+    let foo = Foo { field1: 0, field2: 0 };
+    // Accessing the field directly is fine.
+    let val = foo.field1;
+    // A reference to a packed field causes a error.
+    let val = &foo.field1; // ERROR
+    // An implicit `&` is added in format strings, causing the same error.
+    println!("{}", foo.field1); // ERROR
+}
+```
+
+Creating a reference to an insufficiently aligned packed field is
+[undefined behavior] and therefore 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.
+
+```
+#[repr(packed)]
+pub struct Foo {
+    field1: u64,
+    field2: u8,
+}
+
+unsafe {
+    let foo = Foo { field1: 0, field2: 0 };
+
+    // Instead of a reference, we can create a raw pointer...
+    let ptr = std::ptr::addr_of!(foo.field1);
+    // ... and then (crucially!) access it in an explicitly unaligned way.
+    let val = unsafe { ptr.read_unaligned() };
+    // This would *NOT* be correct:
+    // let val = unsafe { *ptr }; // Undefined Behavior due to unaligned load!
+
+    // For formatting, we can create a copy to avoid the direct reference.
+    let copy = foo.field1;
+    println!("{}", copy);
+    // Creating a copy can be written in a single line with curly braces.
+    // (This is equivalent to the two lines above.)
+    println!("{}", { foo.field1 });
+}
+```
+
+### Additional information
+
+Note that this error is specifically about *references* to packed fields.
+Direct by-value access of those fields is fine, since then the compiler has
+enough information to generate the correct kind of access.
+
+See [issue #82523] for more information.
+
+[packed]: https://doc.rust-lang.org/reference/type-layout.html#the-alignment-modifiers
+[undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
+[issue #82523]: https://github.com/rust-lang/rust/issues/82523

compiler/rustc_lint/src/lib.rs

@@ -367,7 +367,6 @@ 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");
     store.register_renamed("disjoint_capture_migration", "rust_2021_incompatible_closure_captures");
     store.register_renamed("or_patterns_back_compat", "rust_2021_incompatible_or_patterns");
     store.register_renamed("non_fmt_panic", "non_fmt_panics");
@@ -530,6 +529,16 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
         "converted into hard error, see issue #71800 \
          <https://github.com/rust-lang/rust/issues/71800> for more information",
     );
+    store.register_removed(
+        "safe_packed_borrows",
+        "converted into hard error, see issue #82523 \
+         <https://github.com/rust-lang/rust/issues/82523> for more information",
+    );
+    store.register_removed(
+        "unaligned_references",
+        "converted into hard error, see issue #82523 \
+         <https://github.com/rust-lang/rust/issues/82523> for more information",
+    );
 }
 
 fn register_internals(store: &mut LintStore) {

compiler/rustc_lint_defs/src/builtin.rs

@@ -1146,51 +1146,6 @@ declare_lint! {
     "lints that have been renamed or removed"
 }
 
-declare_lint! {
-    /// The `unaligned_references` lint detects unaligned references to fields
-    /// of [packed] structs.
-    ///
-    /// [packed]: https://doc.rust-lang.org/reference/type-layout.html#the-alignment-modifiers
-    ///
-    /// ### Example
-    ///
-    /// ```compile_fail
-    /// #[repr(packed)]
-    /// pub struct Foo {
-    ///     field1: u64,
-    ///     field2: u8,
-    /// }
-    ///
-    /// fn main() {
-    ///     unsafe {
-    ///         let foo = Foo { field1: 0, field2: 0 };
-    ///         let _ = &foo.field1;
-    ///         println!("{}", foo.field1); // An implicit `&` is added here, triggering the lint.
-    ///     }
-    /// }
-    /// ```
-    ///
-    /// {{produces}}
-    ///
-    /// ### Explanation
-    ///
-    /// 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 #82523]: https://github.com/rust-lang/rust/issues/82523
-    pub UNALIGNED_REFERENCES,
-    Deny,
-    "detects unaligned references to fields of packed structs",
-    @future_incompatible = FutureIncompatibleInfo {
-        reference: "issue #82523 <https://github.com/rust-lang/rust/issues/82523>",
-        reason: FutureIncompatibilityReason::FutureReleaseErrorReportNow,
-    };
-    report_in_external_macro
-}
-
 declare_lint! {
     /// The `const_item_mutation` lint detects attempts to mutate a `const`
     /// item.
@@ -3266,7 +3221,6 @@ declare_lint_pass! {
         PUB_USE_OF_PRIVATE_EXTERN_CRATE,
         INVALID_TYPE_PARAM_DEFAULT,
         RENAMED_AND_REMOVED_LINTS,
-        UNALIGNED_REFERENCES,
         CONST_ITEM_MUTATION,
         PATTERNS_IN_FNS_WITHOUT_BODY,
         MISSING_FRAGMENT_SPECIFIER,

compiler/rustc_mir_transform/src/check_packed_ref.rs

@@ -1,9 +1,9 @@
+use rustc_errors::struct_span_err;
 use rustc_hir::def_id::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 crate::util;
 use crate::MirLint;
@@ -31,11 +31,6 @@ struct PackedRefChecker<'a, 'tcx> {
 }
 
 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);
-
-    // FIXME: when we make this a hard error, this should have its
-    // own error code.
-
     let extra = if tcx.generics_of(def_id).own_requires_monomorphization() {
         "with type or const parameters"
     } else {
@@ -46,14 +41,7 @@ fn unsafe_derive_on_repr_packed(tcx: TyCtxt<'_>, def_id: LocalDefId) {
         tcx.item_name(tcx.trait_id_of_impl(def_id.to_def_id()).expect("derived trait name")),
         extra
     );
-
-    tcx.struct_span_lint_hir(
-        UNALIGNED_REFERENCES,
-        lint_hir_id,
-        tcx.def_span(def_id),
-        message,
-        |lint| lint,
-    );
+    struct_span_err!(tcx.sess, tcx.def_span(def_id), E0791, "{message}").emit();
 }
 
 impl<'tcx> Visitor<'tcx> for PackedRefChecker<'_, 'tcx> {
@@ -82,31 +70,22 @@ impl<'tcx> Visitor<'tcx> for PackedRefChecker<'_, 'tcx> {
                     // 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,
-                        "reference to packed field is unaligned",
-                        |lint| {
-                            lint
-                                .note(
-                                    "fields of packed structs are not properly aligned, and 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)"
-                                )
-                        },
-                    );
+                    struct_span_err!(
+                        self.tcx.sess,
+                        self.source_info.span,
+                        E0791,
+                        "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)",
+                    ).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)"
+                    )
+                    .emit();
                 }
             }
         }

src/test/ui/binding/issue-53114-safety-checks.rs

@@ -21,13 +21,11 @@ fn let_wild_gets_unsafe_field() {
     let u2 = U { a: I(1) };
     let p = P { a: &2, b: &3 };
     let _ = &p.b;  //~ ERROR    reference to packed field
-    //~^  WARN will become a hard error
     let _ = u1.a;  // #53114: should eventually signal error as well
     let _ = &u2.a; //~ ERROR  [E0133]
 
     // variation on above with `_` in substructure
     let (_,) = (&p.b,);  //~ ERROR     reference to packed field
-    //~^  WARN will become a hard error
     let (_,) = (u1.a,);  //~ ERROR   [E0133]
     let (_,) = (&u2.a,); //~ ERROR   [E0133]
 }
@@ -37,13 +35,11 @@ fn match_unsafe_field_to_wild() {
     let u2 = U { a: I(1) };
     let p = P { a: &2, b: &3 };
     match &p.b  { _ => { } } //~ ERROR     reference to packed field
-    //~^  WARN will become a hard error
     match u1.a  { _ => { } } //~ ERROR   [E0133]
     match &u2.a { _ => { } } //~ ERROR   [E0133]
 
     // variation on above with `_` in substructure
     match (&p.b,)  { (_,) => { } } //~ ERROR     reference to packed field
-    //~^  WARN will become a hard error
     match (u1.a,)  { (_,) => { } } //~ ERROR   [E0133]
     match (&u2.a,) { (_,) => { } } //~ ERROR   [E0133]
 }