Skip to content

Commit

Permalink
Use constant eval to do strict validity checks
Browse files Browse the repository at this point in the history
  • Loading branch information
5225225 committed Jul 14, 2022
1 parent c2f428d commit 27412d1
Show file tree
Hide file tree
Showing 13 changed files with 161 additions and 94 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3664,6 +3664,7 @@ dependencies = [
"rustc_arena",
"rustc_ast",
"rustc_attr",
"rustc_const_eval",
"rustc_data_structures",
"rustc_errors",
"rustc_fs_util",
Expand Down
15 changes: 2 additions & 13 deletions compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ pub(crate) use llvm::codegen_llvm_intrinsic_call;
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::subst::SubstsRef;
use rustc_span::symbol::{kw, sym, Symbol};
use rustc_target::abi::InitKind;

use crate::prelude::*;
use cranelift_codegen::ir::AtomicRmwOp;
Expand Down Expand Up @@ -672,12 +671,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
return;
}

if intrinsic == sym::assert_zero_valid
&& !layout.might_permit_raw_init(
fx,
InitKind::Zero,
fx.tcx.sess.opts.unstable_opts.strict_init_checks) {

if intrinsic == sym::assert_zero_valid && !fx.tcx.permits_zero_init(layout) {
with_no_trimmed_paths!({
crate::base::codegen_panic(
fx,
Expand All @@ -688,12 +682,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
return;
}

if intrinsic == sym::assert_uninit_valid
&& !layout.might_permit_raw_init(
fx,
InitKind::Uninit,
fx.tcx.sess.opts.unstable_opts.strict_init_checks) {

if intrinsic == sym::assert_uninit_valid && !fx.tcx.permits_uninit_init(layout) {
with_no_trimmed_paths!({
crate::base::codegen_panic(
fx,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ rustc_metadata = { path = "../rustc_metadata" }
rustc_query_system = { path = "../rustc_query_system" }
rustc_target = { path = "../rustc_target" }
rustc_session = { path = "../rustc_session" }
rustc_const_eval = { path = "../rustc_const_eval" }

[dependencies.object]
version = "0.29.0"
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use rustc_span::source_map::Span;
use rustc_span::{sym, Symbol};
use rustc_symbol_mangling::typeid_for_fnabi;
use rustc_target::abi::call::{ArgAbi, FnAbi, PassMode};
use rustc_target::abi::{self, HasDataLayout, InitKind, WrappingRange};
use rustc_target::abi::{self, HasDataLayout, WrappingRange};
use rustc_target::spec::abi::Abi;

/// Used by `FunctionCx::codegen_terminator` for emitting common patterns
Expand Down Expand Up @@ -528,7 +528,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
source_info: mir::SourceInfo,
target: Option<mir::BasicBlock>,
cleanup: Option<mir::BasicBlock>,
strict_validity: bool,
) -> bool {
// Emit a panic or a no-op for `assert_*` intrinsics.
// These are intrinsics that compile to panics so that we can get a message
Expand All @@ -547,12 +546,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
});
if let Some(intrinsic) = panic_intrinsic {
use AssertIntrinsic::*;

let ty = instance.unwrap().substs.type_at(0);
let layout = bx.layout_of(ty);
let do_panic = match intrinsic {
Inhabited => layout.abi.is_uninhabited(),
ZeroValid => !layout.might_permit_raw_init(bx, InitKind::Zero, strict_validity),
UninitValid => !layout.might_permit_raw_init(bx, InitKind::Uninit, strict_validity),
ZeroValid => !bx.tcx().permits_zero_init(layout),
UninitValid => !bx.tcx().permits_uninit_init(layout),
};
if do_panic {
let msg_str = with_no_visible_paths!({
Expand Down Expand Up @@ -687,7 +687,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
source_info,
target,
cleanup,
self.cx.tcx().sess.opts.unstable_opts.strict_init_checks,
) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {
}

impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {
pub(super) fn new(const_eval_limit: Limit, can_access_statics: bool) -> Self {
pub(crate) fn new(const_eval_limit: Limit, can_access_statics: bool) -> Self {
CompileTimeInterpreter {
steps_remaining: const_eval_limit.0,
stack: Vec::new(),
Expand Down
56 changes: 27 additions & 29 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc_middle::ty::layout::LayoutOf as _;
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{Ty, TyCtxt};
use rustc_span::symbol::{sym, Symbol};
use rustc_target::abi::{Abi, Align, InitKind, Primitive, Size};
use rustc_target::abi::{Abi, Align, Primitive, Size};

use super::{
util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx, Machine, OpTy, PlaceTy,
Expand Down Expand Up @@ -413,35 +413,33 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
),
)?;
}
if intrinsic_name == sym::assert_zero_valid
&& !layout.might_permit_raw_init(
self,
InitKind::Zero,
self.tcx.sess.opts.unstable_opts.strict_init_checks,
)
{
M::abort(
self,
format!(
"aborted execution: attempted to zero-initialize type `{}`, which is invalid",
ty
),
)?;

if intrinsic_name == sym::assert_zero_valid {
let should_panic = !self.tcx.permits_zero_init(layout);

if should_panic {
M::abort(
self,
format!(
"aborted execution: attempted to zero-initialize type `{}`, which is invalid",
ty
),
)?;
}
}
if intrinsic_name == sym::assert_uninit_valid
&& !layout.might_permit_raw_init(
self,
InitKind::Uninit,
self.tcx.sess.opts.unstable_opts.strict_init_checks,
)
{
M::abort(
self,
format!(
"aborted execution: attempted to leave type `{}` uninitialized, which is invalid",
ty
),
)?;

if intrinsic_name == sym::assert_uninit_valid {
let should_panic = !self.tcx.permits_uninit_init(layout);

if should_panic {
M::abort(
self,
format!(
"aborted execution: attempted to leave type `{}` uninitialized, which is invalid",
ty
),
)?;
}
}
}
sym::simd_insert => {
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_const_eval/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@ extern crate rustc_middle;
pub mod const_eval;
mod errors;
pub mod interpret;
mod might_permit_raw_init;
pub mod transform;
pub mod util;

use rustc_middle::ty;
use rustc_middle::ty::query::Providers;
use rustc_target::abi::InitKind;

pub fn provide(providers: &mut Providers) {
const_eval::provide(providers);
Expand All @@ -59,4 +61,8 @@ 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.permits_uninit_init =
|tcx, ty| might_permit_raw_init::might_permit_raw_init(tcx, ty, InitKind::Uninit);
providers.permits_zero_init =
|tcx, ty| might_permit_raw_init::might_permit_raw_init(tcx, ty, InitKind::Zero);
}
40 changes: 40 additions & 0 deletions compiler/rustc_const_eval/src/might_permit_raw_init.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
use crate::const_eval::CompileTimeInterpreter;
use crate::interpret::{InterpCx, MemoryKind, OpTy};
use rustc_middle::ty::layout::LayoutCx;
use rustc_middle::ty::{layout::TyAndLayout, ParamEnv, TyCtxt};
use rustc_session::Limit;
use rustc_target::abi::InitKind;

pub fn might_permit_raw_init<'tcx>(
tcx: TyCtxt<'tcx>,
ty: TyAndLayout<'tcx>,
kind: InitKind,
) -> bool {
let strict = tcx.sess.opts.unstable_opts.strict_init_checks;

if strict {
let machine = CompileTimeInterpreter::new(Limit::new(0), false);

let mut cx = InterpCx::new(tcx, rustc_span::DUMMY_SP, ParamEnv::reveal_all(), machine);

let allocated = cx
.allocate(ty, MemoryKind::Machine(crate::const_eval::MemoryKind::Heap))
.expect("OOM: failed to allocate for uninit check");

if kind == InitKind::Zero {
cx.write_bytes_ptr(
allocated.ptr,
std::iter::repeat(0_u8).take(ty.layout.size().bytes_usize()),
)
.expect("failed to write bytes for zero valid check");
}

let ot: OpTy<'_, _> = allocated.into();

// Assume that if it failed, it's a validation failure.
cx.validate_operand(&ot).is_ok()
} else {
let layout_cx = LayoutCx { tcx, param_env: ParamEnv::reveal_all() };
ty.might_permit_raw_init(&layout_cx, kind)
}
}
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2053,4 +2053,12 @@ rustc_queries! {
desc { |tcx| "looking up generator diagnostic data of `{}`", tcx.def_path_str(key) }
separate_provide_extern
}

query permits_uninit_init(key: TyAndLayout<'tcx>) -> bool {
desc { "checking to see if {:?} permits being left uninit", key.ty }
}

query permits_zero_init(key: TyAndLayout<'tcx>) -> bool {
desc { "checking to see if {:?} permits being left zeroed", key.ty }
}
}
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use crate::traits::query::{
use crate::traits::specialization_graph;
use crate::traits::{self, ImplSource};
use crate::ty::fast_reject::SimplifiedType;
use crate::ty::layout::TyAndLayout;
use crate::ty::subst::{GenericArg, SubstsRef};
use crate::ty::util::AlwaysRequiresDrop;
use crate::ty::GeneratorDiagnosticData;
Expand Down
12 changes: 11 additions & 1 deletion compiler/rustc_query_impl/src/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_middle::mir;
use rustc_middle::traits;
use rustc_middle::ty::fast_reject::SimplifiedType;
use rustc_middle::ty::subst::{GenericArg, SubstsRef};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_middle::ty::{self, layout::TyAndLayout, Ty, TyCtxt};
use rustc_span::symbol::{Ident, Symbol};
use rustc_span::{Span, DUMMY_SP};

Expand Down Expand Up @@ -385,6 +385,16 @@ impl<'tcx> Key for Ty<'tcx> {
}
}

impl<'tcx> Key for TyAndLayout<'tcx> {
#[inline(always)]
fn query_crate_is_local(&self) -> bool {
true
}
fn default_span(&self, _: TyCtxt<'_>) -> Span {
DUMMY_SP
}
}

impl<'tcx> Key for (Ty<'tcx>, Ty<'tcx>) {
#[inline(always)]
fn query_crate_is_local(&self) -> bool {
Expand Down
38 changes: 15 additions & 23 deletions compiler/rustc_target/src/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1372,7 +1372,7 @@ pub struct PointeeInfo {

/// Used in `might_permit_raw_init` to indicate the kind of initialisation
/// that is checked to be valid
#[derive(Copy, Clone, Debug)]
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum InitKind {
Zero,
Uninit,
Expand Down Expand Up @@ -1487,14 +1487,18 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
///
/// `init_kind` indicates if the memory is zero-initialized or left uninitialized.
///
/// `strict` is an opt-in debugging flag added in #97323 that enables more checks.
/// This code is intentionally conservative, and will not detect
/// * zero init of an enum whose 0 variant does not allow zero initialization
/// * making uninitialized types who have a full valid range (ints, floats, raw pointers)
/// * Any form of invalid value being made inside an array (unless the value is uninhabited)
///
/// This is conservative: in doubt, it will answer `true`.
/// A strict form of these checks that uses const evaluation exists in
/// `rustc_const_eval::might_permit_raw_init`, and a tracking issue for making these checks
/// stricter is <https://github.com/rust-lang/rust/issues/66151>.
///
/// FIXME: Once we removed all the conservatism, we could alternatively
/// create an all-0/all-undef constant and run the const value validator to see if
/// this is a valid value for the given type.
pub fn might_permit_raw_init<C>(self, cx: &C, init_kind: InitKind, strict: bool) -> bool
/// FIXME: Once all the conservatism is removed from here, and the checks are ran by default,
/// we can use the const evaluation checks always instead.
pub fn might_permit_raw_init<C>(self, cx: &C, init_kind: InitKind) -> bool
where
Self: Copy,
Ty: TyAbiInterface<'a, C>,
Expand All @@ -1507,13 +1511,8 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
s.valid_range(cx).contains(0)
}
InitKind::Uninit => {
if strict {
// The type must be allowed to be uninit (which means "is a union").
s.is_uninit_valid()
} else {
// The range must include all values.
s.is_always_valid(cx)
}
// The range must include all values.
s.is_always_valid(cx)
}
}
};
Expand All @@ -1534,19 +1533,12 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
// If we have not found an error yet, we need to recursively descend into fields.
match &self.fields {
FieldsShape::Primitive | FieldsShape::Union { .. } => {}
FieldsShape::Array { count, .. } => {
FieldsShape::Array { .. } => {
// FIXME(#66151): For now, we are conservative and do not check arrays by default.
if strict
&& *count > 0
&& !self.field(cx, 0).might_permit_raw_init(cx, init_kind, strict)
{
// Found non empty array with a type that is unhappy about this kind of initialization
return false;
}
}
FieldsShape::Arbitrary { offsets, .. } => {
for idx in 0..offsets.len() {
if !self.field(cx, idx).might_permit_raw_init(cx, init_kind, strict) {
if !self.field(cx, idx).might_permit_raw_init(cx, init_kind) {
// We found a field that is unhappy with this kind of initialization.
return false;
}
Expand Down
Loading

0 comments on commit 27412d1

Please sign in to comment.