Make alignment checks a future incompat lint

This commit is contained in:
Oli Scherer 2022-11-21 16:51:16 +00:00
parent ed71e32e14
commit d66824dbc4
13 changed files with 180 additions and 70 deletions

View File

@ -1,3 +1,4 @@
use crate::const_eval::CheckAlignment;
use std::borrow::Cow;
use either::{Left, Right};
@ -76,7 +77,7 @@ fn eval_body_using_ecx<'mir, 'tcx>(
None => InternKind::Constant,
}
};
ecx.machine.check_alignment = false; // interning doesn't need to respect alignment
ecx.machine.check_alignment = CheckAlignment::No; // interning doesn't need to respect alignment
intern_const_alloc_recursive(ecx, intern_kind, &ret)?;
// we leave alignment checks off, since this `ecx` will not be used for further evaluation anyway
@ -102,11 +103,7 @@ pub(super) fn mk_eval_cx<'mir, 'tcx>(
tcx,
root_span,
param_env,
CompileTimeInterpreter::new(
tcx.const_eval_limit(),
can_access_statics,
/*check_alignment:*/ false,
),
CompileTimeInterpreter::new(tcx.const_eval_limit(), can_access_statics, CheckAlignment::No),
)
}
@ -311,7 +308,11 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
CompileTimeInterpreter::new(
tcx.const_eval_limit(),
/*can_access_statics:*/ is_static,
/*check_alignment:*/ true,
if tcx.sess.opts.unstable_opts.extra_const_ub_checks {
CheckAlignment::Error
} else {
CheckAlignment::FutureIncompat
},
),
);

View File

