Skip to content

Commit

Permalink
Auto merge of #80235 - RalfJung:validate-promoteds, r=oli-obk
Browse files Browse the repository at this point in the history
validate promoteds

Turn on const-value validation for promoteds. This is made possible now that #67534 is resolved.

I don't think this is a breaking change. We don't promote any unsafe operation any more (since #77526 landed). We *do* promote `const fn` calls under some circumstances (in `const`/`static` initializers), but union field access and similar operations are not allowed in `const fn`. So now is a perfect time to add this check. :D

r? `@oli-obk`
Fixes #67465
  • Loading branch information
bors committed Dec 25, 2020
2 parents 1832bdd + 97cae9c commit bb17823
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 27 deletions.
32 changes: 13 additions & 19 deletions compiler/rustc_mir/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,25 +383,19 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
Ok(mplace) => {
// Since evaluation had no errors, valiate the resulting constant:
let validation = try {
// FIXME do not validate promoteds until a decision on
// https://github.com/rust-lang/rust/issues/67465 and
// https://github.com/rust-lang/rust/issues/67534 is made.
// Promoteds can contain unexpected `UnsafeCell` and reference `static`s, but their
// otherwise restricted form ensures that this is still sound. We just lose the
// extra safety net of some of the dynamic checks. They can also contain invalid
// values, but since we do not usually check intermediate results of a computation
// for validity, it might be surprising to do that here.
if cid.promoted.is_none() {
let mut ref_tracking = RefTracking::new(mplace);
let mut inner = false;
while let Some((mplace, path)) = ref_tracking.todo.pop() {
let mode = match tcx.static_mutability(cid.instance.def_id()) {
Some(_) => CtfeValidationMode::Regular, // a `static`
None => CtfeValidationMode::Const { inner },
};
ecx.const_validate_operand(mplace.into(), path, &mut ref_tracking, mode)?;
inner = true;
}
let mut ref_tracking = RefTracking::new(mplace);
let mut inner = false;
while let Some((mplace, path)) = ref_tracking.todo.pop() {
let mode = match tcx.static_mutability(cid.instance.def_id()) {
Some(_) if cid.promoted.is_some() => {
// Promoteds in statics are allowed to point to statics.
CtfeValidationMode::Const { inner, allow_static_ptrs: true }
}
Some(_) => CtfeValidationMode::Regular, // a `static`
None => CtfeValidationMode::Const { inner, allow_static_ptrs: false },
};
ecx.const_validate_operand(mplace.into(), path, &mut ref_tracking, mode)?;
inner = true;
}
};
if let Err(error) = validation {
Expand Down
20 changes: 12 additions & 8 deletions compiler/rustc_mir/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,12 @@ pub enum PathElem {
pub enum CtfeValidationMode {
/// Regular validation, nothing special happening.
Regular,
/// Validation of a `const`. `inner` says if this is an inner, indirect allocation (as opposed
/// to the top-level const allocation).
/// Being an inner allocation makes a difference because the top-level allocation of a `const`
/// is copied for each use, but the inner allocations are implicitly shared.
Const { inner: bool },
/// Validation of a `const`.
/// `inner` says if this is an inner, indirect allocation (as opposed to the top-level const
/// allocation). Being an inner allocation makes a difference because the top-level allocation
/// of a `const` is copied for each use, but the inner allocations are implicitly shared.
/// `allow_static_ptrs` says if pointers to statics are permitted (which is the case for promoteds in statics).
Const { inner: bool, allow_static_ptrs: bool },
}

/// State for tracking recursive validation of references
Expand Down Expand Up @@ -437,7 +438,10 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
if let Some(GlobalAlloc::Static(did)) = alloc_kind {
assert!(!self.ecx.tcx.is_thread_local_static(did));
assert!(self.ecx.tcx.is_static(did));
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. })) {
if matches!(
self.ctfe_mode,
Some(CtfeValidationMode::Const { allow_static_ptrs: false, .. })
) {
// See const_eval::machine::MemoryExtra::can_access_statics for why
// this check is so important.
// This check is reachable when the const just referenced the static,
Expand Down Expand Up @@ -742,9 +746,9 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
// Sanity check: `builtin_deref` does not know any pointers that are not primitive.
assert!(op.layout.ty.builtin_deref(true).is_none());

// Special check preventing `UnsafeCell` in constants
// Special check preventing `UnsafeCell` in the inner part of constants
if let Some(def) = op.layout.ty.ty_adt_def() {
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { inner: true }))
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { inner: true, .. }))
&& Some(def.did) == self.ecx.tcx.lang_items().unsafe_cell_type()
{
throw_validation_failure!(self.path, { "`UnsafeCell` in a `const`" });
Expand Down

0 comments on commit bb17823

Please sign in to comment.