Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 54ea0f9

Browse files
authoredOct 28, 2020
Rollup merge of #78351 - RalfJung:validity-unsafe-cell, r=oli-obk
Move "mutable thing in const" check from interning to validity This moves the check for mutable things (such as `UnsafeCell` or `&mut`) in a`const` from interning to validity. That means we can give more targeted error messages (pointing out *where* the problem lies), and we can simplify interning a bit. Also fix the interning mode used for promoteds in statics. r? @oli-obk
2 parents 86a4a38 + 744dfd8 commit 54ea0f9

File tree

10 files changed

+135
-144
lines changed

10 files changed

+135
-144
lines changed
 

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -210,16 +210,6 @@ pub struct Body<'tcx> {
210210
/// We hold in this field all the constants we are not able to evaluate yet.
211211
pub required_consts: Vec<Constant<'tcx>>,
212212

213-
/// The user may be writing e.g. `&[(SOME_CELL, 42)][i].1` and this would get promoted, because
214-
/// we'd statically know that no thing with interior mutability will ever be available to the
215-
/// user without some serious unsafe code. Now this means that our promoted is actually
216-
/// `&[(SOME_CELL, 42)]` and the MIR using it will do the `&promoted[i].1` projection because
217-
/// the index may be a runtime value. Such a promoted value is illegal because it has reachable
218-
/// interior mutability. This flag just makes this situation very obvious where the previous
219-
/// implementation without the flag hid this situation silently.
220-
/// FIXME(oli-obk): rewrite the promoted during promotion to eliminate the cell components.
221-
pub ignore_interior_mut_in_const_validation: bool,
222-
223213
/// Does this body use generic parameters. This is used for the `ConstEvaluatable` check.
224214
///
225215
/// Note that this does not actually mean that this body is not computable right now.
@@ -276,7 +266,6 @@ impl<'tcx> Body<'tcx> {
276266
var_debug_info,
277267
span,
278268
required_consts: Vec::new(),
279-
ignore_interior_mut_in_const_validation: false,
280269
is_polymorphic: false,
281270
predecessor_cache: PredecessorCache::new(),
282271
};
@@ -306,7 +295,6 @@ impl<'tcx> Body<'tcx> {
306295
required_consts: Vec::new(),
307296
generator_kind: None,
308297
var_debug_info: Vec::new(),
309-
ignore_interior_mut_in_const_validation: false,
310298
is_polymorphic: false,
311299
predecessor_cache: PredecessorCache::new(),
312300
};

‎compiler/rustc_mir/src/const_eval/eval_queries.rs

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use super::{CompileTimeEvalContext, CompileTimeInterpreter, ConstEvalErr, MemoryExtra};
22
use crate::interpret::eval_nullary_intrinsic;
33
use crate::interpret::{
4-
intern_const_alloc_recursive, Allocation, ConstAlloc, ConstValue, GlobalId, Immediate,
5-
InternKind, InterpCx, InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking, Scalar,
4+
intern_const_alloc_recursive, Allocation, ConstAlloc, ConstValue, CtfeValidationMode, GlobalId,
5+
Immediate, InternKind, InterpCx, InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking, Scalar,
66
ScalarMaybeUninit, StackPopCleanup,
77
};
88

