Skip to content

Commit d6bcb8b

Browse files
saethlinsaethlin
committed
Try to turn on validation in CTFE for unsafe code
Instead of only validating values which actually get stored to a const, this PR attempts to turn on full validation in the presence of unsafe code, as detected by walking HIR when available.
1 parent 5f33adc commit d6bcb8b

File tree

75 files changed

+887
-1639
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

75 files changed

+887
-1639
lines changed

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

Lines changed: 113 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,17 @@ use rustc_hir::def::DefKind;
22
use rustc_middle::mir;
33
use rustc_middle::ty::{self, Ty, TyCtxt};
44
use std::borrow::Borrow;
5+
use std::cell::RefCell;
56
use std::collections::hash_map::Entry;
67
use std::hash::Hash;
78

89
use rustc_data_structures::fx::FxHashMap;
910
use std::fmt;
1011

1112
use rustc_ast::Mutability;
12-
use rustc_hir::def_id::DefId;
13+
use rustc_hir::def_id::{DefId, LocalDefId};
14+
use rustc_hir::intravisit::Visitor;
15+
use rustc_hir::Node;
1316
use rustc_middle::mir::AssertMessage;
1417
use rustc_session::Limit;
1518
use rustc_span::symbol::{sym, Symbol};
@@ -18,7 +21,7 @@ use rustc_target::spec::abi::Abi;
1821

1922
use crate::interpret::{
2023
self, compile_time_machine, AllocId, ConstAllocation, Frame, ImmTy, InterpCx, InterpResult,
21-
OpTy, PlaceTy, Pointer, Scalar, StackPopUnwind,
24+
Machine, OpTy, PlaceTy, Pointer, Scalar, StackPopUnwind,
2225
};
2326

2427
use super::error::*;
@@ -101,6 +104,8 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {
101104
/// * Pointers to allocations inside of statics can never leak outside, to a non-static global.
102105
/// This boolean here controls the second part.
103106
pub(super) can_access_statics: bool,
107+
108+
unsafe_detector: RefCell<UnsafeDetector>,
104109
}
105110

106111
impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {
@@ -109,6 +114,7 @@ impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {
109114
steps_remaining: const_eval_limit.0,
110115
stack: Vec::new(),
111116
can_access_statics,
117+
unsafe_detector: RefCell::new(UnsafeDetector::default()),
112118
}
113119
}
114120
}
@@ -229,9 +235,102 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
229235
}
230236
}
231237

238+
struct FindUnsafeVisitor<'tcx> {
239+
tcx: TyCtxt<'tcx>,
240+
found_unsafe: bool,
241+
}
242+
243+
impl<'tcx> Visitor<'tcx> for FindUnsafeVisitor<'tcx> {
244+
type NestedFilter = rustc_middle::hir::nested_filter::All;
245+
246+
fn nested_visit_map(&mut self) -> Self::Map {
247+
self.tcx.hir()
248+
}
249+
250+
fn visit_block(&mut self, block: &'tcx rustc_hir::Block<'tcx>) {
251+
rustc_hir::intravisit::walk_block(self, block);
252+
if let rustc_hir::BlockCheckMode::UnsafeBlock(_) = block.rules {
253+
self.found_unsafe = true;
254+
}
255+
}
256+
}
257+
258+
#[cold]
259+
#[inline(never)]
260+
fn may_contain_unsafe<'mir, 'tcx>(
261+
ecx: &InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>,
262+
def_id: DefId,
263+
) -> bool {
264+
let hir = ecx.tcx.hir();
265+
if let Some(Node::Item(item)) = hir.get_if_local(def_id) {
266+
let mut visitor = FindUnsafeVisitor { tcx: *ecx.tcx, found_unsafe: false };
267+
visitor.visit_item(&item);
268+
visitor.found_unsafe
269+
} else {
270+
true
271+
}
272+
}
273+
274+
#[derive(Default)]
275+
struct UnsafeDetector {
276+
loaded_mir_with_unsafe: Option<bool>,
277+
known_safe_defs: FxHashMap<LocalDefId, bool>,
278+
}
279+
280+
impl UnsafeDetector {
281+
#[cold]
282+
#[inline(never)]
283+
fn analyze_def<'mir, 'tcx>(
284+
&mut self,
285+
ecx: &InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>,
286+
def_id: DefId,
287+
) {
288+
if let Some(local_id) = def_id.as_local() {
289+
let found_unsafe = *self
290+
.known_safe_defs
291+
.entry(local_id)
292+
.or_insert_with(|| may_contain_unsafe(ecx, def_id));
293+
self.loaded_mir_with_unsafe = Some(found_unsafe);
294+
} else {
295+
self.loaded_mir_with_unsafe = Some(true);
296+
}
297+
}
298+
299+
#[cold]
300+
#[inline(never)]
301+
fn analyze_stack<'mir, 'tcx>(ecx: &InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>) {
302+
let mut this = ecx.machine.unsafe_detector.borrow_mut();
303+
let stack = CompileTimeInterpreter::stack(ecx);
304+
if stack.len() == 1 {
305+
let frame = stack.last().unwrap();
306+
this.analyze_def(ecx, frame.instance.def_id());
307+
} else {
308+
this.loaded_mir_with_unsafe = Some(true);
309+
}
310+
}
311+
312+
#[inline]
313+
fn is_init(&self) -> bool {
314+
self.loaded_mir_with_unsafe.is_some()
315+
}
316+
317+
#[inline]
318+
fn mir_needs_validation(&self) -> bool {
319+
self.loaded_mir_with_unsafe == Some(true)
320+
}
321+
}
322+
232323
impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, 'tcx> {
233324
compile_time_machine!(<'mir, 'tcx>);
234325

