validation: descend from consts into statics

6 files changed, 74 insertions, 91 deletions

diff --git a/compiler/rustc_const_eval/src/const_eval/mod.rs b/compiler/rustc_const_eval/src/const_eval/mod.rs
index 826b4b278ed..cd50701040e 100644
--- a/compiler/rustc_const_eval/src/const_eval/mod.rs
+++ b/compiler/rustc_const_eval/src/const_eval/mod.rs
@@ -1,11 +1,12 @@
 // Not in interpret to make sure we do not use private implementation details
 
-use crate::interpret::InterpCx;
 use rustc_middle::mir;
-use rustc_middle::mir::interpret::{InterpError, InterpErrorInfo};
+use rustc_middle::mir::interpret::InterpErrorInfo;
 use rustc_middle::query::TyCtxtAt;
 use rustc_middle::ty::{self, Ty};
 
+use crate::interpret::{format_interp_error, InterpCx};
+
 mod error;
 mod eval_queries;
 mod fn_queries;
@@ -25,24 +26,17 @@ pub(crate) enum ValTreeCreationError {
     NodesOverflow,
     /// Values of this type, or this particular value, are not supported as valtrees.
     NonSupportedType,
-    /// The value pointed to non-read-only memory, so we cannot make it a valtree.
-    NotReadOnly,
-    Other,
 }
 pub(crate) type ValTreeCreationResult<'tcx> = Result<ty::ValTree<'tcx>, ValTreeCreationError>;
 
 impl From<InterpErrorInfo<'_>> for ValTreeCreationError {
     fn from(err: InterpErrorInfo<'_>) -> Self {
-        match err.kind() {
-            InterpError::MachineStop(err) => {
-                let err = err.downcast_ref::<ConstEvalErrKind>().unwrap();
-                match err {
-                    ConstEvalErrKind::ConstAccessesMutGlobal => ValTreeCreationError::NotReadOnly,
-                    _ => ValTreeCreationError::Other,
-                }
-            }
-            _ => ValTreeCreationError::Other,
-        }
+        ty::tls::with(|tcx| {
+            bug!(
+                "Unexpected Undefined Behavior error during valtree construction: {}",
+                format_interp_error(tcx.dcx(), err),
+            )
+        })
     }
 }
 
diff --git a/compiler/rustc_const_eval/src/const_eval/valtrees.rs b/compiler/rustc_const_eval/src/const_eval/valtrees.rs
index 31fcdc9a3bd..514a6a7df76 100644
--- a/compiler/rustc_const_eval/src/const_eval/valtrees.rs
+++ b/compiler/rustc_const_eval/src/const_eval/valtrees.rs
@@ -9,7 +9,7 @@ use super::eval_queries::{mk_eval_cx, op_to_const};
 use super::machine::CompileTimeEvalContext;
 use super::{ValTreeCreationError, ValTreeCreationResult, VALTREE_MAX_NODES};
 use crate::const_eval::CanAccessMutGlobal;
-use crate::errors::{MaxNumNodesInConstErr, MutableDataInConstErr};
+use crate::errors::MaxNumNodesInConstErr;
 use crate::interpret::MPlaceTy;
 use crate::interpret::{
     intern_const_alloc_recursive, ImmTy, Immediate, InternKind, MemPlaceMeta, MemoryKind, PlaceTy,
@@ -249,18 +249,6 @@ pub(crate) fn eval_to_valtree<'tcx>(
                         tcx.dcx().emit_err(MaxNumNodesInConstErr { span, global_const_id });
                     Err(handled.into())
                 }
-                ValTreeCreationError::NotReadOnly => {
-                    let handled =
-                        tcx.dcx().emit_err(MutableDataInConstErr { span, global_const_id });
-                    Err(handled.into())
-                }
-                ValTreeCreationError::Other => {
-                    let handled = tcx.dcx().span_delayed_bug(
-                        span.unwrap_or(DUMMY_SP),
-                        "unexpected error during valtree construction",
-                    );
-                    Err(handled.into())
-                }
                 ValTreeCreationError::NonSupportedType => Ok(None),
             }
         }
diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs
index 296329f5d1e..fe72941bbab 100644
--- a/compiler/rustc_const_eval/src/errors.rs
+++ b/compiler/rustc_const_eval/src/errors.rs
@@ -118,14 +118,6 @@ pub(crate) struct MaxNumNodesInConstErr {
 }
 
 #[derive(Diagnostic)]
