bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

interning cleanup: we no longer need to distinguish Const and ConstInner; we no longer need the ignore_interior_mut_in_const hack

Browse source
This commit is contained in:
Ralf Jung 2020-10-25 11:12:19 +01:00
parent 9b501edf08
commit 18fd58e9d1
7 changed files with 33 additions and 81 deletions
Download patch file Download diff file Expand all files Collapse all files

12
compiler/rustc_middle/src/mir/mod.rs
View file

@ -210,16 +210,6 @@ pub struct Body<'tcx> {
/// We hold in this field all the constants we are not able to evaluate yet.
pub required_consts: Vec<Constant<'tcx>>,
/// The user may be writing e.g. `&[(SOME_CELL, 42)][i].1` and this would get promoted, because
/// we'd statically know that no thing with interior mutability will ever be available to the
/// user without some serious unsafe code. Now this means that our promoted is actually
/// `&[(SOME_CELL, 42)]` and the MIR using it will do the `&promoted[i].1` projection because
/// the index may be a runtime value. Such a promoted value is illegal because it has reachable
/// interior mutability. This flag just makes this situation very obvious where the previous
/// implementation without the flag hid this situation silently.
/// FIXME(oli-obk): rewrite the promoted during promotion to eliminate the cell components.
pub ignore_interior_mut_in_const_validation: bool,
/// Does this body use generic parameters. This is used for the `ConstEvaluatable` check.
///
/// Note that this does not actually mean that this body is not computable right now.
@ -276,7 +266,6 @@ impl<'tcx> Body<'tcx> {
var_debug_info,
span,
required_consts: Vec::new(),
ignore_interior_mut_in_const_validation: false,
is_polymorphic: false,
predecessor_cache: PredecessorCache::new(),
};
@ -306,7 +295,6 @@ impl<'tcx> Body<'tcx> {
required_consts: Vec::new(),
generator_kind: None,
var_debug_info: Vec::new(),
ignore_interior_mut_in_const_validation: false,
is_polymorphic: false,
predecessor_cache: PredecessorCache::new(),
};

15
compiler/rustc_mir/src/const_eval/eval_queries.rs
View file

@ -67,12 +67,7 @@ fn eval_body_using_ecx<'mir, 'tcx>(
None => InternKind::Constant,
}
};
intern_const_alloc_recursive(
ecx,
intern_kind,
ret,
body.ignore_interior_mut_in_const_validation,
);
intern_const_alloc_recursive(ecx, intern_kind, ret);
debug!("eval_body_using_ecx done: {:?}", *ret);
Ok(ret)
@ -373,7 +368,13 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
// 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 is made
// 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;

2
compiler/rustc_mir/src/const_eval/mod.rs
View file

@ -29,7 +29,7 @@ pub(crate) fn const_caller_location(
let mut ecx = mk_eval_cx(tcx, DUMMY_SP, ty::ParamEnv::reveal_all(), false);
let loc_place = ecx.alloc_caller_location(file, line, col);
intern_const_alloc_recursive(&mut ecx, InternKind::Constant, loc_place, false);
intern_const_alloc_recursive(&mut ecx, InternKind::Constant, loc_place);
ConstValue::Scalar(loc_place.ptr)
}

72
compiler/rustc_mir/src/interpret/intern.rs
View file

