Skip to content

Commit b361638

Browse files
52252255225225
committed
Use constant eval to do strict validity checks
1 parent 73443a0 commit b361638

File tree

9 files changed

+135
-76
lines changed

9 files changed

+135
-76
lines changed

‎Cargo.lock

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3680,6 +3680,7 @@ dependencies = [
36803680
"rustc_arena",
36813681
"rustc_ast",
36823682
"rustc_attr",
3683+
"rustc_const_eval",
36833684
"rustc_data_structures",
36843685
"rustc_errors",
36853686
"rustc_fs_util",

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -675,8 +675,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
675675
if intrinsic == sym::assert_zero_valid
676676
&& !layout.might_permit_raw_init(
677677
fx,
678-
InitKind::Zero,
679-
fx.tcx.sess.opts.debugging_opts.strict_init_checks) {
678+
InitKind::Zero) {
680679

681680
with_no_trimmed_paths!({
682681
crate::base::codegen_panic(
@@ -691,8 +690,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
691690
if intrinsic == sym::assert_uninit_valid
692691
&& !layout.might_permit_raw_init(
693692
fx,
694-
InitKind::Uninit,
695-
fx.tcx.sess.opts.debugging_opts.strict_init_checks) {
693+
InitKind::Uninit) {
696694

697695
with_no_trimmed_paths!({
698696
crate::base::codegen_panic(

‎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: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -549,10 +549,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
549549
use AssertIntrinsic::*;
550550
let ty = instance.unwrap().substs.type_at(0);
551551
let layout = bx.layout_of(ty);
552-
let do_panic = match intrinsic {
553-
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),
552+
let do_panic = match (&intrinsic, strict_validity) {
553+
(Inhabited, _) => layout.abi.is_uninhabited(),
554+
(ZeroValid, true) => {
555+
!rustc_const_eval::is_zero_valid(bx.tcx(), source_info.span, layout)
556+
}
557+
(UninitValid, true) => {
558+
!rustc_const_eval::is_uninit_valid(bx.tcx(), source_info.span, layout)
559+
}
560+
(ZeroValid, false) => !layout.might_permit_raw_init(bx, InitKind::Zero),
561+
(UninitValid, false) => !layout.might_permit_raw_init(bx, InitKind::Uninit),
556562
};
557563
if do_panic {
558564
let msg_str = with_no_visible_paths!({

‎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: 34 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -413,35 +413,41 @@ 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.debugging_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 = if self.tcx.sess.opts.debugging_opts.strict_init_checks {
419+
!crate::is_zero_valid(*self.tcx, self.cur_span(), layout)
420+
} else {
421+
!layout.might_permit_raw_init(self, InitKind::Zero)
422+
};
423+
424+
if should_panic {
425+
M::abort(
426+
self,
427+
format!(
428+
"aborted execution: attempted to zero-initialize type `{}`, which is invalid",
429+
ty
430+
),
431+
)?;
432+
}
430433
}
431-
if intrinsic_name == sym::assert_uninit_valid
432-
&& !layout.might_permit_raw_init(
433-
self,
434-
InitKind::Uninit,
435-
self.tcx.sess.opts.debugging_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-
)?;
434+
435+
if intrinsic_name == sym::assert_uninit_valid {
436+
let should_panic = if self.tcx.sess.opts.debugging_opts.strict_init_checks {
437+
!crate::is_uninit_valid(*self.tcx, self.cur_span(), layout)
438+
} else {
439+
!layout.might_permit_raw_init(self, InitKind::Uninit)
440+
};
441+
442+
if should_panic {
443+
M::abort(
444+
self,
445+
format!(
446+
"aborted execution: attempted to leave type `{}` uninitialized, which is invalid",
447+
ty
448+
),
449+
)?;
450+
}
445451
}
446452
}
447453
sym::simd_insert => {

‎compiler/rustc_const_eval/src/lib.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,41 @@ pub fn provide(providers: &mut Providers) {
6060
const_eval::deref_mir_constant(tcx, param_env, value)
6161
};
6262
}
63+
64+
use crate::const_eval::CompileTimeInterpreter;
65+
use crate::interpret::{InterpCx, MemoryKind, OpTy};
66+
use rustc_middle::ty::{layout::TyAndLayout, ParamEnv, TyCtxt};
67+
use rustc_session::Limit;
68+
use rustc_span::Span;
69+
70+
pub fn is_uninit_valid<'tcx>(tcx: TyCtxt<'tcx>, root_span: Span, ty: TyAndLayout<'tcx>) -> bool {
71+
let machine = CompileTimeInterpreter::new(Limit::new(0), false);
72+
let mut cx = InterpCx::new(tcx, root_span, ParamEnv::reveal_all(), machine);
73+
let allocated = cx
74+
.allocate(ty, MemoryKind::Machine(const_eval::MemoryKind::Heap))
75+
.expect("failed to allocate for uninit check");
76+
let ot: OpTy<'_, _> = allocated.into();
77+
cx.validate_operand(&ot).is_ok()
78+
}
79+
80+
pub fn is_zero_valid<'tcx>(tcx: TyCtxt<'tcx>, root_span: Span, ty: TyAndLayout<'tcx>) -> bool {
81+
let machine = CompileTimeInterpreter::new(Limit::new(0), false);
82+
83+
let mut cx = InterpCx::new(tcx, root_span, ParamEnv::reveal_all(), machine);
84+
85+
// We could panic here... Or we could just return "yeah it's valid whatever". Or let
86+
// codegen_panic_intrinsic return an error that halts compilation.
87+
// I'm not exactly sure *when* this can fail. OOM?
88+
let allocated = cx
89+
.allocate(ty, MemoryKind::Machine(const_eval::MemoryKind::Heap))
90+
.expect("failed to allocate for uninit check");
91+
92+
// Again, unclear what to do here if it fails.
93+
cx.write_bytes_ptr(allocated.ptr, std::iter::repeat(0_u8).take(ty.layout.size().bytes_usize()))
94+
.expect("failed to write bytes for zero valid check");
95+
96+
let ot: OpTy<'_, _> = allocated.into();
97+
98+
// Assume that if it failed, it's a validation failure.
99+
cx.validate_operand(&ot).is_ok()
100+
}

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

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1494,7 +1494,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
14941494
/// FIXME: Once we removed all the conservatism, we could alternatively
14951495
/// create an all-0/all-undef constant and run the const value validator to see if
14961496
/// 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
1497+
pub fn might_permit_raw_init<C>(self, cx: &C, init_kind: InitKind) -> bool
14981498
where
14991499
Self: Copy,
15001500
Ty: TyAbiInterface<'a, C>,
@@ -1507,13 +1507,8 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
15071507
s.valid_range(cx).contains(0)
15081508
}
15091509
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-
}
1510+
// The range must include all values.
1511+
s.is_always_valid(cx)
15171512
}
15181513
}
15191514
};
@@ -1534,19 +1529,12 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
15341529
// If we have not found an error yet, we need to recursively descend into fields.
15351530
match &self.fields {
15361531
FieldsShape::Primitive | FieldsShape::Union { .. } => {}
1537-
FieldsShape::Array { count, .. } => {
1532+
FieldsShape::Array { .. } => {
15381533
// 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-
}
15461534
}
15471535
FieldsShape::Arbitrary { offsets, .. } => {
15481536
for idx in 0..offsets.len() {
1549-
if !self.field(cx, idx).might_permit_raw_init(cx, init_kind, strict) {
1537+
if !self.field(cx, idx).might_permit_raw_init(cx, init_kind) {
15501538
// We found a field that is unhappy with this kind of initialization.
15511539
return false;
15521540
}

‎src/test/ui/intrinsics/panic-uninitialized-zeroed.rs

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,13 @@ enum LR_NonZero {
5757

5858
struct ZeroSized;
5959

60+
#[allow(dead_code)]
61+
#[repr(i32)]
62+
enum ZeroIsValid {
63+
Zero(u8) = 0,
64+
One(NonNull<()>) = 1,
65+
}
66+
6067
fn test_panic_msg<T>(op: impl (FnOnce() -> T) + panic::UnwindSafe, msg: &str) {
6168
let err = panic::catch_unwind(op).err();
6269
assert_eq!(
@@ -152,33 +159,12 @@ fn main() {
152159
"attempted to zero-initialize type `*const dyn core::marker::Send`, which is invalid"
153160
);
154161

155-
/* FIXME(#66151) we conservatively do not error here yet.
156-
test_panic_msg(
157-
|| mem::uninitialized::<LR_NonZero>(),
158-
"attempted to leave type `LR_NonZero` uninitialized, which is invalid"
159-
);
160-
test_panic_msg(
161-
|| mem::zeroed::<LR_NonZero>(),
162-
"attempted to zero-initialize type `LR_NonZero`, which is invalid"
163-
);
164-
165-
test_panic_msg(
166-
|| mem::uninitialized::<ManuallyDrop<LR_NonZero>>(),
167-
"attempted to leave type `std::mem::ManuallyDrop<LR_NonZero>` uninitialized, \
168-
which is invalid"
169-
);
170-
test_panic_msg(
171-
|| mem::zeroed::<ManuallyDrop<LR_NonZero>>(),
172-
"attempted to zero-initialize type `std::mem::ManuallyDrop<LR_NonZero>`, \
173-
which is invalid"
174-
);
175-
*/
176-
177162
test_panic_msg(
178163
|| mem::uninitialized::<(NonNull<u32>, u32, u32)>(),
179164
"attempted to leave type `(core::ptr::non_null::NonNull<u32>, u32, u32)` uninitialized, \
180165
which is invalid"
181166
);
167+
182168
test_panic_msg(
183169
|| mem::zeroed::<(NonNull<u32>, u32, u32)>(),
184170
"attempted to zero-initialize type `(core::ptr::non_null::NonNull<u32>, u32, u32)`, \
@@ -196,11 +182,23 @@ fn main() {
196182
which is invalid"
197183
);
198184

185+
test_panic_msg(
186+
|| mem::uninitialized::<LR_NonZero>(),
187+
"attempted to leave type `LR_NonZero` uninitialized, which is invalid"
188+
);
189+
190+
test_panic_msg(
191+
|| mem::uninitialized::<ManuallyDrop<LR_NonZero>>(),
192+
"attempted to leave type `core::mem::manually_drop::ManuallyDrop<LR_NonZero>` uninitialized, \
193+
which is invalid"
194+
);
195+
199196
test_panic_msg(
200197
|| mem::uninitialized::<NoNullVariant>(),
201198
"attempted to leave type `NoNullVariant` uninitialized, \
202199
which is invalid"
203200
);
201+
204202
test_panic_msg(
205203
|| mem::zeroed::<NoNullVariant>(),
206204
"attempted to zero-initialize type `NoNullVariant`, \
@@ -212,10 +210,12 @@ fn main() {
212210
|| mem::uninitialized::<bool>(),
213211
"attempted to leave type `bool` uninitialized, which is invalid"
214212
);
213+
215214
test_panic_msg(
216215
|| mem::uninitialized::<LR>(),
217216
"attempted to leave type `LR` uninitialized, which is invalid"
218217
);
218+
219219
test_panic_msg(
220220
|| mem::uninitialized::<ManuallyDrop<LR>>(),
221221
"attempted to leave type `core::mem::manually_drop::ManuallyDrop<LR>` uninitialized, which is invalid"
@@ -229,6 +229,7 @@ fn main() {
229229
let _val = mem::zeroed::<Option<&'static i32>>();
230230
let _val = mem::zeroed::<MaybeUninit<NonNull<u32>>>();
231231
let _val = mem::zeroed::<[!; 0]>();
232+
let _val = mem::zeroed::<ZeroIsValid>();
232233
let _val = mem::uninitialized::<MaybeUninit<bool>>();
233234
let _val = mem::uninitialized::<[!; 0]>();
234235
let _val = mem::uninitialized::<()>();
@@ -259,12 +260,32 @@ fn main() {
259260
|| mem::zeroed::<[NonNull<()>; 1]>(),
260261
"attempted to zero-initialize type `[core::ptr::non_null::NonNull<()>; 1]`, which is invalid"
261262
);
263+
264+
// FIXME(#66151) we conservatively do not error here yet (by default).
265+
test_panic_msg(
266+
|| mem::zeroed::<LR_NonZero>(),
267+
"attempted to zero-initialize type `LR_NonZero`, which is invalid"
268+
);
269+
270+
test_panic_msg(
271+
|| mem::zeroed::<ManuallyDrop<LR_NonZero>>(),
272+
"attempted to zero-initialize type `core::mem::manually_drop::ManuallyDrop<LR_NonZero>`, \
273+
which is invalid"
274+
);
262275
} else {
263276
// These are UB because they have not been officially blessed, but we await the resolution
264277
// of <https://github.com/rust-lang/unsafe-code-guidelines/issues/71> before doing
265278
// anything about that.
266279
let _val = mem::uninitialized::<i32>();
267280
let _val = mem::uninitialized::<*const ()>();
281+
282+
// These are UB, but best to test them to ensure we don't become unintentionally
283+
// stricter.
284+
285+
// It's currently unchecked to create invalid enums and values inside arrays.
286+
let _val = mem::zeroed::<LR_NonZero>();
287+
let _val = mem::zeroed::<[LR_NonZero; 1]>();
288+
let _val = mem::zeroed::<[NonNull<()>; 1]>();
268289
}
269290
}
270291
}

0 commit comments

Comments
 (0)