const interning: decide about mutability purely based on the kind of interning, not the types we see by RalfJung · Pull Request #116745 · rust-lang/rust
rustbot
added
S-waiting-on-review
labels
Oct 14, 2023
RalfJung
changed the title
Draft: const interning: decide about mutability purely based on the kind of interning, not the types we see
const interning: decide about mutability purely based on the kind of interning, not the types we see
bors added a commit to rust-lang-ci/rust that referenced this pull request
Oct 15, 2023const interning: decide about mutability purely based on the kind of interning, not the types we see r? `@oli-obk` this is what I meant on Zulip. For now I left the type visitor in the code; removing it and switching to a simple interning loop will mean we accept code that we currently reject, such as this ```rust const CONST_RAW: *const Vec<i32> = &Vec::new() as *const _; ``` I see no reason for us to reject such code, but accepting it should go through t-lang FCP, so I want to do that in a follow-up PR. This PR does change behavior in the following situations: 1. Shared references inside `static mut` are no longer put in read-only memory. This affects for instance `static mut FOO: &i32 = &0;`. We never *promised* that this would be read-only, and `static mut` is [an anti-pattern anyway](rust-lang#53639), so I think this is fine. If you want read-only memory, write this as `static INNER: i32 = 0; static mut FOO: &i32 = &INNER;`. 2. Potentially, mutable things in a `static` are now marked read-only. That would be a problem. But I am not sure if that can happen? The code mentions `static FOO: *const AtomicUsize = &AtomicUsize::new(42)`, but that is rejected for being non-`Sync`. [This variant](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=112e930ae1b3ef285812ab404ca296fa) also gets rejected, and same for [this one](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=0dac8d173a2b3099b9c2854fdad7a87c). I think we should reject all cases where a `static` introduces mutable state, except for the outermost allocation itself which can have interior mutability (and which is the one allocation where we have fully reliable type information). What I still want to do in this PR before it is ready for review it is ensure we detect situations where `&mut` or `&UnsafeCell` points to immutable allocations. That should detect if we have any instance of case (2). That check should be part of the regular type validity check though, not part of interning.
bors added a commit to rust-lang-ci/rust that referenced this pull request
Dec 17, 2023const interning: decide about mutability purely based on the kind of interning, not the types we see r? `@oli-obk` this is what I meant on Zulip. For now I left the type visitor in the code; removing it and switching to a simple interning loop will mean we accept code that we currently reject, such as this ```rust const CONST_RAW: *const Vec<i32> = &Vec::new() as *const _; ``` I see no reason for us to reject such code, but accepting it should go through t-lang FCP, so I want to do that in a follow-up PR. This PR does change behavior in the following situations: 1. Shared references inside `static mut` are no longer put in read-only memory. This affects for instance `static mut FOO: &i32 = &0;`. We never *promised* that this would be read-only, and `static mut` is [an anti-pattern anyway](rust-lang#53639), so I think this is fine. If you want read-only memory, write this as `static INNER: i32 = 0; static mut FOO: &i32 = &INNER;`. 2. Potentially, mutable things in a `static` are now marked read-only. That would be a problem. But I am not sure if that can happen? The code mentions `static FOO: *const AtomicUsize = &AtomicUsize::new(42)`, but that is rejected for being non-`Sync`. [This variant](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=112e930ae1b3ef285812ab404ca296fa) also gets rejected, and same for [this one](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=0dac8d173a2b3099b9c2854fdad7a87c). I think we should reject all cases where a `static` introduces mutable state, except for the outermost allocation itself which can have interior mutability (and which is the one allocation where we have fully reliable type information). What I still want to do in this PR before it is ready for review it is ensure we detect situations where `&mut` or `&UnsafeCell` points to immutable allocations. That should detect if we have any instance of case (2). That check should be part of the regular type validity check though, not part of interning.
RalfJung
removed
the
S-blocked
label
Dec 17, 2023GuillaumeGomez pushed a commit to GuillaumeGomez/rustc_codegen_gcc that referenced this pull request
Feb 15, 2024compile-time evaluation: detect writes through immutable pointers This has two motivations: - it unblocks rust-lang/rust#116745 (and therefore takes a big step towards `const_mut_refs` stabilization), because we can now detect if the memory that we find in `const` can be interned as "immutable" - it would detect the UB that was uncovered in rust-lang/rust#117905, which was caused by accidental stabilization of `copy` functions in `const` that can only be called with UB When UB is detected, we emit a future-compat warn-by-default lint. This is not a breaking change, so completely in line with [the const-UB RFC](https://rust-lang.github.io/rfcs/3016-const-ub.html), meaning we don't need t-lang FCP here. I made the lint immediately show up for dependencies since it is nearly impossible to even trigger this lint without `const_mut_refs` -- the accidentally stabilized `copy` functions are the only way this can happen, so the crates that popped up in #117905 are the only causes of such UB (in the code that crater covers), and the three cases of UB that we know about have all been fixed in their respective crates already. The way this is implemented is by making use of the fact that our interpreter is already generic over the notion of provenance. For CTFE we now use the new `CtfeProvenance` type which is conceptually an `AllocId` plus a boolean `immutable` flag (but packed for a more efficient representation). This means we can mark a pointer as immutable when it is created as a shared reference. The flag will be propagated to all pointers derived from this one. We can then check the immutable flag on each write to reject writes through immutable pointers. I just hope perf works out.
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request
Apr 7, 2024compile-time evaluation: detect writes through immutable pointers This has two motivations: - it unblocks rust-lang/rust#116745 (and therefore takes a big step towards `const_mut_refs` stabilization), because we can now detect if the memory that we find in `const` can be interned as "immutable" - it would detect the UB that was uncovered in rust-lang/rust#117905, which was caused by accidental stabilization of `copy` functions in `const` that can only be called with UB When UB is detected, we emit a future-compat warn-by-default lint. This is not a breaking change, so completely in line with [the const-UB RFC](https://rust-lang.github.io/rfcs/3016-const-ub.html), meaning we don't need t-lang FCP here. I made the lint immediately show up for dependencies since it is nearly impossible to even trigger this lint without `const_mut_refs` -- the accidentally stabilized `copy` functions are the only way this can happen, so the crates that popped up in #117905 are the only causes of such UB (in the code that crater covers), and the three cases of UB that we know about have all been fixed in their respective crates already. The way this is implemented is by making use of the fact that our interpreter is already generic over the notion of provenance. For CTFE we now use the new `CtfeProvenance` type which is conceptually an `AllocId` plus a boolean `immutable` flag (but packed for a more efficient representation). This means we can mark a pointer as immutable when it is created as a shared reference. The flag will be propagated to all pointers derived from this one. We can then check the immutable flag on each write to reject writes through immutable pointers. I just hope perf works out.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request
Apr 27, 2024compile-time evaluation: detect writes through immutable pointers This has two motivations: - it unblocks rust-lang/rust#116745 (and therefore takes a big step towards `const_mut_refs` stabilization), because we can now detect if the memory that we find in `const` can be interned as "immutable" - it would detect the UB that was uncovered in rust-lang/rust#117905, which was caused by accidental stabilization of `copy` functions in `const` that can only be called with UB When UB is detected, we emit a future-compat warn-by-default lint. This is not a breaking change, so completely in line with [the const-UB RFC](https://rust-lang.github.io/rfcs/3016-const-ub.html), meaning we don't need t-lang FCP here. I made the lint immediately show up for dependencies since it is nearly impossible to even trigger this lint without `const_mut_refs` -- the accidentally stabilized `copy` functions are the only way this can happen, so the crates that popped up in #117905 are the only causes of such UB (in the code that crater covers), and the three cases of UB that we know about have all been fixed in their respective crates already. The way this is implemented is by making use of the fact that our interpreter is already generic over the notion of provenance. For CTFE we now use the new `CtfeProvenance` type which is conceptually an `AllocId` plus a boolean `immutable` flag (but packed for a more efficient representation). This means we can mark a pointer as immutable when it is created as a shared reference. The flag will be propagated to all pointers derived from this one. We can then check the immutable flag on each write to reject writes through immutable pointers. I just hope perf works out.
christian-schilling pushed a commit to christian-schilling/rustc_codegen_cranelift that referenced this pull request
Jan 27, 2026compile-time evaluation: detect writes through immutable pointers This has two motivations: - it unblocks rust-lang/rust#116745 (and therefore takes a big step towards `const_mut_refs` stabilization), because we can now detect if the memory that we find in `const` can be interned as "immutable" - it would detect the UB that was uncovered in rust-lang/rust#117905, which was caused by accidental stabilization of `copy` functions in `const` that can only be called with UB When UB is detected, we emit a future-compat warn-by-default lint. This is not a breaking change, so completely in line with [the const-UB RFC](https://rust-lang.github.io/rfcs/3016-const-ub.html), meaning we don't need t-lang FCP here. I made the lint immediately show up for dependencies since it is nearly impossible to even trigger this lint without `const_mut_refs` -- the accidentally stabilized `copy` functions are the only way this can happen, so the crates that popped up in #117905 are the only causes of such UB (in the code that crater covers), and the three cases of UB that we know about have all been fixed in their respective crates already. The way this is implemented is by making use of the fact that our interpreter is already generic over the notion of provenance. For CTFE we now use the new `CtfeProvenance` type which is conceptually an `AllocId` plus a boolean `immutable` flag (but packed for a more efficient representation). This means we can mark a pointer as immutable when it is created as a shared reference. The flag will be propagated to all pointers derived from this one. We can then check the immutable flag on each write to reject writes through immutable pointers. I just hope perf works out.
christian-schilling pushed a commit to christian-schilling/rustc_codegen_cranelift that referenced this pull request
Jan 27, 2026compile-time evaluation: detect writes through immutable pointers This has two motivations: - it unblocks rust-lang/rust#116745 (and therefore takes a big step towards `const_mut_refs` stabilization), because we can now detect if the memory that we find in `const` can be interned as "immutable" - it would detect the UB that was uncovered in rust-lang/rust#117905, which was caused by accidental stabilization of `copy` functions in `const` that can only be called with UB When UB is detected, we emit a future-compat warn-by-default lint. This is not a breaking change, so completely in line with [the const-UB RFC](https://rust-lang.github.io/rfcs/3016-const-ub.html), meaning we don't need t-lang FCP here. I made the lint immediately show up for dependencies since it is nearly impossible to even trigger this lint without `const_mut_refs` -- the accidentally stabilized `copy` functions are the only way this can happen, so the crates that popped up in #117905 are the only causes of such UB (in the code that crater covers), and the three cases of UB that we know about have all been fixed in their respective crates already. The way this is implemented is by making use of the fact that our interpreter is already generic over the notion of provenance. For CTFE we now use the new `CtfeProvenance` type which is conceptually an `AllocId` plus a boolean `immutable` flag (but packed for a more efficient representation). This means we can mark a pointer as immutable when it is created as a shared reference. The flag will be propagated to all pointers derived from this one. We can then check the immutable flag on each write to reject writes through immutable pointers. I just hope perf works out.
Kobzol pushed a commit to Kobzol/portable-simd that referenced this pull request
Feb 3, 2026compile-time evaluation: detect writes through immutable pointers This has two motivations: - it unblocks rust-lang/rust#116745 (and therefore takes a big step towards `const_mut_refs` stabilization), because we can now detect if the memory that we find in `const` can be interned as "immutable" - it would detect the UB that was uncovered in rust-lang/rust#117905, which was caused by accidental stabilization of `copy` functions in `const` that can only be called with UB When UB is detected, we emit a future-compat warn-by-default lint. This is not a breaking change, so completely in line with [the const-UB RFC](https://rust-lang.github.io/rfcs/3016-const-ub.html), meaning we don't need t-lang FCP here. I made the lint immediately show up for dependencies since it is nearly impossible to even trigger this lint without `const_mut_refs` -- the accidentally stabilized `copy` functions are the only way this can happen, so the crates that popped up in #117905 are the only causes of such UB (in the code that crater covers), and the three cases of UB that we know about have all been fixed in their respective crates already. The way this is implemented is by making use of the fact that our interpreter is already generic over the notion of provenance. For CTFE we now use the new `CtfeProvenance` type which is conceptually an `AllocId` plus a boolean `immutable` flag (but packed for a more efficient representation). This means we can mark a pointer as immutable when it is created as a shared reference. The flag will be propagated to all pointers derived from this one. We can then check the immutable flag on each write to reject writes through immutable pointers. I just hope perf works out.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters