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

Stabilize &mut (and *mut) as well as &Cell (and *const Cell) in const #129195

Merged
merged 4 commits into from
Sep 15, 2024

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Aug 17, 2024

This stabilizes const_mut_refs and const_refs_to_cell. That allows a bunch of new things in const contexts:

  • Mentioning &mut types
  • Creating &mut and *mut values
  • Creating &T and *const T values where T contains interior mutability
  • Dereferencing &mut and *mut values (both for reads and writes)

The same rules as at runtime apply: mutating immutable data is UB. This includes mutation through pointers derived from shared references; the following is diagnosed with a hard error:

#[allow(invalid_reference_casting)]
const _: () = {
    let mut val = 15;
    let ptr = &val as *const i32 as *mut i32;
    unsafe { *ptr = 16; }
};

The main limitation that is enforced is that the final value of a const (or non-mut static) may not contain &mut values nor interior mutable & values. This is necessary because the memory those references point to becomes read-only when the constant is done computing, so (interior) mutable references to such memory would be pretty dangerous. We take a multi-layered approach here to ensuring no mutable references escape the initializer expression:

  • A static analysis rejects (interior) mutable references when the referee looks like it may outlive the current MIR body.
  • To be extra sure, this static check is complemented by a "safety net" of dynamic checks. ("Dynamic" in the sense of "running during/after const-evaluation, e.g. at runtime of this code" -- in contrast to "static" which works entirely by looking at the MIR without evaluating it.)
    • After the final value is computed, we do a type-driven traversal of the entire value, and if we find any &mut or interior-mutable & we error out.
    • However, the type-driven traversal cannot traverse union or raw pointers, so there is a second dynamic check where if the final value of the const contains any pointer that was not derived from a shared reference, we complain. This is currently a future-compat lint, but will become an ICE in const-eval interning: accept interior mutable pointers in final value #128543. On the off-chance that it's actually possible to trigger this lint on stable, I'd prefer if we could make it an ICE before stabilizing const_mut_refs, but it's not a hard blocker. This part of the "safety net" is only active for mutable references since with shared references, it has false positives.

Altogether this should prevent people from leaking (interior) mutable references out of the const initializer.

While updating the tests I learned that surprisingly, this code gets rejected:

const _: Vec<i32> = {
    let mut x = Vec::<i32>::new(); //~ ERROR destructor of `Vec<i32>` cannot be evaluated at compile-time
    let r = &mut x;
    let y = x;
    y
};

The analysis that rejects destructors in const is very conservative when it sees an &mut being created to x, and then considers x to be always live. See here for a longer explanation. const_precise_live_drops will solve this, so I consider this problem to be tracked by #73255.

Cc @rust-lang/wg-const-eval @rust-lang/lang
Cc #57349
Cc #80384

@rustbot
Copy link
Collaborator

rustbot commented Aug 17, 2024

r? @estebank

rustbot has assigned @estebank.
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-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 Aug 17, 2024
@rustbot
Copy link
Collaborator

rustbot commented Aug 17, 2024

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@RalfJung RalfJung changed the title Stabilize &mut (and *mut) in const Stabilize &mut (and *mut) in const Aug 17, 2024
@RalfJung RalfJung added T-lang Relevant to the language team, which will review and decide on the PR/issue. I-lang-nominated Nominated for discussion during a lang team meeting. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 17, 2024
@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Aug 17, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 17, 2024
…inter, r=<try>

make writes_through_immutable_pointer a hard error

This turns the lint added in rust-lang#118324 into a hard error. This has been reported in cargo's future-compat reports since Rust 1.76 (released in February). Given that const_mut_refs is still unstable, it should be impossible to even hit this error on stable: we did accidentally stabilize some functions that can cause this error, but that got reverted in rust-lang#117905. Still, let's do a crater run just to be sure.

Given that this should only affect unstable code, I don't think it needs an FCP, but let's Cc `@rust-lang/lang` anyway -- any objection to making this unambiguous UB into a hard error during const-eval? This can be viewed as part of rust-lang#129195 which is already nominated for discussion.
@RalfJung RalfJung removed the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Aug 17, 2024
@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Aug 17, 2024
@rust-log-analyzer

This comment has been minimized.

