add const_make_global; err for const_allocate ptrs if didn't call

Co-Authored-By: Ralf Jung <[email protected]>
Co-Authored-By: Oli Scherer <[email protected]>

Author: 3 people

compiler/rustc_const_eval/messages.ftl

@@ -56,6 +56,17 @@ const_eval_const_context = {$kind ->
     *[other] {""}
 }
 
+const_eval_const_heap_ptr_in_final = encountered `const_allocate` pointer in final value that was not made global
+    .note = use `const_make_global` to make allocated pointers immutable before returning
+
+const_eval_const_make_global_ptr_already_made_global = attempting to call `const_make_global` twice on the same allocation {$alloc}
+
+const_eval_const_make_global_ptr_is_non_heap = pointer passed to `const_make_global` does not point to a heap allocation: {$ptr}
+
+const_eval_const_make_global_with_dangling_ptr = pointer passed to `const_make_global` is dangling: {$ptr}
+
+const_eval_const_make_global_with_offset = making {$ptr} global which does not point to the beginning of an object
+
 const_eval_copy_nonoverlapping_overlapping =
     `copy_nonoverlapping` called on overlapping ranges
 

compiler/rustc_const_eval/src/const_eval/error.rs

@@ -2,7 +2,7 @@ use std::mem;
 
 use rustc_errors::{Diag, DiagArgName, DiagArgValue, DiagMessage, IntoDiagArg};
 use rustc_middle::mir::AssertKind;
-use rustc_middle::mir::interpret::{Provenance, ReportedErrorInfo};
+use rustc_middle::mir::interpret::{AllocId, Provenance, ReportedErrorInfo};
 use rustc_middle::query::TyCtxtAt;
 use rustc_middle::ty::layout::LayoutError;
 use rustc_middle::ty::{ConstInt, TyCtxt};
@@ -22,8 +22,22 @@ pub enum ConstEvalErrKind {
     ModifiedGlobal,
     RecursiveStatic,
     AssertFailure(AssertKind<ConstInt>),
-    Panic { msg: Symbol, line: u32, col: u32, file: Symbol },
+    Panic {
+        msg: Symbol,
+        line: u32,
+        col: u32,
+        file: Symbol,
+    },
     WriteThroughImmutablePointer,
+    /// Called `const_make_global` twice.
+    ConstMakeGlobalPtrAlreadyMadeGlobal(AllocId),
+    /// Called `const_make_global` on a non-heap pointer.
+    ConstMakeGlobalPtrIsNonHeap(String),
+    /// Called `const_make_global` on a dangling pointer.
+    ConstMakeGlobalWithDanglingPtr(String),
+    /// Called `const_make_global` on a pointer that does not start at the
+    /// beginning of an object.
+    ConstMakeGlobalWithOffset(String),
 }
 
 impl MachineStopType for ConstEvalErrKind {
@@ -38,6 +52,12 @@ impl MachineStopType for ConstEvalErrKind {
             RecursiveStatic => const_eval_recursive_static,
             AssertFailure(x) => x.diagnostic_message(),
             WriteThroughImmutablePointer => const_eval_write_through_immutable_pointer,
+            ConstMakeGlobalPtrAlreadyMadeGlobal { .. } => {
+                const_eval_const_make_global_ptr_already_made_global
+            }
+            ConstMakeGlobalPtrIsNonHeap(_) => const_eval_const_make_global_ptr_is_non_heap,
+            ConstMakeGlobalWithDanglingPtr(_) => const_eval_const_make_global_with_dangling_ptr,
+            ConstMakeGlobalWithOffset(_) => const_eval_const_make_global_with_offset,
         }
     }
     fn add_args(self: Box<Self>, adder: &mut dyn FnMut(DiagArgName, DiagArgValue)) {
@@ -51,6 +71,14 @@ impl MachineStopType for ConstEvalErrKind {
             Panic { msg, .. } => {
                 adder("msg".into(), msg.into_diag_arg(&mut None));
             }
+            ConstMakeGlobalPtrIsNonHeap(ptr)
+            | ConstMakeGlobalWithOffset(ptr)
+            | ConstMakeGlobalWithDanglingPtr(ptr) => {
+                adder("ptr".into(), ptr.into_diag_arg(&mut None));
+            }
+            ConstMakeGlobalPtrAlreadyMadeGlobal(alloc) => {
+                adder("alloc".into(), alloc.into_diag_arg(&mut None));
+            }
         }
     }
 }

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -93,7 +93,7 @@ fn eval_body_using_ecx<'tcx, R: InterpretationResult<'tcx>>(
     // Since evaluation had no errors, validate the resulting constant.
     const_validate_mplace(ecx, &ret, cid)?;
 
-    // Only report this after validation, as validaiton produces much better diagnostics.
+    // Only report this after validation, as validation produces much better diagnostics.
     // FIXME: ensure validation always reports this and stop making interning care about it.
 
     match intern_result {

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -169,13 +169,15 @@ pub type CompileTimeInterpCx<'tcx> = InterpCx<'tcx, CompileTimeMachine<'tcx>>;
 
 #[derive(Debug, PartialEq, Eq, Copy, Clone)]
 pub enum MemoryKind {
-    Heap,
+    Heap { was_made_global: bool },
 }
 
 impl fmt::Display for MemoryKind {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            MemoryKind::Heap => write!(f, "heap allocation"),
+            MemoryKind::Heap { was_made_global } => {
+                write!(f, "heap allocation{}", if *was_made_global { " (made global)" } else { "" })
+            }
         }
     }
 }
