Auto merge of #115524 - RalfJung:misalign, r=wesleywiser

const-eval: make misalignment a hard error

It's been a future-incompat error (showing up in cargo's reports) since #104616, Rust 1.68, released in March.  That should be long enough.

The question for the lang team is simply -- should we move ahead with this, making const-eval alignment failures a hard error? (It turns out some of them accidentally already were hard errors since #104616. But not all so this is still a breaking change. Crater found no regression.)

Author: bors

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -1,6 +1,3 @@
-use crate::const_eval::CheckAlignment;
-use crate::errors::ConstEvalError;
-
 use either::{Left, Right};
 
 use rustc_hir::def::DefKind;
@@ -15,7 +12,9 @@ use rustc_span::source_map::Span;
 use rustc_target::abi::{self, Abi};
 
 use super::{CanAccessStatics, CompileTimeEvalContext, CompileTimeInterpreter};
+use crate::const_eval::CheckAlignment;
 use crate::errors;
+use crate::errors::ConstEvalError;
 use crate::interpret::eval_nullary_intrinsic;
 use crate::interpret::{
     intern_const_alloc_recursive, CtfeValidationMode, GlobalId, Immediate, InternKind, InterpCx,
@@ -290,14 +289,7 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
         key.param_env,
         // Statics (and promoteds inside statics) may access other statics, because unlike consts
         // they do not have to behave "as if" they were evaluated at runtime.
-        CompileTimeInterpreter::new(
-            CanAccessStatics::from(is_static),
-            if tcx.sess.opts.unstable_opts.extra_const_ub_checks {
-                CheckAlignment::Error
-            } else {
-                CheckAlignment::FutureIncompat
-            },
-        ),
+        CompileTimeInterpreter::new(CanAccessStatics::from(is_static), CheckAlignment::Error),
     );
 
     let res = ecx.load_mir(cid.instance.def, cid.promoted);

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -1,10 +1,9 @@
 use rustc_hir::def::DefKind;
-use rustc_hir::{LangItem, CRATE_HIR_ID};
+use rustc_hir::LangItem;
 use rustc_middle::mir;
 use rustc_middle::mir::interpret::PointerArithmetic;
 use rustc_middle::ty::layout::{FnAbiOf, TyAndLayout};
 use rustc_middle::ty::{self, TyCtxt};
-use rustc_session::lint::builtin::INVALID_ALIGNMENT;
 use std::borrow::Borrow;
 use std::hash::Hash;
 use std::ops::ControlFlow;
@@ -21,11 +20,11 @@ use rustc_target::abi::{Align, Size};
 use rustc_target::spec::abi::Abi as CallAbi;
 
 use crate::errors::{LongRunning, LongRunningWarn};
+use crate::fluent_generated as fluent;
 use crate::interpret::{
     self, compile_time_machine, AllocId, ConstAllocation, FnArg, FnVal, Frame, ImmTy, InterpCx,
     InterpResult, OpTy, PlaceTy, Pointer, Scalar,
 };
-use crate::{errors, fluent_generated as fluent};
 
 use super::error::*;
 
@@ -65,22 +64,11 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {
 
 #[derive(Copy, Clone)]
 pub enum CheckAlignment {
-    /// Ignore alignment when following relocations.
+    /// Ignore all alignment requirements.
     /// This is mainly used in interning.
     No,
     /// Hard error when dereferencing a misaligned pointer.
     Error,
-    /// Emit a future incompat lint when dereferencing a misaligned pointer.
-    FutureIncompat,
-}
-
-impl CheckAlignment {
-    pub fn should_check(&self) -> bool {
-        match self {
-            CheckAlignment::No => false,
-            CheckAlignment::Error | CheckAlignment::FutureIncompat => true,
-        }
-    }
 }
 
 #[derive(Copy, Clone, PartialEq)]
@@ -358,48 +346,15 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
     const PANIC_ON_ALLOC_FAIL: bool = false; // will be raised as a proper error
 
     #[inline(always)]
-    fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> CheckAlignment {
-        ecx.machine.check_alignment
+    fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
+        matches!(ecx.machine.check_alignment, CheckAlignment::Error)
     }
 
     #[inline(always)]
     fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>, layout: TyAndLayout<'tcx>) -> bool {
         ecx.tcx.sess.opts.unstable_opts.extra_const_ub_checks || layout.abi.is_uninhabited()
     }
 
-    fn alignment_check_failed(
-        ecx: &InterpCx<'mir, 'tcx, Self>,
-        has: Align,
-        required: Align,
-        check: CheckAlignment,
-    ) -> InterpResult<'tcx, ()> {
-        let err = err_ub!(AlignmentCheckFailed { has, required }).into();
-        match check {
-            CheckAlignment::Error => Err(err),
-            CheckAlignment::No => span_bug!(
-                ecx.cur_span(),
-                "`alignment_check_failed` called when no alignment check requested"
-            ),
-            CheckAlignment::FutureIncompat => {
-                let (_, backtrace) = err.into_parts();
-                backtrace.print_backtrace();
-                let (span, frames) = super::get_span_and_frames(&ecx);
-
-                ecx.tcx.emit_spanned_lint(
-                    INVALID_ALIGNMENT,
-                    ecx.stack().iter().find_map(|frame| frame.lint_root()).unwrap_or(CRATE_HIR_ID),
-                    span,
-                    errors::AlignmentCheckFailed {
-                        has: has.bytes(),
-                        required: required.bytes(),
-                        frames,
-                    },
-                );
-                Ok(())
-            }
-        }
-    }
-
     fn load_mir(
         ecx: &InterpCx<'mir, 'tcx, Self>,
         instance: ty::InstanceDef<'tcx>,

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -12,11 +12,9 @@ use rustc_middle::mir;
 use rustc_middle::ty::layout::TyAndLayout;
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_span::def_id::DefId;
-use rustc_target::abi::{Align, Size};
+use rustc_target::abi::Size;
 use rustc_target::spec::abi::Abi as CallAbi;
 
-use crate::const_eval::CheckAlignment;
-
 use super::{
     AllocBytes, AllocId, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy, InterpCx,
     InterpResult, MPlaceTy, MemoryKind, OpTy, PlaceTy, Pointer, Provenance,
@@ -135,21 +133,14 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
     const POST_MONO_CHECKS: bool = true;
 
     /// Whether memory accesses should be alignment-checked.
-    fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> CheckAlignment;
+    fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
 
     /// Whether, when checking alignment, we should look at the actual address and thus support
     /// custom alignment logic based on whatever the integer address happens to be.
     ///
     /// If this returns true, Provenance::OFFSET_IS_ADDR must be true.
     fn use_addr_for_alignment_check(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
 
-    fn alignment_check_failed(
-        ecx: &InterpCx<'mir, 'tcx, Self>,
-        has: Align,
-        required: Align,
-        check: CheckAlignment,
-    ) -> InterpResult<'tcx, ()>;
-
     /// Whether to enforce the validity invariant for a specific layout.
     fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>, layout: TyAndLayout<'tcx>) -> bool;
 

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -18,7 +18,6 @@ use rustc_middle::mir::display_allocation;
 use rustc_middle::ty::{self, Instance, ParamEnv, Ty, TyCtxt};
 use rustc_target::abi::{Align, HasDataLayout, Size};
 
-use crate::const_eval::CheckAlignment;
 use crate::fluent_generated as fluent;
 
 use super::{
@@ -373,8 +372,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         self.check_and_deref_ptr(
             ptr,
             size,
-            align,
-            M::enforce_alignment(self),
+            M::enforce_alignment(self).then_some(align),
             CheckInAllocMsg::MemoryAccessTest,
             |alloc_id, offset, prov| {
                 let (size, align) = self
@@ -395,17 +393,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         align: Align,
         msg: CheckInAllocMsg,
     ) -> InterpResult<'tcx> {
-        self.check_and_deref_ptr(
-            ptr,
-            size,
-            align,
-            CheckAlignment::Error,
-            msg,
-            |alloc_id, _, _| {
-                let (size, align) = self.get_live_alloc_size_and_align(alloc_id, msg)?;
-                Ok((size, align, ()))
-            },
-        )?;
+        self.check_and_deref_ptr(ptr, size, Some(align), msg, |alloc_id, _, _| {
+            let (size, align) = self.get_live_alloc_size_and_align(alloc_id, msg)?;
+            Ok((size, align, ()))
+        })?;
         Ok(())
     }
 
@@ -419,8 +410,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         &self,
         ptr: Pointer<Option<M::Provenance>>,
         size: Size,
-        align: Align,
-        check: CheckAlignment,
+        align: Option<Align>,
         msg: CheckInAllocMsg,
         alloc_size: impl FnOnce(
             AllocId,
@@ -436,8 +426,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     throw_ub!(DanglingIntPointer(addr, msg));
                 }
                 // Must be aligned.
-                if check.should_check() {
-                    self.check_offset_align(addr, align, check)?;
+                if let Some(align) = align {
+                    self.check_offset_align(addr, align)?;
                 }
                 None
             }
@@ -460,16 +450,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 }
                 // Test align. Check this last; if both bounds and alignment are violated
                 // we want the error to be about the bounds.
-                if check.should_check() {
+                if let Some(align) = align {
                     if M::use_addr_for_alignment_check(self) {
                         // `use_addr_for_alignment_check` can only be true if `OFFSET_IS_ADDR` is true.
-                        self.check_offset_align(ptr.addr().bytes(), align, check)?;
+                        self.check_offset_align(ptr.addr().bytes(), align)?;
                     } else {
                         // Check allocation alignment and offset alignment.
                         if alloc_align.bytes() < align.bytes() {
-                            M::alignment_check_failed(self, alloc_align, align, check)?;
+                            throw_ub!(AlignmentCheckFailed { has: alloc_align, required: align });
                         }
-                        self.check_offset_align(offset.bytes(), align, check)?;
+                        self.check_offset_align(offset.bytes(), align)?;
                     }
                 }
 
@@ -480,18 +470,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         })
     }
 
-    fn check_offset_align(
-        &self,
-        offset: u64,
-        align: Align,
-        check: CheckAlignment,
-    ) -> InterpResult<'tcx> {
+    fn check_offset_align(&self, offset: u64, align: Align) -> InterpResult<'tcx> {
         if offset % align.bytes() == 0 {
             Ok(())
         } else {
             // The biggest power of two through which `offset` is divisible.
             let offset_pow2 = 1 << offset.trailing_zeros();
-            M::alignment_check_failed(self, Align::from_bytes(offset_pow2).unwrap(), align, check)
+            throw_ub!(AlignmentCheckFailed {
+                has: Align::from_bytes(offset_pow2).unwrap(),
+                required: align
+            });
         }
     }
 }
@@ -609,8 +597,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         let ptr_and_alloc = self.check_and_deref_ptr(
             ptr,
             size,
-            align,
-            M::enforce_alignment(self),
+            M::enforce_alignment(self).then_some(align),
             CheckInAllocMsg::MemoryAccessTest,
             |alloc_id, offset, prov| {
                 let alloc = self.get_alloc_raw(alloc_id)?;

compiler/rustc_const_eval/src/interpret/place.rs

@@ -500,8 +500,7 @@ where
             .size_and_align_of_mplace(&mplace)?
             .unwrap_or((mplace.layout.size, mplace.layout.align.abi));
         // Due to packed places, only `mplace.align` matters.
-        let align =
-            if M::enforce_alignment(self).should_check() { mplace.align } else { Align::ONE };
+        let align = if M::enforce_alignment(self) { mplace.align } else { Align::ONE };
         self.check_ptr_access_align(mplace.ptr(), size, align, CheckInAllocMsg::DerefTest)?;
         Ok(())
     }

compiler/rustc_lint/src/lib.rs

@@ -506,6 +506,11 @@ fn register_builtins(store: &mut LintStore) {
         "replaced with another group of lints, see RFC \
          <https://rust-lang.github.io/rfcs/2145-type-privacy.html> for more information",
     );
+    store.register_removed(
+        "invalid_alignment",
+        "converted into hard error, see PR #104616 \
+         <https://github.com/rust-lang/rust/pull/104616> for more information",
+    );
 }
 
 fn register_internals(store: &mut LintStore) {

compiler/rustc_lint_defs/src/builtin.rs

@@ -986,45 +986,6 @@ declare_lint! {
     "detects trivial casts of numeric types which could be removed"
 }
 
-declare_lint! {
-    /// The `invalid_alignment` lint detects dereferences of misaligned pointers during
-    /// constant evaluation.
-    ///
-    /// ### Example
-    ///
-    /// ```rust,compile_fail
-    /// #![feature(const_mut_refs)]
-    /// const FOO: () = unsafe {
-    ///     let x = &[0_u8; 4];
-    ///     let y = x.as_ptr().cast::<u32>();
-    ///     let mut z = 123;
-    ///     y.copy_to_nonoverlapping(&mut z, 1); // the address of a `u8` array is unknown
-    ///     // and thus we don't know if it is aligned enough for copying a `u32`.
-    /// };
-    /// ```
-    ///
-    /// {{produces}}
-    ///
-    /// ### Explanation
-    ///
-    /// The compiler allowed dereferencing raw pointers irrespective of alignment
-    /// during const eval due to the const evaluator at the time not making it easy
-    /// or cheap to check. Now that it is both, this is not accepted anymore.
-    ///
-    /// Since it was undefined behaviour to begin with, this breakage does not violate
-    /// Rust's stability guarantees. Using undefined behaviour can cause arbitrary
-    /// behaviour, including failure to build.
-    ///
-    /// [future-incompatible]: ../index.md#future-incompatible-lints
-    pub INVALID_ALIGNMENT,
-    Deny,
-    "raw pointers must be aligned before dereferencing",
-    @future_incompatible = FutureIncompatibleInfo {
-        reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
-        reference: "issue #68585 <https://github.com/rust-lang/rust/issues/104616>",
-    };
-}
-
 declare_lint! {
     /// The `exported_private_dependencies` lint detects private dependencies
     /// that are exposed in a public interface.
@@ -3430,7 +3391,6 @@ declare_lint_pass! {
         INDIRECT_STRUCTURAL_MATCH,
         INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
         INLINE_NO_SANITIZE,
-        INVALID_ALIGNMENT,
         INVALID_DOC_ATTRIBUTES,
         INVALID_MACRO_EXPORT_ARGUMENTS,
         INVALID_TYPE_PARAM_DEFAULT,