Change `{Box,Arc,Rc,Weak}::into_raw` to only work with `A = Global` #141219

Amanieu · 2025-05-18T11:15:49Z

Also applies to Vec::into_raw_parts.

The expectation is that you can round-trip these methods with from_raw, but this is only true when using the global allocator. With custom allocators you should instead be using into_raw_with_allocator and from_raw_in.

The implementation of Box::leak is changed to use Box::into_raw_with_allocator and explicitly leak the allocator (which was already the existing behavior). This is because, for leak to be safe, the allocator must not free its underlying backing store. The Allocator trait only guarantees that allocated memory remains valid until the allocator is dropped.

rustbot · 2025-05-18T11:15:54Z

r? @joboet

rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Amanieu · 2025-05-18T17:54:20Z

I don't know why this is failing on Miri... I only moved the methods and didn't change any implementation (except Box::leak).

Amanieu · 2025-05-19T08:34:32Z

@RalfJung Do you have any idea why these miri tests would fail? The only relevant change is moving Box::into_raw from Box<T, A> to Box<T, Global>.

RalfJung · 2025-05-19T10:17:36Z

Yeah it has to do with this comment. This is part of the general issue that adding allocators to Box was done without considering the fact that Box is a primitive type that has special treatment in the operational semantics (and all over the compiler).

Amanieu · 2025-05-19T10:28:15Z

Sure, but the implementation of Box::into_raw hasn't changed at all in this PR: https://github.com/Amanieu/rust/blob/ac178fc93c2e3ee1a7e53a14db69551a3207fb66/library/alloc/src/boxed.rs#L1152

I don't see why changing the bounds on that methods from Box<T, A> to Box<T, Global> makes miri tests now fail.

RalfJung · 2025-05-19T11:35:52Z

Miri has no choice but use some terrible hacks to work around the poor state that Box is in due to custom allocators at the moment. Those hacks are sensitive to whether code is generic or monomorphic. I'll try to take a look and see if we can update those hacks to be compatible with this PR.

RalfJung · 2025-05-19T11:56:41Z

I pushed a commit that should hopefully fix CI.

joboet

r=me pending approval from the rest of libs-api

library/alloc/src/boxed.rs

safinaskar · 2025-05-25T05:16:48Z

@rustbot label +A-box +A-allocators +A-MIR +A-miri

bors · 2025-06-14T08:42:45Z

☔ The latest upstream changes (presumably #142483) made this pull request unmergeable. Please resolve the merge conflicts.

dtolnay · 2025-07-01T15:49:52Z

Looks good to me as a libs-api member!

This affects unstable code only: feature(allocator_api) (even though the methods involved are stable when used with default allocator), so it does not require a team FCP. Ready to merge as soon as you have rebased.

Also applies to `Vec::into_raw_parts`. The expectation is that you can round-trip these methods with `from_raw`, but this is only true when using the global allocator. With custom allocators you should instead be using `into_raw_with_allocator` and `from_raw_in`. The implementation of `Box::leak` is changed to use `Box::into_raw_with_allocator` and explicitly leak the allocator (which was already the existing behavior). This is because, for `leak` to be safe, the allocator must not free its underlying backing store. The `Allocator` trait only guarantees that allocated memory remains valid until the allocator is dropped.

Amanieu · 2025-07-01T16:30:32Z

@bors r=joboet

bors · 2025-07-01T16:30:34Z

📌 Commit 15bd619 has been approved by joboet

It is now in the queue for this repository.

RalfJung · 2025-07-01T17:48:59Z

I had pushed a Miri fix into this PR in the past... did that get force-pushed away?

RalfJung · 2025-07-01T17:50:43Z

Yeah, looks like it...

@Amanieu I would recommend never using push -f, and always using push --force-with-lease; that greatly reduces the risk of issues like this. This alias in .gitconfig helps:

[alias]
	pushf = push --force-with-lease

RalfJung · 2025-07-01T17:51:41Z

Meanwhile, please cherry-pick 432a1c0, I hope that still fixes the problem. I won't push to this branch again to avoid repeating history. ;)

Amanieu · 2025-07-01T17:56:54Z

@bors r=joboet

bors · 2025-07-01T17:56:57Z

📌 Commit 8797d54 has been approved by joboet

It is now in the queue for this repository.

Amanieu · 2025-07-01T18:35:52Z

[alias]
	pushf = push --force-with-lease

Thanks, I've added that and will try to remember to use it in the future!

GrigorenkoPV · 2025-07-02T14:09:10Z

library/alloc/src/boxed.rs

+        // Avoid `into_raw_with_allocator` as that interacts poorly with Miri's Stacked Borrows.
+        let mut b = mem::ManuallyDrop::new(b);
+        // We go through the built-in deref for `Box`, which is crucial for Miri to recognize this
+        // operation for it's alias tracking.


Nit: its* 😅

Rollup of 11 pull requests Successful merges: - #131923 (Derive `Copy` and `Hash` for `IntErrorKind`) - #138340 (Remove some unsized tuple impls now that we don't support unsizing tuples anymore) - #141219 (Change `{Box,Arc,Rc,Weak}::into_raw` to only work with `A = Global`) - #142212 (bootstrap: validate `rust.codegen-backends` & `target.<triple>.codegen-backends`) - #142237 (Detect more cases of unused_parens around types) - #142964 (Attribute rework: a parser for single attributes without arguments) - #143070 (Rewrite `macro_rules!` parser to not use the MBE engine itself) - #143235 (Assemble const bounds via normal item bounds in old solver too) - #143261 (Feed `explicit_predicates_of` instead of `predicates_of`) - #143276 (loop match: handle opaque patterns) - #143306 (Add `track_caller` attributes to trace origin of Clippy lints) r? `@ghost` `@rustbot` modify labels: rollup try-job: aarch64-apple try-job: x86_64-msvc-1 try-job: x86_64-gnu try-job: dist-i586-gnu-i586-i686-musl try-job: test-various

Rollup merge of #141219 - Amanieu:leak_alloc, r=joboet Change `{Box,Arc,Rc,Weak}::into_raw` to only work with `A = Global` Also applies to `Vec::into_raw_parts`. The expectation is that you can round-trip these methods with `from_raw`, but this is only true when using the global allocator. With custom allocators you should instead be using `into_raw_with_allocator` and `from_raw_in`. The implementation of `Box::leak` is changed to use `Box::into_raw_with_allocator` and explicitly leak the allocator (which was already the existing behavior). This is because, for `leak` to be safe, the allocator must not free its underlying backing store. The `Allocator` trait only guarantees that allocated memory remains valid until the allocator is dropped.

Fix issue introduced by rust-lang/rust#141219 - [x] Changelog updated

rustbot assigned joboet May 18, 2025

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

Amanieu force-pushed the leak_alloc branch from b8f1fff to 87983ab Compare May 18, 2025 11:20

Amanieu mentioned this pull request May 18, 2025

Box::leak will not call allocator destruction function #106207

Closed

This comment has been minimized.

Sign in to view

Amanieu force-pushed the leak_alloc branch from 87983ab to ee01141 Compare May 18, 2025 13:16

This comment has been minimized.

Sign in to view

Amanieu force-pushed the leak_alloc branch from ee01141 to 638c2e8 Compare May 18, 2025 13:42

This comment has been minimized.

Sign in to view

Amanieu force-pushed the leak_alloc branch from 638c2e8 to ac178fc Compare May 18, 2025 17:47

This comment has been minimized.

Sign in to view

joboet approved these changes May 23, 2025

View reviewed changes

library/alloc/src/boxed.rs Show resolved Hide resolved

RalfJung force-pushed the leak_alloc branch from e3c5921 to 432a1c0 Compare May 23, 2025 17:35

RalfJung mentioned this pull request May 24, 2025

RFC: No (opsem) Magic Boxes rust-lang/rfcs#3712

Open

This was referenced May 24, 2025

Tracking issue for vec_into_raw_parts #65816

Open

Draft: Make into_parts methods on Vec associated functions #141509

Open

rustbot added A-allocators Area: Custom and system allocators A-box Area: Our favorite opsem complication A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-miri Area: The miri tool labels May 25, 2025

Amanieu added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Jun 30, 2025

dtolnay approved these changes Jul 1, 2025

View reviewed changes

Amanieu force-pushed the leak_alloc branch from 432a1c0 to 15bd619 Compare July 1, 2025 16:29

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 1, 2025

Amanieu removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Jul 1, 2025

This comment has been minimized.

Sign in to view

make Box::into_raw compatible with Stacked Borrows again

8797d54

GrigorenkoPV reviewed Jul 2, 2025

View reviewed changes

matthiaskrgr mentioned this pull request Jul 2, 2025

Rollup of 11 pull requests #143338

Merged

bors merged commit 1a686c6 into rust-lang:master Jul 3, 2025
10 checks passed

rustbot added this to the 1.90.0 milestone Jul 3, 2025

gwilymk mentioned this pull request Jul 4, 2025

Use into_raw_with_allocator agbrs/agb#1060

Merged

1 task

gwilymk added a commit to agbrs/agb that referenced this pull request Jul 4, 2025

Use into_raw_with_allocator (#1060)

58e27b9

Fix issue introduced by rust-lang/rust#141219 - [x] Changelog updated

Change {Box,Arc,Rc,Weak}::into_raw to only work with A = Global #141219

Change {Box,Arc,Rc,Weak}::into_raw to only work with A = Global #141219

Conversation

Amanieu commented May 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented May 18, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Amanieu commented May 18, 2025

Uh oh!

This comment has been minimized.

Amanieu commented May 19, 2025

Uh oh!

RalfJung commented May 19, 2025

Uh oh!

Amanieu commented May 19, 2025

Uh oh!

RalfJung commented May 19, 2025

Uh oh!

RalfJung commented May 19, 2025

Uh oh!

joboet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

safinaskar commented May 25, 2025

Uh oh!

bors commented Jun 14, 2025

Uh oh!

dtolnay commented Jul 1, 2025

Uh oh!

Amanieu commented Jul 1, 2025

Uh oh!

bors commented Jul 1, 2025

Uh oh!

This comment has been minimized.

RalfJung commented Jul 1, 2025

Uh oh!

RalfJung commented Jul 1, 2025

Uh oh!

RalfJung commented Jul 1, 2025

Uh oh!

Amanieu commented Jul 1, 2025

Uh oh!

bors commented Jul 1, 2025

Uh oh!

Amanieu commented Jul 1, 2025

Uh oh!

GrigorenkoPV Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Change `{Box,Arc,Rc,Weak}::into_raw` to only work with `A = Global` #141219

Change `{Box,Arc,Rc,Weak}::into_raw` to only work with `A = Global` #141219

Amanieu commented May 18, 2025 •

edited

Loading