validation: descend from consts into statics

Author: RalfJung

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

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
@@ -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)
+                );
             }
         }
     }

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

@@ -417,6 +417,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

@@ -3,8 +3,9 @@
 
 use std::fmt::Debug;
 
-use rustc_const_eval::interpret::{ImmTy, Projectable};
-use rustc_const_eval::interpret::{InterpCx, InterpResult, Scalar};
+use rustc_const_eval::interpret::{
+    format_interp_error, ImmTy, InterpCx, InterpResult, Projectable, Scalar,
+};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir::def::DefKind;
 use rustc_hir::HirId;
@@ -246,7 +247,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

@@ -1,13 +1,15 @@
+// normalize-stderr-test "(the raw bytes of the constant) \(size: [0-9]*, align: [0-9]*\)" -> "$1 (size: $$SIZE, align: $$ALIGN)"
+// normalize-stderr-test "([0-9a-f][0-9a-f] |╾─*ALLOC[0-9]+(\+[a-z0-9]+)?(<imm>)?─*╼ )+ *│.*" -> "HEX_DUMP"
 #![feature(const_refs_to_static, const_mut_refs, sync_unsafe_cell)]
 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 {