@@ -59,23 +59,15 @@ fn eval_body_using_ecx<'mir, 'tcx>(
5959
ecx.run()?;
6060

6161
// Intern the result
62-
// FIXME: since the DefId of a promoted is the DefId of its owner, this
63-
// means that promoteds in statics are actually interned like statics!
64-
// However, this is also currently crucial because we promote mutable
65-
// non-empty slices in statics to extend their lifetime, and this
66-
// ensures that they are put into a mutable allocation.
67-
// For other kinds of promoteds in statics (like array initializers), this is rather silly.
68-
let intern_kind = match tcx.static_mutability(cid.instance.def_id()) {
69-
Some(m) => InternKind::Static(m),
70-
None if cid.promoted.is_some() => InternKind::Promoted,
71-
_ => InternKind::Constant,
62+
let intern_kind = if cid.promoted.is_some() {
63+
InternKind::Promoted
64+
} else {
65+
match tcx.static_mutability(cid.instance.def_id()) {
66+
Some(m) => InternKind::Static(m),
67+
None => InternKind::Constant,
68+
}
7269
};
73-
intern_const_alloc_recursive(
74-
ecx,
75-
intern_kind,
76-
ret,
77-
body.ignore_interior_mut_in_const_validation,
78-
);
70+
intern_const_alloc_recursive(ecx, intern_kind, ret);
7971

8072
debug!("eval_body_using_ecx done: {:?}", *ret);
8173
Ok(ret)
@@ -376,16 +368,23 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
376368
// Since evaluation had no errors, valiate the resulting constant:
377369
let validation = try {
378370
// FIXME do not validate promoteds until a decision on
379-
// https://github.com/rust-lang/rust/issues/67465 is made
371+
// https://github.com/rust-lang/rust/issues/67465 and
372+
// https://github.com/rust-lang/rust/issues/67534 is made.
373+
// Promoteds can contain unexpected `UnsafeCell` and reference `static`s, but their
374+
// otherwise restricted form ensures that this is still sound. We just lose the
375+
// extra safety net of some of the dynamic checks. They can also contain invalid
376+
// values, but since we do not usually check intermediate results of a computation
377+
// for validity, it might be surprising to do that here.
380378
if cid.promoted.is_none() {
381379
let mut ref_tracking = RefTracking::new(mplace);
380+
let mut inner = false;
382381
while let Some((mplace, path)) = ref_tracking.todo.pop() {
383-
ecx.const_validate_operand(
384-
mplace.into(),
385-
path,
386-
&mut ref_tracking,
387-
/*may_ref_to_static*/ ecx.memory.extra.can_access_statics,
388-
)?;
382+
let mode = match tcx.static_mutability(cid.instance.def_id()) {
383+
Some(_) => CtfeValidationMode::Regular, // a `static`
384+
None => CtfeValidationMode::Const { inner },
385+
};
386+
ecx.const_validate_operand(mplace.into(), path, &mut ref_tracking, mode)?;
387+
inner = true;
389388
}
390389
}
391390
};

‎compiler/rustc_mir/src/const_eval/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ pub(crate) fn const_caller_location(
2929
let mut ecx = mk_eval_cx(tcx, DUMMY_SP, ty::ParamEnv::reveal_all(), false);
3030

3131
let loc_place = ecx.alloc_caller_location(file, line, col);
32-
intern_const_alloc_recursive(&mut ecx, InternKind::Constant, loc_place, false);
32+
intern_const_alloc_recursive(&mut ecx, InternKind::Constant, loc_place);
3333
ConstValue::Scalar(loc_place.ptr)
3434
}
3535

‎compiler/rustc_mir/src/interpret/intern.rs

Lines changed: 28 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,23 @@
22
//!
33
//! After a const evaluation has computed a value, before we destroy the const evaluator's session
44
//! memory, we need to extract all memory allocations to the global memory pool so they stay around.
5+
//!
6+
//! In principle, this is not very complicated: we recursively walk the final value, follow all the
7+
//! pointers, and move all reachable allocations to the global `tcx` memory. The only complication
8+
//! is picking the right mutability for the allocations in a `static` initializer: we want to make
9+
//! as many allocations as possible immutable so LLVM can put them into read-only memory. At the
10+
//! same time, we need to make memory that could be mutated by the program mutable to avoid
11+
//! incorrect compilations. To achieve this, we do a type-based traversal of the final value,
12+
//! tracking mutable and shared references and `UnsafeCell` to determine the current mutability.
13+
//! (In principle, we could skip this type-based part for `const` and promoteds, as they need to be
14+
//! always immutable. At least for `const` however we use this opportunity to reject any `const`
15+
//! that contains allocations whose mutability we cannot identify.)
516
617
use super::validity::RefTracking;
718
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
819
use rustc_hir as hir;
920
use rustc_middle::mir::interpret::InterpResult;
10-
use rustc_middle::ty::{self, layout::TyAndLayout, query::TyCtxtAt, Ty};
21+
use rustc_middle::ty::{self, layout::TyAndLayout, Ty};
1122
use rustc_target::abi::Size;
1223

1324
use rustc_ast::Mutability;
@@ -40,11 +51,6 @@ struct InternVisitor<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> {
4051
/// This field stores whether we are *currently* inside an `UnsafeCell`. This can affect
4152
/// the intern mode of references we encounter.
4253
inside_unsafe_cell: bool,
43-
44-
/// This flag is to avoid triggering UnsafeCells are not allowed behind references in constants
45-
/// for promoteds.
46-
/// It's a copy of `mir::Body`'s ignore_interior_mut_in_const_validation field
47-
ignore_interior_mut_in_const: bool,
4854
}
4955

5056
#[derive(Copy, Clone, Debug, PartialEq, Hash, Eq)]
@@ -53,22 +59,14 @@ enum InternMode {
5359
/// this is *immutable*, and below mutable references inside an `UnsafeCell`, this
5460
/// is *mutable*.
5561
Static(hir::Mutability),
56-
/// The "base value" of a const, which can have `UnsafeCell` (as in `const FOO: Cell<i32>`),
57-
/// but that interior mutability is simply ignored.
58-
ConstBase,
59-
/// The "inner values" of a const with references, where `UnsafeCell` is an error.
60-
ConstInner,
62+
/// A `const`.
63+
Const,
6164
}
6265

6366
/// Signalling data structure to ensure we don't recurse
6467
/// into the memory of other constants or statics
6568
struct IsStaticOrFn;
6669

67-
fn mutable_memory_in_const(tcx: TyCtxtAt<'_>, kind: &str) {
68-
// FIXME: show this in validation instead so we can point at where in the value the error is?
69-
tcx.sess.span_err(tcx.span, &format!("mutable memory ({}) is not allowed in constant", kind));
70-
}
71-
7270
/// Intern an allocation without looking at its children.
7371
/// `mode` is the mode of the environment where we found this pointer.
7472
/// `mutablity` is the mutability of the place to be interned; even if that says
@@ -129,9 +127,7 @@ fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>(
129127
// See const_eval::machine::MemoryExtra::can_access_statics for why
130128
// immutability is so important.
131129

132-
// There are no sensible checks we can do here; grep for `mutable_memory_in_const` to
133-
// find the checks we are doing elsewhere to avoid even getting here for memory
134-
// that "wants" to be mutable.
130+
// Validation will ensure that there is no `UnsafeCell` on an immutable allocation.
135131
alloc.mutability = Mutability::Not;
136132
};
137133
// link the alloc id to the actual allocation
@@ -167,17 +163,13 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir
167163
mplace: MPlaceTy<'tcx>,
168164
fields: impl Iterator<Item = InterpResult<'tcx, Self::V>>,
169165
) -> InterpResult<'tcx> {
166+
// ZSTs cannot contain pointers, so we can skip them.
167+
if mplace.layout.is_zst() {
168+
return Ok(());
169+
}
170+
170171
if let Some(def) = mplace.layout.ty.ty_adt_def() {
171172
if Some(def.did) == self.ecx.tcx.lang_items().unsafe_cell_type() {
172-
if self.mode == InternMode::ConstInner && !self.ignore_interior_mut_in_const {
173-
// We do not actually make this memory mutable. But in case the user
174-
// *expected* it to be mutable, make sure we error. This is just a
175-
// sanity check to prevent users from accidentally exploiting the UB
176-
// they caused. It also helps us to find cases where const-checking
177-
// failed to prevent an `UnsafeCell` (but as `ignore_interior_mut_in_const`
178-
// shows that part is not airtight).
179-
mutable_memory_in_const(self.ecx.tcx, "`UnsafeCell`");
180-
}
181173
// We are crossing over an `UnsafeCell`, we can mutate again. This means that
182174
// References we encounter inside here are interned as pointing to mutable
183175
// allocations.
@@ -189,11 +181,6 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir
189181
}
190182
}
191183

192-
// ZSTs cannot contain pointers, so we can skip them.
193-
if mplace.layout.is_zst() {
194-
return Ok(());
195-
}
196-
197184
self.walk_aggregate(mplace, fields)
198185
}
199186

