From 5f593da4e6976fe27f7cd31ce0fb9a9293b3a00b Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sun, 26 Feb 2023 21:50:19 +0000 Subject: [PATCH] Unify all validity check intrinsics Also merges the inhabitedness check into the query to further unify the code paths. --- .../src/intrinsics/mod.rs | 80 +++++++------------ compiler/rustc_codegen_ssa/src/mir/block.rs | 42 +++------- .../src/interpret/intrinsics.rs | 74 ++++++----------- compiler/rustc_const_eval/src/lib.rs | 4 +- ..._init.rs => check_validity_requirement.rs} | 30 ++++--- compiler/rustc_const_eval/src/util/mod.rs | 4 +- compiler/rustc_middle/src/query/keys.rs | 4 +- compiler/rustc_middle/src/query/mod.rs | 4 +- compiler/rustc_middle/src/ty/layout.rs | 22 ++++- compiler/rustc_middle/src/ty/query.rs | 2 +- .../rustc_mir_transform/src/instcombine.rs | 16 +--- 11 files changed, 119 insertions(+), 163 deletions(-) rename compiler/rustc_const_eval/src/util/{might_permit_raw_init.rs => check_validity_requirement.rs} (86%) diff --git a/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs b/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs index f00e932107058..e74aabf2fcb0d 100644 --- a/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs +++ b/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs @@ -22,7 +22,7 @@ pub(crate) use cpuid::codegen_cpuid_call; pub(crate) use llvm::codegen_llvm_intrinsic_call; use rustc_middle::ty; -use rustc_middle::ty::layout::{HasParamEnv, InitKind}; +use rustc_middle::ty::layout::{HasParamEnv, ValidityRequirement}; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::subst::SubstsRef; use rustc_span::symbol::{kw, sym, Symbol}; @@ -628,57 +628,39 @@ fn codegen_regular_intrinsic_call<'tcx>( intrinsic_args!(fx, args => (); intrinsic); let ty = substs.type_at(0); - let layout = fx.layout_of(ty); - if layout.abi.is_uninhabited() { - with_no_trimmed_paths!({ - crate::base::codegen_panic_nounwind( - fx, - &format!("attempted to instantiate uninhabited type `{}`", layout.ty), - source_info, - ) - }); - return; - } - if intrinsic == sym::assert_zero_valid - && !fx - .tcx - .check_validity_of_init((InitKind::Zero, fx.param_env().and(ty))) - .expect("expected to have layout during codegen") - { - with_no_trimmed_paths!({ - crate::base::codegen_panic_nounwind( - fx, - &format!( - "attempted to zero-initialize type `{}`, which is invalid", - layout.ty - ), - source_info, - ); - }); - return; - } + let requirement = ValidityRequirement::from_intrinsic(intrinsic); - if intrinsic == sym::assert_mem_uninitialized_valid - && !fx + if let Some(requirement) = requirement { + let do_panic = !fx .tcx - .check_validity_of_init(( - InitKind::UninitMitigated0x01Fill, - fx.param_env().and(ty), - )) - .expect("expected to have layout during codegen") - { - with_no_trimmed_paths!({ - crate::base::codegen_panic_nounwind( - fx, - &format!( - "attempted to leave type `{}` uninitialized, which is invalid", - layout.ty - ), - source_info, - ) - }); - return; + .check_validity_requirement((requirement, fx.param_env().and(ty))) + .expect("expect to have layout during codegen"); + + if do_panic { + let layout = fx.layout_of(ty); + + with_no_trimmed_paths!({ + crate::base::codegen_panic_nounwind( + fx, + &if layout.abi.is_uninhabited() { + format!("attempted to instantiate uninhabited type `{}`", layout.ty) + } else if requirement == ValidityRequirement::Zero { + format!( + "attempted to zero-initialize type `{}`, which is invalid", + layout.ty + ) + } else { + format!( + "attempted to leave type `{}` uninitialized, which is invalid", + layout.ty + ) + }, + source_info, + ) + }); + return; + } } } diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index b1abbd673a53a..57a19a4ab1eab 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -14,7 +14,7 @@ use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece}; use rustc_hir::lang_items::LangItem; use rustc_index::vec::Idx; use rustc_middle::mir::{self, AssertKind, SwitchTargets}; -use rustc_middle::ty::layout::{HasTyCtxt, InitKind, LayoutOf}; +use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, ValidityRequirement}; use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths}; use rustc_middle::ty::{self, Instance, Ty, TypeVisitableExt}; use rustc_session::config::OptLevel; @@ -655,44 +655,24 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // Emit a panic or a no-op for `assert_*` intrinsics. // These are intrinsics that compile to panics so that we can get a message // which mentions the offending type, even from a const context. - #[derive(Debug, PartialEq)] - enum AssertIntrinsic { - Inhabited, - ZeroValid, - MemUninitializedValid, - } - let panic_intrinsic = intrinsic.and_then(|i| match i { - sym::assert_inhabited => Some(AssertIntrinsic::Inhabited), - sym::assert_zero_valid => Some(AssertIntrinsic::ZeroValid), - sym::assert_mem_uninitialized_valid => Some(AssertIntrinsic::MemUninitializedValid), - _ => None, - }); - if let Some(intrinsic) = panic_intrinsic { - use AssertIntrinsic::*; - + let panic_intrinsic = intrinsic.and_then(|s| ValidityRequirement::from_intrinsic(s)); + if let Some(requirement) = panic_intrinsic { let ty = instance.unwrap().substs.type_at(0); + + let do_panic = !bx + .tcx() + .check_validity_requirement((requirement, bx.param_env().and(ty))) + .expect("expect to have layout during codegen"); + let layout = bx.layout_of(ty); - let do_panic = match intrinsic { - Inhabited => layout.abi.is_uninhabited(), - ZeroValid => !bx - .tcx() - .check_validity_of_init((InitKind::Zero, bx.param_env().and(ty))) - .expect("expected to have layout during codegen"), - MemUninitializedValid => !bx - .tcx() - .check_validity_of_init(( - InitKind::UninitMitigated0x01Fill, - bx.param_env().and(ty), - )) - .expect("expected to have layout during codegen"), - }; + Some(if do_panic { let msg_str = with_no_visible_paths!({ with_no_trimmed_paths!({ if layout.abi.is_uninhabited() { // Use this error even for the other intrinsics as it is more precise. format!("attempted to instantiate uninhabited type `{}`", ty) - } else if intrinsic == ZeroValid { + } else if requirement == ValidityRequirement::Zero { format!("attempted to zero-initialize type `{}`, which is invalid", ty) } else { format!( diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index 26c84b4ce6127..c65d677e8ea75 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -11,7 +11,7 @@ use rustc_middle::mir::{ BinOp, NonDivergingIntrinsic, }; use rustc_middle::ty; -use rustc_middle::ty::layout::{InitKind, LayoutOf as _}; +use rustc_middle::ty::layout::{LayoutOf as _, ValidityRequirement}; use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::{Ty, TyCtxt}; use rustc_span::symbol::{sym, Symbol}; @@ -418,57 +418,35 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { | sym::assert_zero_valid | sym::assert_mem_uninitialized_valid => { let ty = instance.substs.type_at(0); - let layout = self.layout_of(ty)?; - - // For *all* intrinsics we first check `is_uninhabited` to give a more specific - // error message. - if layout.abi.is_uninhabited() { - // The run-time intrinsic panics just to get a good backtrace; here we abort - // since there is no problem showing a backtrace even for aborts. - M::abort( - self, - format!( + let requirement = ValidityRequirement::from_intrinsic(intrinsic_name).unwrap(); + + let should_panic = !self + .tcx + .check_validity_requirement((requirement, self.param_env.and(ty))) + .map_err(|_| err_inval!(TooGeneric))?; + + if should_panic { + let layout = self.layout_of(ty)?; + + let msg = match requirement { + // For *all* intrinsics we first check `is_uninhabited` to give a more specific + // error message. + _ if layout.abi.is_uninhabited() => format!( "aborted execution: attempted to instantiate uninhabited type `{}`", ty ), - )?; - } - - if intrinsic_name == sym::assert_zero_valid { - let should_panic = !self - .tcx - .check_validity_of_init((InitKind::Zero, self.param_env.and(ty))) - .map_err(|_| err_inval!(TooGeneric))?; - - if should_panic { - M::abort( - self, - format!( - "aborted execution: attempted to zero-initialize type `{}`, which is invalid", - ty - ), - )?; - } - } + ValidityRequirement::Inhabited => bug!("handled earlier"), + ValidityRequirement::Zero => format!( + "aborted execution: attempted to zero-initialize type `{}`, which is invalid", + ty + ), + ValidityRequirement::UninitMitigated0x01Fill => format!( + "aborted execution: attempted to leave type `{}` uninitialized, which is invalid", + ty + ), + }; - if intrinsic_name == sym::assert_mem_uninitialized_valid { - let should_panic = !self - .tcx - .check_validity_of_init(( - InitKind::UninitMitigated0x01Fill, - self.param_env.and(ty), - )) - .map_err(|_| err_inval!(TooGeneric))?; - - if should_panic { - M::abort( - self, - format!( - "aborted execution: attempted to leave type `{}` uninitialized, which is invalid", - ty - ), - )?; - } + M::abort(self, msg)?; } } sym::simd_insert => { diff --git a/compiler/rustc_const_eval/src/lib.rs b/compiler/rustc_const_eval/src/lib.rs index 092a7dc3d3b51..ed9efe568fb30 100644 --- a/compiler/rustc_const_eval/src/lib.rs +++ b/compiler/rustc_const_eval/src/lib.rs @@ -61,7 +61,7 @@ pub fn provide(providers: &mut Providers) { let (param_env, value) = param_env_and_value.into_parts(); const_eval::deref_mir_constant(tcx, param_env, value) }; - providers.check_validity_of_init = |tcx, (init_kind, param_env_and_ty)| { - util::might_permit_raw_init(tcx, init_kind, param_env_and_ty) + providers.check_validity_requirement = |tcx, (init_kind, param_env_and_ty)| { + util::check_validity_requirement(tcx, init_kind, param_env_and_ty) }; } diff --git a/compiler/rustc_const_eval/src/util/might_permit_raw_init.rs b/compiler/rustc_const_eval/src/util/check_validity_requirement.rs similarity index 86% rename from compiler/rustc_const_eval/src/util/might_permit_raw_init.rs rename to compiler/rustc_const_eval/src/util/check_validity_requirement.rs index a78bf927ca1dc..dcd15b919f4e3 100644 --- a/compiler/rustc_const_eval/src/util/might_permit_raw_init.rs +++ b/compiler/rustc_const_eval/src/util/check_validity_requirement.rs @@ -1,4 +1,4 @@ -use rustc_middle::ty::layout::{InitKind, LayoutCx, LayoutError, LayoutOf, TyAndLayout}; +use rustc_middle::ty::layout::{LayoutCx, LayoutError, LayoutOf, TyAndLayout, ValidityRequirement}; use rustc_middle::ty::{ParamEnv, ParamEnvAnd, Ty, TyCtxt}; use rustc_session::Limit; use rustc_target::abi::{Abi, FieldsShape, Scalar, Variants}; @@ -18,16 +18,23 @@ use crate::interpret::{InterpCx, MemoryKind, OpTy}; /// Rust UB as long as there is no risk of miscompilations. The `strict_init_checks` can be set to /// do a full check against Rust UB instead (in which case we will also ignore the 0x01-filling and /// to the full uninit check). -pub fn might_permit_raw_init<'tcx>( +pub fn check_validity_requirement<'tcx>( tcx: TyCtxt<'tcx>, - kind: InitKind, + kind: ValidityRequirement, param_env_and_ty: ParamEnvAnd<'tcx, Ty<'tcx>>, ) -> Result> { + let layout = tcx.layout_of(param_env_and_ty)?; + + // There is nothing strict or lax about inhabitedness. + if kind == ValidityRequirement::Inhabited { + return Ok(!layout.abi.is_uninhabited()); + } + if tcx.sess.opts.unstable_opts.strict_init_checks { - might_permit_raw_init_strict(tcx.layout_of(param_env_and_ty)?, tcx, kind) + might_permit_raw_init_strict(layout, tcx, kind) } else { let layout_cx = LayoutCx { tcx, param_env: param_env_and_ty.param_env }; - might_permit_raw_init_lax(tcx.layout_of(param_env_and_ty)?, &layout_cx, kind) + might_permit_raw_init_lax(layout, &layout_cx, kind) } } @@ -36,7 +43,7 @@ pub fn might_permit_raw_init<'tcx>( fn might_permit_raw_init_strict<'tcx>( ty: TyAndLayout<'tcx>, tcx: TyCtxt<'tcx>, - kind: InitKind, + kind: ValidityRequirement, ) -> Result> { let machine = CompileTimeInterpreter::new( Limit::new(0), @@ -50,7 +57,7 @@ fn might_permit_raw_init_strict<'tcx>( .allocate(ty, MemoryKind::Machine(crate::const_eval::MemoryKind::Heap)) .expect("OOM: failed to allocate for uninit check"); - if kind == InitKind::Zero { + if kind == ValidityRequirement::Zero { cx.write_bytes_ptr( allocated.ptr, std::iter::repeat(0_u8).take(ty.layout.size().bytes_usize()), @@ -72,15 +79,18 @@ fn might_permit_raw_init_strict<'tcx>( fn might_permit_raw_init_lax<'tcx>( this: TyAndLayout<'tcx>, cx: &LayoutCx<'tcx, TyCtxt<'tcx>>, - init_kind: InitKind, + init_kind: ValidityRequirement, ) -> Result> { let scalar_allows_raw_init = move |s: Scalar| -> bool { match init_kind { - InitKind::Zero => { + ValidityRequirement::Inhabited => { + bug!("ValidityRequirement::Inhabited should have been handled above") + } + ValidityRequirement::Zero => { // The range must contain 0. s.valid_range(cx).contains(0) } - InitKind::UninitMitigated0x01Fill => { + ValidityRequirement::UninitMitigated0x01Fill => { // The range must include an 0x01-filled buffer. let mut val: u128 = 0x01; for _ in 1..s.size(cx).bytes() { diff --git a/compiler/rustc_const_eval/src/util/mod.rs b/compiler/rustc_const_eval/src/util/mod.rs index 51735e33e0f71..c0aabd77ceead 100644 --- a/compiler/rustc_const_eval/src/util/mod.rs +++ b/compiler/rustc_const_eval/src/util/mod.rs @@ -1,14 +1,14 @@ mod alignment; mod call_kind; +mod check_validity_requirement; pub mod collect_writes; mod compare_types; mod find_self_call; -mod might_permit_raw_init; mod type_name; pub use self::alignment::is_disaligned; pub use self::call_kind::{call_kind, CallDesugaringKind, CallKind}; +pub use self::check_validity_requirement::check_validity_requirement; pub use self::compare_types::{is_equal_up_to_subtyping, is_subtype}; pub use self::find_self_call::find_self_call; -pub use self::might_permit_raw_init::might_permit_raw_init; pub use self::type_name::type_name; diff --git a/compiler/rustc_middle/src/query/keys.rs b/compiler/rustc_middle/src/query/keys.rs index 111ea6b8cddc0..78ee8a6a8fd64 100644 --- a/compiler/rustc_middle/src/query/keys.rs +++ b/compiler/rustc_middle/src/query/keys.rs @@ -4,7 +4,7 @@ use crate::infer::canonical::Canonical; use crate::mir; use crate::traits; use crate::ty::fast_reject::SimplifiedType; -use crate::ty::layout::{InitKind, TyAndLayout}; +use crate::ty::layout::{TyAndLayout, ValidityRequirement}; use crate::ty::subst::{GenericArg, SubstsRef}; use crate::ty::{self, Ty, TyCtxt}; use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE}; @@ -698,7 +698,7 @@ impl Key for HirId { } } -impl<'tcx> Key for (InitKind, ty::ParamEnvAnd<'tcx, Ty<'tcx>>) { +impl<'tcx> Key for (ValidityRequirement, ty::ParamEnvAnd<'tcx, Ty<'tcx>>) { type CacheSelector = DefaultCacheSelector; // Just forward to `Ty<'tcx>` diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index d4435a54b4ab6..f4c1ad0f6c0b9 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -2173,8 +2173,8 @@ rustc_queries! { separate_provide_extern } - query check_validity_of_init(key: (InitKind, ty::ParamEnvAnd<'tcx, Ty<'tcx>>)) -> Result> { - desc { "checking to see if `{}` permits being left {}", key.1.value, key.0 } + query check_validity_requirement(key: (ValidityRequirement, ty::ParamEnvAnd<'tcx, Ty<'tcx>>)) -> Result> { + desc { "checking validity requirement for `{}`: {}", key.1.value, key.0 } } query compare_impl_const( diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index f0b52455889aa..090272a6fa6d9 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -7,6 +7,7 @@ use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_index::vec::Idx; use rustc_session::config::OptLevel; +use rustc_span::symbol::{sym, Symbol}; use rustc_span::{Span, DUMMY_SP}; use rustc_target::abi::call::FnAbi; use rustc_target::abi::*; @@ -172,16 +173,29 @@ pub const MAX_SIMD_LANES: u64 = 1 << 0xF; /// Used in `might_permit_raw_init` to indicate the kind of initialisation /// that is checked to be valid #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)] -pub enum InitKind { +pub enum ValidityRequirement { + Inhabited, Zero, UninitMitigated0x01Fill, } -impl fmt::Display for InitKind { +impl ValidityRequirement { + pub fn from_intrinsic(intrinsic: Symbol) -> Option { + match intrinsic { + sym::assert_inhabited => Some(Self::Inhabited), + sym::assert_zero_valid => Some(Self::Zero), + sym::assert_mem_uninitialized_valid => Some(Self::UninitMitigated0x01Fill), + _ => None, + } + } +} + +impl fmt::Display for ValidityRequirement { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::Zero => f.write_str("zeroed"), - Self::UninitMitigated0x01Fill => f.write_str("filled with 0x01"), + Self::Inhabited => f.write_str("is inhabited"), + Self::Zero => f.write_str("allows being left zeroed"), + Self::UninitMitigated0x01Fill => f.write_str("allows being filled with 0x01"), } } } diff --git a/compiler/rustc_middle/src/ty/query.rs b/compiler/rustc_middle/src/ty/query.rs index d743c30684958..2bc51baf87905 100644 --- a/compiler/rustc_middle/src/ty/query.rs +++ b/compiler/rustc_middle/src/ty/query.rs @@ -32,7 +32,7 @@ use crate::traits::specialization_graph; use crate::traits::{self, ImplSource}; use crate::ty::context::TyCtxtFeed; use crate::ty::fast_reject::SimplifiedType; -use crate::ty::layout::InitKind; +use crate::ty::layout::ValidityRequirement; use crate::ty::subst::{GenericArg, SubstsRef}; use crate::ty::util::AlwaysRequiresDrop; use crate::ty::GeneratorDiagnosticData; diff --git a/compiler/rustc_mir_transform/src/instcombine.rs b/compiler/rustc_mir_transform/src/instcombine.rs index 05286b71d47e4..4182da1957e39 100644 --- a/compiler/rustc_mir_transform/src/instcombine.rs +++ b/compiler/rustc_mir_transform/src/instcombine.rs @@ -6,9 +6,9 @@ use rustc_middle::mir::{ BinOp, Body, Constant, ConstantKind, LocalDecls, Operand, Place, ProjectionElem, Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, UnOp, }; -use rustc_middle::ty::layout::InitKind; +use rustc_middle::ty::layout::ValidityRequirement; use rustc_middle::ty::{self, ParamEnv, SubstsRef, Ty, TyCtxt}; -use rustc_span::symbol::{sym, Symbol}; +use rustc_span::symbol::Symbol; pub struct InstCombine; @@ -256,16 +256,8 @@ fn intrinsic_assert_panics<'tcx>( ty: Ty<'tcx>, intrinsic_name: Symbol, ) -> Option { - Some(match intrinsic_name { - sym::assert_inhabited => tcx.layout_of(param_env.and(ty)).ok()?.abi.is_uninhabited(), - sym::assert_zero_valid => { - !tcx.check_validity_of_init((InitKind::Zero, param_env.and(ty))).ok()? - } - sym::assert_mem_uninitialized_valid => !tcx - .check_validity_of_init((InitKind::UninitMitigated0x01Fill, param_env.and(ty))) - .ok()?, - _ => return None, - }) + let requirement = ValidityRequirement::from_intrinsic(intrinsic_name)?; + Some(!tcx.check_validity_requirement((requirement, param_env.and(ty))).ok()?) } fn resolve_rust_intrinsic<'tcx>(