Skip to content

Commit 5badbfa

Browse files
committed
add const_leak and reject interning non-leaked const_allocate ptrs
1 parent 8df4a58 commit 5badbfa

30 files changed

+353
-27
lines changed

compiler/rustc_const_eval/messages.ftl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,13 @@ const_eval_const_context = {$kind ->
5656
*[other] {""}
5757
}
5858
59+
const_eval_const_heap_ptr_in_final = encountered `const_allocate` pointer in final value that was not made global
60+
.note = use `const_make_global` to make allocated pointers immutable before returning
61+
62+
const_eval_const_make_global_ptr_already_imm = attempting to call `const_make_global` twice on the same allocation {$alloc}
63+
64+
const_eval_const_make_global_ptr_is_non_heap = invalid pointer passed to `const_make_global`
65+
5966
const_eval_copy_nonoverlapping_overlapping =
6067
`copy_nonoverlapping` called on overlapping ranges
6168
@@ -338,6 +345,7 @@ const_eval_realloc_or_alloc_with_offset =
338345
{$kind ->
339346
[dealloc] deallocating
340347
[realloc] reallocating
348+
[leak] leaking
341349
*[other] {""}
342350
} {$ptr} which does not point to the beginning of an object
343351

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ fn eval_body_using_ecx<'tcx, R: InterpretationResult<'tcx>>(
9393
// Since evaluation had no errors, validate the resulting constant.
9494
const_validate_mplace(ecx, &ret, cid)?;
9595

96-
// Only report this after validation, as validaiton produces much better diagnostics.
96+
// Only report this after validation, as validation produces much better diagnostics.
9797
// FIXME: ensure validation always reports this and stop making interning care about it.
9898

9999
match intern_result {

compiler/rustc_const_eval/src/const_eval/machine.rs

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,13 +169,19 @@ pub type CompileTimeInterpCx<'tcx> = InterpCx<'tcx, CompileTimeMachine<'tcx>>;
169169

170170
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
171171
pub enum MemoryKind {
172-
Heap,
172+
Heap {
173+
was_made_immut: bool
174+
},
173175
}
174176

175177
impl fmt::Display for MemoryKind {
176178
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
177179
match self {
178-
MemoryKind::Heap => write!(f, "heap allocation"),
180+
MemoryKind::Heap { was_made_immut } => write!(f, "{}heap allocation", if *was_made_immut {
181+
"immutable "
182+
} else {
183+
""
184+
}),
179185
}
180186
}
181187
}
@@ -184,7 +190,7 @@ impl interpret::MayLeak for MemoryKind {
184190
#[inline(always)]
185191
fn may_leak(self) -> bool {
186192
match self {
187-
MemoryKind::Heap => false,
193+
MemoryKind::Heap { was_made_immut } => was_made_immut,
188194
}
189195
}
190196
}
@@ -420,7 +426,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
420426
let ptr = ecx.allocate_ptr(
421427
Size::from_bytes(size),
422428
align,
423-
interpret::MemoryKind::Machine(MemoryKind::Heap),
429+
interpret::MemoryKind::Machine(MemoryKind::Heap { was_made_immut: false }),
424430
AllocInit::Uninit,
425431
)?;
426432
ecx.write_pointer(ptr, dest)?;
@@ -453,10 +459,32 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
453459
ecx.deallocate_ptr(
454460
ptr,
455461
Some((size, align)),
456-
interpret::MemoryKind::Machine(MemoryKind::Heap),
462+
interpret::MemoryKind::Machine(MemoryKind::Heap { was_made_immut: false }),
457463
)?;
458464
}
459465
}
466+
467+
sym::const_make_global => {
468+
let ptr = ecx.read_pointer(&args[0])?;
469+
let size = ecx.read_scalar(&args[1])?.to_target_usize(ecx)?;
470+
let align = ecx.read_scalar(&args[2])?.to_target_usize(ecx)?;
471+
472+
// FIXME: do we need these arguments?
473+
let _size: Size = Size::from_bytes(size);
474+
let _align = match Align::from_bytes(align) {
475+
Ok(a) => a,
476+
Err(err) => throw_ub_custom!(
477+
fluent::const_eval_invalid_align_details,
478+
name = "const_make_global",
479+
err_kind = err.diag_ident(),
480+
align = err.align()
481+
),
482+
};
483+
484+
ecx.make_const_heap_ptr_immutable(ptr)?;
485+
ecx.write_pointer(ptr, dest)?;
486+
}
487+
460488
// The intrinsic represents whether the value is known to the optimizer (LLVM).
461489
// We're not doing any optimizations here, so there is no optimizer that could know the value.
462490
// (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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,15 @@ pub(crate) struct MutablePtrInFinal {
4343
pub kind: InternKind,
4444
}
4545