@@ -213,7 +200,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir
213200
if let Scalar::Ptr(vtable) = mplace.meta.unwrap_meta() {
214201
// Explicitly choose const mode here, since vtables are immutable, even
215202
// if the reference of the fat pointer is mutable.
216-
self.intern_shallow(vtable.alloc_id, InternMode::ConstInner, None);
203+
self.intern_shallow(vtable.alloc_id, InternMode::Const, None);
217204
} else {
218205
// Validation will error (with a better message) on an invalid vtable pointer.
219206
// Let validation show the error message, but make sure it *does* error.
@@ -225,7 +212,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir
225212
// Only recurse for allocation-backed pointers.
226213
if let Scalar::Ptr(ptr) = mplace.ptr {
227214
// Compute the mode with which we intern this. Our goal here is to make as many
228-
// statics as we can immutable so they can be placed in const memory by LLVM.
215+
// statics as we can immutable so they can be placed in read-only memory by LLVM.
229216
let ref_mode = match self.mode {
230217
InternMode::Static(mutbl) => {
231218
// In statics, merge outer mutability with reference mutability and
@@ -259,27 +246,11 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir
259246
}
260247
}
261248
}
262-
InternMode::ConstBase | InternMode::ConstInner => {
263-
// Ignore `UnsafeCell`, everything is immutable. Do some sanity checking
264-
// for mutable references that we encounter -- they must all be ZST.
265-
// This helps to prevent users from accidentally exploiting UB that they
266-
// caused (by somehow getting a mutable reference in a `const`).
267-
if ref_mutability == Mutability::Mut {
268-
match referenced_ty.kind() {
269-
ty::Array(_, n) if n.eval_usize(*tcx, self.ecx.param_env) == 0 => {}
270-
ty::Slice(_)
271-
if mplace.meta.unwrap_meta().to_machine_usize(self.ecx)?
272-
== 0 => {}
273-
_ => mutable_memory_in_const(tcx, "`&mut`"),
274-
}
275-
} else {
276-
// A shared reference. We cannot check `freeze` here due to references
277-
// like `&dyn Trait` that are actually immutable. We do check for
278-
// concrete `UnsafeCell` when traversing the pointee though (if it is
279-
// a new allocation, not yet interned).
280-
}
281-
// Go on with the "inner" rules.
282-
InternMode::ConstInner
249+
InternMode::Const => {
250+
// Ignore `UnsafeCell`, everything is immutable. Validity does some sanity
251+
// checking for mutable references that we encounter -- they must all be
252+
// ZST.
253+
InternMode::Const
283254
}
284255
};
285256
match self.intern_shallow(ptr.alloc_id, ref_mode, Some(referenced_ty)) {
@@ -318,7 +289,6 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
318289
ecx: &mut InterpCx<'mir, 'tcx, M>,
319290
intern_kind: InternKind,
320291
ret: MPlaceTy<'tcx>,
321-
ignore_interior_mut_in_const: bool,
322292
) where
323293
'tcx: 'mir,
324294
{
@@ -327,7 +297,7 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
327297
InternKind::Static(mutbl) => InternMode::Static(mutbl),
328298
// `Constant` includes array lengths.
329299
// `Promoted` includes non-`Copy` array initializers and `rustc_args_required_const` arguments.
330-
InternKind::Constant | InternKind::Promoted => InternMode::ConstBase,
300+
InternKind::Constant | InternKind::Promoted => InternMode::Const,
331301
};
332302

333303
// Type based interning.
@@ -357,7 +327,6 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
357327
ecx,
358328
mode,
359329
leftover_allocations,
360-
ignore_interior_mut_in_const,
361330
inside_unsafe_cell: false,
362331
}
363332
.visit_value(mplace);

