Skip to content

Commit 75a5dd0

Browse files
borsbors
committed
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.)
2 parents fcab248 + bd33846 commit 75a5dd0

File tree

12 files changed

+114
-193
lines changed

12 files changed

+114
-193
lines changed

‎compiler/rustc_const_eval/src/const_eval/eval_queries.rs

+3-11
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
use crate::const_eval::CheckAlignment;
2-
use crate::errors::ConstEvalError;
3-
41
use either::{Left, Right};
52

63
use rustc_hir::def::DefKind;
@@ -15,7 +12,9 @@ use rustc_span::source_map::Span;
1512
use rustc_target::abi::{self, Abi};
1613

1714
use super::{CanAccessStatics, CompileTimeEvalContext, CompileTimeInterpreter};
15+
use crate::const_eval::CheckAlignment;
1816
use crate::errors;
17+
use crate::errors::ConstEvalError;
1918
use crate::interpret::eval_nullary_intrinsic;
2019
use crate::interpret::{
2120
intern_const_alloc_recursive, CtfeValidationMode, GlobalId, Immediate, InternKind, InterpCx,
@@ -290,14 +289,7 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
290289
key.param_env,
291290
// Statics (and promoteds inside statics) may access other statics, because unlike consts
292291
// they do not have to behave "as if" they were evaluated at runtime.
293-
CompileTimeInterpreter::new(
294-
CanAccessStatics::from(is_static),
295-
if tcx.sess.opts.unstable_opts.extra_const_ub_checks {
296-
CheckAlignment::Error
297-
} else {
298-
CheckAlignment::FutureIncompat
299-
},
300-
),
292+
CompileTimeInterpreter::new(CanAccessStatics::from(is_static), CheckAlignment::Error),
301293
);
302294

303295
let res = ecx.load_mir(cid.instance.def, cid.promoted);

‎compiler/rustc_const_eval/src/const_eval/machine.rs

+5-50
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
use rustc_hir::def::DefKind;
2-
use rustc_hir::{LangItem, CRATE_HIR_ID};
2+
use rustc_hir::LangItem;
33
use rustc_middle::mir;
44
use rustc_middle::mir::interpret::PointerArithmetic;
55
use rustc_middle::ty::layout::{FnAbiOf, TyAndLayout};
66
use rustc_middle::ty::{self, TyCtxt};
7-
use rustc_session::lint::builtin::INVALID_ALIGNMENT;
87
use std::borrow::Borrow;
98
use std::hash::Hash;
109
use std::ops::ControlFlow;
@@ -21,11 +20,11 @@ use rustc_target::abi::{Align, Size};
2120
use rustc_target::spec::abi::Abi as CallAbi;
2221

2322
use crate::errors::{LongRunning, LongRunningWarn};
23+
use crate::fluent_generated as fluent;
2424
use crate::interpret::{
2525
self, compile_time_machine, AllocId, ConstAllocation, FnArg, FnVal, Frame, ImmTy, InterpCx,
2626
InterpResult, OpTy, PlaceTy, Pointer, Scalar,
2727
};
28-
use crate::{errors, fluent_generated as fluent};
2928

3029
use super::error::*;
3130

@@ -65,22 +64,11 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {
6564

6665
#[derive(Copy, Clone)]
6766
pub enum CheckAlignment {
68-
/// Ignore alignment when following relocations.
67+
/// Ignore all alignment requirements.
6968
/// This is mainly used in interning.
7069
No,
7170
/// Hard error when dereferencing a misaligned pointer.
7271
Error,
73-
/// Emit a future incompat lint when dereferencing a misaligned pointer.
74-
FutureIncompat,
75-
}
76-
77-
impl CheckAlignment {
78-
pub fn should_check(&self) -> bool {
79-
match self {
80-
CheckAlignment::No => false,
81-
CheckAlignment::Error | CheckAlignment::FutureIncompat => true,
82-
}
83-
}
8472
}
8573

8674
#[derive(Copy, Clone, PartialEq)]
@@ -358,48 +346,15 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
358346
const PANIC_ON_ALLOC_FAIL: bool = false; // will be raised as a proper error
359347

360348
#[inline(always)]
361-
fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> CheckAlignment {
362-
ecx.machine.check_alignment
349+
fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
350+
matches!(ecx.machine.check_alignment, CheckAlignment::Error)
363351
}
364352

365353
#[inline(always)]
366354
fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>, layout: TyAndLayout<'tcx>) -> bool {
367355
ecx.tcx.sess.opts.unstable_opts.extra_const_ub_checks || layout.abi.is_uninhabited()
368356
}
369357