@@ -184,7 +186,7 @@ impl interpret::MayLeak for MemoryKind {
     #[inline(always)]
     fn may_leak(self) -> bool {
         match self {
-            MemoryKind::Heap => false,
+            MemoryKind::Heap { was_made_global } => was_made_global,
         }
     }
 }
@@ -420,7 +422,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
                 let ptr = ecx.allocate_ptr(
                     Size::from_bytes(size),
                     align,
-                    interpret::MemoryKind::Machine(MemoryKind::Heap),
+                    interpret::MemoryKind::Machine(MemoryKind::Heap { was_made_global: false }),
                     AllocInit::Uninit,
                 )?;
                 ecx.write_pointer(ptr, dest)?;
@@ -453,10 +455,17 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
                     ecx.deallocate_ptr(
                         ptr,
                         Some((size, align)),
-                        interpret::MemoryKind::Machine(MemoryKind::Heap),
+                        interpret::MemoryKind::Machine(MemoryKind::Heap { was_made_global: false }),
                     )?;
                 }
             }
+
+            sym::const_make_global => {
+                let ptr = ecx.read_pointer(&args[0])?;
+                ecx.make_const_heap_ptr_global(ptr)?;
+                ecx.write_pointer(ptr, dest)?;
+            }
+
             // The intrinsic represents whether the value is known to the optimizer (LLVM).
             // We're not doing any optimizations here, so there is no optimizer that could know the value.
             // (We know the value here in the machine of course, but this is the runtime of that code,

compiler/rustc_const_eval/src/errors.rs

@@ -43,6 +43,14 @@ pub(crate) struct MutablePtrInFinal {
     pub kind: InternKind,
 }
 
+#[derive(Diagnostic)]
+#[diag(const_eval_const_heap_ptr_in_final)]
+#[note]
+pub(crate) struct ConstHeapPtrInFinal {
+    #[primary_span]
+    pub span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(const_eval_unstable_in_stable_exposed)]
 pub(crate) struct UnstableInStableExposed {

compiler/rustc_const_eval/src/interpret/intern.rs

@@ -31,7 +31,7 @@ use super::{
 };
 use crate::const_eval;
 use crate::const_eval::DummyMachine;
-use crate::errors::NestedStaticInThreadLocal;
+use crate::errors::{ConstHeapPtrInFinal, NestedStaticInThreadLocal};
 
 pub trait CompileTimeMachine<'tcx, T> = Machine<
         'tcx,
@@ -55,14 +55,43 @@ impl HasStaticRootDefId for const_eval::CompileTimeMachine<'_> {
     }
 }
 
+pub enum DisallowInternReason {
+    ConstHeap,
+}
+
+/// A trait for controlling whether memory allocated in the interpreter can be interned.
+///
+/// This prevents us from interning `const_allocate` pointers that have not been made
+/// global through `const_make_global`.
+pub trait CanIntern {
+    fn disallows_intern(&self) -> Option<DisallowInternReason>;
+}
+
+impl CanIntern for const_eval::MemoryKind {
+    fn disallows_intern(&self) -> Option<DisallowInternReason> {
+        match self {
+            const_eval::MemoryKind::Heap { was_made_global: false } => {
+                Some(DisallowInternReason::ConstHeap)
+            }
+            const_eval::MemoryKind::Heap { was_made_global: true } => None,
+        }
+    }
+}
+
+impl CanIntern for ! {
+    fn disallows_intern(&self) -> Option<DisallowInternReason> {
+        *self
+    }
+}
+
 /// Intern an allocation. Returns `Err` if the allocation does not exist in the local memory.
 ///
 /// `mutability` can be used to force immutable interning: if it is `Mutability::Not`, the
 /// allocation is interned immutably; if it is `Mutability::Mut`, then the allocation *must be*
 /// already mutable (as a sanity check).
 ///
 /// Returns an iterator over all relocations referred to by this allocation.