‎compiler/rustc_mir/src/interpret/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ pub use self::machine::{compile_time_machine, AllocMap, Machine, MayLeak, StackP
2424
pub use self::memory::{AllocCheck, FnVal, Memory, MemoryKind};
2525
pub use self::operand::{ImmTy, Immediate, OpTy, Operand};
2626
pub use self::place::{MPlaceTy, MemPlace, MemPlaceMeta, Place, PlaceTy};
27-
pub use self::validity::RefTracking;
27+
pub use self::validity::{CtfeValidationMode, RefTracking};
2828
pub use self::visitor::{MutValueVisitor, ValueVisitor};
2929

3030
crate use self::intrinsics::eval_nullary_intrinsic;

‎compiler/rustc_mir/src/interpret/validity.rs

Lines changed: 58 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,17 @@ pub enum PathElem {
113113
DynDowncast,
114114
}
115115

116+
/// Extra things to check for during validation of CTFE results.
117+
pub enum CtfeValidationMode {
118+
/// Regular validation, nothing special happening.
119+
Regular,
120+
/// Validation of a `const`. `inner` says if this is an inner, indirect allocation (as opposed
121+
/// to the top-level const allocation).
122+
/// Being an inner allocation makes a difference because the top-level allocation of a `const`
123+
/// is copied for each use, but the inner allocations are implicitly shared.
124+
Const { inner: bool },
125+
}
126+
116127
/// State for tracking recursive validation of references
117128
pub struct RefTracking<T, PATH = ()> {
118129
pub seen: FxHashSet<T>,
@@ -202,9 +213,9 @@ struct ValidityVisitor<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> {
202213
/// starts must not be changed! `visit_fields` and `visit_array` rely on
203214
/// this stack discipline.
204215
path: Vec<PathElem>,
205-
ref_tracking_for_consts:
206-
Option<&'rt mut RefTracking<MPlaceTy<'tcx, M::PointerTag>, Vec<PathElem>>>,
207-
may_ref_to_static: bool,
216+
ref_tracking: Option<&'rt mut RefTracking<MPlaceTy<'tcx, M::PointerTag>, Vec<PathElem>>>,
217+
/// `None` indicates this is not validating for CTFE (but for runtime).
218+
ctfe_mode: Option<CtfeValidationMode>,
208219
ecx: &'rt InterpCx<'mir, 'tcx, M>,
209220
}
210221

@@ -418,27 +429,15 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
418429
{ "a dangling {} (use-after-free)", kind },
419430
);
420431
// Recursive checking
421-
if let Some(ref mut ref_tracking) = self.ref_tracking_for_consts {
432+
if let Some(ref mut ref_tracking) = self.ref_tracking {
422433
if let Some(ptr) = ptr {
423434
// not a ZST
424435
// Skip validation entirely for some external statics
425436
let alloc_kind = self.ecx.tcx.get_global_alloc(ptr.alloc_id);
426437
if let Some(GlobalAlloc::Static(did)) = alloc_kind {
427438
assert!(!self.ecx.tcx.is_thread_local_static(did));
428439
assert!(self.ecx.tcx.is_static(did));
429-
if self.may_ref_to_static {
430-
// We skip checking other statics. These statics must be sound by
431-
// themselves, and the only way to get broken statics here is by using
432-
// unsafe code.
433-
// The reasons we don't check other statics is twofold. For one, in all
434-
// sound cases, the static was already validated on its own, and second, we
435-
// trigger cycle errors if we try to compute the value of the other static
436-
// and that static refers back to us.
437-
// We might miss const-invalid data,
438-
// but things are still sound otherwise (in particular re: consts
439-
// referring to statics).
440-
return Ok(());
441-
} else {
440+
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. })) {
442441
// See const_eval::machine::MemoryExtra::can_access_statics for why
443442
// this check is so important.
444443
// This check is reachable when the const just referenced the static,
@@ -447,6 +446,17 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
447446
{ "a {} pointing to a static variable", kind }
448447
);
449448
}
449+
// We skip checking other statics. These statics must be sound by
450+
// themselves, and the only way to get broken statics here is by using
451+
// unsafe code.
452+
// The reasons we don't check other statics is twofold. For one, in all
453+
// sound cases, the static was already validated on its own, and second, we
454+
// trigger cycle errors if we try to compute the value of the other static
455+
// and that static refers back to us.
456+
// We might miss const-invalid data,
457+
// but things are still sound otherwise (in particular re: consts
458+
// referring to statics).
459+
return Ok(());
450460
}
451461
}
452462
// Proceed recursively even for ZST, no reason to skip them!
@@ -504,7 +514,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
504514
let value = self.ecx.read_scalar(value)?;
505515
// NOTE: Keep this in sync with the array optimization for int/float
506516
// types below!
507-
if self.ref_tracking_for_consts.is_some() {
517+
if self.ctfe_mode.is_some() {
508518
// Integers/floats in CTFE: Must be scalar bits, pointers are dangerous
509519
let is_bits = value.check_init().map_or(false, |v| v.is_bits());
510520
if !is_bits {
@@ -532,7 +542,17 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
532542
}
533543
Ok(true)
534544
}
535-
ty::Ref(..) => {
545+
ty::Ref(_, ty, mutbl) => {
546+
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. }))
547+
&& *mutbl == hir::Mutability::Mut
548+
{
549+
// A mutable reference inside a const? That does not seem right (except if it is
550+
// a ZST).
551+
let layout = self.ecx.layout_of(ty)?;
552+
if !layout.is_zst() {
553+
throw_validation_failure!(self.path, { "mutable reference in a `const`" });
554+
}
555+
}
536556
self.check_safe_pointer(value, "reference")?;
537557
Ok(true)
538558
}
@@ -723,6 +743,15 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
723743
// Sanity check: `builtin_deref` does not know any pointers that are not primitive.
724744
assert!(op.layout.ty.builtin_deref(true).is_none());
725745

746+
// Special check preventing `UnsafeCell` in constants
747+
if let Some(def) = op.layout.ty.ty_adt_def() {
748+
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { inner: true }))
749+
&& Some(def.did) == self.ecx.tcx.lang_items().unsafe_cell_type()
750+
{
751+
throw_validation_failure!(self.path, { "`UnsafeCell` in a `const`" });
752+
}
753+
}
754+
726755
// Recursively walk the value at its type.
727756
self.walk_value(op)?;
728757

@@ -814,7 +843,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
814843
self.ecx,
815844
ptr,
816845
size,
817-
/*allow_uninit_and_ptr*/ self.ref_tracking_for_consts.is_none(),
846+
/*allow_uninit_and_ptr*/ self.ctfe_mode.is_none(),
818847
) {
819848
// In the happy case, we needn't check anything else.
820849
Ok(()) => {}
@@ -865,16 +894,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
865894
&self,
866895
op: OpTy<'tcx, M::PointerTag>,
867896
path: Vec<PathElem>,
868-
ref_tracking_for_consts: Option<
869-
&mut RefTracking<MPlaceTy<'tcx, M::PointerTag>, Vec<PathElem>>,
870-
>,
871-
may_ref_to_static: bool,
897+
ref_tracking: Option<&mut RefTracking<MPlaceTy<'tcx, M::PointerTag>, Vec<PathElem>>>,
898+
ctfe_mode: Option<CtfeValidationMode>,
872899
) -> InterpResult<'tcx> {
873900
trace!("validate_operand_internal: {:?}, {:?}", *op, op.layout.ty);
874901

875902
// Construct a visitor
876-
let mut visitor =
877-
ValidityVisitor { path, ref_tracking_for_consts, may_ref_to_static, ecx: self };
903+
let mut visitor = ValidityVisitor { path, ref_tracking, ctfe_mode, ecx: self };
878904

879905
// Try to cast to ptr *once* instead of all the time.
880906
let op = self.force_op_ptr(op).unwrap_or(op);
@@ -902,23 +928,25 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
902928
/// `ref_tracking` is used to record references that we encounter so that they
903929
/// can be checked recursively by an outside driving loop.
904930
///
905-
/// `may_ref_to_static` controls whether references are allowed to point to statics.
931+
/// `constant` controls whether this must satisfy the rules for constants:
932+
/// - no pointers to statics.
933+
/// - no `UnsafeCell` or non-ZST `&mut`.
906934
#[inline(always)]
907935
pub fn const_validate_operand(
908936
&self,
909937
op: OpTy<'tcx, M::PointerTag>,
910938
path: Vec<PathElem>,
911939
ref_tracking: &mut RefTracking<MPlaceTy<'tcx, M::PointerTag>, Vec<PathElem>>,
912-
may_ref_to_static: bool,
940+
ctfe_mode: CtfeValidationMode,
913941
) -> InterpResult<'tcx> {
914-
self.validate_operand_internal(op, path, Some(ref_tracking), may_ref_to_static)
942+
self.validate_operand_internal(op, path, Some(ref_tracking), Some(ctfe_mode))
915943
}
916944