370-
fn alignment_check_failed(
371-
ecx: &InterpCx<'mir, 'tcx, Self>,
372-
has: Align,
373-
required: Align,
374-
check: CheckAlignment,
375-
) -> InterpResult<'tcx, ()> {
376-
let err = err_ub!(AlignmentCheckFailed { has, required }).into();
377-
match check {
378-
CheckAlignment::Error => Err(err),
379-
CheckAlignment::No => span_bug!(
380-
ecx.cur_span(),
381-
"`alignment_check_failed` called when no alignment check requested"
382-
),
383-
CheckAlignment::FutureIncompat => {
384-
let (_, backtrace) = err.into_parts();
385-
backtrace.print_backtrace();
386-
let (span, frames) = super::get_span_and_frames(&ecx);
387-
388-
ecx.tcx.emit_spanned_lint(
389-
INVALID_ALIGNMENT,
390-
ecx.stack().iter().find_map(|frame| frame.lint_root()).unwrap_or(CRATE_HIR_ID),
391-
span,
392-
errors::AlignmentCheckFailed {
393-
has: has.bytes(),
394-
required: required.bytes(),
395-
frames,
396-
},
397-
);
398-
Ok(())
399-
}
400-
}
401-
}
402-
403358
fn load_mir(
404359
ecx: &InterpCx<'mir, 'tcx, Self>,
405360
instance: ty::InstanceDef<'tcx>,

‎compiler/rustc_const_eval/src/interpret/machine.rs

+2-11
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,9 @@ use rustc_middle::mir;
1212
use rustc_middle::ty::layout::TyAndLayout;
1313
use rustc_middle::ty::{self, TyCtxt};
1414
use rustc_span::def_id::DefId;
15-
use rustc_target::abi::{Align, Size};
15+
use rustc_target::abi::Size;
1616
use rustc_target::spec::abi::Abi as CallAbi;
1717

18-
use crate::const_eval::CheckAlignment;
19-
2018
use super::{
2119
AllocBytes, AllocId, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy, InterpCx,
2220
InterpResult, MPlaceTy, MemoryKind, OpTy, PlaceTy, Pointer, Provenance,
@@ -135,21 +133,14 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
135133
const POST_MONO_CHECKS: bool = true;
136134

137135
/// Whether memory accesses should be alignment-checked.
138-
fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> CheckAlignment;
136+
fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
139137

140138
/// Whether, when checking alignment, we should look at the actual address and thus support
141139
/// custom alignment logic based on whatever the integer address happens to be.
142140
///
143141
/// If this returns true, Provenance::OFFSET_IS_ADDR must be true.
144142
fn use_addr_for_alignment_check(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
145143

146-
fn alignment_check_failed(
147-
ecx: &InterpCx<'mir, 'tcx, Self>,
148-
has: Align,
149-
required: Align,
150-
check: CheckAlignment,
151-
) -> InterpResult<'tcx, ()>;
152-
153144
/// Whether to enforce the validity invariant for a specific layout.
154145
fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>, layout: TyAndLayout<'tcx>) -> bool;
155146

‎compiler/rustc_const_eval/src/interpret/memory.rs

+18-31
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ use rustc_middle::mir::display_allocation;
1818
use rustc_middle::ty::{self, Instance, ParamEnv, Ty, TyCtxt};
1919
use rustc_target::abi::{Align, HasDataLayout, Size};
2020

21-
use crate::const_eval::CheckAlignment;
2221
use crate::fluent_generated as fluent;
2322

2423
use super::{
@@ -373,8 +372,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
373372
self.check_and_deref_ptr(
374373
ptr,
375374
size,
376-
align,
377-
M::enforce_alignment(self),
375+
M::enforce_alignment(self).then_some(align),
378376
CheckInAllocMsg::MemoryAccessTest,
379377
|alloc_id, offset, prov| {
380378
let (size, align) = self
@@ -395,17 +393,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
395393
align: Align,
396394
msg: CheckInAllocMsg,
397395
) -> InterpResult<'tcx> {
398-
self.check_and_deref_ptr(
399-
ptr,
400-
size,
401-
align,
402-
CheckAlignment::Error,
403-
msg,
404-
|alloc_id, _, _| {
405-
let (size, align) = self.get_live_alloc_size_and_align(alloc_id, msg)?;
406-
Ok((size, align, ()))
407-
},
408-
)?;
396+
self.check_and_deref_ptr(ptr, size, Some(align), msg, |alloc_id, _, _| {
397+
let (size, align) = self.get_live_alloc_size_and_align(alloc_id, msg)?;
398+
Ok((size, align, ()))
399+
})?;
409400
Ok(())
410401
}
411402

@@ -419,8 +410,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
419410
&self,
420411
ptr: Pointer<Option<M::Provenance>>,
421412
size: Size,
422-
align: Align,
423-
check: CheckAlignment,
413+
align: Option<Align>,
424414
msg: CheckInAllocMsg,
425415
alloc_size: impl FnOnce(
426416
AllocId,
@@ -436,8 +426,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
436426
throw_ub!(DanglingIntPointer(addr, msg));
437427
}
438428
// Must be aligned.
439-
if check.should_check() {
440-
self.check_offset_align(addr, align, check)?;
429+
if let Some(align) = align {
430+
self.check_offset_align(addr, align)?;
441431
}
442432
None
443433
}
@@ -460,16 +450,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
460450
}
461451
// Test align. Check this last; if both bounds and alignment are violated
462452
// we want the error to be about the bounds.
463-
if check.should_check() {
453+
if let Some(align) = align {
464454
if M::use_addr_for_alignment_check(self) {
465455
// `use_addr_for_alignment_check` can only be true if `OFFSET_IS_ADDR` is true.
466-
self.check_offset_align(ptr.addr().bytes(), align, check)?;
456+
self.check_offset_align(ptr.addr().bytes(), align)?;
467457
} else {
468458
// Check allocation alignment and offset alignment.
469459
if alloc_align.bytes() < align.bytes() {
470-
M::alignment_check_failed(self, alloc_align, align, check)?;
460+
throw_ub!(AlignmentCheckFailed { has: alloc_align, required: align });
471461
}
472-
self.check_offset_align(offset.bytes(), align, check)?;
462+
self.check_offset_align(offset.bytes(), align)?;
473463
}
474464
}
475465

@@ -480,18 +470,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
480470
})
481471
}
482472

483-
fn check_offset_align(
484-
&self,
485-
offset: u64,
486-
align: Align,
487-
check: CheckAlignment,
488-
) -> InterpResult<'tcx> {
473+
fn check_offset_align(&self, offset: u64, align: Align) -> InterpResult<'tcx> {
489474
if offset % align.bytes() == 0 {
490475
Ok(())
491476
} else {
492477
// The biggest power of two through which `offset` is divisible.
493478
let offset_pow2 = 1 << offset.trailing_zeros();
494-
M::alignment_check_failed(self, Align::from_bytes(offset_pow2).unwrap(), align, check)
479+
throw_ub!(AlignmentCheckFailed {
480+
has: Align::from_bytes(offset_pow2).unwrap(),
481+
required: align
482+
});
495483
}
496484
}
497485
}
@@ -609,8 +597,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
609597
let ptr_and_alloc = self.check_and_deref_ptr(
610598
ptr,
611599
size,
612-
align,
613-
M::enforce_alignment(self),
600+
M::enforce_alignment(self).then_some(align),
614601
CheckInAllocMsg::MemoryAccessTest,
615602
|alloc_id, offset, prov| {
616603
let alloc = self.get_alloc_raw(alloc_id)?;

‎compiler/rustc_const_eval/src/interpret/place.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -500,8 +500,7 @@ where
500500
.size_and_align_of_mplace(&mplace)?
501501
.unwrap_or((mplace.layout.size, mplace.layout.align.abi));
502502
// Due to packed places, only `mplace.align` matters.
503-
let align =
504-
if M::enforce_alignment(self).should_check() { mplace.align } else { Align::ONE };
503+
let align = if M::enforce_alignment(self) { mplace.align } else { Align::ONE };
505504
self.check_ptr_access_align(mplace.ptr(), size, align, CheckInAllocMsg::DerefTest)?;
506505
Ok(())
507506
}

‎compiler/rustc_lint/src/lib.rs

+5
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,11 @@ fn register_builtins(store: &mut LintStore) {
506506
"replaced with another group of lints, see RFC \
507507
<https://rust-lang.github.io/rfcs/2145-type-privacy.html> for more information",
508508
);
509+
store.register_removed(
510+
"invalid_alignment",
511+
"converted into hard error, see PR #104616 \
512+
<https://github.com/rust-lang/rust/pull/104616> for more information",
513+
);
509514
}
510515