-fn intern_shallow<'tcx, T, M: CompileTimeMachine<'tcx, T>>(
+fn intern_shallow<'tcx, T: CanIntern, M: CompileTimeMachine<'tcx, T>>(
     ecx: &mut InterpCx<'tcx, M>,
     alloc_id: AllocId,
     mutability: Mutability,
@@ -71,9 +100,26 @@ fn intern_shallow<'tcx, T, M: CompileTimeMachine<'tcx, T>>(
     trace!("intern_shallow {:?}", alloc_id);
     // remove allocation
     // FIXME(#120456) - is `swap_remove` correct?
-    let Some((_kind, mut alloc)) = ecx.memory.alloc_map.swap_remove(&alloc_id) else {
+    let Some((kind, mut alloc)) = ecx.memory.alloc_map.swap_remove(&alloc_id) else {
         return Err(());
     };
+
+    match kind {
+        MemoryKind::Machine(x) if let Some(reason) = x.disallows_intern() => match reason {
+            // attempting to intern a `const_allocate`d pointer that was not made global via
+            // `const_make_global`. We emit an error here but don't return an `Err`. The `Err`
+            // is for pointers that we can't intern at all (i.e. dangling pointers). We still
+            // (recursively) intern this pointer because we don't have to worry about the
+            // additional paperwork involved with _not_ interning it, such as storing it in
+            // the dead memory map and having to deal with additional "dangling pointer"
+            // messages if someone tries to store the non-made-global ptr in the final value.
+            DisallowInternReason::ConstHeap => {
+                ecx.tcx.dcx().emit_err(ConstHeapPtrInFinal { span: ecx.tcx.span });
+            }
+        },
+        MemoryKind::Machine(_) | MemoryKind::Stack | MemoryKind::CallerLocation => {}
+    }
+
     // Set allocation mutability as appropriate. This is used by LLVM to put things into
     // read-only memory, and also by Miri when evaluating other globals that
     // access this one.
@@ -99,7 +145,7 @@ fn intern_shallow<'tcx, T, M: CompileTimeMachine<'tcx, T>>(
     } else {
         ecx.tcx.set_alloc_id_memory(alloc_id, alloc);
     }
-    Ok(alloc.0.0.provenance().ptrs().iter().map(|&(_, prov)| prov))
+    Ok(alloc.inner().provenance().ptrs().iter().map(|&(_, prov)| prov))
 }
 
 /// Creates a new `DefId` and feeds all the right queries to make this `DefId`
@@ -181,7 +227,7 @@ pub fn intern_const_alloc_recursive<'tcx, M: CompileTimeMachine<'tcx, const_eval
         }
         InternKind::Static(Mutability::Not) => {
             (
-                // Outermost allocation is mutable if `!Freeze`.
+                // Outermost allocation is mutable if `!Freeze` i.e. contains interior mutable types.
                 if ret.layout.ty.is_freeze(*ecx.tcx, ecx.typing_env) {
                     Mutability::Not
                 } else {
@@ -321,7 +367,7 @@ pub fn intern_const_alloc_recursive<'tcx, M: CompileTimeMachine<'tcx, const_eval
 
 /// Intern `ret`. This function assumes that `ret` references no other allocation.
 #[instrument(level = "debug", skip(ecx))]
-pub fn intern_const_alloc_for_constprop<'tcx, T, M: CompileTimeMachine<'tcx, T>>(
+pub fn intern_const_alloc_for_constprop<'tcx, T: CanIntern, M: CompileTimeMachine<'tcx, T>>(
     ecx: &mut InterpCx<'tcx, M>,
     alloc_id: AllocId,
 ) -> InterpResult<'tcx, ()> {

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -26,6 +26,7 @@ use super::{
     Misalignment, Pointer, PointerArithmetic, Provenance, Scalar, alloc_range, err_ub,
     err_ub_custom, interp_ok, throw_ub, throw_ub_custom, throw_unsup, throw_unsup_format,
 };
+use crate::const_eval::ConstEvalErrKind;
 use crate::fluent_generated as fluent;
 
 #[derive(Debug, PartialEq, Copy, Clone)]
@@ -311,6 +312,49 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         interp_ok(new_ptr)
     }
 
+    /// mark the `const_allocate`d pointer immutable so we can intern it.
+    pub fn make_const_heap_ptr_global(
+        &mut self,
+        ptr: Pointer<Option<M::Provenance>>,
+    ) -> InterpResult<'tcx>
+    where
+        M: Machine<'tcx, MemoryKind = crate::const_eval::MemoryKind>,
+    {
+        let (alloc_id, offset, _) = self.ptr_get_alloc_id(ptr, 0)?;
+        if offset.bytes() != 0 {
+            return Err(ConstEvalErrKind::ConstMakeGlobalWithOffset(format!("{ptr:?}"))).into();
+        }
+
+        let not_local_heap =
+            matches!(self.tcx.try_get_global_alloc(alloc_id), Some(GlobalAlloc::Memory(_)));
+
+        if not_local_heap {
+            return Err(ConstEvalErrKind::ConstMakeGlobalPtrIsNonHeap(format!("{ptr:?}"))).into();
+        }
+
+        let (kind, alloc) = self.memory.alloc_map.get_mut_or(alloc_id, || {
+            Err(ConstEvalErrKind::ConstMakeGlobalWithDanglingPtr(format!("{ptr:?}")))
+        })?;
+
+        alloc.mutability = Mutability::Not;
+
+        match kind {
+            MemoryKind::Stack | MemoryKind::CallerLocation => {
+                return Err(ConstEvalErrKind::ConstMakeGlobalPtrIsNonHeap(format!("{ptr:?}")))
+                    .into();
+            }
+            MemoryKind::Machine(crate::const_eval::MemoryKind::Heap { was_made_global }) => {
+                if *was_made_global {
+                    return Err(ConstEvalErrKind::ConstMakeGlobalPtrAlreadyMadeGlobal(alloc_id))
+                        .into();
+                }
+                *was_made_global = true;
+            }
+        }
+
+        interp_ok(())
+    }
+
     #[instrument(skip(self), level = "debug")]
     pub fn deallocate_ptr(
         &mut self,

compiler/rustc_const_eval/src/util/check_validity_requirement.rs

@@ -52,9 +52,8 @@ fn check_validity_requirement_strict<'tcx>(
 
     let mut cx = InterpCx::new(cx.tcx(), DUMMY_SP, cx.typing_env, machine);
 
-    let allocated = cx
-        .allocate(ty, MemoryKind::Machine(crate::const_eval::MemoryKind::Heap))
-        .expect("OOM: failed to allocate for uninit check");
+    let allocated =
+        cx.allocate(ty, MemoryKind::Stack).expect("OOM: failed to allocate for uninit check");
 
     if kind == ValidityRequirement::Zero {
         cx.write_bytes_ptr(
@@ -185,7 +184,10 @@ pub(crate) fn validate_scalar_in_layout<'tcx>(
         bug!("could not compute layout of {scalar:?}:{ty:?}")
     };
     let allocated = cx
-        .allocate(layout, MemoryKind::Machine(crate::const_eval::MemoryKind::Heap))
+        .allocate(
+            layout,
+            MemoryKind::Machine(crate::const_eval::MemoryKind::Heap { was_made_global: false }),
+        )
         .expect("OOM: failed to allocate for uninit check");
 
     cx.write_scalar(scalar, &allocated).unwrap();

compiler/rustc_hir_analysis/src/check/intrinsic.rs

@@ -422,6 +422,9 @@ pub(crate) fn check_intrinsic_type(
             vec![Ty::new_mut_ptr(tcx, tcx.types.u8), tcx.types.usize, tcx.types.usize],
             tcx.types.unit,
         ),
+        sym::const_make_global => {
+            (0, 0, vec![Ty::new_mut_ptr(tcx, tcx.types.u8)], Ty::new_imm_ptr(tcx, tcx.types.u8))
+        }
 
         sym::ptr_offset_from => (
             1,

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

@@ -257,7 +257,7 @@ pub enum InvalidProgramInfo<'tcx> {
 /// Details of why a pointer had to be in-bounds.
 #[derive(Debug, Copy, Clone)]
 pub enum CheckInAllocMsg {
-    /// We are access memory.
+    /// We are accessing memory.
     MemoryAccess,
     /// We are doing pointer arithmetic.
     InboundsPointerArithmetic,