917945
/// This function checks the data at `op` to be runtime-valid.
918946
/// `op` is assumed to cover valid memory if it is an indirect operand.
919947
/// It will error if the bits at the destination do not match the ones described by the layout.
920948
#[inline(always)]
921949
pub fn validate_operand(&self, op: OpTy<'tcx, M::PointerTag>) -> InterpResult<'tcx> {
922-
self.validate_operand_internal(op, vec![], None, false)
950+
self.validate_operand_internal(op, vec![], None, None)
923951
}
924952
}

‎compiler/rustc_mir/src/transform/const_prop.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use rustc_hir::def::DefKind;
99
use rustc_hir::HirId;
1010
use rustc_index::bit_set::BitSet;
1111
use rustc_index::vec::IndexVec;
12-
use rustc_middle::mir::interpret::{InterpResult, Scalar};
1312
use rustc_middle::mir::visit::{
1413
MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor,
1514
};
@@ -28,9 +27,10 @@ use rustc_trait_selection::traits;
2827

2928
use crate::const_eval::ConstEvalErr;
3029
use crate::interpret::{
31-
self, compile_time_machine, truncate, AllocId, Allocation, ConstValue, Frame, ImmTy, Immediate,
32-
InterpCx, LocalState, LocalValue, MemPlace, Memory, MemoryKind, OpTy, Operand as InterpOperand,
33-
PlaceTy, Pointer, ScalarMaybeUninit, StackPopCleanup,
30+
self, compile_time_machine, truncate, AllocId, Allocation, ConstValue, CtfeValidationMode,
31+
Frame, ImmTy, Immediate, InterpCx, InterpResult, LocalState, LocalValue, MemPlace, Memory,
32+
MemoryKind, OpTy, Operand as InterpOperand, PlaceTy, Pointer, Scalar, ScalarMaybeUninit,
33+
StackPopCleanup,
3434
};
3535
use crate::transform::MirPass;
3636

