Skip to content

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

Merged
bors merged 2 commits into
rust-lang:masterfrom
Amanieu:leak_alloc
Jul 3, 2025
Merged

Change {Box,Arc,Rc,Weak}::into_raw to only work with A = Global#141219
bors merged 2 commits into
rust-lang:masterfrom
Amanieu:leak_alloc

Conversation

@Amanieu

@Amanieu Amanieu commented May 18, 2025

Copy link
Copy Markdown
Member

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

rustbot commented May 18, 2025

Copy link
Copy Markdown
Collaborator

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

@rustbot 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
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Amanieu

Amanieu commented May 18, 2025

Copy link
Copy Markdown
Member Author

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

@rust-log-analyzer

This comment has been minimized.

@Amanieu

Amanieu commented May 19, 2025

Copy link
Copy Markdown
Member Author

@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

Copy link
Copy Markdown
Member

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

Amanieu commented May 19, 2025

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member

I pushed a commit that should hopefully fix CI.

@joboet joboet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread library/alloc/src/boxed.rs
@safinaskar

Copy link
Copy Markdown
Contributor

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

@rustbot 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
@bors

bors commented Jun 14, 2025

Copy link
Copy Markdown
Collaborator

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

@dtolnay

dtolnay commented Jul 1, 2025

Copy link
Copy Markdown
Member

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

Amanieu commented Jul 1, 2025

Copy link
Copy Markdown
Member Author

@bors r=joboet

@bors

bors commented Jul 1, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 15bd619 has been approved by joboet

It is now in the queue for this repository.

@bors 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 Amanieu removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Jul 1, 2025
@rust-log-analyzer

This comment has been minimized.

@RalfJung

RalfJung commented Jul 1, 2025

Copy link
Copy Markdown
Member

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

@RalfJung

RalfJung commented Jul 1, 2025

Copy link
Copy Markdown
Member

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

RalfJung commented Jul 1, 2025

Copy link
Copy Markdown
Member

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

Amanieu commented Jul 1, 2025

Copy link
Copy Markdown
Member Author

@bors r=joboet

@bors

bors commented Jul 1, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 8797d54 has been approved by joboet

It is now in the queue for this repository.

@Amanieu

Amanieu commented Jul 1, 2025

Copy link
Copy Markdown
Member Author
[alias]
	pushf = push --force-with-lease

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

// 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: its* 😅

bors added a commit that referenced this pull request Jul 2, 2025
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
bors added a commit that referenced this pull request Jul 2, 2025
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
@bors bors merged commit 1a686c6 into rust-lang:master Jul 3, 2025
10 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 3, 2025
rust-timer added a commit that referenced this pull request Jul 3, 2025
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.
gwilymk added a commit to agbrs/agb that referenced this pull request Jul 4, 2025
Fix issue introduced by rust-lang/rust#141219

- [x] Changelog updated
github-actions Bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Jul 11, 2025
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.
github-actions Bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Jul 11, 2025
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#131923 (Derive `Copy` and `Hash` for `IntErrorKind`)
 - rust-lang#138340 (Remove some unsized tuple impls now that we don't support unsizing tuples anymore)
 - rust-lang#141219 (Change `{Box,Arc,Rc,Weak}::into_raw` to only work with `A = Global`)
 - rust-lang#142212 (bootstrap: validate `rust.codegen-backends` & `target.<triple>.codegen-backends`)
 - rust-lang#142237 (Detect more cases of unused_parens around types)
 - rust-lang#142964 (Attribute rework: a parser for single attributes without arguments)
 - rust-lang#143070 (Rewrite `macro_rules!` parser to not use the MBE engine itself)
 - rust-lang#143235 (Assemble const bounds via normal item bounds in old solver too)
 - rust-lang#143261 (Feed `explicit_predicates_of` instead of `predicates_of`)
 - rust-lang#143276 (loop match: handle opaque patterns)
 - rust-lang#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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 S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants