Skip to content

Commit 27412d1

Browse files
52252255225225
committed
Use constant eval to do strict validity checks
1 parent c2f428d commit 27412d1

File tree

13 files changed

+161
-94
lines changed

13 files changed

+161
-94
lines changed

‎Cargo.lock

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3664,6 +3664,7 @@ dependencies = [
36643664
"rustc_arena",
36653665
"rustc_ast",
36663666
"rustc_attr",
3667+
"rustc_const_eval",
36673668
"rustc_data_structures",
36683669
"rustc_errors",
36693670
"rustc_fs_util",

‎compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ pub(crate) use llvm::codegen_llvm_intrinsic_call;
5858
use rustc_middle::ty::print::with_no_trimmed_paths;
5959
use rustc_middle::ty::subst::SubstsRef;
6060
use rustc_span::symbol::{kw, sym, Symbol};
61-
use rustc_target::abi::InitKind;
6261

6362
use crate::prelude::*;
6463
use cranelift_codegen::ir::AtomicRmwOp;
@@ -672,12 +671,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
672671
return;
673672
}
674673

675-
if intrinsic == sym::assert_zero_valid
676-
&& !layout.might_permit_raw_init(
677-
fx,
678-
InitKind::Zero,
679-
fx.tcx.sess.opts.unstable_opts.strict_init_checks) {
680-
674+
if intrinsic == sym::assert_zero_valid && !fx.tcx.permits_zero_init(layout) {
681675
with_no_trimmed_paths!({
682676
crate::base::codegen_panic(
683677
fx,
@@ -688,12 +682,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
688682
return;
689683
}
690684

691-
if intrinsic == sym::assert_uninit_valid
692-
&& !layout.might_permit_raw_init(
693-
fx,
694-
InitKind::Uninit,
695-
fx.tcx.sess.opts.unstable_opts.strict_init_checks) {
696-
685+
if intrinsic == sym::assert_uninit_valid && !fx.tcx.permits_uninit_init(layout) {
697686
with_no_trimmed_paths!({
698687
crate::base::codegen_panic(
699688
fx,

‎compiler/rustc_codegen_ssa/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ rustc_metadata = { path = "../rustc_metadata" }
4040
rustc_query_system = { path = "../rustc_query_system" }
4141
rustc_target = { path = "../rustc_target" }
4242
rustc_session = { path = "../rustc_session" }
43+
rustc_const_eval = { path = "../rustc_const_eval" }
4344

4445
[dependencies.object]
4546
version = "0.29.0"

‎compiler/rustc_codegen_ssa/src/mir/block.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use rustc_span::source_map::Span;
2222
use rustc_span::{sym, Symbol};
2323
use rustc_symbol_mangling::typeid_for_fnabi;
2424
use rustc_target::abi::call::{ArgAbi, FnAbi, PassMode};
25-
use rustc_target::abi::{self, HasDataLayout, InitKind, WrappingRange};
25+
use rustc_target::abi::{self, HasDataLayout, WrappingRange};
2626
use rustc_target::spec::abi::Abi;
2727

2828
/// Used by `FunctionCx::codegen_terminator` for emitting common patterns
@@ -528,7 +528,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
528528
source_info: mir::SourceInfo,
529529
target: Option<mir::BasicBlock>,
530530
cleanup: Option<mir::BasicBlock>,
531-
strict_validity: bool,
532531
) -> bool {
533532
// Emit a panic or a no-op for `assert_*` intrinsics.
534533
// These are intrinsics that compile to panics so that we can get a message
@@ -547,12 +546,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
547546
});
548547
if let Some(intrinsic) = panic_intrinsic {
549548
use AssertIntrinsic::*;
549+
550550
let ty = instance.unwrap().substs.type_at(0);
551551
let layout = bx.layout_of(ty);
552552
let do_panic = match intrinsic {
553553
Inhabited => layout.abi.is_uninhabited(),
554-
ZeroValid => !layout.might_permit_raw_init(bx, InitKind::Zero, strict_validity),
555-
UninitValid => !layout.might_permit_raw_init(bx, InitKind::Uninit, strict_validity),
554+
ZeroValid => !bx.tcx().permits_zero_init(layout),
555+
UninitValid => !bx.tcx().permits_uninit_init(layout),
556556
};
557557
if do_panic {
558558
let msg_str = with_no_visible_paths!({
@@ -687,7 +687,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
687687
source_info,
688688
target,
689689
cleanup,
690-
self.cx.tcx().sess.opts.unstable_opts.strict_init_checks,
691690
) {
692691
return;
693692
}

‎compiler/rustc_const_eval/src/const_eval/machine.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {
104104
}
105105

106106
impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {
107-
pub(super) fn new(const_eval_limit: Limit, can_access_statics: bool) -> Self {
107+
pub(crate) fn new(const_eval_limit: Limit, can_access_statics: bool) -> Self {
108108
CompileTimeInterpreter {
109109
steps_remaining: const_eval_limit.0,
110110
stack: Vec::new(),

‎compiler/rustc_const_eval/src/interpret/intrinsics.rs

Lines changed: 27 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use rustc_middle::ty::layout::LayoutOf as _;
1515
use rustc_middle::ty::subst::SubstsRef;
1616
use rustc_middle::ty::{Ty, TyCtxt};
1717
use rustc_span::symbol::{sym, Symbol};
18-
use rustc_target::abi::{Abi, Align, InitKind, Primitive, Size};
18+
use rustc_target::abi::{Abi, Align, Primitive, Size};
1919

2020
use super::{
2121
util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx, Machine, OpTy, PlaceTy,
@@ -413,35 +413,33 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
413413
),
414414
)?;
415415
}
416-
if intrinsic_name == sym::assert_zero_valid
417-
&& !layout.might_permit_raw_init(
418-
self,
419-
InitKind::Zero,
420-
self.tcx.sess.opts.unstable_opts.strict_init_checks,
421-
)
422-
{
423-
M::abort(
424-
self,
425-
format!(
426-
"aborted execution: attempted to zero-initialize type `{}`, which is invalid",
427-
ty
428-
),
429-
)?;
416+
417+
if intrinsic_name == sym::assert_zero_valid {
418+
let should_panic = !self.tcx.permits_zero_init(layout);
419+
420+
if should_panic {
421+
M::abort(
422+
self,
423+
format!(
424+
"aborted execution: attempted to zero-initialize type `{}`, which is invalid",
425+
ty
426+
),
427+
)?;
428+
}
430429
}
431-
if intrinsic_name == sym::assert_uninit_valid
432-
&& !layout.might_permit_raw_init(
433-
self,
434-
InitKind::Uninit,
435-
self.tcx.sess.opts.unstable_opts.strict_init_checks,
436-
)
437-
{
438-
M::abort(
439-
self,
440-
format!(
441-
"aborted execution: attempted to leave type `{}` uninitialized, which is invalid",
442-
ty
443-
),
444-
)?;
430+
431+
if intrinsic_name == sym::assert_uninit_valid {
432+
let should_panic = !self.tcx.permits_uninit_init(layout);
433+
434+
if should_panic {
435+
M::abort(
436+
self,
437+
format!(
438+
"aborted execution: attempted to leave type `{}` uninitialized, which is invalid",
439+
ty
440+
),
441+
)?;
442+
}
445443
}
446444
}
447445
sym::simd_insert => {

‎compiler/rustc_const_eval/src/lib.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,13 @@ extern crate rustc_middle;
3333
pub mod const_eval;
3434
mod errors;
3535
pub mod interpret;
36+
mod might_permit_raw_init;
3637
pub mod transform;
3738
pub mod util;
3839

3940
use rustc_middle::ty;
4041
use rustc_middle::ty::query::Providers;
42+
use rustc_target::abi::InitKind;
4143

4244
pub fn provide(providers: &mut Providers) {
4345
const_eval::provide(providers);
@@ -59,4 +61,8 @@ pub fn provide(providers: &mut Providers) {
5961
let (param_env, value) = param_env_and_value.into_parts();
6062
const_eval::deref_mir_constant(tcx, param_env, value)
6163
};
64+
providers.permits_uninit_init =
65+
|tcx, ty| might_permit_raw_init::might_permit_raw_init(tcx, ty, InitKind::Uninit);
66+
providers.permits_zero_init =
67+
|tcx, ty| might_permit_raw_init::might_permit_raw_init(tcx, ty, InitKind::Zero);
6268
}

‎compiler/rustc_const_eval/src/might_permit_raw_init.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
use crate::const_eval::CompileTimeInterpreter;
2+
use crate::interpret::{InterpCx, MemoryKind, OpTy};
3+
use rustc_middle::ty::layout::LayoutCx;
4+
use rustc_middle::ty::{layout::TyAndLayout, ParamEnv, TyCtxt};
5+
use rustc_session::Limit;
6+
use rustc_target::abi::InitKind;
7+
8+
pub fn might_permit_raw_init<'tcx>(
9+
tcx: TyCtxt<'tcx>,
10+
ty: TyAndLayout<'tcx>,
11+
kind: InitKind,
12+
) -> bool {
13+
let strict = tcx.sess.opts.unstable_opts.strict_init_checks;
14+
15+
if strict {
16+
let machine = CompileTimeInterpreter::new(Limit::new(0), false);
17+
18+
let mut cx = InterpCx::new(tcx, rustc_span::DUMMY_SP, ParamEnv::reveal_all(), machine);
19+
20+
let allocated = cx
21+
.allocate(ty, MemoryKind::Machine(crate::const_eval::MemoryKind::Heap))
22+
.expect("OOM: failed to allocate for uninit check");
23+
24+
if kind == InitKind::Zero {
25+
cx.write_bytes_ptr(
26+
allocated.ptr,
27+
std::iter::repeat(0_u8).take(ty.layout.size().bytes_usize()),
28+
)
29+
.expect("failed to write bytes for zero valid check");
30+
}
31+
32+
let ot: OpTy<'_, _> = allocated.into();
33+
34+
// Assume that if it failed, it's a validation failure.
35+
cx.validate_operand(&ot).is_ok()
36+
} else {
37+
let layout_cx = LayoutCx { tcx, param_env: ParamEnv::reveal_all() };
38+
ty.might_permit_raw_init(&layout_cx, kind)
39+
}
40+
}

‎compiler/rustc_middle/src/query/mod.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2053,4 +2053,12 @@ rustc_queries! {
20532053
desc { |tcx| "looking up generator diagnostic data of `{}`", tcx.def_path_str(key) }
20542054
separate_provide_extern
20552055
}
2056+
2057+
query permits_uninit_init(key: TyAndLayout<'tcx>) -> bool {
2058+
desc { "checking to see if {:?} permits being left uninit", key.ty }
2059+
}
2060+
2061+
query permits_zero_init(key: TyAndLayout<'tcx>) -> bool {
2062+
desc { "checking to see if {:?} permits being left zeroed", key.ty }
2063+
}
20562064
}

‎compiler/rustc_middle/src/ty/query.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ use crate::traits::query::{
2828
use crate::traits::specialization_graph;
2929
use crate::traits::{self, ImplSource};
3030
use crate::ty::fast_reject::SimplifiedType;
31+
use crate::ty::layout::TyAndLayout;
3132
use crate::ty::subst::{GenericArg, SubstsRef};
3233
use crate::ty::util::AlwaysRequiresDrop;
3334
use crate::ty::GeneratorDiagnosticData;

‎compiler/rustc_query_impl/src/keys.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use rustc_middle::mir;
66
use rustc_middle::traits;
77
use rustc_middle::ty::fast_reject::SimplifiedType;
88
use rustc_middle::ty::subst::{GenericArg, SubstsRef};
9-
use rustc_middle::ty::{self, Ty, TyCtxt};
9+
use rustc_middle::ty::{self, layout::TyAndLayout, Ty, TyCtxt};
1010
use rustc_span::symbol::{Ident, Symbol};
1111
use rustc_span::{Span, DUMMY_SP};
1212

@@ -385,6 +385,16 @@ impl<'tcx> Key for Ty<'tcx> {
385385
}
386386
}
387387

388+
impl<'tcx> Key for TyAndLayout<'tcx> {
389+
#[inline(always)]
390+
fn query_crate_is_local(&self) -> bool {
391+
true
392+
}
393+
fn default_span(&self, _: TyCtxt<'_>) -> Span {
394+
DUMMY_SP
395+
}
396+
}
397+
388398
impl<'tcx> Key for (Ty<'tcx>, Ty<'tcx>) {
389399
#[inline(always)]
390400
fn query_crate_is_local(&self) -> bool {

‎compiler/rustc_target/src/abi/mod.rs

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,7 +1372,7 @@ pub struct PointeeInfo {
13721372

13731373
/// Used in `might_permit_raw_init` to indicate the kind of initialisation
13741374
/// that is checked to be valid
1375-
#[derive(Copy, Clone, Debug)]
1375+
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
13761376
pub enum InitKind {
13771377
Zero,
13781378
Uninit,
@@ -1487,14 +1487,18 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
14871487
///
14881488
/// `init_kind` indicates if the memory is zero-initialized or left uninitialized.
14891489
///
1490-
/// `strict` is an opt-in debugging flag added in #97323 that enables more checks.
1490+
/// This code is intentionally conservative, and will not detect
1491+
/// * zero init of an enum whose 0 variant does not allow zero initialization
1492+
/// * making uninitialized types who have a full valid range (ints, floats, raw pointers)
1493+
/// * Any form of invalid value being made inside an array (unless the value is uninhabited)
14911494
///
1492-
/// This is conservative: in doubt, it will answer `true`.
1495+
/// A strict form of these checks that uses const evaluation exists in
1496+
/// `rustc_const_eval::might_permit_raw_init`, and a tracking issue for making these checks
1497+
/// stricter is <https://github.com/rust-lang/rust/issues/66151>.
14931498
///
1494-
/// FIXME: Once we removed all the conservatism, we could alternatively
1495-
/// create an all-0/all-undef constant and run the const value validator to see if
1496-
/// this is a valid value for the given type.
1497-
pub fn might_permit_raw_init<C>(self, cx: &C, init_kind: InitKind, strict: bool) -> bool
1499+
/// FIXME: Once all the conservatism is removed from here, and the checks are ran by default,
1500+
/// we can use the const evaluation checks always instead.
1501+
pub fn might_permit_raw_init<C>(self, cx: &C, init_kind: InitKind) -> bool
14981502
where
14991503
Self: Copy,
15001504
Ty: TyAbiInterface<'a, C>,
@@ -1507,13 +1511,8 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
15071511
s.valid_range(cx).contains(0)
15081512
}
15091513
InitKind::Uninit => {
1510-
if strict {
1511-
// The type must be allowed to be uninit (which means "is a union").
1512-
s.is_uninit_valid()
1513-
} else {
1514-
// The range must include all values.
1515-
s.is_always_valid(cx)
1516-
}
1514+
// The range must include all values.
1515+
s.is_always_valid(cx)
15171516
}
15181517
}
15191518
};
@@ -1534,19 +1533,12 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
15341533
// If we have not found an error yet, we need to recursively descend into fields.
15351534
match &self.fields {
15361535
FieldsShape::Primitive | FieldsShape::Union { .. } => {}
1537-
FieldsShape::Array { count, .. } => {
1536+
FieldsShape::Array { .. } => {
15381537
// FIXME(#66151): For now, we are conservative and do not check arrays by default.
1539-
if strict
1540-
&& *count > 0
1541-
&& !self.field(cx, 0).might_permit_raw_init(cx, init_kind, strict)
1542-
{
1543-
// Found non empty array with a type that is unhappy about this kind of initialization
1544-
return false;
1545-
}
15461538
}
15471539
FieldsShape::Arbitrary { offsets, .. } => {
15481540
for idx in 0..offsets.len() {
1549-
if !self.field(cx, idx).might_permit_raw_init(cx, init_kind, strict) {
1541+
if !self.field(cx, idx).might_permit_raw_init(cx, init_kind) {
15501542
// We found a field that is unhappy with this kind of initialization.
15511543
return false;
15521544
}

0 commit comments

Comments
 (0)