-#[diag(const_eval_mutable_data_in_const)]
-pub(crate) struct MutableDataInConstErr {
-    #[primary_span]
-    pub span: Option<Span>,
-    pub global_const_id: String,
-}
-
-#[derive(Diagnostic)]
 #[diag(const_eval_unallowed_fn_pointer_call)]
 pub(crate) struct UnallowedFnPointerCall {
     #[primary_span]
@@ -619,6 +611,7 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
 
             PointerAsInt { .. } => const_eval_validation_pointer_as_int,
             PartialPointer => const_eval_validation_partial_pointer,
+            ConstRefToMutable => const_eval_validation_const_ref_to_mutable,
             MutableRefInConst => const_eval_validation_mutable_ref_in_const,
             MutableRefToImmutable => const_eval_validation_mutable_ref_to_immutable,
             NullFnPtr => const_eval_validation_null_fn_ptr,
@@ -773,6 +766,7 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
             NullPtr { .. }
             | PtrToStatic { .. }
             | MutableRefInConst
+            | ConstRefToMutable
             | MutableRefToImmutable
             | NullFnPtr
             | NeverVal
diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs
index dd989ab80fd..dd9dfe3fe79 100644
--- a/compiler/rustc_const_eval/src/interpret/eval_context.rs
+++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs
@@ -4,6 +4,7 @@ use std::{fmt, mem};
 use either::{Either, Left, Right};
 
 use hir::CRATE_HIR_ID;
+use rustc_errors::DiagCtxt;
 use rustc_hir::{self as hir, def_id::DefId, definitions::DefPathData};
 use rustc_index::IndexVec;
 use rustc_middle::mir;
@@ -430,6 +431,26 @@ pub(super) fn from_known_layout<'tcx>(
     }
 }
 
+/// Turn the given error into a human-readable string. Expects the string to be printed, so if
+/// `RUSTC_CTFE_BACKTRACE` is set this will show a backtrace of the rustc internals that
+/// triggered the error.
+///
+/// This is NOT the preferred way to render an error; use `report` from `const_eval` instead.
+/// However, this is useful when error messages appear in ICEs.
+pub fn format_interp_error<'tcx>(dcx: &DiagCtxt, e: InterpErrorInfo<'tcx>) -> String {
+    let (e, backtrace) = e.into_parts();
+    backtrace.print_backtrace();
+    // FIXME(fee1-dead), HACK: we want to use the error as title therefore we can just extract the
+    // label and arguments from the InterpError.
+    #[allow(rustc::untranslatable_diagnostic)]
+    let mut diag = dcx.struct_allow("");
+    let msg = e.diagnostic_message();
+    e.add_args(dcx, &mut diag);
+    let s = dcx.eagerly_translate_to_string(msg, diag.args());
+    diag.cancel();
+    s
+}
+
 impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     pub fn new(
         tcx: TyCtxt<'tcx>,
@@ -462,27 +483,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             .map_or(CRATE_HIR_ID, |def_id| self.tcx.local_def_id_to_hir_id(def_id))
     }
 
