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

Tracking Issue for const_refs_to_cell #80384

Closed
1 of 3 tasks
oli-obk opened this issue Dec 26, 2020 · 4 comments · Fixed by rust-lang/reference#1590
Closed
1 of 3 tasks

Tracking Issue for const_refs_to_cell #80384

oli-obk opened this issue Dec 26, 2020 · 4 comments · Fixed by rust-lang/reference#1590
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) B-experimental Blocker: In-tree experiment; RFC pending or unneeded. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Dec 26, 2020

This is a tracking issue for taking references to values of types that (may) contain UnsafeCell within const contexts.
The feature gate for the issue is #![feature(const_refs_to_cell)].

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved questions

  • The feature gate permits const BAR: AtomicUsize = ..; static FOO: &AtomicUsize = &BAR; and only prevents const FOO: &AtomicUsize = &BAR;. This decision was made because the implementation is simpler this way and we would already allow the static FOO case if BAR were a static itself.
    • This could be problematic, because LLVM ends up deduplicating some unnamed allocations like the one in static FOO
  • The soundness of this feature depends on the fact that if a StorageDead exists for a local, then for every assignment to the local, on each pass from that assignment to any return, there will be a StorageDead for this local.
  • Once we get heap allocations in constants, it would be possible for users to write a const fn foo() -> &'static Cell<i32>, which could then be used to initialize a const FOO: &'static Cell<i32> which is unsound (and only caught by dynamic checks). The tracking issue for const heap is Tracking Issue for const_heap #79597

Implementation history

@oli-obk oli-obk added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Dec 26, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 4, 2021
…e, r=RalfJung

Allow references to interior mutable data behind a feature gate

supercedes rust-lang#80373 by simply not checking for interior mutability on borrows of locals that have `StorageDead` and thus can never be leaked to the final value of the constant

tracking issue: rust-lang#80384

r? `@RalfJung`
@kupiakos
Copy link
Contributor

kupiakos commented Mar 16, 2022

Does this need to apply to raw references/ptr::addr_of!? Given only a *const UnsafeCell<T>, there is not yet a stable way to mutate the referent:

#![feature(const_refs_to_cell)]

use core::cell::UnsafeCell;
use core::ptr;

fn main() {
    const _: i32 = {
        let x = UnsafeCell::new(5i32);
        let p: *const UnsafeCell<i32> = ptr::addr_of!(x);

        unsafe {
            // No stable way to mutate `p`

            // Requires const_mut_refs
            // *(p as *mut i32) = 10

            // Requires const_ptr_write
            // (p as *mut i32).write(10)

            // Requires const_intrinsic_copy, which just left FCP
            // ptr::copy(&10, p as *mut i32, 1);

            *(p as *const i32)
        }
    };
}

@RalfJung
Copy link
Member

I would be rather nervous about relying on roundabout arguments like this -- that's the kind of reasoning which quickly leads to accidental stabilization.

For some motivation why we care about this feature, see Gilnaa/memoffset#4 (comment).

@kupiakos
Copy link
Contributor

kupiakos commented Sep 20, 2022

Do you suspect const_mut_ref would stabilize before this?

This limitation would be much less frustrating if it didn't trigger when taking the address of any generic T, even where I have full certainty that the input cannot contains UnsafeCell. If I could bound on Freeze to communicate this impossibility this would solve the problem (and plenty of other soundness bounds I run into). Is there any way to get the bytes out?

I'm trying to read a generic integer's bytes without mutation and am prevented from doing so on the impossible possibility that an interior mutable type is used. Fortunately it can be gotten around for now by passing in a reference to the const fn, but a &u8 arg is a questionable API surface. For context, I'm attempting to create a stable, generic, const-compatible checked integer cast, but this is the last blocker for an ergonomic API.

copybara-service bot pushed a commit to google/crubit that referenced this issue Jul 7, 2023
Implementation of `memoffset::offset_of` (from the `memoffset` crate)
ends up taking a reference to a struct.  In `const` contexts (such as
the context of the assertion) this runs into
rust-lang/rust#80384.  This CL works around
this by opting the generated code into
`#![feature(const_refs_to_cell)]`.

After this CL bindings generated by `cc_bindings_from_rs` will require
a "nightly" version of the Rust compiler.  This is unfortunate, but
this dependency already exists on `rs_bindings_from_cc` side (e.g.
requiring `impl_trait_in_assoc_type` and/or `type_alias_impl_trait`
unstable features).

This CL unblocks implementing `Drop` support.  `Drop` support adds
bindings for additional types, some of which run into this bug.

PiperOrigin-RevId: 546337289
Change-Id: I1684b30a1ac096cc5115aabbe6e5c6504286947c
@fmease fmease added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. A-const-eval Area: Constant evaluation (MIR interpretation) B-experimental Blocker: In-tree experiment; RFC pending or unneeded. B-unstable Blocker: Implemented in the nightly compiler and unstable. S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. and removed S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. labels Mar 1, 2024
@RalfJung
Copy link
Member

Status update: const_mut_refs will hopefully to be stabilized soon. However, I think I'd prefer to wait a bit whether there are any issues showing up there before we also allow references to interior mutable types. In particular, the "safety net" described in #129195 doesn't work as well for shared references due to #121610 and rust-lang/unsafe-code-guidelines#493. Therefore, I'd like to increase our confidence in the static checks before stabilizing something without a solid safety net.

See here for a Zulip discussion about what it would take to make sure that when we are allowing references to interior mutable data, that data is indeed "transient" and does not escape the initializer expression.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 15, 2024
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
rust-timer added a commit to rust-lang-ci/rust that referenced this issue 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-const-eval Area: Constant evaluation (MIR interpretation) B-experimental Blocker: In-tree experiment; RFC pending or unneeded. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants