add `const_leak` and reject interning non-leaked `const_allocate` ptrs #143595

fee1-dead · 2025-07-07T16:23:17Z

Implements as discussed on Zulip: #t-compiler/const-eval > const heap

r? @rust-lang/wg-const-eval

rustbot · 2025-07-07T16:23:23Z

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

juntyr · 2025-07-07T17:18:13Z

I'm really excited about this experiment and the movement on const alloc, so thanks for working on it <3

oli-obk · 2025-07-07T17:31:42Z

compiler/rustc_const_eval/src/const_eval/machine.rs

+                    ),
+                };
+
+                // If an allocation is created in an another const,


For now we could just error on this. I don't think this should really happen. Either you leaked, then you don't have an owned value anymore, or you didn't, then it got an error

oli-obk · 2025-07-07T17:35:25Z

tests/ui/consts/const-eval/heap/alloc_intrinsic_untyped.rs

@@ -7,5 +7,6 @@ use std::intrinsics;

 const BAR: *mut i32 = unsafe { intrinsics::const_allocate(4, 4) as *mut i32 };
 //~^ error: mutable pointer in final value of constant
+//~| error: encountered `const_allocate` pointer in final value that was not leaked


We also need a test that only errors with this error message and looks otherwise benign

oli-obk · 2025-07-07T17:37:21Z

compiler/rustc_const_eval/src/const_eval/machine.rs

+                        size,
+                        align,
+                        interpret::MemoryKind::Machine(MemoryKind::Heap),
+                        interpret::MemoryKind::Machine(MemoryKind::HeapLeaked),


Do we need a separate memory kind? I think the interner will be happy with anything that didn't get deallocated

Similar to how it interns stack allocations from the trailing expression

oli-obk · 2025-07-07T17:39:39Z

compiler/rustc_const_eval/src/interpret/intern.rs

        return Err(());
    };
+
+    if matches!(kind, MemoryKind::Machine(x) if x.is_const_heap()) {


Please turn this into an exhaustive match on the base memory kind enum.

Why is this not reusing the may_leak method?

This isn't about leaking though, it's about interning. I don't think it is the same condition as what Miri's leak check needs?

We intern stack allocations and those are marked as may_leak = false

Please turn this into an exhaustive match on the base memory kind enum.

I agree with this part though -- we should have a list of that we are allowed to intern. So the trait should be called AllowIntern or so, and it should be defined in intern.rs as that's the one place it is needed (not machine.rs).

RalfJung · 2025-07-07T18:26:34Z

compiler/rustc_const_eval/src/interpret/memory.rs

+        kind: MemoryKind<M::MemoryKind>,
+        new_kind: MemoryKind<M::MemoryKind>,
+    ) -> InterpResult<'tcx, Pointer<M::Provenance>> {
+        self.reallocate_ptr_inner(


Why does this reallocate? I don't think that is the right semantics. It will be very hard to use this to intern a graph of allocations since you need to mutate the all pointers into the allocation. And it will be impossible to do this in a cyclic graph.

It will be very hard to use this to intern a graph of allocations since you need to mutate the all pointers into the allocation.

True. I originally went with a "just change the original memory kind" approach but got really scared by the following lines:

rust/compiler/rustc_const_eval/src/interpret/memory.rs

Lines 1609 to 1614 in 688ea65

/// The result must be used immediately; it is not allowed to convert

/// the returned data back into a `Pointer` and store that in machine state.

/// (In fact that's not even possible since `M::ProvenanceExtra` is generic and

/// we don't have an operation to turn it back into `M::Provenance`.)

#[inline(always)]

pub fn ptr_get_alloc_id(

Hm, I don't see how that comment would conflict with "change the memory kind"?

I made it so that it now makes the allocation global (tcx managed).

RalfJung · 2025-07-07T18:30:07Z

In terms of naming, I think we said that Box::leak would not be performing this operation, so "leak" seems like a suboptimal choice. Maybe something like "make_global" or "into_static" or so?

The original plan I proposed also makes the allocation immutable when it gets marked as global -- basically, this operation should indicate that you are done preparing this allocation and it is ready to be interned. (We actually intern it later as that's easier, but for the API this does not matter.)

RalfJung · 2025-07-08T06:11:30Z

compiler/rustc_const_eval/src/interpret/memory.rs

+        alloc.mutability = Mutability::Not;
+        let alloc = self.tcx.mk_const_alloc(alloc);
+        self.tcx.set_alloc_id_memory(alloc_id, alloc);
+        interp_ok(())


This risks breaking key invariants like "tcx-managed allocations do not have any dangling pointers". So as nice as this is I think it won't work.

I'm pretty sure both ptr_get_alloc_id and self.memory.alloc_map.remove(&alloc_id) calls above ensure that this allocation is valid, unless you mean that the allocation itself could contain values that are dangling pointers to other stuff?

Yes that's what I mean.

rustbot assigned lcnr Jul 7, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 7, 2025

fee1-dead force-pushed the push-sylpykzkmynr branch 3 times, most recently from 67537c4 to 911c6a6 Compare July 7, 2025 16:33

oli-obk reviewed Jul 7, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

oli-obk reviewed Jul 7, 2025

View reviewed changes

RalfJung reviewed Jul 7, 2025

View reviewed changes

fee1-dead force-pushed the push-sylpykzkmynr branch 2 times, most recently from aa4e8a9 to a1ef719 Compare July 8, 2025 06:07

RalfJung reviewed Jul 8, 2025

View reviewed changes

fee1-dead force-pushed the push-sylpykzkmynr branch from a1ef719 to 575f024 Compare July 8, 2025 06:14

add const_leak and reject interning non-leaked const_allocate ptrs

b1cc426

fee1-dead force-pushed the push-sylpykzkmynr branch from 575f024 to b1cc426 Compare July 8, 2025 06:22

	/// The result must be used immediately; it is not allowed to convert
	/// the returned data back into a `Pointer` and store that in machine state.
	/// (In fact that's not even possible since `M::ProvenanceExtra` is generic and
	/// we don't have an operation to turn it back into `M::Provenance`.)
	#[inline(always)]
	pub fn ptr_get_alloc_id(

add const_leak and reject interning non-leaked const_allocate ptrs #143595

Are you sure you want to change the base?

add const_leak and reject interning non-leaked const_allocate ptrs #143595

Conversation

fee1-dead commented Jul 7, 2025 • edited by oli-obk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 7, 2025

Uh oh!

juntyr commented Jul 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Jul 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

add `const_leak` and reject interning non-leaked `const_allocate` ptrs #143595

add `const_leak` and reject interning non-leaked `const_allocate` ptrs #143595

fee1-dead commented Jul 7, 2025 •

edited by oli-obk

Loading