diff options
Diffstat (limited to 'compiler/rustc_const_eval/src')
33 files changed, 527 insertions, 358 deletions
diff --git a/compiler/rustc_const_eval/src/const_eval/error.rs b/compiler/rustc_const_eval/src/const_eval/error.rs index c60d6e4fe..0579f7815 100644 --- a/compiler/rustc_const_eval/src/const_eval/error.rs +++ b/compiler/rustc_const_eval/src/const_eval/error.rs @@ -36,16 +36,16 @@ impl<'tcx> Into<InterpErrorInfo<'tcx>> for ConstEvalErrKind { impl fmt::Display for ConstEvalErrKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { use self::ConstEvalErrKind::*; - match *self { + match self { ConstAccessesStatic => write!(f, "constant accesses static"), ModifiedGlobal => { write!(f, "modifying a static's initial value from another static's initializer") } - AssertFailure(ref msg) => write!(f, "{:?}", msg), + AssertFailure(msg) => write!(f, "{:?}", msg), Panic { msg, line, col, file } => { write!(f, "the evaluated program panicked at '{}', {}:{}:{}", msg, file, line, col) } - Abort(ref msg) => write!(f, "{}", msg), + Abort(msg) => write!(f, "{}", msg), } } } @@ -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 @@ -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()) + } + } } } diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index c27790d88..18e01567c 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -1,5 +1,5 @@ +use crate::const_eval::CheckAlignment; use std::borrow::Cow; -use std::convert::TryInto; use either::{Left, Right}; @@ -77,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 @@ -103,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), ) } @@ -312,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 + }, ), ); diff --git a/compiler/rustc_const_eval/src/const_eval/fn_queries.rs b/compiler/rustc_const_eval/src/const_eval/fn_queries.rs index f1674d04f..351c70130 100644 --- a/compiler/rustc_const_eval/src/const_eval/fn_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/fn_queries.rs @@ -41,6 +41,7 @@ fn constness(tcx: TyCtxt<'_>, def_id: DefId) -> hir::Constness { }; if is_const { hir::Constness::Const } else { hir::Constness::NotConst } } + hir::Node::Expr(e) if let hir::ExprKind::Closure(c) = e.kind => c.constness, _ => { if let Some(fn_kind) = node.fn_kind() { if fn_kind.constness() == hir::Constness::Const { diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 3dfded2d9..4709514c8 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -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; @@ -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, @@ -204,7 +225,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> { /// `align_offset(ptr, target_align)` needs special handling in const eval, because the pointer /// may not have an address. /// - /// If `ptr` does have a known address, then we return `CONTINUE` and the function call should + /// If `ptr` does have a known address, then we return `Continue(())` and the function call should /// proceed as normal. /// /// If `ptr` doesn't have an address, but its underlying allocation's alignment is at most @@ -252,18 +273,18 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> { ret, StackPopUnwind::NotAllowed, )?; - Ok(ControlFlow::BREAK) + Ok(ControlFlow::Break(())) } else { // Not alignable in const, return `usize::MAX`. let usize_max = Scalar::from_machine_usize(self.machine_usize_max(), self); self.write_scalar(usize_max, dest)?; self.return_to_block(ret)?; - Ok(ControlFlow::BREAK) + Ok(ControlFlow::Break(())) } } Err(_addr) => { // The pointer has an address, continue with function call. - Ok(ControlFlow::CONTINUE) + Ok(ControlFlow::Continue(())) } } } @@ -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 } @@ -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>, @@ -357,7 +408,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, // Only check non-glue functions if let ty::InstanceDef::Item(def) = instance.def { // Execution might have wandered off into other crates, so we cannot do a stability- - // sensitive check here. But we can at least rule out functions that are not const + // sensitive check here. But we can at least rule out functions that are not const // at all. if !ecx.tcx.is_const_fn_raw(def.did) { // allow calling functions inside a trait marked with #[const_trait]. @@ -482,7 +533,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, let eval_to_int = |op| ecx.read_immediate(&ecx.eval_operand(op, None)?).map(|x| x.to_const_int()); let err = match msg { - BoundsCheck { ref len, ref index } => { + BoundsCheck { len, index } => { let len = eval_to_int(len)?; let index = eval_to_int(index)?; BoundsCheck { len, index } diff --git a/compiler/rustc_const_eval/src/const_eval/valtrees.rs b/compiler/rustc_const_eval/src/const_eval/valtrees.rs index f4da11883..498c00873 100644 --- a/compiler/rustc_const_eval/src/const_eval/valtrees.rs +++ b/compiler/rustc_const_eval/src/const_eval/valtrees.rs @@ -142,12 +142,11 @@ pub(crate) fn const_to_valtree_inner<'tcx>( | ty::Foreign(..) | ty::Infer(ty::FreshIntTy(_)) | ty::Infer(ty::FreshFloatTy(_)) - | ty::Projection(..) + // FIXME(oli-obk): we could look behind opaque types + | ty::Alias(..) | ty::Param(_) | ty::Bound(..) | ty::Placeholder(..) - // FIXME(oli-obk): we could look behind opaque types - | ty::Opaque(..) | ty::Infer(_) // FIXME(oli-obk): we can probably encode closures just like structs | ty::Closure(..) @@ -307,11 +306,10 @@ pub fn valtree_to_const_value<'tcx>( | ty::Foreign(..) | ty::Infer(ty::FreshIntTy(_)) | ty::Infer(ty::FreshFloatTy(_)) - | ty::Projection(..) + | ty::Alias(..) | ty::Param(_) | ty::Bound(..) | ty::Placeholder(..) - | ty::Opaque(..) | ty::Infer(_) | ty::Closure(..) | ty::Generator(..) diff --git a/compiler/rustc_const_eval/src/interpret/cast.rs b/compiler/rustc_const_eval/src/interpret/cast.rs index 269ae15d4..b2c847d3f 100644 --- a/compiler/rustc_const_eval/src/interpret/cast.rs +++ b/compiler/rustc_const_eval/src/interpret/cast.rs @@ -1,5 +1,4 @@ use std::assert_matches::assert_matches; -use std::convert::TryFrom; use rustc_apfloat::ieee::{Double, Single}; use rustc_apfloat::{Float, FloatConvert}; @@ -333,7 +332,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Immediate::new_slice(ptr, length.eval_usize(*self.tcx, self.param_env), self); self.write_immediate(val, dest) } - (&ty::Dynamic(ref data_a, ..), &ty::Dynamic(ref data_b, ..)) => { + (ty::Dynamic(data_a, ..), ty::Dynamic(data_b, ..)) => { let val = self.read_immediate(src)?; if data_a.principal() == data_b.principal() { // A NOP cast that doesn't actually change anything, should be allowed even with mismatching vtables. @@ -348,7 +347,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let new_vptr = self.get_vtable_ptr(ty, data_b.principal())?; self.write_immediate(Immediate::new_dyn_trait(old_data, new_vptr, self), dest) } - (_, &ty::Dynamic(ref data, _, ty::Dyn)) => { + (_, &ty::Dynamic(data, _, ty::Dyn)) => { // Initial cast from sized to dyn trait let vtable = self.get_vtable_ptr(src_pointee_ty, data.principal())?; let ptr = self.read_scalar(src)?; diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 0b2809f1d..d13fed7a9 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -196,7 +196,7 @@ impl<'tcx, Prov: Provenance + 'static> LocalState<'tcx, Prov> { } } - /// Overwrite the local. If the local can be overwritten in place, return a reference + /// Overwrite the local. If the local can be overwritten in place, return a reference /// to do so; otherwise return the `MemPlace` to consult instead. /// /// Note: This may only be invoked from the `Machine::access_local_mut` hook and not from @@ -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> { @@ -583,7 +592,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ); // Recurse to get the size of the dynamically sized field (must be - // the last field). Can't have foreign types here, how would we + // the last field). Can't have foreign types here, how would we // adjust alignment and size for them? let field = layout.field(self, layout.fields.count() - 1); let Some((unsized_size, mut unsized_align)) = self.size_and_align_of(metadata, &field)? else { @@ -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 }); diff --git a/compiler/rustc_const_eval/src/interpret/intern.rs b/compiler/rustc_const_eval/src/interpret/intern.rs index 458cc6180..54528b1db 100644 --- a/compiler/rustc_const_eval/src/interpret/intern.rs +++ b/compiler/rustc_const_eval/src/interpret/intern.rs @@ -59,7 +59,7 @@ struct InternVisitor<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx, const_ev #[derive(Copy, Clone, Debug, PartialEq, Hash, Eq)] enum InternMode { - /// A static and its current mutability. Below shared references inside a `static mut`, + /// A static and its current mutability. Below shared references inside a `static mut`, /// this is *immutable*, and below mutable references inside an `UnsafeCell`, this /// is *mutable*. Static(hir::Mutability), @@ -296,7 +296,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory } } InternMode::Const => { - // Ignore `UnsafeCell`, everything is immutable. Validity does some sanity + // Ignore `UnsafeCell`, everything is immutable. Validity does some sanity // checking for mutable references that we encounter -- they must all be // ZST. InternMode::Const @@ -330,7 +330,7 @@ pub enum InternKind { /// Intern `ret` and everything it references. /// -/// This *cannot raise an interpreter error*. Doing so is left to validation, which +/// This *cannot raise an interpreter error*. Doing so is left to validation, which /// tracks where in the value we are and thus can show much better error messages. #[instrument(level = "debug", skip(ecx))] pub fn intern_const_alloc_recursive< @@ -379,7 +379,7 @@ pub fn intern_const_alloc_recursive< inside_unsafe_cell: false, } .visit_value(&mplace); - // We deliberately *ignore* interpreter errors here. When there is a problem, the remaining + // We deliberately *ignore* interpreter errors here. When there is a problem, the remaining // references are "leftover"-interned, and later validation will show a proper error // and point at the right part of the value causing the problem. match res { @@ -454,7 +454,7 @@ pub fn intern_const_alloc_recursive< return Err(reported); } else if ecx.tcx.try_get_global_alloc(alloc_id).is_none() { // We have hit an `AllocId` that is neither in local or global memory and isn't - // marked as dangling by local memory. That should be impossible. + // marked as dangling by local memory. That should be impossible. span_bug!(ecx.tcx.span, "encountered unknown alloc id {:?}", alloc_id); } } diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index 7940efcd2..cc7b6c91b 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -2,8 +2,6 @@ //! looking at their MIR. Intrinsics/functions supported here are shared by CTFE //! and miri. -use std::convert::TryFrom; - use rustc_hir::def_id::DefId; use rustc_middle::mir::{ self, @@ -81,14 +79,10 @@ pub(crate) fn eval_nullary_intrinsic<'tcx>( } sym::variant_count => match tp_ty.kind() { // Correctly handles non-monomorphic calls, so there is no need for ensure_monomorphic_enough. - ty::Adt(ref adt, _) => { - ConstValue::from_machine_usize(adt.variants().len() as u64, &tcx) + ty::Adt(adt, _) => ConstValue::from_machine_usize(adt.variants().len() as u64, &tcx), + ty::Alias(..) | ty::Param(_) | ty::Placeholder(_) | ty::Infer(_) => { + throw_inval!(TooGeneric) } - ty::Projection(_) - | ty::Opaque(_, _) - | ty::Param(_) - | ty::Placeholder(_) - | ty::Infer(_) => throw_inval!(TooGeneric), ty::Bound(_, _) => bug!("bound ty during ctfe"), ty::Bool | ty::Char @@ -307,7 +301,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } sym::offset => { let ptr = self.read_pointer(&args[0])?; - let offset_count = self.read_scalar(&args[1])?.to_machine_isize(self)?; + let offset_count = self.read_machine_isize(&args[1])?; let pointee_ty = substs.type_at(0); let offset_ptr = self.ptr_offset_inbounds(ptr, pointee_ty, offset_count)?; @@ -315,7 +309,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } sym::arith_offset => { let ptr = self.read_pointer(&args[0])?; - let offset_count = self.read_scalar(&args[1])?.to_machine_isize(self)?; + let offset_count = self.read_machine_isize(&args[1])?; let pointee_ty = substs.type_at(0); let pointee_size = i64::try_from(self.layout_of(pointee_ty)?.size.bytes()).unwrap(); @@ -432,7 +426,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { sym::transmute => { self.copy_op(&args[0], dest, /*allow_transmute*/ true)?; } - sym::assert_inhabited | sym::assert_zero_valid | sym::assert_uninit_valid => { + sym::assert_inhabited + | sym::assert_zero_valid + | sym::assert_mem_uninitialized_valid => { let ty = instance.substs.type_at(0); let layout = self.layout_of(ty)?; @@ -464,7 +460,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } - if intrinsic_name == sym::assert_uninit_valid { + if intrinsic_name == sym::assert_mem_uninitialized_valid { let should_panic = !self.tcx.permits_uninit_init(layout); if should_panic { @@ -672,7 +668,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { count: &OpTy<'tcx, <M as Machine<'mir, 'tcx>>::Provenance>, nonoverlapping: bool, ) -> InterpResult<'tcx> { - let count = self.read_scalar(&count)?.to_machine_usize(self)?; + let count = self.read_machine_usize(&count)?; let layout = self.layout_of(src.layout.ty.builtin_deref(true).unwrap().ty)?; let (size, align) = (layout.size, layout.align.abi); // `checked_mul` enforces a too small bound (the correct one would probably be machine_isize_max), @@ -700,7 +696,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let dst = self.read_pointer(&dst)?; let byte = self.read_scalar(&byte)?.to_u8()?; - let count = self.read_scalar(&count)?.to_machine_usize(self)?; + let count = self.read_machine_usize(&count)?; // `checked_mul` enforces a too small bound (the correct one would probably be machine_isize_max), // but no actual allocation can be big enough for the difference to be noticeable. diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics/caller_location.rs b/compiler/rustc_const_eval/src/interpret/intrinsics/caller_location.rs index 7d94a22c4..77c7b4bac 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics/caller_location.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics/caller_location.rs @@ -1,5 +1,3 @@ -use std::convert::TryFrom; - use rustc_ast::Mutability; use rustc_hir::lang_items::LangItem; use rustc_middle::mir::TerminatorKind; diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index 0604d5ee6..248953de8 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -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, @@ -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. @@ -130,6 +132,13 @@ pub trait Machine<'mir, 'tcx>: 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 fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool; @@ -171,7 +180,7 @@ pub trait Machine<'mir, 'tcx>: Sized { unwind: StackPopUnwind, ) -> InterpResult<'tcx, Option<(&'mir mir::Body<'tcx>, ty::Instance<'tcx>)>>; - /// Execute `fn_val`. It is the hook's responsibility to advance the instruction + /// Execute `fn_val`. It is the hook's responsibility to advance the instruction /// pointer as appropriate. fn call_extra_fn( ecx: &mut InterpCx<'mir, 'tcx, Self>, @@ -430,7 +439,7 @@ pub trait Machine<'mir, 'tcx>: Sized { } /// A lot of the flexibility above is just needed for `Miri`, but all "compile-time" machines -/// (CTFE and ConstProp) use the same instance. Here, we share that code. +/// (CTFE and ConstProp) use the same instance. Here, we share that code. pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) { type Provenance = AllocId; type ProvenanceExtra = (); diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 528c1cb06..291bfb2b5 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -18,6 +18,8 @@ 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 super::{ alloc_range, AllocId, AllocMap, AllocRange, Allocation, CheckInAllocMsg, GlobalAlloc, InterpCx, InterpResult, Machine, MayLeak, Pointer, PointerArithmetic, Provenance, Scalar, @@ -144,7 +146,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Call this to turn untagged "global" pointers (obtained via `tcx`) into - /// the machine pointer to the allocation. Must never be used + /// the machine pointer to the allocation. Must never be used /// for any other pointers, nor for TLS statics. /// /// Using the resulting pointer represents a *direct* access to that memory @@ -349,11 +351,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { size: Size, align: Align, ) -> InterpResult<'tcx, Option<(AllocId, Size, M::ProvenanceExtra)>> { - let align = M::enforce_alignment(&self).then_some(align); self.check_and_deref_ptr( ptr, size, align, + M::enforce_alignment(self), CheckInAllocMsg::MemoryAccessTest, |alloc_id, offset, prov| { let (size, align) = self.get_live_alloc_size_and_align(alloc_id)?; @@ -373,10 +375,17 @@ 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, Some(align), msg, |alloc_id, _, _| { - let (size, align) = self.get_live_alloc_size_and_align(alloc_id)?; - Ok((size, align, ())) - })?; + 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)?; + Ok((size, align, ())) + }, + )?; Ok(()) } @@ -388,7 +397,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &self, ptr: Pointer<Option<M::Provenance>>, size: Size, - align: Option<Align>, + align: Align, + check: CheckAlignment, msg: CheckInAllocMsg, alloc_size: impl FnOnce( AllocId, @@ -396,19 +406,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, @@ -417,8 +414,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { throw_ub!(DanglingIntPointer(addr, msg)); } // Must be aligned. - if let Some(align) = align { - check_offset_align(addr, align)?; + if check.should_check() { + self.check_offset_align(addr, align, check)?; } None } @@ -441,16 +438,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 let Some(align) = align { + 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 }); + M::alignment_check_failed(self, alloc_align, align, check)?; } - check_offset_align(offset.bytes(), align)?; + self.check_offset_align(offset.bytes(), align, check)?; } } @@ -460,6 +457,21 @@ 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(); + M::alignment_check_failed(self, Align::from_bytes(offset_pow2).unwrap(), align, check) + } + } } /// Allocation accessors @@ -524,7 +536,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &self, id: AllocId, ) -> InterpResult<'tcx, &Allocation<M::Provenance, M::AllocExtra>> { - // The error type of the inner closure here is somewhat funny. We have two + // The error type of the inner closure here is somewhat funny. We have two // ways of "erroring": An actual error, or because we got a reference from // `get_global_alloc` that we can actually use directly without inserting anything anywhere. // So the error type is `InterpResult<'tcx, &Allocation<M::Provenance>>`. @@ -560,11 +572,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { size: Size, align: Align, ) -> InterpResult<'tcx, Option<AllocRef<'a, 'tcx, M::Provenance, M::AllocExtra>>> { - let align = M::enforce_alignment(self).then_some(align); let ptr_and_alloc = self.check_and_deref_ptr( ptr, size, align, + M::enforce_alignment(self), CheckInAllocMsg::MemoryAccessTest, |alloc_id, offset, prov| { let alloc = self.get_alloc_raw(alloc_id)?; @@ -851,7 +863,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> std::fmt::Debug for DumpAllocs<'a, write!(fmt, "{id:?}")?; match self.ecx.memory.alloc_map.get(id) { - Some(&(kind, ref alloc)) => { + Some((kind, alloc)) => { // normal alloc write!(fmt, " ({}, ", kind)?; write_allocation_track_relocs( diff --git a/compiler/rustc_const_eval/src/interpret/operand.rs b/compiler/rustc_const_eval/src/interpret/operand.rs index 221e359d2..befc0928f 100644 --- a/compiler/rustc_const_eval/src/interpret/operand.rs +++ b/compiler/rustc_const_eval/src/interpret/operand.rs @@ -39,7 +39,7 @@ pub enum Immediate<Prov: Provenance = AllocId> { impl<Prov: Provenance> From<Scalar<Prov>> for Immediate<Prov> { #[inline(always)] fn from(val: Scalar<Prov>) -> Self { - Immediate::Scalar(val.into()) + Immediate::Scalar(val) } } @@ -53,7 +53,7 @@ impl<Prov: Provenance> Immediate<Prov> { } pub fn new_slice(val: Scalar<Prov>, len: u64, cx: &impl HasDataLayout) -> Self { - Immediate::ScalarPair(val.into(), Scalar::from_machine_usize(len, cx).into()) + Immediate::ScalarPair(val, Scalar::from_machine_usize(len, cx)) } pub fn new_dyn_trait( @@ -61,7 +61,7 @@ impl<Prov: Provenance> Immediate<Prov> { vtable: Pointer<Option<Prov>>, cx: &impl HasDataLayout, ) -> Self { - Immediate::ScalarPair(val.into(), Scalar::from_maybe_pointer(vtable, cx)) + Immediate::ScalarPair(val, Scalar::from_maybe_pointer(vtable, cx)) } #[inline] @@ -341,10 +341,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { alloc_range(b_offset, b_size), /*read_provenance*/ b.is_ptr(), )?; - Some(ImmTy { - imm: Immediate::ScalarPair(a_val.into(), b_val.into()), - layout: mplace.layout, - }) + Some(ImmTy { imm: Immediate::ScalarPair(a_val, b_val), layout: mplace.layout }) } _ => { // Neither a scalar nor scalar pair. @@ -407,6 +404,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(self.read_immediate(op)?.to_scalar()) } + // Pointer-sized reads are fairly common and need target layout access, so we wrap them in + // convenience functions. + /// Read a pointer from a place. pub fn read_pointer( &self, @@ -414,6 +414,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ) -> InterpResult<'tcx, Pointer<Option<M::Provenance>>> { self.read_scalar(op)?.to_pointer(self) } + /// Read a pointer-sized unsigned integer from a place. + pub fn read_machine_usize(&self, op: &OpTy<'tcx, M::Provenance>) -> InterpResult<'tcx, u64> { + self.read_scalar(op)?.to_machine_usize(self) + } + /// Read a pointer-sized signed integer from a place. + pub fn read_machine_isize(&self, op: &OpTy<'tcx, M::Provenance>) -> InterpResult<'tcx, i64> { + self.read_scalar(op)?.to_machine_isize(self) + } /// Turn the wide MPlace into a string (must already be dereferenced!) pub fn read_str(&self, mplace: &MPlaceTy<'tcx, M::Provenance>) -> InterpResult<'tcx, &str> { @@ -480,7 +488,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(OpTy { op, layout: place.layout, align: Some(place.align) }) } - /// Evaluate a place with the goal of reading from it. This lets us sometimes + /// Evaluate a place with the goal of reading from it. This lets us sometimes /// avoid allocations. pub fn eval_place_to_op( &self, @@ -525,11 +533,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { layout: Option<TyAndLayout<'tcx>>, ) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> { use rustc_middle::mir::Operand::*; - let op = match *mir_op { + let op = match mir_op { // FIXME: do some more logic on `move` to invalidate the old location - Copy(place) | Move(place) => self.eval_place_to_op(place, layout)?, + &Copy(place) | &Move(place) => self.eval_place_to_op(place, layout)?, - Constant(ref constant) => { + Constant(constant) => { let c = self.subst_from_current_frame_and_normalize_erasing_regions(constant.literal)?; @@ -569,8 +577,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ty::ConstKind::Unevaluated(uv) => { let instance = self.resolve(uv.def, uv.substs)?; let cid = GlobalId { instance, promoted: None }; - self.ctfe_query(span, |tcx| tcx.eval_to_valtree(self.param_env.and(cid)))? - .unwrap_or_else(|| bug!("unable to create ValTree for {uv:?}")) + self.ctfe_query(span, |tcx| { + tcx.eval_to_valtree(self.param_env.with_const().and(cid)) + })? + .unwrap_or_else(|| bug!("unable to create ValTree for {uv:?}")) } ty::ConstKind::Bound(..) | ty::ConstKind::Infer(..) => { span_bug!(self.cur_span(), "unexpected ConstKind in ctfe: {val:?}") diff --git a/compiler/rustc_const_eval/src/interpret/operator.rs b/compiler/rustc_const_eval/src/interpret/operator.rs index 1f1d06651..e8ff70e3a 100644 --- a/compiler/rustc_const_eval/src/interpret/operator.rs +++ b/compiler/rustc_const_eval/src/interpret/operator.rs @@ -1,5 +1,3 @@ -use std::convert::TryFrom; - use rustc_apfloat::Float; use rustc_middle::mir; use rustc_middle::mir::interpret::{InterpResult, Scalar}; @@ -38,7 +36,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { if let Abi::ScalarPair(..) = dest.layout.abi { // We can use the optimized path and avoid `place_field` (which might do // `force_allocation`). - let pair = Immediate::ScalarPair(val.into(), Scalar::from_bool(overflowed).into()); + let pair = Immediate::ScalarPair(val, Scalar::from_bool(overflowed)); self.write_immediate(pair, dest)?; } else { assert!(self.tcx.sess.opts.unstable_opts.randomize_layout); diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index c47cfe8bb..274af61ee 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -141,7 +141,7 @@ impl<Prov: Provenance> MemPlace<Prov> { match self.meta { MemPlaceMeta::None => Immediate::from(Scalar::from_maybe_pointer(self.ptr, cx)), MemPlaceMeta::Meta(meta) => { - Immediate::ScalarPair(Scalar::from_maybe_pointer(self.ptr, cx).into(), meta.into()) + Immediate::ScalarPair(Scalar::from_maybe_pointer(self.ptr, cx), meta) } } } @@ -233,7 +233,7 @@ impl<'tcx, Prov: Provenance> MPlaceTy<'tcx, Prov> { _ => bug!("len not supported on unsized type {:?}", self.layout.ty), } } else { - // Go through the layout. There are lots of types that support a length, + // Go through the layout. There are lots of types that support a length, // e.g., SIMD types. (But not all repr(simd) types even have FieldsShape::Array!) match self.layout.fields { abi::FieldsShape::Array { count, .. } => Ok(count), @@ -294,7 +294,7 @@ where M: Machine<'mir, 'tcx, Provenance = Prov>, { /// Take a value, which represents a (thin or wide) reference, and make it a place. - /// Alignment is just based on the type. This is the inverse of `MemPlace::to_ref()`. + /// Alignment is just based on the type. This is the inverse of `MemPlace::to_ref()`. /// /// Only call this if you are sure the place is "valid" (aligned and inbounds), or do not /// want to ever use the place for memory access! @@ -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(()) } @@ -708,7 +703,7 @@ where &mut Operand::Immediate(local_val) => { // We need to make an allocation. - // We need the layout of the local. We can NOT use the layout we got, + // We need the layout of the local. We can NOT use the layout we got, // that might e.g., be an inner field of a struct with `Scalar` layout, // that has different alignment than the outer field. let local_layout = diff --git a/compiler/rustc_const_eval/src/interpret/projection.rs b/compiler/rustc_const_eval/src/interpret/projection.rs index 2ffd73eef..291464ab5 100644 --- a/compiler/rustc_const_eval/src/interpret/projection.rs +++ b/compiler/rustc_const_eval/src/interpret/projection.rs @@ -363,7 +363,7 @@ where Index(local) => { let layout = self.layout_of(self.tcx.types.usize)?; let n = self.local_to_op(self.frame(), local, Some(layout))?; - let n = self.read_scalar(&n)?.to_machine_usize(self)?; + let n = self.read_machine_usize(&n)?; self.place_index(base, n)? } ConstantIndex { offset, min_length, from_end } => { @@ -392,7 +392,7 @@ where Index(local) => { let layout = self.layout_of(self.tcx.types.usize)?; let n = self.local_to_op(self.frame(), local, Some(layout))?; - let n = self.read_scalar(&n)?.to_machine_usize(self)?; + let n = self.read_machine_usize(&n)?; self.operand_index(base, n)? } ConstantIndex { offset, min_length, from_end } => { diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs index 81b44a494..fad4cb06c 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -111,7 +111,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { M::retag_place_contents(self, *kind, &dest)?; } - Intrinsic(box ref intrinsic) => self.emulate_nondiverging_intrinsic(intrinsic)?, + Intrinsic(box intrinsic) => self.emulate_nondiverging_intrinsic(intrinsic)?, // Statements we do not track. AscribeUserType(..) => {} @@ -163,8 +163,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.copy_op(&op, &dest, /*allow_transmute*/ false)?; } - CopyForDeref(ref place) => { - let op = self.eval_place_to_op(*place, Some(dest.layout))?; + CopyForDeref(place) => { + let op = self.eval_place_to_op(place, Some(dest.layout))?; self.copy_op(&op, &dest, /* allow_transmute*/ false)?; } diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index 57e40e168..da320cd1c 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -29,10 +29,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Goto { target } => self.go_to_block(target), - SwitchInt { ref discr, ref targets, switch_ty } => { + SwitchInt { ref discr, ref targets } => { let discr = self.read_immediate(&self.eval_operand(discr, None)?)?; trace!("SwitchInt({:?})", *discr); - assert_eq!(discr.layout.ty, switch_ty); // Branch to the `otherwise` case by default, if no match is found. let mut target_block = targets.otherwise(); @@ -120,11 +119,20 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } Drop { place, target, unwind } => { + let frame = self.frame(); + let ty = place.ty(&frame.body.local_decls, *self.tcx).ty; + let ty = self.subst_from_frame_and_normalize_erasing_regions(frame, ty)?; + let instance = Instance::resolve_drop_in_place(*self.tcx, ty); + if let ty::InstanceDef::DropGlue(_, None) = instance.def { + // This is the branch we enter if and only if the dropped type has no drop glue + // whatsoever. This can happen as a result of monomorphizing a drop of a + // generic. In order to make sure that generic and non-generic code behaves + // roughly the same (and in keeping with Mir semantics) we do nothing here. + self.go_to_block(target); + return Ok(()); + } let place = self.eval_place(place)?; - let ty = place.layout.ty; trace!("TerminatorKind::drop: {:?}, type {}", place, ty); - - let instance = Instance::resolve_drop_in_place(*self.tcx, ty); self.drop_in_place(&place, instance, target, unwind)?; } @@ -438,7 +446,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // they go to. // For where they come from: If the ABI is RustCall, we untuple the - // last incoming argument. These two iterators do not have the same type, + // last incoming argument. These two iterators do not have the same type, // so to keep the code paths uniform we accept an allocation // (for RustCall ABI only). let caller_args: Cow<'_, [OpTy<'tcx, M::Provenance>]> = @@ -473,7 +481,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { .filter(|arg_and_abi| !matches!(arg_and_abi.1.mode, PassMode::Ignore)); // Now we have to spread them out across the callee's locals, - // taking into account the `spread_arg`. If we could write + // taking into account the `spread_arg`. If we could write // this is a single iterator (that handles `spread_arg`), then // `pass_argument` would be the loop body. It takes care to // not advance `caller_iter` for ZSTs. @@ -640,8 +648,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { unwind: Option<mir::BasicBlock>, ) -> InterpResult<'tcx> { trace!("drop_in_place: {:?},\n {:?}, {:?}", *place, place.layout.ty, instance); - // We take the address of the object. This may well be unaligned, which is fine - // for us here. However, unaligned accesses will probably make the actual drop + // We take the address of the object. This may well be unaligned, which is fine + // for us here. However, unaligned accesses will probably make the actual drop // implementation fail -- a problem shared by rustc. let place = self.force_allocation(place)?; diff --git a/compiler/rustc_const_eval/src/interpret/util.rs b/compiler/rustc_const_eval/src/interpret/util.rs index 2bc521d5b..cabc65e2c 100644 --- a/compiler/rustc_const_eval/src/interpret/util.rs +++ b/compiler/rustc_const_eval/src/interpret/util.rs @@ -1,6 +1,5 @@ use rustc_middle::mir::interpret::InterpResult; use rustc_middle::ty::{self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor}; -use std::convert::TryInto; use std::ops::ControlFlow; /// Checks whether a type contains generic parameters which require substitution. @@ -27,7 +26,7 @@ where fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> { if !ty.needs_subst() { - return ControlFlow::CONTINUE; + return ControlFlow::Continue(()); } match *ty.kind() { @@ -41,16 +40,15 @@ where let index = index .try_into() .expect("more generic parameters than can fit into a `u32`"); - let is_used = unused_params.contains(index).map_or(true, |unused| !unused); // Only recurse when generic parameters in fns, closures and generators // are used and require substitution. // Just in case there are closures or generators within this subst, // recurse. - if is_used && subst.needs_subst() { + if unused_params.is_used(index) && subst.needs_subst() { return subst.visit_with(self); } } - ControlFlow::CONTINUE + ControlFlow::Continue(()) } _ => ty.super_visit_with(self), } diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs index 0e85c7d11..19e359986 100644 --- a/compiler/rustc_const_eval/src/interpret/validity.rs +++ b/compiler/rustc_const_eval/src/interpret/validity.rs @@ -4,7 +4,6 @@ //! That's useful because it means other passes (e.g. promotion) can rely on `const`s //! to be const-safe. -use std::convert::TryFrom; use std::fmt::{Display, Write}; use std::num::NonZeroUsize; @@ -176,7 +175,7 @@ fn write_path(out: &mut String, path: &[PathElem]) { TupleElem(idx) => write!(out, ".{}", idx), ArrayElem(idx) => write!(out, "[{}]", idx), // `.<deref>` does not match Rust syntax, but it is more readable for long paths -- and - // some of the other items here also are not Rust syntax. Actually we can't + // some of the other items here also are not Rust syntax. Actually we can't // even use the usual syntax because we are just showing the projections, // not the root. Deref => write!(out, ".<deref>"), @@ -420,7 +419,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' ) } // Recursive checking - if let Some(ref mut ref_tracking) = self.ref_tracking { + if let Some(ref_tracking) = self.ref_tracking.as_deref_mut() { // Proceed recursively even for ZST, no reason to skip them! // `!` is a ZST and we want to validate it. if let Ok((alloc_id, _offset, _prov)) = self.ecx.ptr_try_get_alloc_id(place.ptr) { @@ -485,7 +484,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' } /// Check if this is a value of primitive type, and if yes check the validity of the value - /// at that type. Return `true` if the type is indeed primitive. + /// at that type. Return `true` if the type is indeed primitive. fn try_visit_primitive( &mut self, value: &OpTy<'tcx, M::Provenance>, @@ -602,8 +601,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' | ty::Placeholder(..) | ty::Bound(..) | ty::Param(..) - | ty::Opaque(..) - | ty::Projection(..) + | ty::Alias(..) | ty::GeneratorWitness(..) => bug!("Encountered invalid type {:?}", ty), } } @@ -625,7 +623,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' // Can only happen during CTFE. // We support 2 kinds of ranges here: full range, and excluding zero. if start == 1 && end == max_value { - // Only null is the niche. So make sure the ptr is NOT null. + // Only null is the niche. So make sure the ptr is NOT null. if self.ecx.scalar_may_be_null(scalar)? { throw_validation_failure!(self.path, { "a potentially null pointer" } @@ -761,7 +759,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> // Recursively walk the value at its type. self.walk_value(op)?; - // *After* all of this, check the ABI. We need to check the ABI to handle + // *After* all of this, check the ABI. We need to check the ABI to handle // types like `NonNull` where the `Scalar` info is more restrictive than what // the fields say (`rustc_layout_scalar_valid_range_start`). // But in most cases, this will just propagate what the fields say, @@ -859,10 +857,10 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> // Optimization: we just check the entire range at once. // NOTE: Keep this in sync with the handling of integer and float // types above, in `visit_primitive`. - // In run-time mode, we accept pointers in here. This is actually more + // In run-time mode, we accept pointers in here. This is actually more // permissive than a per-element check would be, e.g., we accept // a &[u8] that contains a pointer even though bytewise checking would - // reject it. However, that's good: We don't inherently want + // reject it. However, that's good: We don't inherently want // to reject those pointers, we just do not have the machinery to // talk about parts of a pointer. // We also accept uninit, for consistency with the slow path. diff --git a/compiler/rustc_const_eval/src/interpret/visitor.rs b/compiler/rustc_const_eval/src/interpret/visitor.rs index 1a10851a9..f9efc2418 100644 --- a/compiler/rustc_const_eval/src/interpret/visitor.rs +++ b/compiler/rustc_const_eval/src/interpret/visitor.rs @@ -481,12 +481,12 @@ macro_rules! make_value_visitor { }; // Visit the fields of this value. - match v.layout().fields { + match &v.layout().fields { FieldsShape::Primitive => {} - FieldsShape::Union(fields) => { + &FieldsShape::Union(fields) => { self.visit_union(v, fields)?; } - FieldsShape::Arbitrary { ref offsets, .. } => { + FieldsShape::Arbitrary { offsets, .. } => { // FIXME: We collect in a vec because otherwise there are lifetime // errors: Projecting to a field needs access to `ecx`. let fields: Vec<InterpResult<'tcx, Self::V>> = diff --git a/compiler/rustc_const_eval/src/lib.rs b/compiler/rustc_const_eval/src/lib.rs index 443c01fdb..57b91df2d 100644 --- a/compiler/rustc_const_eval/src/lib.rs +++ b/compiler/rustc_const_eval/src/lib.rs @@ -6,7 +6,6 @@ Rust MIR: a lowered representation of Rust. #![feature(assert_matches)] #![feature(box_patterns)] -#![feature(control_flow_enum)] #![feature(decl_macro)] #![feature(exact_size_is_empty)] #![feature(let_chains)] @@ -20,6 +19,7 @@ Rust MIR: a lowered representation of Rust. #![feature(trusted_step)] #![feature(try_blocks)] #![feature(yeet_expr)] +#![feature(if_let_guard)] #![feature(is_some_and)] #![recursion_limit = "256"] diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index 54213d55a..79f1737e3 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -242,7 +242,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> { // impl trait is gone in MIR, so check the return type of a const fn by its signature // instead of the type of the return place. self.span = body.local_decls[RETURN_PLACE].source_info.span; - let return_ty = tcx.fn_sig(def_id).output(); + let return_ty = self.ccx.fn_sig().output(); self.check_local_or_return_ty(return_ty.skip_binder(), RETURN_PLACE); } @@ -442,7 +442,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { self.super_rvalue(rvalue, location); - match *rvalue { + match rvalue { Rvalue::ThreadLocalRef(_) => self.check_op(ops::ThreadLocalAccess), Rvalue::Use(_) @@ -451,18 +451,15 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { | Rvalue::Discriminant(..) | Rvalue::Len(_) => {} - Rvalue::Aggregate(ref kind, ..) => { - if let AggregateKind::Generator(def_id, ..) = kind.as_ref() { - if let Some(generator_kind) = self.tcx.generator_kind(def_id.to_def_id()) { - if matches!(generator_kind, hir::GeneratorKind::Async(..)) { - self.check_op(ops::Generator(generator_kind)); - } - } + Rvalue::Aggregate(kind, ..) => { + if let AggregateKind::Generator(def_id, ..) = kind.as_ref() + && let Some(generator_kind @ hir::GeneratorKind::Async(..)) = self.tcx.generator_kind(def_id.to_def_id()) + { + self.check_op(ops::Generator(generator_kind)); } } - Rvalue::Ref(_, kind @ BorrowKind::Mut { .. }, ref place) - | Rvalue::Ref(_, kind @ BorrowKind::Unique, ref place) => { + Rvalue::Ref(_, kind @ (BorrowKind::Mut { .. } | BorrowKind::Unique), place) => { let ty = place.ty(self.body, self.tcx).ty; let is_allowed = match ty.kind() { // Inside a `static mut`, `&mut [...]` is allowed. @@ -491,12 +488,12 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { } } - Rvalue::AddressOf(Mutability::Mut, ref place) => { + Rvalue::AddressOf(Mutability::Mut, place) => { self.check_mut_borrow(place.local, hir::BorrowKind::Raw) } - Rvalue::Ref(_, BorrowKind::Shared | BorrowKind::Shallow, ref place) - | Rvalue::AddressOf(Mutability::Not, ref place) => { + Rvalue::Ref(_, BorrowKind::Shared | BorrowKind::Shallow, place) + | Rvalue::AddressOf(Mutability::Not, place) => { let borrowed_place_has_mut_interior = qualifs::in_place::<HasMutInterior, _>( &self.ccx, &mut |local| self.qualifs.has_mut_interior(self.ccx, local, location), @@ -564,7 +561,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf, _) => {} Rvalue::ShallowInitBox(_, _) => {} - Rvalue::UnaryOp(_, ref operand) => { + Rvalue::UnaryOp(_, operand) => { let ty = operand.ty(self.body, self.tcx); if is_int_bool_or_char(ty) { // Int, bool, and char operations are fine. @@ -575,8 +572,8 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { } } - Rvalue::BinaryOp(op, box (ref lhs, ref rhs)) - | Rvalue::CheckedBinaryOp(op, box (ref lhs, ref rhs)) => { + Rvalue::BinaryOp(op, box (lhs, rhs)) + | Rvalue::CheckedBinaryOp(op, box (lhs, rhs)) => { let lhs_ty = lhs.ty(self.body, self.tcx); let rhs_ty = rhs.ty(self.body, self.tcx); @@ -585,13 +582,16 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { } else if lhs_ty.is_fn_ptr() || lhs_ty.is_unsafe_ptr() { assert_eq!(lhs_ty, rhs_ty); assert!( - op == BinOp::Eq - || op == BinOp::Ne - || op == BinOp::Le - || op == BinOp::Lt - || op == BinOp::Ge - || op == BinOp::Gt - || op == BinOp::Offset + matches!( + op, + BinOp::Eq + | BinOp::Ne + | BinOp::Le + | BinOp::Lt + | BinOp::Ge + | BinOp::Gt + | BinOp::Offset + ) ); self.check_op(ops::RawPtrComparison); @@ -730,6 +730,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { substs, span: *fn_span, from_hir_call: *from_hir_call, + feature: Some(sym::const_trait_impl), }); return; } @@ -782,6 +783,20 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { ); return; } + Ok(Some(ImplSource::Closure(data))) => { + if !tcx.is_const_fn_raw(data.closure_def_id) { + self.check_op(ops::FnCallNonConst { + caller, + callee, + substs, + span: *fn_span, + from_hir_call: *from_hir_call, + feature: None, + }); + + return; + } + } Ok(Some(ImplSource::UserDefined(data))) => { let callee_name = tcx.item_name(callee); if let Some(&did) = tcx @@ -802,6 +817,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { substs, span: *fn_span, from_hir_call: *from_hir_call, + feature: None, }); return; } @@ -844,6 +860,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { substs, span: *fn_span, from_hir_call: *from_hir_call, + feature: None, }); return; } @@ -903,6 +920,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { substs, span: *fn_span, from_hir_call: *from_hir_call, + feature: None, }); return; } diff --git a/compiler/rustc_const_eval/src/transform/check_consts/mod.rs b/compiler/rustc_const_eval/src/transform/check_consts/mod.rs index 655ec345e..54868e418 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/mod.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/mod.rs @@ -8,7 +8,7 @@ use rustc_attr as attr; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_middle::mir; -use rustc_middle::ty::{self, TyCtxt}; +use rustc_middle::ty::{self, PolyFnSig, TyCtxt}; use rustc_span::Symbol; pub use self::qualifs::Qualif; @@ -64,6 +64,17 @@ impl<'mir, 'tcx> ConstCx<'mir, 'tcx> { fn is_async(&self) -> bool { self.tcx.asyncness(self.def_id()).is_async() } + + pub fn fn_sig(&self) -> PolyFnSig<'tcx> { + let did = self.def_id().to_def_id(); + if self.tcx.is_closure(did) { + let ty = self.tcx.type_of(did); + let ty::Closure(_, substs) = ty.kind() else { bug!("type_of closure not ty::Closure") }; + substs.as_closure().sig() + } else { + self.tcx.fn_sig(did) + } + } } pub fn rustc_allow_const_fn_unstable( @@ -115,7 +126,7 @@ fn is_parent_const_stable_trait(tcx: TyCtxt<'_>, def_id: DefId) -> bool { let local_def_id = def_id.expect_local(); let hir_id = tcx.local_def_id_to_hir_id(local_def_id); - let Some(parent) = tcx.hir().find_parent_node(hir_id) else { return false }; + let Some(parent) = tcx.hir().opt_parent_id(hir_id) else { return false }; let parent_def = tcx.hir().get(parent); if !matches!( diff --git a/compiler/rustc_const_eval/src/transform/check_consts/ops.rs b/compiler/rustc_const_eval/src/transform/check_consts/ops.rs index b19d270e6..0cb5d2ff8 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/ops.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/ops.rs @@ -111,6 +111,7 @@ pub struct FnCallNonConst<'tcx> { pub substs: SubstsRef<'tcx>, pub span: Span, pub from_hir_call: bool, + pub feature: Option<Symbol>, } impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> { @@ -119,7 +120,7 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> { ccx: &ConstCx<'_, 'tcx>, _: Span, ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { - let FnCallNonConst { caller, callee, substs, span, from_hir_call } = *self; + let FnCallNonConst { caller, callee, substs, span, from_hir_call, feature } = *self; let ConstCx { tcx, param_env, .. } = *ccx; let diag_trait = |err, self_ty: Ty<'_>, trait_id| { @@ -318,6 +319,13 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> { ccx.const_kind(), )); + if let Some(feature) = feature && ccx.tcx.sess.is_nightly_build() { + err.help(&format!( + "add `#![feature({})]` to the crate attributes to enable", + feature, + )); + } + if let ConstContext::Static(_) = ccx.const_kind() { err.note("consider wrapping this expression in `Lazy::new(|| ...)` from the `once_cell` crate: https://crates.io/crates/once_cell"); } diff --git a/compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs b/compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs index d4570c598..cf4e875c9 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs @@ -95,7 +95,7 @@ impl<'tcx> Visitor<'tcx> for CheckLiveDrops<'_, 'tcx> { } // Drop elaboration is not precise enough to accept code like - // `src/test/ui/consts/control-flow/drop-pass.rs`; e.g., when an `Option<Vec<T>>` is + // `tests/ui/consts/control-flow/drop-pass.rs`; e.g., when an `Option<Vec<T>>` is // initialized with `None` and never changed, it still emits drop glue. // Hence we additionally check the qualifs here to allow more code to pass. if self.qualifs.needs_non_const_drop(self.ccx, dropped_place.local, location) { diff --git a/compiler/rustc_const_eval/src/transform/promote_consts.rs b/compiler/rustc_const_eval/src/transform/promote_consts.rs index 6777fae74..fae6117f8 100644 --- a/compiler/rustc_const_eval/src/transform/promote_consts.rs +++ b/compiler/rustc_const_eval/src/transform/promote_consts.rs @@ -133,7 +133,7 @@ impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> { } _ => { /* mark as unpromotable below */ } } - } else if let TempState::Defined { ref mut uses, .. } = *temp { + } else if let TempState::Defined { uses, .. } = temp { // We always allow borrows, even mutable ones, as we need // to promote mutable borrows of some ZSTs e.g., `&mut []`. let allowed_use = match context { @@ -216,12 +216,6 @@ impl<'tcx> Validator<'_, 'tcx> { return Err(Unpromotable); } - // We cannot promote things that need dropping, since the promoted value - // would not get dropped. - if self.qualif_local::<qualifs::NeedsDrop>(place.local) { - return Err(Unpromotable); - } - Ok(()) } _ => bug!(), @@ -262,13 +256,17 @@ impl<'tcx> Validator<'_, 'tcx> { } } } else { - let span = self.body.local_decls[local].source_info.span; - span_bug!(span, "{:?} not promotable, qualif_local shouldn't have been called", local); + false } } fn validate_local(&mut self, local: Local) -> Result<(), Unpromotable> { if let TempState::Defined { location: loc, uses, valid } = self.temps[local] { + // We cannot promote things that need dropping, since the promoted value + // would not get dropped. + if self.qualif_local::<qualifs::NeedsDrop>(local) { + return Err(Unpromotable); + } valid.or_else(|_| { let ok = { let block = &self.body[loc.block]; @@ -750,7 +748,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { if loc.statement_index < num_stmts { let (mut rvalue, source_info) = { let statement = &mut self.source[loc.block].statements[loc.statement_index]; - let StatementKind::Assign(box (_, ref mut rhs)) = statement.kind else { + let StatementKind::Assign(box (_, rhs)) = &mut statement.kind else { span_bug!( statement.source_info.span, "{:?} is not an assignment", @@ -780,9 +778,9 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { self.source[loc.block].terminator().clone() } else { let terminator = self.source[loc.block].terminator_mut(); - let target = match terminator.kind { - TerminatorKind::Call { target: Some(target), .. } => target, - ref kind => { + let target = match &terminator.kind { + TerminatorKind::Call { target: Some(target), .. } => *target, + kind => { span_bug!(terminator.source_info.span, "{:?} not promotable", kind); } }; @@ -816,7 +814,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { ..terminator }; } - ref kind => { + kind => { span_bug!(terminator.source_info.span, "{:?} not promotable", kind); } }; @@ -849,54 +847,50 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { let local_decls = &mut self.source.local_decls; let loc = candidate.location; let statement = &mut blocks[loc.block].statements[loc.statement_index]; - match statement.kind { - StatementKind::Assign(box ( - _, - Rvalue::Ref(ref mut region, borrow_kind, ref mut place), - )) => { - // Use the underlying local for this (necessarily interior) borrow. - let ty = local_decls[place.local].ty; - let span = statement.source_info.span; - - let ref_ty = tcx.mk_ref( - tcx.lifetimes.re_erased, - ty::TypeAndMut { ty, mutbl: borrow_kind.to_mutbl_lossy() }, - ); + let StatementKind::Assign(box (_, Rvalue::Ref(region, borrow_kind, place))) = &mut statement.kind else { + bug!() + }; - *region = tcx.lifetimes.re_erased; - - let mut projection = vec![PlaceElem::Deref]; - projection.extend(place.projection); - place.projection = tcx.intern_place_elems(&projection); - - // Create a temp to hold the promoted reference. - // This is because `*r` requires `r` to be a local, - // otherwise we would use the `promoted` directly. - let mut promoted_ref = LocalDecl::new(ref_ty, span); - promoted_ref.source_info = statement.source_info; - let promoted_ref = local_decls.push(promoted_ref); - assert_eq!(self.temps.push(TempState::Unpromotable), promoted_ref); - - let promoted_ref_statement = Statement { - source_info: statement.source_info, - kind: StatementKind::Assign(Box::new(( - Place::from(promoted_ref), - Rvalue::Use(promoted_operand(ref_ty, span)), - ))), - }; - self.extra_statements.push((loc, promoted_ref_statement)); - - Rvalue::Ref( - tcx.lifetimes.re_erased, - borrow_kind, - Place { - local: mem::replace(&mut place.local, promoted_ref), - projection: List::empty(), - }, - ) - } - _ => bug!(), - } + // Use the underlying local for this (necessarily interior) borrow. + let ty = local_decls[place.local].ty; + let span = statement.source_info.span; + + let ref_ty = tcx.mk_ref( + tcx.lifetimes.re_erased, + ty::TypeAndMut { ty, mutbl: borrow_kind.to_mutbl_lossy() }, + ); + + *region = tcx.lifetimes.re_erased; + + let mut projection = vec![PlaceElem::Deref]; + projection.extend(place.projection); + place.projection = tcx.intern_place_elems(&projection); + + // Create a temp to hold the promoted reference. + // This is because `*r` requires `r` to be a local, + // otherwise we would use the `promoted` directly. + let mut promoted_ref = LocalDecl::new(ref_ty, span); + promoted_ref.source_info = statement.source_info; + let promoted_ref = local_decls.push(promoted_ref); + assert_eq!(self.temps.push(TempState::Unpromotable), promoted_ref); + + let promoted_ref_statement = Statement { + source_info: statement.source_info, + kind: StatementKind::Assign(Box::new(( + Place::from(promoted_ref), + Rvalue::Use(promoted_operand(ref_ty, span)), + ))), + }; + self.extra_statements.push((loc, promoted_ref_statement)); + + Rvalue::Ref( + tcx.lifetimes.re_erased, + *borrow_kind, + Place { + local: mem::replace(&mut place.local, promoted_ref), + projection: List::empty(), + }, + ) }; assert_eq!(self.new_block(), START_BLOCK); diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 5c9263dc5..dd168a9ac 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -1,7 +1,8 @@ //! Validates the MIR to ensure that invariants are upheld. -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_index::bit_set::BitSet; +use rustc_index::vec::IndexVec; use rustc_infer::traits::Reveal; use rustc_middle::mir::interpret::Scalar; use rustc_middle::mir::visit::NonUseContext::VarDebugInfo; @@ -9,8 +10,8 @@ use rustc_middle::mir::visit::{PlaceContext, Visitor}; use rustc_middle::mir::{ traversal, AggregateKind, BasicBlock, BinOp, Body, BorrowKind, CastKind, CopyNonOverlapping, Local, Location, MirPass, MirPhase, NonDivergingIntrinsic, Operand, Place, PlaceElem, PlaceRef, - ProjectionElem, RuntimePhase, Rvalue, SourceScope, Statement, StatementKind, Terminator, - TerminatorKind, UnOp, START_BLOCK, + ProjectionElem, RetagKind, RuntimePhase, Rvalue, SourceScope, Statement, StatementKind, + Terminator, TerminatorKind, UnOp, START_BLOCK, }; use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeVisitable}; use rustc_mir_dataflow::impls::MaybeStorageLive; @@ -18,7 +19,7 @@ use rustc_mir_dataflow::storage::always_storage_live_locals; use rustc_mir_dataflow::{Analysis, ResultsCursor}; use rustc_target::abi::{Size, VariantIdx}; -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] enum EdgeKind { Unwind, Normal, @@ -52,23 +53,25 @@ impl<'tcx> MirPass<'tcx> for Validator { }; let always_live_locals = always_storage_live_locals(body); - let storage_liveness = MaybeStorageLive::new(always_live_locals) + let storage_liveness = MaybeStorageLive::new(std::borrow::Cow::Owned(always_live_locals)) .into_engine(tcx, body) .iterate_to_fixpoint() .into_results_cursor(body); - TypeChecker { + let mut checker = TypeChecker { when: &self.when, body, tcx, param_env, mir_phase, + unwind_edge_count: 0, reachable_blocks: traversal::reachable_as_bitset(body), storage_liveness, place_cache: Vec::new(), value_cache: Vec::new(), - } - .visit_body(body); + }; + checker.visit_body(body); + checker.check_cleanup_control_flow(); } } @@ -78,8 +81,9 @@ struct TypeChecker<'a, 'tcx> { tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, mir_phase: MirPhase, + unwind_edge_count: usize, reachable_blocks: BitSet<BasicBlock>, - storage_liveness: ResultsCursor<'a, 'tcx, MaybeStorageLive>, + storage_liveness: ResultsCursor<'a, 'tcx, MaybeStorageLive<'static>>, place_cache: Vec<PlaceRef<'tcx>>, value_cache: Vec<u128>, } @@ -102,7 +106,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ); } - fn check_edge(&self, location: Location, bb: BasicBlock, edge_kind: EdgeKind) { + fn check_edge(&mut self, location: Location, bb: BasicBlock, edge_kind: EdgeKind) { if bb == START_BLOCK { self.fail(location, "start block must not have predecessors") } @@ -111,10 +115,12 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { match (src.is_cleanup, bb.is_cleanup, edge_kind) { // Non-cleanup blocks can jump to non-cleanup blocks along non-unwind edges (false, false, EdgeKind::Normal) - // Non-cleanup blocks can jump to cleanup blocks along unwind edges - | (false, true, EdgeKind::Unwind) // Cleanup blocks can jump to cleanup blocks along non-unwind edges | (true, true, EdgeKind::Normal) => {} + // Non-cleanup blocks can jump to cleanup blocks along unwind edges + (false, true, EdgeKind::Unwind) => { + self.unwind_edge_count += 1; + } // All other jumps are invalid _ => { self.fail( @@ -134,6 +140,88 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { } } + fn check_cleanup_control_flow(&self) { + if self.unwind_edge_count <= 1 { + return; + } + let doms = self.body.basic_blocks.dominators(); + let mut post_contract_node = FxHashMap::default(); + // Reusing the allocation across invocations of the closure + let mut dom_path = vec![]; + let mut get_post_contract_node = |mut bb| { + let root = loop { + if let Some(root) = post_contract_node.get(&bb) { + break *root; + } + let parent = doms.immediate_dominator(bb); + dom_path.push(bb); + if !self.body.basic_blocks[parent].is_cleanup { + break bb; + } + bb = parent; + }; + for bb in dom_path.drain(..) { + post_contract_node.insert(bb, root); + } + root + }; + + let mut parent = IndexVec::from_elem(None, &self.body.basic_blocks); + for (bb, bb_data) in self.body.basic_blocks.iter_enumerated() { + if !bb_data.is_cleanup || !self.reachable_blocks.contains(bb) { + continue; + } + let bb = get_post_contract_node(bb); + for s in bb_data.terminator().successors() { + let s = get_post_contract_node(s); + if s == bb { + continue; + } + let parent = &mut parent[bb]; + match parent { + None => { + *parent = Some(s); + } + Some(e) if *e == s => (), + Some(e) => self.fail( + Location { block: bb, statement_index: 0 }, + format!( + "Cleanup control flow violation: The blocks dominated by {:?} have edges to both {:?} and {:?}", + bb, + s, + *e + ) + ), + } + } + } + + // Check for cycles + let mut stack = FxHashSet::default(); + for i in 0..parent.len() { + let mut bb = BasicBlock::from_usize(i); + stack.clear(); + stack.insert(bb); + loop { + let Some(parent)= parent[bb].take() else { + break + }; + let no_cycle = stack.insert(parent); + if !no_cycle { + self.fail( + Location { block: bb, statement_index: 0 }, + format!( + "Cleanup control flow violation: Cycle involving edge {:?} -> {:?}", + bb, parent, + ), + ); + break; + } + bb = parent; + } + } + } + /// Check if src can be assigned into dest. /// This is not precise, it will accept some incorrect assignments. fn mir_assign_valid_types(&self, src: Ty<'tcx>, dest: Ty<'tcx>) -> bool { @@ -241,7 +329,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { }; let kind = match parent_ty.ty.kind() { - &ty::Opaque(def_id, substs) => { + &ty::Alias(ty::Opaque, ty::AliasTy { def_id, substs, .. }) => { self.tcx.bound_type_of(def_id).subst(self.tcx, substs).kind() } kind => kind, @@ -652,7 +740,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { self.fail(location, "`SetDiscriminant`is not allowed until deaggregation"); } let pty = place.ty(&self.body.local_decls, self.tcx).ty.kind(); - if !matches!(pty, ty::Adt(..) | ty::Generator(..) | ty::Opaque(..)) { + if !matches!(pty, ty::Adt(..) | ty::Generator(..) | ty::Alias(ty::Opaque, ..)) { self.fail( location, format!( @@ -667,10 +755,13 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { self.fail(location, "`Deinit`is not allowed until deaggregation"); } } - StatementKind::Retag(_, _) => { + StatementKind::Retag(kind, _) => { // FIXME(JakobDegen) The validator should check that `self.mir_phase < // DropsLowered`. However, this causes ICEs with generation of drop shims, which // seem to fail to set their `MirPhase` correctly. + if *kind == RetagKind::Raw || *kind == RetagKind::TwoPhase { + self.fail(location, format!("explicit `{:?}` is forbidden", kind)); + } } StatementKind::StorageLive(..) | StatementKind::StorageDead(..) @@ -686,17 +777,8 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { TerminatorKind::Goto { target } => { self.check_edge(location, *target, EdgeKind::Normal); } - TerminatorKind::SwitchInt { targets, switch_ty, discr } => { - let ty = discr.ty(&self.body.local_decls, self.tcx); - if ty != *switch_ty { - self.fail( - location, - format!( - "encountered `SwitchInt` terminator with type mismatch: {:?} != {:?}", - ty, switch_ty, - ), - ); - } + TerminatorKind::SwitchInt { targets, discr } => { + let switch_ty = discr.ty(&self.body.local_decls, self.tcx); let target_width = self.tcx.sess.target.pointer_width; diff --git a/compiler/rustc_const_eval/src/util/aggregate.rs b/compiler/rustc_const_eval/src/util/aggregate.rs index c43de3368..10783c5ed 100644 --- a/compiler/rustc_const_eval/src/util/aggregate.rs +++ b/compiler/rustc_const_eval/src/util/aggregate.rs @@ -3,7 +3,6 @@ use rustc_middle::mir::*; use rustc_middle::ty::{Ty, TyCtxt}; use rustc_target::abi::VariantIdx; -use std::convert::TryFrom; use std::iter::TrustedLen; /// Expand `lhs = Rvalue::Aggregate(kind, operands)` into assignments to the fields. diff --git a/compiler/rustc_const_eval/src/util/call_kind.rs b/compiler/rustc_const_eval/src/util/call_kind.rs index b38a6c551..38d9b0449 100644 --- a/compiler/rustc_const_eval/src/util/call_kind.rs +++ b/compiler/rustc_const_eval/src/util/call_kind.rs @@ -5,7 +5,7 @@ use rustc_hir::def_id::DefId; use rustc_hir::{lang_items, LangItem}; use rustc_middle::ty::subst::SubstsRef; -use rustc_middle::ty::{self, AssocItemContainer, DefIdTree, Instance, ParamEnv, Ty, TyCtxt}; +use rustc_middle::ty::{AssocItemContainer, Instance, ParamEnv, Ty, TyCtxt}; use rustc_span::symbol::Ident; use rustc_span::{sym, DesugaringKind, Span}; @@ -39,9 +39,7 @@ pub enum CallKind<'tcx> { Normal { self_arg: Option<Ident>, desugaring: Option<(CallDesugaringKind, Ty<'tcx>)>, - /// Whether the self type of the method call has an `.as_ref()` method. - /// Used for better diagnostics. - is_option_or_result: bool, + method_did: DefId, }, /// A call to `Fn(..)::call(..)`, desugared from `my_closure(a, b, c)` FnCall { fn_trait_id: DefId, self_ty: Ty<'tcx> }, @@ -133,16 +131,6 @@ pub fn call_kind<'tcx>( } else { None }; - let parent_did = tcx.parent(method_did); - let parent_self_ty = (tcx.def_kind(parent_did) == rustc_hir::def::DefKind::Impl) - .then_some(parent_did) - .and_then(|did| match tcx.type_of(did).kind() { - ty::Adt(def, ..) => Some(def.did()), - _ => None, - }); - let is_option_or_result = parent_self_ty.map_or(false, |def_id| { - matches!(tcx.get_diagnostic_name(def_id), Some(sym::Option | sym::Result)) - }); - CallKind::Normal { self_arg, desugaring, is_option_or_result } + CallKind::Normal { self_arg, desugaring, method_did } }) } diff --git a/compiler/rustc_const_eval/src/util/compare_types.rs b/compiler/rustc_const_eval/src/util/compare_types.rs index be786569c..f5f3d5de6 100644 --- a/compiler/rustc_const_eval/src/util/compare_types.rs +++ b/compiler/rustc_const_eval/src/util/compare_types.rs @@ -58,6 +58,6 @@ pub fn is_subtype<'tcx>( // even if they're constrained in our current function. // // It seems very unlikely that this hides any bugs. - let _ = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types(); + let _ = infcx.take_opaque_types(); errors.is_empty() } diff --git a/compiler/rustc_const_eval/src/util/might_permit_raw_init.rs b/compiler/rustc_const_eval/src/util/might_permit_raw_init.rs index 6ca712233..4ce107ea6 100644 --- a/compiler/rustc_const_eval/src/util/might_permit_raw_init.rs +++ b/compiler/rustc_const_eval/src/util/might_permit_raw_init.rs @@ -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); diff --git a/compiler/rustc_const_eval/src/util/type_name.rs b/compiler/rustc_const_eval/src/util/type_name.rs index 14c8c8802..c4122f664 100644 --- a/compiler/rustc_const_eval/src/util/type_name.rs +++ b/compiler/rustc_const_eval/src/util/type_name.rs @@ -58,8 +58,7 @@ impl<'tcx> Printer<'tcx> for AbsolutePathPrinter<'tcx> { // Types with identity (print the module path). ty::Adt(ty::AdtDef(Interned(&ty::AdtDefData { did: def_id, .. }, _)), substs) | ty::FnDef(def_id, substs) - | ty::Opaque(def_id, substs) - | ty::Projection(ty::ProjectionTy { item_def_id: def_id, substs }) + | ty::Alias(_, ty::AliasTy { def_id, substs, .. }) | ty::Closure(def_id, substs) | ty::Generator(def_id, substs, _) => self.print_def_path(def_id, substs), ty::Foreign(def_id) => self.print_def_path(def_id, &[]), |