326+
fn enforce_validity(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> bool {
327+
let unsafe_detector = ecx.machine.unsafe_detector.get_mut();
328+
if !unsafe_detector.is_init() {
329+
UnsafeDetector::analyze_stack(ecx);
330+
}
331+
ecx.machine.unsafe_detector.get_mut().mir_needs_validation()
332+
}
333+
235334
type MemoryKind = MemoryKind;
236335

237336
const PANIC_ON_ALLOC_FAIL: bool = false; // will be raised as a proper error
@@ -240,10 +339,12 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
240339
ecx: &InterpCx<'mir, 'tcx, Self>,
241340
instance: ty::InstanceDef<'tcx>,
242341
) -> InterpResult<'tcx, &'tcx mir::Body<'tcx>> {
243-
match instance {
342+
ecx.machine.unsafe_detector.borrow_mut().analyze_def(ecx, instance.def_id());
343+
344+
let mir = match instance {
244345
ty::InstanceDef::Item(def) => {
245346
if ecx.tcx.is_ctfe_mir_available(def.did) {
246-
Ok(ecx.tcx.mir_for_ctfe_opt_const_arg(def))
347+
ecx.tcx.mir_for_ctfe_opt_const_arg(def)
247348
} else if ecx.tcx.def_kind(def.did) == DefKind::AssocConst {
248349
let guar = ecx.tcx.sess.delay_span_bug(
249350
rustc_span::DUMMY_SP,
@@ -252,12 +353,16 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
252353
throw_inval!(AlreadyReported(guar));
253354
} else {
254355
let path = ecx.tcx.def_path_str(def.did);
255-
Err(ConstEvalErrKind::NeedsRfc(format!("calling extern function `{}`", path))
256-
.into())
356+
return Err(ConstEvalErrKind::NeedsRfc(format!(
357+
"calling extern function `{}`",
358+
path
359+
))
360+
.into());
257361
}
258362
}
259-
_ => Ok(ecx.tcx.instance_mir(instance)),
260-
}
363+
_ => ecx.tcx.instance_mir(instance),
364+
};
365+
Ok(mir)
261366
}
262367