46+
47+
#[derive(Diagnostic)]
48+
#[diag(const_eval_const_heap_ptr_in_final)]
49+
#[note]
50+
pub(crate) struct ConstHeapPtrInFinal {
51+
#[primary_span]
52+
pub span: Span,
53+
}
54+
4655
#[derive(Diagnostic)]
4756
#[diag(const_eval_unstable_in_stable_exposed)]
4857
pub(crate) struct UnstableInStableExposed {
@@ -493,6 +502,8 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
493502
}
494503
AbiMismatchArgument { .. } => const_eval_incompatible_types,
495504
AbiMismatchReturn { .. } => const_eval_incompatible_return_types,
505+
ConstMakeGlobalPtrIsNonHeap => const_eval_const_make_global_ptr_is_non_heap,
506+
ConstMakeGlobalPtrAlreadyImm { .. } => const_eval_const_make_global_ptr_already_imm,
496507
}
497508
}
498509

@@ -518,7 +529,8 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
518529
| InvalidUninitBytes(None)
519530
| DeadLocal
520531
| UninhabitedEnumVariantWritten(_)
521-
| UninhabitedEnumVariantRead(_) => {}
532+
| UninhabitedEnumVariantRead(_)
533+
| ConstMakeGlobalPtrIsNonHeap => {}
522534

523535
ArithOverflow { intrinsic } => {
524536
diag.arg("intrinsic", intrinsic);
@@ -620,6 +632,9 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
620632
diag.arg("caller_ty", caller_ty.to_string());
621633
diag.arg("callee_ty", callee_ty.to_string());
622634
}
635+
ConstMakeGlobalPtrAlreadyImm(alloc) => {
636+
diag.arg("alloc", alloc);
637+
}
623638
}
624639
}
625640
}

compiler/rustc_const_eval/src/interpret/intern.rs

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use super::{
3131
};
3232
use crate::const_eval;
3333
use crate::const_eval::DummyMachine;
34-
use crate::errors::NestedStaticInThreadLocal;
34+
use crate::errors::{ConstHeapPtrInFinal, NestedStaticInThreadLocal};
3535

3636
pub trait CompileTimeMachine<'tcx, T> = Machine<
3737
'tcx,
@@ -55,14 +55,37 @@ impl HasStaticRootDefId for const_eval::CompileTimeMachine<'_> {
5555
}
5656
}
5757