@ -7,7 +7,7 @@ use super::validity::RefTracking;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir as hir;
use rustc_middle::mir::interpret::InterpResult;
use rustc_middle::ty::{self, layout::TyAndLayout, query::TyCtxtAt, Ty};
use rustc_middle::ty::{self, layout::TyAndLayout, Ty};
use rustc_target::abi::Size;
use rustc_ast::Mutability;
@ -40,11 +40,6 @@ struct InternVisitor<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> {
/// This field stores whether we are *currently* inside an `UnsafeCell`. This can affect
/// the intern mode of references we encounter.
inside_unsafe_cell: bool,
/// This flag is to avoid triggering UnsafeCells are not allowed behind references in constants
/// for promoteds.
/// It's a copy of `mir::Body`'s ignore_interior_mut_in_const_validation field
ignore_interior_mut_in_const: bool,
}
#[derive(Copy, Clone, Debug, PartialEq, Hash, Eq)]
@ -53,22 +48,14 @@ enum InternMode {
/// this is *immutable*, and below mutable references inside an `UnsafeCell`, this
/// is *mutable*.
Static(hir::Mutability),
/// The "base value" of a const, which can have `UnsafeCell` (as in `const FOO: Cell<i32>`),
/// but that interior mutability is simply ignored.
ConstBase,
/// The "inner values" of a const with references, where `UnsafeCell` is an error.
ConstInner,
/// A `const`.
Const,
}
/// Signalling data structure to ensure we don't recurse
/// into the memory of other constants or statics
struct IsStaticOrFn;
fn mutable_memory_in_const(tcx: TyCtxtAt<'_>, kind: &str) {
// FIXME: show this in validation instead so we can point at where in the value the error is?
tcx.sess.span_err(tcx.span, &format!("mutable memory ({}) is not allowed in constant", kind));
}
/// Intern an allocation without looking at its children.
/// `mode` is the mode of the environment where we found this pointer.
/// `mutablity` is the mutability of the place to be interned; even if that says
@ -165,17 +152,13 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir
mplace: MPlaceTy<'tcx>,
fields: impl Iterator<Item = InterpResult<'tcx, Self::V>>,
) -> InterpResult<'tcx> {
// ZSTs cannot contain pointers, so we can skip them.
if mplace.layout.is_zst() {
return Ok(());
}
if let Some(def) = mplace.layout.ty.ty_adt_def() {
if Some(def.did) == self.ecx.tcx.lang_items().unsafe_cell_type() {
if self.mode == InternMode::ConstInner && !self.ignore_interior_mut_in_const {
// We do not actually make this memory mutable. But in case the user
// *expected* it to be mutable, make sure we error. This is just a
// sanity check to prevent users from accidentally exploiting the UB
// they caused. It also helps us to find cases where const-checking
// failed to prevent an `UnsafeCell` (but as `ignore_interior_mut_in_const`
// shows that part is not airtight).
//mutable_memory_in_const(self.ecx.tcx, "`UnsafeCell`");
}
// We are crossing over an `UnsafeCell`, we can mutate again. This means that
// References we encounter inside here are interned as pointing to mutable
// allocations.
@ -187,11 +170,6 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir
}
}
// ZSTs cannot contain pointers, so we can skip them.
if mplace.layout.is_zst() {
return Ok(());
}
self.walk_aggregate(mplace, fields)
}
@ -211,7 +189,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir
if let Scalar::Ptr(vtable) = mplace.meta.unwrap_meta() {
// Explicitly choose const mode here, since vtables are immutable, even
// if the reference of the fat pointer is mutable.
self.intern_shallow(vtable.alloc_id, InternMode::ConstInner, None);
self.intern_shallow(vtable.alloc_id, InternMode::Const, None);
} else {
// Validation will error (with a better message) on an invalid vtable pointer.
// Let validation show the error message, but make sure it *does* error.
@ -223,7 +201,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir
// Only recurse for allocation-backed pointers.
if let Scalar::Ptr(ptr) = mplace.ptr {
// Compute the mode with which we intern this. Our goal here is to make as many
// statics as we can immutable so they can be placed in const memory by LLVM.
// statics as we can immutable so they can be placed in read-only memory by LLVM.
let ref_mode = match self.mode {
InternMode::Static(mutbl) => {
// In statics, merge outer mutability with reference mutability and
@ -257,27 +235,11 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir
}
}
}
InternMode::ConstBase | InternMode::ConstInner => {
// Ignore `UnsafeCell`, everything is immutable. Do some sanity checking
// for mutable references that we encounter -- they must all be ZST.
// This helps to prevent users from accidentally exploiting UB that they
// caused (by somehow getting a mutable reference in a `const`).
if ref_mutability == Mutability::Mut {
/*match referenced_ty.kind() {
ty::Array(_, n) if n.eval_usize(*tcx, self.ecx.param_env) == 0 => {}
ty::Slice(_)
if mplace.meta.unwrap_meta().to_machine_usize(self.ecx)?
== 0 => {}
_ => mutable_memory_in_const(tcx, "`&mut`"),
}*/
} else {
// A shared reference. We cannot check `freeze` here due to references
// like `&dyn Trait` that are actually immutable. We do check for
// concrete `UnsafeCell` when traversing the pointee though (if it is
// a new allocation, not yet interned).
}
// Go on with the "inner" rules.
InternMode::ConstInner
InternMode::Const => {
// Ignore `UnsafeCell`, everything is immutable. Validity does some sanity
// checking for mutable references that we encounter -- they must all be
// ZST.
InternMode::Const
}
};
match self.intern_shallow(ptr.alloc_id, ref_mode, Some(referenced_ty)) {
@ -316,7 +278,6 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
ecx: &mut InterpCx<'mir, 'tcx, M>,
intern_kind: InternKind,
ret: MPlaceTy<'tcx>,
ignore_interior_mut_in_const: bool,
) where
'tcx: 'mir,
{
@ -325,7 +286,7 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
InternKind::Static(mutbl) => InternMode::Static(mutbl),
// `Constant` includes array lengths.
// `Promoted` includes non-`Copy` array initializers and `rustc_args_required_const` arguments.
InternKind::Constant | InternKind::Promoted => InternMode::ConstBase,
InternKind::Constant | InternKind::Promoted => InternMode::Const,
};
// Type based interning.
@ -355,7 +316,6 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
ecx,
mode,
leftover_allocations,
ignore_interior_mut_in_const,
inside_unsafe_cell: false,
}
.visit_value(mplace);

