Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add_retag: ensure box-to-raw-ptr casts are preserved for Miri #122647

Merged
merged 2 commits into from
Mar 18, 2024

Conversation

RalfJung
Copy link
Member

In #122233 I added retag_box_to_raw not realizing that we can already do addr_of_mut!(*bx) to turn a box into a raw pointer without an intermediate reference. We just need to ensure this information is preserved past the ElaborateBoxDerefs pass.

r? @oli-obk

@rustbot 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 Mar 17, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 17, 2024

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

The Miri subtree was changed

cc @rust-lang/miri

&elaborate_box_derefs::ElaborateBoxDerefs,
&coroutine::StateTransform,
&add_retag::AddRetag,
&Lint(known_panics_lint::KnownPanicsLint),
Copy link
Member Author

@RalfJung RalfJung Mar 17, 2024

Choose a reason for hiding this comment

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

I was not sure what to do with the pass order here. AddRetag needs to be before ElaborateBoxDerefs removes all traces of primitive Box derefs. Whether they should be before or after StateTransform, I don't know. I decided to use the order which doesn't change anything on stable (where AddRetag is a NOP).

@oli-obk
Copy link
Contributor

oli-obk commented Mar 18, 2024

@bors r+ rollup

thanks for the thorough explanations

@bors
Copy link
Contributor

bors commented Mar 18, 2024

📌 Commit bcf8015 has been approved by oli-obk

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 Mar 18, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 18, 2024
add_retag: ensure box-to-raw-ptr casts are preserved for Miri

In rust-lang#122233 I added `retag_box_to_raw` not realizing that we can already do `addr_of_mut!(*bx)` to turn a box into a raw pointer without an intermediate reference. We just need to ensure this information is preserved past the ElaborateBoxDerefs pass.

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 18, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#122480 (Avoid various uses of `Option<Span>` in favor of using `DUMMY_SP` in the few cases that used `None`)
 - rust-lang#122567 (Remove fixme about LLVM basic block naming)
 - rust-lang#122588 (less useless filter calls in imported_source_file)
 - rust-lang#122647 (add_retag: ensure box-to-raw-ptr casts are preserved for Miri)
 - rust-lang#122649 (Update the minimum external LLVM to 17)
 - rust-lang#122680 (Do not eat nested expressions' results in `MayContainYieldPoint` format args visitor)
 - rust-lang#122683 (add missing test: expected paren or brace in macro)
 - rust-lang#122689 (Add missing `try_visit` calls in visitors.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3fa48ec into rust-lang:master Mar 18, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 18, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 18, 2024
Rollup merge of rust-lang#122647 - RalfJung:box-to-raw-retag, r=oli-obk

add_retag: ensure box-to-raw-ptr casts are preserved for Miri

In rust-lang#122233 I added `retag_box_to_raw` not realizing that we can already do `addr_of_mut!(*bx)` to turn a box into a raw pointer without an intermediate reference. We just need to ensure this information is preserved past the ElaborateBoxDerefs pass.

r? ``@oli-obk``
@RalfJung RalfJung deleted the box-to-raw-retag branch March 19, 2024 19:33
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 16, 2024
Box::into_raw: make Miri understand that this is a box-to-raw cast

Turns out rust-lang#122647 went a bit too far in cleaning up `Box`... we still need a hack in `Box::into_raw`. The nicer fix would be to make Stacked Borrows not care about reference-to-raw-pointer casts, but it's unclear whether that will ever be possible without going to full Tree Borrows.

Fixes rust-lang/miri#3473.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 17, 2024
Rollup merge of rust-lang#124013 - RalfJung:box-to-raw, r=oli-obk

Box::into_raw: make Miri understand that this is a box-to-raw cast

Turns out rust-lang#122647 went a bit too far in cleaning up `Box`... we still need a hack in `Box::into_raw`. The nicer fix would be to make Stacked Borrows not care about reference-to-raw-pointer casts, but it's unclear whether that will ever be possible without going to full Tree Borrows.

Fixes rust-lang/miri#3473.
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Apr 17, 2024
Box::into_raw: make Miri understand that this is a box-to-raw cast

Turns out rust-lang/rust#122647 went a bit too far in cleaning up `Box`... we still need a hack in `Box::into_raw`. The nicer fix would be to make Stacked Borrows not care about reference-to-raw-pointer casts, but it's unclear whether that will ever be possible without going to full Tree Borrows.

Fixes #3473.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants