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 https://github.com/rust-lang/rust/pull/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.)
This commit is contained in:
bors 2023-10-14 00:57:09 +00:00
commit 75a5dd05bc
12 changed files with 114 additions and 193 deletions

View File

@ -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);

View File

@ -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,8 +346,8 @@ 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)]
@ -367,39 +355,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
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>,

View File

@ -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,7 +133,7 @@ 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.
@ -143,13 +141,6 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
/// 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;

View File

@ -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)?;

View File

@ -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(())
}

View File

@ -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) {

View File

@ -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,

View File

@ -2,8 +2,6 @@
//! assertion failures
use either::Right;
use rustc_const_eval::const_eval::CheckAlignment;
use rustc_const_eval::ReportErrorExt;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::def::DefKind;
@ -16,7 +14,7 @@ use rustc_middle::mir::*;
use rustc_middle::ty::layout::{LayoutError, LayoutOf, LayoutOfHelpers, TyAndLayout};
use rustc_middle::ty::{self, GenericArgs, Instance, ParamEnv, Ty, TyCtxt, TypeVisitableExt};
use rustc_span::{def_id::DefId, Span};
use rustc_target::abi::{self, Align, HasDataLayout, Size, TargetDataLayout};
use rustc_target::abi::{self, HasDataLayout, Size, TargetDataLayout};
use rustc_target::spec::abi::Abi as CallAbi;
use crate::dataflow_const_prop::Patch;
@ -141,27 +139,14 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
type MemoryKind = !;
#[inline(always)]
fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> CheckAlignment {
// We do not check for alignment to avoid having to carry an `Align`
// in `ConstValue::Indirect`.
CheckAlignment::No
fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
false // no reason to enforce alignment
}
#[inline(always)]
fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>, _layout: TyAndLayout<'tcx>) -> bool {
false // for now, we don't enforce validity
}
fn alignment_check_failed(
ecx: &InterpCx<'mir, 'tcx, Self>,
_has: Align,
_required: Align,
_check: CheckAlignment,
) -> InterpResult<'tcx, ()> {
span_bug!(
ecx.cur_span(),
"`alignment_check_failed` called when no alignment check requested"
)
}
fn load_mir(
_ecx: &InterpCx<'mir, 'tcx, Self>,

View File

@ -2,7 +2,6 @@
//!
//! Currently, this pass only propagates scalar values.
use rustc_const_eval::const_eval::CheckAlignment;
use rustc_const_eval::interpret::{ImmTy, Immediate, InterpCx, OpTy, Projectable};
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def::DefKind;
@ -17,7 +16,7 @@ use rustc_mir_dataflow::value_analysis::{
use rustc_mir_dataflow::{lattice::FlatSet, Analysis, Results, ResultsVisitor};
use rustc_span::def_id::DefId;
use rustc_span::DUMMY_SP;
use rustc_target::abi::{Align, FieldIdx, VariantIdx};
use rustc_target::abi::{FieldIdx, VariantIdx};
use crate::MirPass;
@ -709,23 +708,13 @@ impl<'mir, 'tcx: 'mir> rustc_const_eval::interpret::Machine<'mir, 'tcx> for Dumm
const PANIC_ON_ALLOC_FAIL: bool = true;
#[inline(always)]
fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> CheckAlignment {
// We do not check for alignment to avoid having to carry an `Align`
// in `ConstValue::ByRef`.
CheckAlignment::No
fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
false // no reason to enforce alignment
}
fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>, _layout: TyAndLayout<'tcx>) -> bool {
unimplemented!()
}
fn alignment_check_failed(
_ecx: &InterpCx<'mir, 'tcx, Self>,
_has: Align,
_required: Align,
_check: CheckAlignment,
) -> interpret::InterpResult<'tcx, ()> {
unimplemented!()
}
fn before_access_global(
_tcx: TyCtxt<'tcx>,

View File

@ -12,7 +12,6 @@ use rand::rngs::StdRng;
use rand::SeedableRng;
use rustc_ast::ast::Mutability;
use rustc_const_eval::const_eval::CheckAlignment;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
#[allow(unused)]
use rustc_data_structures::static_assert_size;
@ -885,12 +884,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
const PANIC_ON_ALLOC_FAIL: bool = false;
#[inline(always)]
fn enforce_alignment(ecx: &MiriInterpCx<'mir, 'tcx>) -> CheckAlignment {
if ecx.machine.check_alignment == AlignmentCheck::None {
CheckAlignment::No
} else {
CheckAlignment::Error
}
fn enforce_alignment(ecx: &MiriInterpCx<'mir, 'tcx>) -> bool {
ecx.machine.check_alignment != AlignmentCheck::None
}
#[inline(always)]
@ -898,15 +893,6 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
ecx.machine.check_alignment == AlignmentCheck::Int
}
fn alignment_check_failed(
_ecx: &InterpCx<'mir, 'tcx, Self>,
has: Align,
required: Align,
_check: CheckAlignment,
) -> InterpResult<'tcx, ()> {
throw_ub!(AlignmentCheckFailed { has, required })
}
#[inline(always)]
fn enforce_validity(ecx: &MiriInterpCx<'mir, 'tcx>, _layout: TyAndLayout<'tcx>) -> bool {
ecx.machine.validate

View File

@ -0,0 +1,36 @@
// normalize-stderr-test "alloc\d+" -> "allocN"
#![feature(const_pointer_byte_offsets)]
#![feature(pointer_byte_offsets)]
#![feature(const_mut_refs)]
const MISALIGNED_LOAD: () = unsafe {
let mem = [0u32; 8];
let ptr = mem.as_ptr().byte_add(1);
let _val = *ptr; //~ERROR: evaluation of constant value failed
//~^NOTE: accessing memory with alignment 1, but alignment 4 is required
};
const MISALIGNED_STORE: () = unsafe {
let mut mem = [0u32; 8];
let ptr = mem.as_mut_ptr().byte_add(1);
*ptr = 0; //~ERROR: evaluation of constant value failed
//~^NOTE: accessing memory with alignment 1, but alignment 4 is required
};
const MISALIGNED_COPY: () = unsafe {
let x = &[0_u8; 4];
let y = x.as_ptr().cast::<u32>();
let mut z = 123;
y.copy_to_nonoverlapping(&mut z, 1);
//~^NOTE
// The actual error points into the implementation of `copy_to_nonoverlapping`.
};
const OOB: () = unsafe {
let mem = [0u32; 1];
let ptr = mem.as_ptr().cast::<u64>();
let _val = *ptr; //~ERROR: evaluation of constant value failed
//~^NOTE: size 4, so pointer to 8 bytes starting at offset 0 is out-of-bounds
};
fn main() {}

View File

@ -0,0 +1,36 @@
error[E0080]: evaluation of constant value failed
--> $DIR/raw-pointer-ub.rs:9:16
|
LL | let _val = *ptr;
| ^^^^ accessing memory with alignment 1, but alignment 4 is required
error[E0080]: evaluation of constant value failed
--> $DIR/raw-pointer-ub.rs:16:5
|
LL | *ptr = 0;
| ^^^^^^^^ accessing memory with alignment 1, but alignment 4 is required
error[E0080]: evaluation of constant value failed
--> $SRC_DIR/core/src/intrinsics.rs:LL:COL
|
= note: accessing memory with alignment 1, but alignment 4 is required
|
note: inside `copy_nonoverlapping::<u32>`
--> $SRC_DIR/core/src/intrinsics.rs:LL:COL
note: inside `ptr::const_ptr::<impl *const u32>::copy_to_nonoverlapping`
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
note: inside `MISALIGNED_COPY`
--> $DIR/raw-pointer-ub.rs:24:5
|
LL | y.copy_to_nonoverlapping(&mut z, 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error[E0080]: evaluation of constant value failed
--> $DIR/raw-pointer-ub.rs:32:16
|
LL | let _val = *ptr;
| ^^^^ dereferencing pointer failed: allocN has size 4, so pointer to 8 bytes starting at offset 0 is out-of-bounds
error: aborting due to 4 previous errors
For more information about this error, try `rustc --explain E0080`.