validation: descend from consts into statics

compiler/rustc_const_eval/messages.ftl

@@ -215,9 +215,6 @@ const_eval_modified_global =
 const_eval_mut_deref =
     mutation through a reference is not allowed in {const_eval_const_context}s
 
-const_eval_mutable_data_in_const =
-    constant refers to mutable data
-
 const_eval_mutable_ptr_in_final = encountered mutable pointer in final value of {const_eval_intern_kind}
 
 const_eval_non_const_fmt_macro_call =
@@ -414,6 +411,9 @@ const_eval_upcast_mismatch =
 ## (We'd love to sort this differently to make that more clear but tidy won't let us...)
 const_eval_validation_box_to_static = {$front_matter}: encountered a box pointing to a static variable in a constant
 const_eval_validation_box_to_uninhabited = {$front_matter}: encountered a box pointing to uninhabited type {$ty}
+
+const_eval_validation_const_ref_to_mutable = {$front_matter}: encountered reference to mutable memory in `const`
+
 const_eval_validation_dangling_box_no_provenance = {$front_matter}: encountered a dangling box ({$pointer} has no provenance)
 const_eval_validation_dangling_box_out_of_bounds = {$front_matter}: encountered a dangling box (going beyond the bounds of its allocation)
 const_eval_validation_dangling_box_use_after_free = {$front_matter}: encountered a dangling box (use-after-free)

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),
+            )
+        })
     }
 }
 

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,
@@ -248,18 +248,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),
             }
         }

compiler/rustc_const_eval/src/errors.rs

@@ -117,14 +117,6 @@ pub(crate) struct MaxNumNodesInConstErr {
     pub global_const_id: String,
 }
 
-#[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 {
@@ -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

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)

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,
 };

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
@@ -979,7 +983,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             Err(err) => {
                 bug!(
                     "Unexpected Undefined Behavior error during validation: {}",
-                    self.format_error(err)
+                    format_interp_error(self.tcx.dcx(), err)
                 );
             }
         }

compiler/rustc_middle/src/mir/interpret/error.rs

@@ -425,6 +425,7 @@ pub enum ValidationErrorKind<'tcx> {
     PtrToUninhabited { ptr_kind: PointerKind, ty: Ty<'tcx> },
     PtrToStatic { ptr_kind: PointerKind },
     MutableRefInConst,
+    ConstRefToMutable,
     MutableRefToImmutable,
     UnsafeCellInImmutable,
     NullFnPtr,

compiler/rustc_mir_transform/src/const_prop_lint.rs

@@ -7,7 +7,7 @@ use either::Left;
 
 use rustc_const_eval::interpret::Immediate;
 use rustc_const_eval::interpret::{
-    InterpCx, InterpResult, MemoryKind, OpTy, Scalar, StackPopCleanup,
+    format_interp_error, InterpCx, InterpResult, MemoryKind, OpTy, Scalar, StackPopCleanup,
 };
 use rustc_const_eval::ReportErrorExt;
 use rustc_hir::def::DefKind;
@@ -236,7 +236,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
                 assert!(
                     !error.kind().formatted_string(),
                     "const-prop encountered formatting error: {}",
-                    self.ecx.format_error(error),
+                    format_interp_error(self.ecx.tcx.dcx(), error),
                 );
                 None
             }

src/tools/miri/src/diagnostics.rs

@@ -290,7 +290,7 @@ pub fn report_error<'tcx, 'mir>(
                 ) =>
             {
                 ecx.handle_ice(); // print interpreter backtrace
-                bug!("This validation error should be impossible in Miri: {}", ecx.format_error(e));
+                bug!("This validation error should be impossible in Miri: {}", format_interp_error(ecx.tcx.dcx(), e));
             }
             UndefinedBehavior(_) => "Undefined Behavior",
             ResourceExhaustion(_) => "resource exhaustion",
@@ -304,7 +304,7 @@ pub fn report_error<'tcx, 'mir>(
             ) => "post-monomorphization error",
             _ => {
                 ecx.handle_ice(); // print interpreter backtrace
-                bug!("This error should be impossible in Miri: {}", ecx.format_error(e));
+                bug!("This error should be impossible in Miri: {}", format_interp_error(ecx.tcx.dcx(), e));
             }
         };
         #[rustfmt::skip]
@@ -370,7 +370,7 @@ pub fn report_error<'tcx, 'mir>(
         _ => {}
     }
 
-    msg.insert(0, ecx.format_error(e));
+    msg.insert(0, format_interp_error(ecx.tcx.dcx(), e));
 
     report_msg(
         DiagLevel::Error,

tests/ui/consts/const_refs_to_static_fail.rs

@@ -4,10 +4,10 @@ use std::cell::SyncUnsafeCell;
 static S: SyncUnsafeCell<i32> = SyncUnsafeCell::new(0);
 static mut S_MUT: i32 = 0;
 
-const C1: &SyncUnsafeCell<i32> = &S;
+const C1: &SyncUnsafeCell<i32> = &S; //~ERROR undefined behavior
+//~| encountered reference to mutable memory
 const C1_READ: () = unsafe {
-    assert!(*C1.get() == 0); //~ERROR evaluation of constant value failed
-    //~^ constant accesses mutable global memory
+    assert!(*C1.get() == 0);
 };
 const C2: *const i32 = unsafe { std::ptr::addr_of!(S_MUT) };
 const C2_READ: () = unsafe {

tests/ui/consts/const_refs_to_static_fail.stderr

@@ -1,8 +1,19 @@
-error[E0080]: evaluation of constant value failed
-  --> $DIR/const_refs_to_static_fail.rs:9:13
+error[E0080]: it is undefined behavior to use this value
+  --> $DIR/const_refs_to_static_fail.rs:7:1
+   |
+LL | const C1: &SyncUnsafeCell<i32> = &S;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered reference to mutable memory in `const`
+   |
+   = 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.
+   = note: the raw bytes of the constant (size: 4, align: 4) {
+               ╾ALLOC0╼                                     │ ╾──╼
+           }
+
+note: erroneous constant encountered
+  --> $DIR/const_refs_to_static_fail.rs:10:14
    |
 LL |     assert!(*C1.get() == 0);
-   |             ^^^^^^^^^ constant accesses mutable global memory
+   |              ^^
 
 error[E0080]: evaluation of constant value failed
   --> $DIR/const_refs_to_static_fail.rs:14:13