511516
fn register_internals(store: &mut LintStore) {

‎compiler/rustc_lint_defs/src/builtin.rs

-40
Original file line numberDiff line numberDiff line change
@@ -986,45 +986,6 @@ declare_lint! {
986986
"detects trivial casts of numeric types which could be removed"
987987
}
988988

989-
declare_lint! {
990-
/// The `invalid_alignment` lint detects dereferences of misaligned pointers during
991-
/// constant evaluation.
992-
///
993-
/// ### Example
994-
///
995-
/// ```rust,compile_fail
996-
/// #![feature(const_mut_refs)]
997-
/// const FOO: () = unsafe {
998-
/// let x = &[0_u8; 4];
999-
/// let y = x.as_ptr().cast::<u32>();
1000-
/// let mut z = 123;
1001-
/// y.copy_to_nonoverlapping(&mut z, 1); // the address of a `u8` array is unknown
1002-
/// // and thus we don't know if it is aligned enough for copying a `u32`.
1003-
/// };
1004-
/// ```
1005-
///
1006-
/// {{produces}}
1007-
///
1008-
/// ### Explanation
1009-
///
1010-
/// The compiler allowed dereferencing raw pointers irrespective of alignment
1011-
/// during const eval due to the const evaluator at the time not making it easy
1012-
/// or cheap to check. Now that it is both, this is not accepted anymore.
1013-
///
1014-
/// Since it was undefined behaviour to begin with, this breakage does not violate
1015-
/// Rust's stability guarantees. Using undefined behaviour can cause arbitrary
1016-
/// behaviour, including failure to build.
1017-
///
1018-
/// [future-incompatible]: ../index.md#future-incompatible-lints
1019-
pub INVALID_ALIGNMENT,
1020-
Deny,
1021-
"raw pointers must be aligned before dereferencing",
1022-
@future_incompatible = FutureIncompatibleInfo {
1023-
reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
1024-
reference: "issue #68585 <https://github.com/rust-lang/rust/issues/104616>",
1025-
};
1026-
}
1027-
1028989
declare_lint! {
1029990
/// The `exported_private_dependencies` lint detects private dependencies
1030991
/// that are exposed in a public interface.
@@ -3430,7 +3391,6 @@ declare_lint_pass! {
34303391
INDIRECT_STRUCTURAL_MATCH,
34313392
INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
34323393
INLINE_NO_SANITIZE,
3433-
INVALID_ALIGNMENT,
34343394
INVALID_DOC_ATTRIBUTES,
34353395
INVALID_MACRO_EXPORT_ARGUMENTS,
34363396
INVALID_TYPE_PARAM_DEFAULT,

0 commit comments

Comments
 (0)