@@ -805,8 +805,9 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
805805
value,
806806
vec![],
807807
// FIXME: is ref tracking too expensive?
808+
// FIXME: what is the point of ref tracking if we do not even check the tracked refs?
808809
&mut interpret::RefTracking::empty(),
809-
/*may_ref_to_static*/ true,
810+
CtfeValidationMode::Regular,
810811
) {
811812
trace!("validation error, attempt failed: {:?}", e);
812813
return;

‎compiler/rustc_mir/src/transform/promote_consts.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,7 +1170,7 @@ pub fn promote_candidates<'tcx>(
11701170
let mut scope = body.source_scopes[candidate.source_info(body).scope].clone();
11711171
scope.parent_scope = None;
11721172

1173-
let mut promoted = Body::new(
1173+
let promoted = Body::new(
11741174
body.source, // `promoted` gets filled in below
11751175
IndexVec::new(),
11761176
IndexVec::from_elem_n(scope, 1),
@@ -1181,7 +1181,6 @@ pub fn promote_candidates<'tcx>(
11811181
body.span,
11821182
body.generator_kind,
11831183
);
1184-
promoted.ignore_interior_mut_in_const_validation = true;
11851184

11861185
let promoter = Promoter {
11871186
promoted,

‎src/test/ui/consts/miri_unleashed/mutable_references_err.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ unsafe impl Sync for Meh {}
1313

1414
// the following will never be ok! no interior mut behind consts, because
1515
// all allocs interned here will be marked immutable.
16-
const MUH: Meh = Meh { //~ ERROR: mutable memory (`UnsafeCell`) is not allowed in constant
16+
const MUH: Meh = Meh { //~ ERROR: it is undefined behavior to use this value
1717
x: &UnsafeCell::new(42),
1818
};
1919

@@ -24,11 +24,11 @@ unsafe impl Sync for Synced {}
2424

2525
// Make sure we also catch this behind a type-erased `dyn Trait` reference.
2626
const SNEAKY: &dyn Sync = &Synced { x: UnsafeCell::new(42) };
27-
//~^ ERROR: mutable memory (`UnsafeCell`) is not allowed in constant
27+
//~^ ERROR: it is undefined behavior to use this value
2828

2929
// Make sure we also catch mutable references.
3030
const BLUNT: &mut i32 = &mut 42;
31-
//~^ ERROR: mutable memory (`&mut`) is not allowed in constant
31+
//~^ ERROR: it is undefined behavior to use this value
3232

3333
fn main() {
3434
unsafe {

‎src/test/ui/consts/miri_unleashed/mutable_references_err.stderr

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,28 @@
1-
error: mutable memory (`UnsafeCell`) is not allowed in constant
1+
error[E0080]: it is undefined behavior to use this value
22
--> $DIR/mutable_references_err.rs:16:1
33
|
44
LL | / const MUH: Meh = Meh {
55
LL | | x: &UnsafeCell::new(42),
66
LL | | };
7-
| |__^
7+
| |__^ type validation failed: encountered `UnsafeCell` in a `const` at .x.<deref>
8+
|
9+
= 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.
810

9-
error: mutable memory (`UnsafeCell`) is not allowed in constant
11+
error[E0080]: it is undefined behavior to use this value
1012
--> $DIR/mutable_references_err.rs:26:1
1113
|
1214
LL | const SNEAKY: &dyn Sync = &Synced { x: UnsafeCell::new(42) };
13-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered `UnsafeCell` in a `const` at .<deref>.<dyn-downcast>.x
16+
|
17+
= 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.
1418

15-
error: mutable memory (`&mut`) is not allowed in constant
19+
error[E0080]: it is undefined behavior to use this value
1620
--> $DIR/mutable_references_err.rs:30:1
1721
|
1822
LL | const BLUNT: &mut i32 = &mut 42;
19-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
23+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered mutable reference in a `const`
24+
|
25+
= 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.
2026

2127
warning: skipping const checks
2228
|
@@ -38,3 +44,4 @@ LL | const BLUNT: &mut i32 = &mut 42;
3844

3945
error: aborting due to 3 previous errors; 1 warning emitted
4046

47+
For more information about this error, try `rustc --explain E0080`.

0 commit comments

Comments
 (0)
Please sign in to comment.