-    /// Turn the given error into a human-readable string. Expects the string to be printed, so if
-    /// `RUSTC_CTFE_BACKTRACE` is set this will show a backtrace of the rustc internals that
-    /// triggered the error.
-    ///
-    /// This is NOT the preferred way to render an error; use `report` from `const_eval` instead.
-    /// However, this is useful when error messages appear in ICEs.
-    pub fn format_error(&self, e: InterpErrorInfo<'tcx>) -> String {
-        let (e, backtrace) = e.into_parts();
-        backtrace.print_backtrace();
-        // FIXME(fee1-dead), HACK: we want to use the error as title therefore we can just extract the
-        // label and arguments from the InterpError.
-        let dcx = self.tcx.dcx();
-        #[allow(rustc::untranslatable_diagnostic)]
-        let mut diag = dcx.struct_allow("");
-        let msg = e.diagnostic_message();
-        e.add_args(dcx, &mut diag);
-        let s = dcx.eagerly_translate_to_string(msg, diag.args());
-        diag.cancel();
-        s
-    }
-
     #[inline(always)]
     pub(crate) fn stack(&self) -> &[Frame<'mir, 'tcx, M::Provenance, M::FrameExtra>] {
         M::stack(self)
diff --git a/compiler/rustc_const_eval/src/interpret/mod.rs b/compiler/rustc_const_eval/src/interpret/mod.rs
index 7d286d103ad..c1b6ce4eb4e 100644
--- a/compiler/rustc_const_eval/src/interpret/mod.rs
+++ b/compiler/rustc_const_eval/src/interpret/mod.rs
@@ -20,7 +20,7 @@ mod visitor;
 
 pub use rustc_middle::mir::interpret::*; // have all the `interpret` symbols in one place: here
 
-pub use self::eval_context::{Frame, FrameInfo, InterpCx, StackPopCleanup};
+pub use self::eval_context::{format_interp_error, Frame, FrameInfo, InterpCx, StackPopCleanup};
 pub use self::intern::{
     intern_const_alloc_for_constprop, intern_const_alloc_recursive, InternKind,
 };
diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs
index 67665c4aed2..90623a1aa9d 100644
--- a/compiler/rustc_const_eval/src/interpret/validity.rs
+++ b/compiler/rustc_const_eval/src/interpret/validity.rs
@@ -27,8 +27,9 @@ use rustc_target::abi::{
 use std::hash::Hash;
 
 use super::{
-    AllocId, CheckInAllocMsg, GlobalAlloc, ImmTy, Immediate, InterpCx, InterpResult, MPlaceTy,
-    Machine, MemPlaceMeta, OpTy, Pointer, Projectable, Scalar, ValueVisitor,
+    format_interp_error, AllocId, CheckInAllocMsg, GlobalAlloc, ImmTy, Immediate, InterpCx,
+    InterpResult, MPlaceTy, Machine, MemPlaceMeta, OpTy, Pointer, Projectable, Scalar,
+    ValueVisitor,
 };
 
 // for the validation errors
@@ -460,46 +461,49 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
                         // Special handling for pointers to statics (irrespective of their type).
                         assert!(!self.ecx.tcx.is_thread_local_static(did));
                         assert!(self.ecx.tcx.is_static(did));
+                        let is_mut =
+                            matches!(self.ecx.tcx.def_kind(did), DefKind::Static(Mutability::Mut))
+                                || !self
+                                    .ecx
+                                    .tcx
+                                    .type_of(did)
+                                    .no_bound_vars()
+                                    .expect("statics should not have generic parameters")
+                                    .is_freeze(*self.ecx.tcx, ty::ParamEnv::reveal_all());
                         // Mutability check.
                         if ptr_expected_mutbl == Mutability::Mut {
-                            if matches!(
-                                self.ecx.tcx.def_kind(did),
-                                DefKind::Static(Mutability::Not)
-                            ) && self
-                                .ecx
-                                .tcx
-                                .type_of(did)
-                                .no_bound_vars()
-                                .expect("statics should not have generic parameters")
-                                .is_freeze(*self.ecx.tcx, ty::ParamEnv::reveal_all())
-                            {
+                            if !is_mut {
                                 throw_validation_failure!(self.path, MutableRefToImmutable);
                             }
                         }
-                        // We skip recursively checking other statics. These statics must be sound by
-                        // themselves, and the only way to get broken statics here is by using
-                        // unsafe code.
-                        // The reasons we don't check other statics is twofold. For one, in all
-                        // sound cases, the static was already validated on its own, and second, we
-                        // trigger cycle errors if we try to compute the value of the other static
-                        // and that static refers back to us.
-                        // We might miss const-invalid data,
-                        // but things are still sound otherwise (in particular re: consts
-                        // referring to statics).
-                        return Ok(());
+                        match self.ctfe_mode {
+                            Some(CtfeValidationMode::Static { .. }) => {
+                                // We skip recursively checking other statics. These statics must be sound by
+                                // themselves, and the only way to get broken statics here is by using
+                                // unsafe code.
+                                // The reasons we don't check other statics is twofold. For one, in all
+                                // sound cases, the static was already validated on its own, and second, we
+                                // trigger cycle errors if we try to compute the value of the other static
+                                // and that static refers back to us.
+                                // This could miss some UB, but that's fine.
+                                return Ok(());
+                            }
+                            Some(CtfeValidationMode::Const { .. }) => {
+                                // For consts on the other hand we have to recursively check;
+                                // pattern matching assumes a valid value. However we better make
+                                // sure this is not mutable.
+                                if is_mut {
+                                    throw_validation_failure!(self.path, ConstRefToMutable);
+                                }
+                            }
+                            None => {}
+                        }
                     }
                     GlobalAlloc::Memory(alloc) => {
                         if alloc.inner().mutability == Mutability::Mut
                             && matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. }))
                         {
-                            // This is impossible: this can only be some inner allocation of a
-                            // `static mut` (everything else either hits the `GlobalAlloc::Static`
-                            // case or is interned immutably). To get such a pointer we'd have to
-                            // load it from a static, but such loads lead to a CTFE error.
-                            span_bug!(
-                                self.ecx.tcx.span,
-                                "encountered reference to mutable memory inside a `const`"
-                            );
+                            throw_validation_failure!(self.path, ConstRefToMutable);
                         }
                         if ptr_expected_mutbl == Mutability::Mut
                             && alloc.inner().mutability == Mutability::Not
@@ -978,7 +982,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             // Complain about any other kind of error -- those are bad because we'd like to
             // report them in a way that shows *where* in the value the issue lies.
             Err(err) => {
-                bug!("Unexpected error during validation: {}", self.format_error(err));
+                bug!(
+                    "Unexpected error during validation: {}",
+                    format_interp_error(self.tcx.dcx(), err)
+                );
             }
         }
     }