@RalfJung RalfJung added relnotes Marks issues that should be documented in the release notes of the next release. to-announce Announce this issue on triage meeting labels Aug 17, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Sep 7, 2024

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

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 7, 2024
@RalfJung
Copy link
Member Author

RalfJung commented Sep 8, 2024

If tidy would let me keep the feature gates in the order they were in, that would have avoided around 50% of the conflicts in this PR...

@RalfJung RalfJung removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 12, 2024
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Sep 14, 2024
@rfcbot
Copy link

rfcbot commented Sep 14, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@bors
Copy link
Contributor

bors commented Sep 14, 2024

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

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 14, 2024
@RalfJung
Copy link
Member Author

@bors r=fee1-dead

@bors
Copy link
Contributor

bors commented Sep 15, 2024

📌 Commit 49316f8 has been approved by fee1-dead

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 15, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 15, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#129195 (Stabilize `&mut` (and `*mut`) as well as `&Cell` (and `*const Cell`) in const)
 - rust-lang#130118 (move Option::unwrap_unchecked into const_option feature gate)
 - rust-lang#130295 (Fix target-cpu fpu features on Armv8-R.)
 - rust-lang#130371 (Correctly account for niche-optimized tags in rustc_transmute)
 - rust-lang#130381 (library: Compute Rust exception class from its string repr)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 011289c into rust-lang:master Sep 15, 2024
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 15, 2024
Rollup merge of rust-lang#129195 - RalfJung:const-mut-refs, r=fee1-dead

Stabilize `&mut` (and `*mut`) as well as `&Cell` (and `*const Cell`) in const

This stabilizes `const_mut_refs` and `const_refs_to_cell`. That allows a bunch of new things in const contexts:
- Mentioning `&mut` types
- Creating `&mut` and `*mut` values
- Creating `&T` and `*const T` values where `T` contains interior mutability
- Dereferencing `&mut` and `*mut` values (both for reads and writes)

The same rules as at runtime apply: mutating immutable data is UB. This includes mutation through pointers derived from shared references; the following is diagnosed with a hard error:
```rust
#[allow(invalid_reference_casting)]
const _: () = {
    let mut val = 15;
    let ptr = &val as *const i32 as *mut i32;
    unsafe { *ptr = 16; }
};
```

The main limitation that is enforced is that the final value of a const (or non-`mut` static) may not contain `&mut` values nor interior mutable `&` values. This is necessary because the memory those references point to becomes *read-only* when the constant is done computing, so (interior) mutable references to such memory would be pretty dangerous. We take a multi-layered approach here to ensuring no mutable references escape the initializer expression:
- A static analysis rejects (interior) mutable references when the referee looks like it may outlive the current MIR body.
- To be extra sure, this static check is complemented by a "safety net" of dynamic checks. ("Dynamic" in the sense of "running during/after const-evaluation, e.g. at runtime of this code" -- in contrast to "static" which works entirely by looking at the MIR without evaluating it.)
  - After the final value is computed, we do a type-driven traversal of the entire value, and if we find any `&mut` or interior-mutable `&` we error out.
  - However, the type-driven traversal cannot traverse `union` or raw pointers, so there is a second dynamic check where if the final value of the const contains any pointer that was not derived from a shared reference, we complain. This is currently a future-compat lint, but will become an ICE in rust-lang#128543. On the off-chance that it's actually possible to trigger this lint on stable, I'd prefer if we could make it an ICE before stabilizing const_mut_refs, but it's not a hard blocker. This part of the "safety net" is only active for mutable references since with shared references, it has false positives.

Altogether this should prevent people from leaking (interior) mutable references out of the const initializer.

While updating the tests I learned that surprisingly, this code gets rejected:
```rust
const _: Vec<i32> = {
    let mut x = Vec::<i32>::new(); //~ ERROR destructor of `Vec<i32>` cannot be evaluated at compile-time
    let r = &mut x;
    let y = x;
    y
};
```
The analysis that rejects destructors in `const` is very conservative when it sees an `&mut` being created to `x`, and then considers `x` to be always live. See [here](rust-lang#65394 (comment)) for a longer explanation. `const_precise_live_drops` will solve this, so I consider this problem to be tracked by rust-lang#73255.

Cc `@rust-lang/wg-const-eval` `@rust-lang/lang`
Cc rust-lang#57349
Cc rust-lang#80384
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-lang Relevant to the language team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.