Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

always check alignment during CTFE #104616

Merged
merged 11 commits into from
Dec 15, 2022
135 changes: 65 additions & 70 deletions compiler/rustc_const_eval/src/const_eval/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,59 @@ impl<'tcx> ConstEvalErr<'tcx> {
self.report_decorated(tcx, message, |_| {})
}

#[instrument(level = "trace", skip(self, decorate))]
pub(super) fn decorate(&self, err: &mut Diagnostic, decorate: impl FnOnce(&mut Diagnostic)) {
trace!("reporting const eval failure at {:?}", self.span);
// Add some more context for select error types.
match self.error {
InterpError::Unsupported(
UnsupportedOpInfo::ReadPointerAsBytes
| UnsupportedOpInfo::PartialPointerOverwrite(_)
| UnsupportedOpInfo::PartialPointerCopy(_),
) => {
err.help("this code performed an operation that depends on the underlying bytes representing a pointer");
err.help("the absolute address of a pointer is not known at compile-time, so such operations are not supported");
}
_ => {}
}
// Add spans for the stacktrace. Don't print a single-line backtrace though.
if self.stacktrace.len() > 1 {
// Helper closure to print duplicated lines.
let mut flush_last_line = |last_frame, times| {
if let Some((line, span)) = last_frame {
err.span_note(span, &line);
// Don't print [... additional calls ...] if the number of lines is small
if times < 3 {
for _ in 0..times {
err.span_note(span, &line);
}
} else {
err.span_note(
span,
format!("[... {} additional calls {} ...]", times, &line),
);
}
}
};

let mut last_frame = None;
let mut times = 0;
for frame_info in &self.stacktrace {
let frame = (frame_info.to_string(), frame_info.span);
if last_frame.as_ref() == Some(&frame) {
times += 1;
} else {
flush_last_line(last_frame, times);
last_frame = Some(frame);
times = 0;
}
}
flush_last_line(last_frame, times);
}
// Let the caller attach any additional information it wants.
decorate(err);
}

/// Create a diagnostic for this const eval error.
///
/// Sets the message passed in via `message` and adds span labels with detailed error
Expand All @@ -101,88 +154,30 @@ impl<'tcx> ConstEvalErr<'tcx> {
message: &str,
decorate: impl FnOnce(&mut Diagnostic),
) -> ErrorHandled {
let finish = |err: &mut Diagnostic, span_msg: Option<String>| {
trace!("reporting const eval failure at {:?}", self.span);
if let Some(span_msg) = span_msg {
err.span_label(self.span, span_msg);
}
// Add some more context for select error types.
match self.error {
InterpError::Unsupported(
UnsupportedOpInfo::ReadPointerAsBytes
| UnsupportedOpInfo::PartialPointerOverwrite(_)
| UnsupportedOpInfo::PartialPointerCopy(_),
) => {
err.help("this code performed an operation that depends on the underlying bytes representing a pointer");
err.help("the absolute address of a pointer is not known at compile-time, so such operations are not supported");
}
_ => {}
}
// Add spans for the stacktrace. Don't print a single-line backtrace though.
if self.stacktrace.len() > 1 {
// Helper closure to print duplicated lines.
let mut flush_last_line = |last_frame, times| {
if let Some((line, span)) = last_frame {
err.span_note(span, &line);
// Don't print [... additional calls ...] if the number of lines is small
if times < 3 {
for _ in 0..times {
err.span_note(span, &line);
}
} else {
err.span_note(
span,
format!("[... {} additional calls {} ...]", times, &line),
);
}
}
};

let mut last_frame = None;
let mut times = 0;
for frame_info in &self.stacktrace {
let frame = (frame_info.to_string(), frame_info.span);
if last_frame.as_ref() == Some(&frame) {
times += 1;
} else {
flush_last_line(last_frame, times);
last_frame = Some(frame);
times = 0;
}
}
flush_last_line(last_frame, times);
}
// Let the caller attach any additional information it wants.
decorate(err);
};

debug!("self.error: {:?}", self.error);
// Special handling for certain errors
match &self.error {
// Don't emit a new diagnostic for these errors
err_inval!(Layout(LayoutError::Unknown(_))) | err_inval!(TooGeneric) => {
return ErrorHandled::TooGeneric;
}
err_inval!(AlreadyReported(error_reported)) => {
return ErrorHandled::Reported(*error_reported);
ErrorHandled::TooGeneric
}
err_inval!(AlreadyReported(error_reported)) => ErrorHandled::Reported(*error_reported),
err_inval!(Layout(LayoutError::SizeOverflow(_))) => {
// We must *always* hard error on these, even if the caller wants just a lint.
// The `message` makes little sense here, this is a more serious error than the
// caller thinks anyway.
// See <https://github.com/rust-lang/rust/pull/63152>.
let mut err = struct_error(tcx, &self.error.to_string());
finish(&mut err, None);
return ErrorHandled::Reported(err.emit());
self.decorate(&mut err, decorate);
ErrorHandled::Reported(err.emit())
}
_ => {}
};

let err_msg = self.error.to_string();

// Report as hard error.
let mut err = struct_error(tcx, message);
finish(&mut err, Some(err_msg));
ErrorHandled::Reported(err.emit())
_ => {
// Report as hard error.
let mut err = struct_error(tcx, message);
err.span_label(self.span, self.error.to_string());
self.decorate(&mut err, decorate);
ErrorHandled::Reported(err.emit())
}
}
}
}
15 changes: 8 additions & 7 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::const_eval::CheckAlignment;
use std::borrow::Cow;

use either::{Left, Right};
Expand Down Expand Up @@ -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

Expand All @@ -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),
)
}

Expand Down Expand Up @@ -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:*/ tcx.sess.opts.unstable_opts.extra_const_ub_checks,
if tcx.sess.opts.unstable_opts.extra_const_ub_checks {
CheckAlignment::Error
} else {
CheckAlignment::FutureIncompat
},
),
);

Expand Down
59 changes: 55 additions & 4 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use rustc_hir::def::DefKind;
use rustc_hir::LangItem;
use rustc_hir::{LangItem, CRATE_HIR_ID};
use rustc_middle::mir;
use rustc_middle::mir::interpret::PointerArithmetic;
use rustc_middle::ty::layout::FnAbiOf;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_session::lint::builtin::INVALID_ALIGNMENT;
use std::borrow::Borrow;
use std::hash::Hash;
use std::ops::ControlFlow;
Expand Down Expand Up @@ -47,14 +48,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,
Expand Down Expand Up @@ -309,7 +330,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
}

Expand All @@ -318,6 +339,36 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
ecx.tcx.sess.opts.unstable_opts.extra_const_ub_checks
}

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 err = ConstEvalErr::new(ecx, err, None);
ecx.tcx.struct_span_lint_hir(
INVALID_ALIGNMENT,
ecx.stack().iter().find_map(|frame| frame.lint_root()).unwrap_or(CRATE_HIR_ID),
err.span,
err.error.to_string(),
|db| {
err.decorate(db, |_| {});
db
},
);
Ok(())
}
}
}

fn load_mir(
ecx: &InterpCx<'mir, 'tcx, Self>,
instance: ty::InstanceDef<'tcx>,
Expand Down
16 changes: 10 additions & 6 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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 });
Expand Down
13 changes: 11 additions & 2 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
use rustc_middle::mir;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::def_id::DefId;
use rustc_target::abi::Size;
use rustc_target::abi::{Align, 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,
Expand Down Expand Up @@ -122,14 +124,21 @@ 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.
///
/// 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
fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;

Expand Down
Loading