From 06ca7b700cae83324b4bad8be3286da4f964bb58 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 20 Dec 2020 15:49:08 +0100 Subject: [PATCH 1/2] validate promoteds --- .../rustc_mir/src/const_eval/eval_queries.rs | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_mir/src/const_eval/eval_queries.rs b/compiler/rustc_mir/src/const_eval/eval_queries.rs index f13b4b7b91924..3a3117d5ccf7e 100644 --- a/compiler/rustc_mir/src/const_eval/eval_queries.rs +++ b/compiler/rustc_mir/src/const_eval/eval_queries.rs @@ -383,25 +383,15 @@ 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_none() => CtfeValidationMode::Regular, // a `static` + _ => CtfeValidationMode::Const { inner }, + }; + ecx.const_validate_operand(mplace.into(), path, &mut ref_tracking, mode)?; + inner = true; } }; if let Err(error) = validation { From 97cae9c555016cfa02a3aa1e41a41157525f8cf4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 20 Dec 2020 19:34:29 +0100 Subject: [PATCH 2/2] promoteds in statics may refer to statics --- .../rustc_mir/src/const_eval/eval_queries.rs | 8 ++++++-- compiler/rustc_mir/src/interpret/validity.rs | 20 +++++++++++-------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_mir/src/const_eval/eval_queries.rs b/compiler/rustc_mir/src/const_eval/eval_queries.rs index 3a3117d5ccf7e..df163f6562842 100644 --- a/compiler/rustc_mir/src/const_eval/eval_queries.rs +++ b/compiler/rustc_mir/src/const_eval/eval_queries.rs @@ -387,8 +387,12 @@ pub fn eval_to_allocation_raw_provider<'tcx>( 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_none() => CtfeValidationMode::Regular, // a `static` - _ => CtfeValidationMode::Const { inner }, + 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; diff --git a/compiler/rustc_mir/src/interpret/validity.rs b/compiler/rustc_mir/src/interpret/validity.rs index 2d235d65c4d38..57aec0953b8c2 100644 --- a/compiler/rustc_mir/src/interpret/validity.rs +++ b/compiler/rustc_mir/src/interpret/validity.rs @@ -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 @@ -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, @@ -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`" });