263368
fn find_mir_or_eval_fn(

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
131131
fn force_int_for_alignment_check(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
132132

133133
/// Whether to enforce the validity invariant
134-
fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
134+
fn enforce_validity(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> bool;
135135

136136
/// Whether to enforce integers and floats being initialized.
137137
fn enforce_number_init(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
@@ -450,7 +450,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
450450
}
451451

452452
#[inline(always)]
453-
fn enforce_validity(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
453+
fn enforce_validity(_ecx: &mut InterpCx<$mir, $tcx, Self>) -> bool {
454454
false // for now, we don't enforce validity
455455
}
456456

‎compiler/rustc_mir_transform/src/const_prop.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,12 @@ impl ConstPropMachine<'_, '_> {
184184

185185
impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx> {
186186
compile_time_machine!(<'mir, 'tcx>);
187+
188+
#[inline(always)]
189+
fn enforce_validity(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> bool {
190+
false
191+
}
192+
187193
const PANIC_ON_ALLOC_FAIL: bool = true; // all allocations are small (see `MAX_ALLOC_LIMIT`)
188194

189195
type MemoryKind = !;

‎compiler/rustc_mir_transform/src/const_prop_lint.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,12 @@ impl ConstPropMachine<'_, '_> {
176176

177177
impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx> {
178178
compile_time_machine!(<'mir, 'tcx>);
179+
180+
#[inline(always)]
181+
fn enforce_validity(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> bool {
182+
false
183+
}
184+
179185
const PANIC_ON_ALLOC_FAIL: bool = true; // all allocations are small (see `MAX_ALLOC_LIMIT`)
180186

181187
type MemoryKind = !;

‎src/test/ui/const-generics/min_const_generics/invalid-patterns.32bit.stderr‎

Lines changed: 12 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,49 +22,29 @@ error[E0308]: mismatched types
2222
LL | get_flag::<42, 0x5ad>();
2323
| ^^^^^ expected `char`, found `u8`
2424

25-
error[E0080]: it is undefined behavior to use this value
26-
--> $DIR/invalid-patterns.rs:38:21
25+
error[E0080]: evaluation of constant value failed
26+
--> $DIR/invalid-patterns.rs:38:32
2727
|
2828
LL | get_flag::<false, { unsafe { char_raw.character } }>();
29-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
30-
|
31-
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
32-
= note: the raw bytes of the constant (size: 4, align: 4) {
33-
__ __ __ __ │ ░░░░
34-
}
29+
| ^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
3530

36-
error[E0080]: it is undefined behavior to use this value
37-
--> $DIR/invalid-patterns.rs:40:14
31+
error[E0080]: evaluation of constant value failed
32+
--> $DIR/invalid-patterns.rs:40:25
3833
|
3934
LL | get_flag::<{ unsafe { bool_raw.boolean } }, 'z'>();
40-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x42, but expected a boolean
41-
|
42-
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
43-
= note: the raw bytes of the constant (size: 1, align: 1) {
44-
42 │ B
45-
}
35+
| ^^^^^^^^^^^^^^^^ type validation failed: encountered 0x42, but expected a boolean
4636

47-
error[E0080]: it is undefined behavior to use this value
48-
--> $DIR/invalid-patterns.rs:42:14
37+
error[E0080]: evaluation of constant value failed
38+
--> $DIR/invalid-patterns.rs:42:25
4939
|
5040
LL | get_flag::<{ unsafe { bool_raw.boolean } }, { unsafe { char_raw.character } }>();
51-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x42, but expected a boolean
52-
|
53-
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
54-
= note: the raw bytes of the constant (size: 1, align: 1) {
55-
42 │ B
56-
}
41+
| ^^^^^^^^^^^^^^^^ type validation failed: encountered 0x42, but expected a boolean
5742

58-
error[E0080]: it is undefined behavior to use this value
59-
--> $DIR/invalid-patterns.rs:42:47
43+
error[E0080]: evaluation of constant value failed
44+
--> $DIR/invalid-patterns.rs:42:58
6045
|
6146
LL | get_flag::<{ unsafe { bool_raw.boolean } }, { unsafe { char_raw.character } }>();
62-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
63-
|
64-
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
65-
= note: the raw bytes of the constant (size: 4, align: 4) {
66-
__ __ __ __ │ ░░░░
67-
}
47+
| ^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
6848

6949
error: aborting due to 8 previous errors
7050

‎src/test/ui/const-generics/min_const_generics/invalid-patterns.64bit.stderr‎

Lines changed: 12 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,49 +22,29 @@ error[E0308]: mismatched types
2222
LL | get_flag::<42, 0x5ad>();
2323
| ^^^^^ expected `char`, found `u8`
2424

25-
error[E0080]: it is undefined behavior to use this value
26-
--> $DIR/invalid-patterns.rs:38:21
25+
error[E0080]: evaluation of constant value failed
26+
--> $DIR/invalid-patterns.rs:38:32
2727
|
2828
LL | get_flag::<false, { unsafe { char_raw.character } }>();
29-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
30-
|
31-
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
32-
= note: the raw bytes of the constant (size: 4, align: 4) {
33-
__ __ __ __ │ ░░░░
34-
}
29+
| ^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
3530

36-
error[E0080]: it is undefined behavior to use this value
37-
--> $DIR/invalid-patterns.rs:40:14
31+
error[E0080]: evaluation of constant value failed
32+
--> $DIR/invalid-patterns.rs:40:25
3833
|
3934
LL | get_flag::<{ unsafe { bool_raw.boolean } }, 'z'>();
40-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x42, but expected a boolean
41-
|
42-
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
43-
= note: the raw bytes of the constant (size: 1, align: 1) {
44-
42 │ B
45-
}
35+
| ^^^^^^^^^^^^^^^^ type validation failed: encountered 0x42, but expected a boolean
4636

47-
error[E0080]: it is undefined behavior to use this value
48-
--> $DIR/invalid-patterns.rs:42:14
37+
error[E0080]: evaluation of constant value failed
38+
--> $DIR/invalid-patterns.rs:42:25
4939
|
5040
LL | get_flag::<{ unsafe { bool_raw.boolean } }, { unsafe { char_raw.character } }>();
51-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x42, but expected a boolean
52-
|
53-
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
54-
= note: the raw bytes of the constant (size: 1, align: 1) {
55-
42 │ B
56-
}
41+
| ^^^^^^^^^^^^^^^^ type validation failed: encountered 0x42, but expected a boolean
5742

58-
error[E0080]: it is undefined behavior to use this value
59-
--> $DIR/invalid-patterns.rs:42:47
43+
error[E0080]: evaluation of constant value failed
44+
--> $DIR/invalid-patterns.rs:42:58
6045
|
6146
LL | get_flag::<{ unsafe { bool_raw.boolean } }, { unsafe { char_raw.character } }>();
62-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
63-
|
64-
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
65-
= note: the raw bytes of the constant (size: 4, align: 4) {
66-
__ __ __ __ │ ░░░░
67-
}
47+
| ^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
6848

6949
error: aborting due to 8 previous errors
7050

‎src/test/ui/const-generics/min_const_generics/invalid-patterns.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ fn main() {
3636

3737

3838
get_flag::<false, { unsafe { char_raw.character } }>();
39-
//~^ ERROR it is undefined behavior
39+
//~^ ERROR evaluation of constant value failed
4040
get_flag::<{ unsafe { bool_raw.boolean } }, 'z'>();
41-
//~^ ERROR it is undefined behavior
41+
//~^ ERROR evaluation of constant value failed
4242
get_flag::<{ unsafe { bool_raw.boolean } }, { unsafe { char_raw.character } }>();
43-
//~^ ERROR it is undefined behavior
44-
//~| ERROR it is undefined behavior
43+
//~^ ERROR evaluation of constant value failed
44+
//~| ERROR evaluation of constant value failed
4545
}

‎src/test/ui/consts/const-err4.32bit.stderr‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,8 @@
1-
error[E0080]: it is undefined behavior to use this value
2-
--> $DIR/const-err4.rs:9:11
1+
error[E0080]: evaluation of constant value failed
2+
--> $DIR/const-err4.rs:9:21
33
|
44
LL | Boo = [unsafe { Foo { b: () }.a }; 4][3],
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized bytes
6-
|
7-
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
8-
= note: the raw bytes of the constant (size: 4, align: 4) {
9-
__ __ __ __ │ ░░░░
10-
}
5+
| ^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized bytes
116

127
error: aborting due to previous error
138

0 commit comments

Comments
 (0)