2
compiler/rustc_mir/src/interpret/mod.rs
View file

@ -24,7 +24,7 @@ pub use self::machine::{compile_time_machine, AllocMap, Machine, MayLeak, StackP
pub use self::memory::{AllocCheck, FnVal, Memory, MemoryKind};
pub use self::operand::{ImmTy, Immediate, OpTy, Operand};
pub use self::place::{MPlaceTy, MemPlace, MemPlaceMeta, Place, PlaceTy};
pub use self::validity::{RefTracking, CtfeValidationMode};
pub use self::validity::{CtfeValidationMode, RefTracking};
pub use self::visitor::{MutValueVisitor, ValueVisitor};
crate use self::intrinsics::eval_nullary_intrinsic;

8
compiler/rustc_mir/src/interpret/validity.rs
View file

@ -119,6 +119,8 @@ pub enum CtfeValidationMode {
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 },
}
@ -541,8 +543,10 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
Ok(true)
}
ty::Ref(_, ty, mutbl) => {
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. })) && *mutbl == hir::Mutability::Mut {
// A mutable reference inside a const? That does not seem right (except of it is
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. }))
&& *mutbl == hir::Mutability::Mut
{
// A mutable reference inside a const? That does not seem right (except if it is
// a ZST).
let layout = self.ecx.layout_of(ty)?;
if !layout.is_zst() {

3
compiler/rustc_mir/src/transform/promote_consts.rs
View file

@ -1170,7 +1170,7 @@ pub fn promote_candidates<'tcx>(
let mut scope = body.source_scopes[candidate.source_info(body).scope].clone();
scope.parent_scope = None;
let mut promoted = Body::new(
let promoted = Body::new(
body.source, // `promoted` gets filled in below
IndexVec::new(),
IndexVec::from_elem_n(scope, 1),
@ -1181,7 +1181,6 @@ pub fn promote_candidates<'tcx>(
body.span,
body.generator_kind,
);
promoted.ignore_interior_mut_in_const_validation = true;
let promoter = Promoter {
promoted,