58+
pub enum DisallowInternReason {
59+
ConstHeap,
60+
}
61+
62+
pub trait CanIntern {
63+
fn disallows_intern(&self) -> Option<DisallowInternReason>;
64+
}
65+
66+
impl CanIntern for const_eval::MemoryKind {
67+
fn disallows_intern(&self) -> Option<DisallowInternReason> {
68+
match self {
69+
const_eval::MemoryKind::Heap { was_made_immut: false } => Some(DisallowInternReason::ConstHeap),
70+
const_eval::MemoryKind::Heap { was_made_immut: true } => None,
71+
}
72+
}
73+
}
74+
75+
impl CanIntern for ! {
76+
fn disallows_intern(&self) -> Option<DisallowInternReason> {
77+
*self
78+
}
79+
}
80+
5881
/// Intern an allocation. Returns `Err` if the allocation does not exist in the local memory.
5982
///
6083
/// `mutability` can be used to force immutable interning: if it is `Mutability::Not`, the
6184
/// allocation is interned immutably; if it is `Mutability::Mut`, then the allocation *must be*
6285
/// already mutable (as a sanity check).
6386
///
6487
/// Returns an iterator over all relocations referred to by this allocation.
65-
fn intern_shallow<'tcx, T, M: CompileTimeMachine<'tcx, T>>(
88+
fn intern_shallow<'tcx, T: CanIntern, M: CompileTimeMachine<'tcx, T>>(
6689
ecx: &mut InterpCx<'tcx, M>,
6790
alloc_id: AllocId,
6891
mutability: Mutability,
@@ -71,9 +94,22 @@ fn intern_shallow<'tcx, T, M: CompileTimeMachine<'tcx, T>>(
7194
trace!("intern_shallow {:?}", alloc_id);
7295
// remove allocation
7396
// FIXME(#120456) - is `swap_remove` correct?
74-
let Some((_kind, mut alloc)) = ecx.memory.alloc_map.swap_remove(&alloc_id) else {
97+
let Some((kind, mut alloc)) = ecx.memory.alloc_map.swap_remove(&alloc_id) else {
7598
return Err(());
7699
};
100+
101+
match kind {
102+
MemoryKind::Machine(x) if let Some(reason) = x.disallows_intern() => match reason {
103+
// attempting to intern a `const_allocate`d pointer that was not made global via
104+
// `const_make_global`. We emit an error here but don't return an `Err` as if we
105+
// did the caller assumes we found a dangling pointer.
106+
DisallowInternReason::ConstHeap => {
107+
ecx.tcx.dcx().emit_err(ConstHeapPtrInFinal { span: ecx.tcx.span });
108+
}
109+
},
110+
MemoryKind::Machine(_) | MemoryKind::Stack | MemoryKind::CallerLocation => {}
111+
}
112+
77113
// Set allocation mutability as appropriate. This is used by LLVM to put things into
78114
// read-only memory, and also by Miri when evaluating other globals that
79115
// access this one.
@@ -99,7 +135,7 @@ fn intern_shallow<'tcx, T, M: CompileTimeMachine<'tcx, T>>(
99135
} else {
100136
ecx.tcx.set_alloc_id_memory(alloc_id, alloc);
101137
}
102-
Ok(alloc.0.0.provenance().ptrs().iter().map(|&(_, prov)| prov))
138+
Ok(alloc.inner().provenance().ptrs().iter().map(|&(_, prov)| prov))
103139
}
104140

105141
/// Creates a new `DefId` and feeds all the right queries to make this `DefId`
@@ -181,7 +217,7 @@ pub fn intern_const_alloc_recursive<'tcx, M: CompileTimeMachine<'tcx, const_eval
181217
}
182218
InternKind::Static(Mutability::Not) => {
183219
(
184-
// Outermost allocation is mutable if `!Freeze`.
220+
// Outermost allocation is mutable if `!Freeze` i.e. contains interior mutable types.
185221
if ret.layout.ty.is_freeze(*ecx.tcx, ecx.typing_env) {
186222
Mutability::Not
187223
} else {
@@ -309,7 +345,12 @@ pub fn intern_const_alloc_recursive<'tcx, M: CompileTimeMachine<'tcx, const_eval
309345
// `static mut`.
310346
just_interned.insert(alloc_id);
311347
match intern_shallow(ecx, alloc_id, inner_mutability, Some(&mut disambiguator)) {
312-
Ok(nested) => todo.extend(nested),
348+
Ok(nested) => {
349+
// filtering here is necessary for self-referential values created through
350+
// `const_allocate`.
351+
// see tests/ui/consts/const-eval/heap/make-global-cyclic.rs
352+
todo.extend(nested.filter(|prov| !just_interned.contains(&prov.alloc_id())))
353+
}
313354
Err(()) => {
314355
ecx.tcx.dcx().delayed_bug("found dangling pointer during const interning");
315356
result = Err(InternResult::FoundDanglingPointer);
@@ -321,7 +362,7 @@ pub fn intern_const_alloc_recursive<'tcx, M: CompileTimeMachine<'tcx, const_eval
321362

322363
/// Intern `ret`. This function assumes that `ret` references no other allocation.
323364
#[instrument(level = "debug", skip(ecx))]
324-
pub fn intern_const_alloc_for_constprop<'tcx, T, M: CompileTimeMachine<'tcx, T>>(
365+
pub fn intern_const_alloc_for_constprop<'tcx, T: CanIntern, M: CompileTimeMachine<'tcx, T>>(
325366
ecx: &mut InterpCx<'tcx, M>,
326367
alloc_id: AllocId,
327368
) -> InterpResult<'tcx, ()> {

compiler/rustc_const_eval/src/interpret/memory.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,46 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
311311
interp_ok(new_ptr)
312312
}
313313

314+
/// mark the `const_allocate`d pointer immutable so we can intern it.
315+
pub fn make_const_heap_ptr_immutable(
316+
&mut self,
317+
ptr: Pointer<Option<M::Provenance>>,
318+
) -> InterpResult<'tcx>
319+
where
320+
M: Machine<'tcx, MemoryKind = crate::const_eval::MemoryKind>,
321+
{
322+
let (alloc_id, _, _) = self.ptr_get_alloc_id(ptr, 0)?;
323+
324+
let not_local_heap = matches!(
325+
self.tcx.try_get_global_alloc(alloc_id),
326+
Some(GlobalAlloc::Memory(_))
327+
);
328+
329+
if not_local_heap {
330+
throw_ub!(ConstMakeGlobalPtrIsNonHeap);
331+
}
332+
333+
let (kind, alloc) = self.memory.alloc_map.get_mut_or(alloc_id, || {
334+
Err(err_ub!(PointerUseAfterFree(alloc_id, CheckInAllocMsg::Dereferenceable)))
335+
})?;
336+
337+
alloc.mutability = Mutability::Not;
338+
339+
match kind {
340+
MemoryKind::Stack | MemoryKind::CallerLocation => {
341+
throw_ub!(ConstMakeGlobalPtrIsNonHeap);
342+
}
343+
MemoryKind::Machine(crate::const_eval::MemoryKind::Heap { was_made_immut }) => {
344+
if *was_made_immut {
345+
throw_ub!(ConstMakeGlobalPtrAlreadyImm(alloc_id));
346+
}
347+
*was_made_immut = true;
348+
}
349+
}
350+
351+
interp_ok(())
352+
}
353+
314354
#[instrument(skip(self), level = "debug")]
315355
pub fn deallocate_ptr(
316356
&mut self,

compiler/rustc_const_eval/src/util/check_validity_requirement.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ fn check_validity_requirement_strict<'tcx>(
5353
let mut cx = InterpCx::new(cx.tcx(), DUMMY_SP, cx.typing_env, machine);
5454

5555
let allocated = cx
56-
.allocate(ty, MemoryKind::Machine(crate::const_eval::MemoryKind::Heap))
56+
.allocate(ty, MemoryKind::Machine(crate::const_eval::MemoryKind::Heap { was_made_immut: false }))
5757
.expect("OOM: failed to allocate for uninit check");
5858

5959
if kind == ValidityRequirement::Zero {
@@ -185,7 +185,7 @@ pub(crate) fn validate_scalar_in_layout<'tcx>(
185185
bug!("could not compute layout of {scalar:?}:{ty:?}")
186186
};
187187
let allocated = cx
188-
.allocate(layout, MemoryKind::Machine(crate::const_eval::MemoryKind::Heap))
188+
.allocate(layout, MemoryKind::Machine(crate::const_eval::MemoryKind::Heap { was_made_immut: false }))
189189
.expect("OOM: failed to allocate for uninit check");
190190

191191
cx.write_scalar(scalar, &allocated).unwrap();

compiler/rustc_hir_analysis/src/check/intrinsic.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,12 @@ pub(crate) fn check_intrinsic_type(
415415
vec![Ty::new_mut_ptr(tcx, tcx.types.u8), tcx.types.usize, tcx.types.usize],
416416
tcx.types.unit,
417417
),
418+
sym::const_make_global => (
419+
0,
420+
0,
421+
vec![Ty::new_mut_ptr(tcx, tcx.types.u8), tcx.types.usize, tcx.types.usize],
422+
Ty::new_imm_ptr(tcx, tcx.types.u8),
423+
),
418424

419425
sym::ptr_offset_from => (
420426
1,

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ pub enum InvalidProgramInfo<'tcx> {
257257
/// Details of why a pointer had to be in-bounds.
258258
#[derive(Debug, Copy, Clone)]
259259
pub enum CheckInAllocMsg {
260-
/// We are access memory.
260+
/// We are accessing memory.
261261
MemoryAccess,
262262
/// We are doing pointer arithmetic.
263263
InboundsPointerArithmetic,
@@ -427,6 +427,10 @@ pub enum UndefinedBehaviorInfo<'tcx> {
427427
AbiMismatchArgument { caller_ty: Ty<'tcx>, callee_ty: Ty<'tcx> },
428428
/// ABI-incompatible return types.
429429
AbiMismatchReturn { caller_ty: Ty<'tcx>, callee_ty: Ty<'tcx> },
430+
/// Called `const_make_global` on a non-heap pointer.
431+
ConstMakeGlobalPtrIsNonHeap,
432+
/// Called `const_make_global` twice.
433+
ConstMakeGlobalPtrAlreadyImm(AllocId),
430434
}
431435

432436
#[derive(Debug, Clone, Copy)]

compiler/rustc_span/src/symbol.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,7 @@ symbols! {
714714
const_indexing,
715715
const_let,
716716
const_loop,
717+
const_make_global,
717718
const_mut_refs,
718719
const_panic,
719720
const_panic_fmt,

0 commit comments

Comments
 (0)