@ -47,14 +47,34 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {
pub(super) can_access_statics: bool,
/// Whether to check alignment during evaluation.
pub(super) check_alignment: bool,
pub(super) check_alignment: CheckAlignment,
}
#[derive(Copy, Clone)]
pub enum CheckAlignment {
/// Ignore alignment when following relocations.
/// 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,
}
}
}
impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {
pub(crate) fn new(
const_eval_limit: Limit,
can_access_statics: bool,
check_alignment: bool,
check_alignment: CheckAlignment,
) -> Self {
CompileTimeInterpreter {
steps_remaining: const_eval_limit.0,
@ -309,7 +329,7 @@ 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>) -> bool {
fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> CheckAlignment {
ecx.machine.check_alignment
}

View File

@ -248,6 +248,15 @@ impl<'mir, 'tcx, Prov: Provenance, Extra> Frame<'mir, 'tcx, Prov, Extra> {
Right(span) => span,
}
}
pub fn lint_root(&self) -> Option<hir::HirId> {
self.current_source_info().and_then(|source_info| {
match &self.body.source_scopes[source_info.scope].local_data {
mir::ClearCrossCrate::Set(data) => Some(data.lint_root),
mir::ClearCrossCrate::Clear => None,
}
})
}
}
impl<'tcx> fmt::Display for FrameInfo<'tcx> {
@ -954,12 +963,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// This deliberately does *not* honor `requires_caller_location` since it is used for much
// more than just panics.
for frame in stack.iter().rev() {
let lint_root = frame.current_source_info().and_then(|source_info| {
match &frame.body.source_scopes[source_info.scope].local_data {
mir::ClearCrossCrate::Set(data) => Some(data.lint_root),
mir::ClearCrossCrate::Clear => None,
}
});
let lint_root = frame.lint_root();
let span = frame.current_span();
frames.push(FrameInfo { span, instance: frame.instance, lint_root });

View File

@ -13,6 +13,8 @@ use rustc_span::def_id::DefId;
use rustc_target::abi::Size;
use rustc_target::spec::abi::Abi as CallAbi;
use crate::const_eval::CheckAlignment;
use super::{
AllocId, AllocRange, Allocation, ConstAllocation, Frame, ImmTy, InterpCx, InterpResult,
MemoryKind, OpTy, Operand, PlaceTy, Pointer, Provenance, Scalar, StackPopUnwind,
@ -122,7 +124,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
const PANIC_ON_ALLOC_FAIL: bool;
/// Whether memory accesses should be alignment-checked.
fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> CheckAlignment;
/// 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.

View File

@ -14,10 +14,15 @@ use std::ptr;
use rustc_ast::Mutability;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir::CRATE_HIR_ID;
use rustc_middle::mir::display_allocation;
use rustc_middle::mir::interpret::UndefinedBehaviorInfo;
use rustc_middle::ty::{self, Instance, ParamEnv, Ty, TyCtxt};
use rustc_session::lint::builtin::INVALID_ALIGNMENT;
use rustc_target::abi::{Align, HasDataLayout, Size};
use crate::const_eval::CheckAlignment;
use super::{
alloc_range, AllocId, AllocMap, AllocRange, Allocation, CheckInAllocMsg, GlobalAlloc, InterpCx,
InterpResult, Machine, MayLeak, Pointer, PointerArithmetic, Provenance, Scalar,
@ -377,7 +382,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
ptr,
size,
align,
/* force_alignment_check */ true,
CheckAlignment::Error,
msg,
|alloc_id, _, _| {
let (size, align) = self.get_live_alloc_size_and_align(alloc_id)?;
@ -396,7 +401,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
ptr: Pointer<Option<M::Provenance>>,
size: Size,
align: Align,
force_alignment_check: bool,
check: CheckAlignment,
msg: CheckInAllocMsg,
alloc_size: impl FnOnce(
AllocId,
@ -404,19 +409,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
M::ProvenanceExtra,
) -> InterpResult<'tcx, (Size, Align, T)>,
) -> InterpResult<'tcx, Option<T>> {
fn check_offset_align<'tcx>(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();
throw_ub!(AlignmentCheckFailed {
has: Align::from_bytes(offset_pow2).unwrap(),
required: align,
})
}
}
Ok(match self.ptr_try_get_alloc_id(ptr) {
Err(addr) => {
// We couldn't get a proper allocation. This is only okay if the access size is 0,
@ -425,8 +417,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
throw_ub!(DanglingIntPointer(addr, msg));
}
// Must be aligned.
if force_alignment_check {
check_offset_align(addr, align)?;
if check.should_check() {
self.check_offset_align(addr, align, check)?;
}
None
}
@ -449,16 +441,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 force_alignment_check {
if check.should_check() {
if M::use_addr_for_alignment_check(self) {
// `use_addr_for_alignment_check` can only be true if `OFFSET_IS_ADDR` is true.
check_offset_align(ptr.addr().bytes(), align)?;
self.check_offset_align(ptr.addr().bytes(), align, check)?;
} else {
// Check allocation alignment and offset alignment.
if alloc_align.bytes() < align.bytes() {
throw_ub!(AlignmentCheckFailed { has: alloc_align, required: align });
self.alignment_check_failed(alloc_align, align, check)?;
}
check_offset_align(offset.bytes(), align)?;
self.check_offset_align(offset.bytes(), align, check)?;
}
}
@ -468,6 +460,55 @@ 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> {
if offset % align.bytes() == 0 {
Ok(())
} else {
// The biggest power of two through which `offset` is divisible.
let offset_pow2 = 1 << offset.trailing_zeros();
self.alignment_check_failed(Align::from_bytes(offset_pow2).unwrap(), align, check)
}
}
fn alignment_check_failed(
&self,
has: Align,
required: Align,
check: CheckAlignment,
) -> InterpResult<'tcx, ()> {
match check {
CheckAlignment::Error => {
throw_ub!(AlignmentCheckFailed { has, required })
}
CheckAlignment::No => span_bug!(
self.cur_span(),
"`alignment_check_failed` called when no alignment check requested"
),
CheckAlignment::FutureIncompat => self.tcx.struct_span_lint_hir(
INVALID_ALIGNMENT,
self.stack().iter().find_map(|frame| frame.lint_root()).unwrap_or(CRATE_HIR_ID),
self.cur_span(),
UndefinedBehaviorInfo::AlignmentCheckFailed { has, required }.to_string(),
|db| {
let mut stacktrace = self.generate_stacktrace();
// Filter out `requires_caller_location` frames.
stacktrace
.retain(|frame| !frame.instance.def.requires_caller_location(*self.tcx));
for frame in stacktrace {
db.span_label(frame.span, format!("inside `{}`", frame.instance));
}
db
},
),
}
Ok(())
}
}
/// Allocation accessors

View File

@ -364,13 +364,8 @@ where
.size_and_align_of_mplace(&mplace)?
.unwrap_or((mplace.layout.size, mplace.layout.align.abi));
assert!(mplace.align <= align, "dynamic alignment less strict than static one?");
let align = M::enforce_alignment(self).then_some(align);
self.check_ptr_access_align(
mplace.ptr,
size,
align.unwrap_or(Align::ONE),
CheckInAllocMsg::DerefTest,
)?;
let align = if M::enforce_alignment(self).should_check() { align } else { Align::ONE };
self.check_ptr_access_align(mplace.ptr, size, align, CheckInAllocMsg::DerefTest)?;
Ok(())
}

View File

@ -3,7 +3,7 @@ use rustc_middle::ty::{ParamEnv, TyCtxt};
use rustc_session::Limit;
use rustc_target::abi::{Abi, FieldsShape, InitKind, Scalar, Variants};
use crate::const_eval::CompileTimeInterpreter;
use crate::const_eval::{CheckAlignment, CompileTimeInterpreter};
use crate::interpret::{InterpCx, MemoryKind, OpTy};
/// Determines if this type permits "raw" initialization by just transmuting some memory into an
@ -41,7 +41,7 @@ fn might_permit_raw_init_strict<'tcx>(
let machine = CompileTimeInterpreter::new(
Limit::new(0),
/*can_access_statics:*/ false,
/*check_alignment:*/ true,
CheckAlignment::Error,
);
let mut cx = InterpCx::new(tcx, rustc_span::DUMMY_SP, ParamEnv::reveal_all(), machine);

View File

@ -1019,6 +1019,42 @@ declare_lint! {
};
}
declare_lint! {
/// The `invalid_alignment` lint detects dereferences of misaligned pointers during
/// constant evluation.
///
/// ### Example
///
/// ```rust,compile_fail
/// const FOO: () = unsafe {
/// let x = [0_u8; 10];
/// let y = x.as_ptr() as *const u32;
/// *y; // the address of a `u8` array is unknown and thus we don't know if
/// // it is aligned enough for reading 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 {
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.

View File

@ -6,6 +6,7 @@ use std::cell::Cell;
use either::Right;
use rustc_ast::Mutability;
use rustc_const_eval::const_eval::CheckAlignment;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::def::DefKind;
use rustc_index::bit_set::BitSet;
@ -186,10 +187,10 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
type MemoryKind = !;
#[inline(always)]
fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
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`.
false
CheckAlignment::No
}
#[inline(always)]

View File

@ -2,6 +2,7 @@
//!
//! Currently, this pass only propagates scalar values.
use rustc_const_eval::const_eval::CheckAlignment;
use rustc_const_eval::interpret::{ConstValue, ImmTy, Immediate, InterpCx, Scalar};
use rustc_data_structures::fx::FxHashMap;
use rustc_middle::mir::visit::{MutVisitor, Visitor};
@ -448,7 +449,7 @@ impl<'mir, 'tcx> rustc_const_eval::interpret::Machine<'mir, 'tcx> for DummyMachi
type MemoryKind = !;
const PANIC_ON_ALLOC_FAIL: bool = true;
fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> CheckAlignment {
unimplemented!()
}

View File

@ -148,20 +148,22 @@ LL | const DATA_FN_PTR: fn() = unsafe { mem::transmute(&13) };
╾─alloc41─╼ │ ╾──╼
}
error[E0080]: evaluation of constant value failed
error: accessing memory with alignment 1, but alignment 4 is required
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
= note: accessing memory with alignment 1, but alignment 4 is required
|
note: inside `std::ptr::read::<u32>`
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
note: inside `ptr::const_ptr::<impl *const u32>::read`
= note: inside `std::ptr::read::<u32>`
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
note: inside `UNALIGNED_READ`
--> $DIR/ub-ref-ptr.rs:65:5
|
= note: inside `ptr::const_ptr::<impl *const u32>::read`
|
::: $DIR/ub-ref-ptr.rs:65:5
|
LL | ptr.read();
| ^^^^^^^^^^
| ---------- inside `UNALIGNED_READ`
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #68585 <https://github.com/rust-lang/rust/issues/104616>
= note: `#[deny(invalid_alignment)]` on by default
error: aborting due to 15 previous errors

View File

@ -148,20 +148,22 @@ LL | const DATA_FN_PTR: fn() = unsafe { mem::transmute(&13) };
╾───────alloc41───────╼ │ ╾──────╼
}
error[E0080]: evaluation of constant value failed
error: accessing memory with alignment 1, but alignment 4 is required
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
= note: accessing memory with alignment 1, but alignment 4 is required
|
note: inside `std::ptr::read::<u32>`
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
note: inside `ptr::const_ptr::<impl *const u32>::read`
= note: inside `std::ptr::read::<u32>`
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
note: inside `UNALIGNED_READ`
--> $DIR/ub-ref-ptr.rs:65:5
|
= note: inside `ptr::const_ptr::<impl *const u32>::read`
|
::: $DIR/ub-ref-ptr.rs:65:5
|
LL | ptr.read();
| ^^^^^^^^^^
| ---------- inside `UNALIGNED_READ`
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #68585 <https://github.com/rust-lang/rust/issues/104616>
= note: `#[deny(invalid_alignment)]` on by default
error: aborting due to 15 previous errors

View File

@ -24,6 +24,7 @@ use rustc_span::def_id::{CrateNum, DefId};
use rustc_span::Symbol;
use rustc_target::abi::Size;
use rustc_target::spec::abi::Abi;
use rustc_const_eval::const_eval::CheckAlignment;
use crate::{
concurrency::{data_race, weak_memory},
@ -752,8 +753,12 @@ 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>) -> bool {
ecx.machine.check_alignment != AlignmentCheck::None
fn enforce_alignment(ecx: &MiriInterpCx<'mir, 'tcx>) -> CheckAlignment {
if ecx.machine.check_alignment == AlignmentCheck::None {
CheckAlignment::No
} else {
CheckAlignment::Error
}
}
#[inline(always)]