const_mut_refs: allow mutable refs to statics

This commit is contained in:
Ralf Jung 2024-02-11 14:27:08 +01:00
parent 4316d0c625
commit 340f8aac7e
13 changed files with 173 additions and 62 deletions

View File

@ -344,7 +344,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
visitor.visit_ty(ty);
}
fn check_mut_borrow(&mut self, local: Local, kind: hir::BorrowKind) {
fn check_mut_borrow(&mut self, place: &Place<'_>, kind: hir::BorrowKind) {
match self.const_kind() {
// In a const fn all borrows are transient or point to the places given via
// references in the arguments (so we already checked them with
@ -355,10 +355,19 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
// to mutable memory.
hir::ConstContext::ConstFn => self.check_op(ops::TransientMutBorrow(kind)),
_ => {
// For indirect places, we are not creating a new permanent borrow, it's just as
// transient as the already existing one. For reborrowing references this is handled
// at the top of `visit_rvalue`, but for raw pointers we handle it here.
// Pointers/references to `static mut` and cases where the `*` is not the first
// projection also end up here.
// Locals with StorageDead do not live beyond the evaluation and can
// thus safely be borrowed without being able to be leaked to the final
// value of the constant.
if self.local_has_storage_dead(local) {
// Note: This is only sound if every local that has a `StorageDead` has a
// `StorageDead` in every control flow path leading to a `return` terminator.
// The good news is that interning will detect if any unexpected mutable
// pointer slips through.
if place.is_indirect() || self.local_has_storage_dead(place.local) {
self.check_op(ops::TransientMutBorrow(kind));
} else {
self.check_op(ops::MutBorrow(kind));
@ -390,6 +399,11 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
trace!("visit_rvalue: rvalue={:?} location={:?}", rvalue, location);
// Special-case reborrows to be more like a copy of a reference.
// FIXME: this does not actually handle all reborrows. It only detects cases where `*` is the outermost
// projection of the borrowed place, it skips deref'ing raw pointers and it skips `static`.
// All those cases are handled below with shared/mutable borrows.
// Once `const_mut_refs` is stable, we should be able to entirely remove this special case.
// (`const_refs_to_cell` is not needed, we already allow all borrows of indirect places anyway.)
match *rvalue {
Rvalue::Ref(_, kind, place) => {
if let Some(reborrowed_place_ref) = place_as_reborrow(self.tcx, self.body, place) {
@ -460,7 +474,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
if !is_allowed {
self.check_mut_borrow(
place.local,
place,
if matches!(rvalue, Rvalue::Ref(..)) {
hir::BorrowKind::Ref
} else {
@ -478,7 +492,14 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
place.as_ref(),
);
if borrowed_place_has_mut_interior {
// If the place is indirect, this is basically a reborrow. We have a reborrow
// special case above, but for raw pointers and pointers/references to `static` and
// when the `*` is not the first projection, `place_as_reborrow` does not recognize
// them as such, so we end up here. This should probably be considered a
// `TransientCellBorrow` (we consider the equivalent mutable case a
// `TransientMutBorrow`), but such reborrows got accidentally stabilized already and
// it is too much of a breaking change to take back.
if borrowed_place_has_mut_interior && !place.is_indirect() {
match self.const_kind() {
// In a const fn all borrows are transient or point to the places given via
// references in the arguments (so we already checked them with
@ -495,6 +516,8 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
// final value.
// Note: This is only sound if every local that has a `StorageDead` has a
// `StorageDead` in every control flow path leading to a `return` terminator.
// The good news is that interning will detect if any unexpected mutable
// pointer slips through.
if self.local_has_storage_dead(place.local) {
self.check_op(ops::TransientCellBorrow);
} else {
@ -948,6 +971,12 @@ fn place_as_reborrow<'tcx>(
) -> Option<PlaceRef<'tcx>> {
match place.as_ref().last_projection() {
Some((place_base, ProjectionElem::Deref)) => {
// FIXME: why do statics and raw pointers get excluded here? This makes
// some code involving mutable pointers unstable, but it is unclear
// why that code is treated differently from mutable references.
// Once TransientMutBorrow and TransientCellBorrow are stable,
// this can probably be cleaned up without any behavioral changes.
// A borrow of a `static` also looks like `&(*_1)` in the MIR, but `_1` is a `const`
// that points to the allocation for the static. Don't treat these as reborrows.
if body.local_decls[place_base.local].is_ref_to_static() {

View File

@ -74,6 +74,13 @@ pub trait Qualif {
adt: AdtDef<'tcx>,
args: GenericArgsRef<'tcx>,
) -> bool;
/// Returns `true` if this `Qualif` behaves sructurally for pointers and references:
/// the pointer/reference qualifies if and only if the pointee qualifies.
///
/// (This is currently `false` for all our instances, but that may change in the future. Also,
/// by keeping it abstract, the handling of `Deref` in `in_place` becomes more clear.)
fn deref_structural<'tcx>(cx: &ConstCx<'_, 'tcx>) -> bool;
}
/// Constant containing interior mutability (`UnsafeCell<T>`).
@ -103,6 +110,10 @@ impl Qualif for HasMutInterior {
// It arises structurally for all other types.
adt.is_unsafe_cell()
}
fn deref_structural<'tcx>(_cx: &ConstCx<'_, 'tcx>) -> bool {
false
}
}
/// Constant containing an ADT that implements `Drop`.
@ -131,6 +142,10 @@ impl Qualif for NeedsDrop {
) -> bool {
adt.has_dtor(cx.tcx)
}
fn deref_structural<'tcx>(_cx: &ConstCx<'_, 'tcx>) -> bool {
false
}
}
/// Constant containing an ADT that implements non-const `Drop`.
@ -210,6 +225,10 @@ impl Qualif for NeedsNonConstDrop {
) -> bool {
adt.has_non_const_dtor(cx.tcx)
}
fn deref_structural<'tcx>(_cx: &ConstCx<'_, 'tcx>) -> bool {
false
}
}
// FIXME: Use `mir::visit::Visitor` for the `in_*` functions if/when it supports early return.
@ -303,6 +322,11 @@ where
return false;
}
if matches!(elem, ProjectionElem::Deref) && !Q::deref_structural(cx) {
// We have to assume that this qualifies.
return true;
}
place = place_base;
}

View File

@ -0,0 +1,43 @@
//@check-pass
//! This is the reduced version of the "Linux kernel vtable" use-case.
#![feature(const_mut_refs, const_refs_to_static)]
use std::ptr::addr_of_mut;
#[repr(C)]
struct ThisModule(i32);
trait Module {
const THIS_MODULE_PTR: *mut ThisModule;
}
struct MyModule;
// Generated by a macro.
extern "C" {
static mut THIS_MODULE: ThisModule;
}
// Generated by a macro.
impl Module for MyModule {
const THIS_MODULE_PTR: *mut ThisModule = unsafe { addr_of_mut!(THIS_MODULE) };
}
struct Vtable {
module: *mut ThisModule,
foo_fn: fn(*mut ()) -> i32,
}
trait Foo {
type Mod: Module;
fn foo(&mut self) -> i32;
}
fn generate_vtable<T: Foo>() -> &'static Vtable {
&Vtable {
module: T::Mod::THIS_MODULE_PTR,
foo_fn: |ptr| unsafe { &mut *ptr.cast::<T>() }.foo(),
}
}
fn main() {}

View File

@ -6,5 +6,6 @@ const C1: &'static mut [usize] = &mut [];
static mut S: usize = 3;
const C2: &'static mut usize = unsafe { &mut S };
//~^ ERROR: referencing statics in constants
//~| ERROR: mutable references are not allowed
fn main() {}

View File

@ -16,7 +16,17 @@ LL | const C2: &'static mut usize = unsafe { &mut S };
= note: `static` and `const` variables can refer to other `const` variables. A `const` variable, however, cannot refer to a `static` variable.
= help: to fix this, the value can be extracted to a `const` and then used.
error: aborting due to 2 previous errors
error[E0658]: mutable references are not allowed in constants
--> $DIR/issue-17718-const-bad-values.rs:7:41
|
LL | const C2: &'static mut usize = unsafe { &mut S };
| ^^^^^^
|
= note: see issue #57349 <https://github.com/rust-lang/rust/issues/57349> for more information
= help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
error: aborting due to 3 previous errors
Some errors have detailed explanations: E0658, E0764.
For more information about an error, try `rustc --explain E0658`.

View File

@ -16,7 +16,7 @@ help: skipping check for `const_mut_refs` feature
|
LL | &mut *(Box::new(0))
| ^^^^^^^^^^^^^^^^^^^
help: skipping check that does not even have a feature gate
help: skipping check for `const_mut_refs` feature
--> $DIR/box.rs:8:5
|
LL | &mut *(Box::new(0))

View File

@ -114,7 +114,7 @@ help: skipping check for `const_refs_to_static` feature
|
LL | const SUBTLE: &mut i32 = unsafe { &mut FOO };
| ^^^
help: skipping check that does not even have a feature gate
help: skipping check for `const_mut_refs` feature
--> $DIR/mutable_references_err.rs:32:35
|
LL | const SUBTLE: &mut i32 = unsafe { &mut FOO };

View File

@ -114,7 +114,7 @@ help: skipping check for `const_refs_to_static` feature
|
LL | const SUBTLE: &mut i32 = unsafe { &mut FOO };
| ^^^
help: skipping check that does not even have a feature gate
help: skipping check for `const_mut_refs` feature
--> $DIR/mutable_references_err.rs:32:35
|
LL | const SUBTLE: &mut i32 = unsafe { &mut FOO };

View File

@ -0,0 +1,40 @@
//@run-pass
#![feature(const_mut_refs)]
#![feature(sync_unsafe_cell)]
use std::cell::SyncUnsafeCell;
use std::ptr;
#[repr(C)]
struct SyncPtr {
foo: *mut u32,
}
unsafe impl Sync for SyncPtr {}
static mut STATIC: u32 = 42;
static INTERIOR_MUTABLE_STATIC: SyncUnsafeCell<u32> = SyncUnsafeCell::new(42);
// A static that mutably points to STATIC.
static PTR: SyncPtr = SyncPtr {
foo: unsafe { ptr::addr_of_mut!(STATIC) },
};
static INTERIOR_MUTABLE_PTR: SyncPtr = SyncPtr {
foo: ptr::addr_of!(INTERIOR_MUTABLE_STATIC) as *mut u32,
};
fn main() {
let ptr = PTR.foo;
unsafe {
assert_eq!(*ptr, 42);
*ptr = 0;
assert_eq!(*PTR.foo, 0);
}
let ptr = INTERIOR_MUTABLE_PTR.foo;
unsafe {
assert_eq!(*ptr, 42);
*ptr = 0;
assert_eq!(*INTERIOR_MUTABLE_PTR.foo, 0);
}
}

View File

@ -1,3 +1,5 @@
#![feature(const_mut_refs)]
static X: i32 = 1;
const C: i32 = 2;
static mut M: i32 = 3;
@ -5,14 +7,12 @@ static mut M: i32 = 3;
const CR: &'static mut i32 = &mut C; //~ ERROR mutable references are not allowed
//~| WARN taking a mutable
static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR E0658
//~| ERROR cannot borrow
//~| ERROR mutable references are not allowed
static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR cannot borrow immutable static item `X` as mutable
static CONST_REF: &'static mut i32 = &mut C; //~ ERROR mutable references are not allowed
//~| WARN taking a mutable
static STATIC_MUT_REF: &'static mut i32 = unsafe { &mut M }; //~ ERROR mutable references are not
static STATIC_MUT_REF: &'static mut i32 = unsafe { &mut M };
//~^ WARN mutable reference of mutable static is discouraged [static_mut_ref]
fn main() {}

View File

@ -14,7 +14,7 @@ LL | static STATIC_MUT_REF: &'static mut i32 = unsafe { addr_of_mut!(M) };
| ~~~~~~~~~~~~~~~
warning: taking a mutable reference to a `const` item
--> $DIR/E0017.rs:5:30
--> $DIR/E0017.rs:7:30
|
LL | const CR: &'static mut i32 = &mut C;
| ^^^^^^
@ -22,36 +22,20 @@ LL | const CR: &'static mut i32 = &mut C;
= note: each usage of a `const` item creates a new temporary
= note: the mutable reference will refer to this temporary, not the original `const` item
note: `const` item defined here
--> $DIR/E0017.rs:2:1
--> $DIR/E0017.rs:4:1
|
LL | const C: i32 = 2;
| ^^^^^^^^^^^^
= note: `#[warn(const_item_mutation)]` on by default
error[E0764]: mutable references are not allowed in the final value of constants
--> $DIR/E0017.rs:5:30
--> $DIR/E0017.rs:7:30
|
LL | const CR: &'static mut i32 = &mut C;
| ^^^^^^
error[E0658]: mutation through a reference is not allowed in statics
--> $DIR/E0017.rs:8:39
|
LL | static STATIC_REF: &'static mut i32 = &mut X;
| ^^^^^^
|
= note: see issue #57349 <https://github.com/rust-lang/rust/issues/57349> for more information
= help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
error[E0764]: mutable references are not allowed in the final value of statics
--> $DIR/E0017.rs:8:39
|
LL | static STATIC_REF: &'static mut i32 = &mut X;
| ^^^^^^
error[E0596]: cannot borrow immutable static item `X` as mutable
--> $DIR/E0017.rs:8:39
--> $DIR/E0017.rs:10:39
|
LL | static STATIC_REF: &'static mut i32 = &mut X;
| ^^^^^^ cannot borrow as mutable
@ -65,7 +49,7 @@ LL | static CONST_REF: &'static mut i32 = &mut C;
= note: each usage of a `const` item creates a new temporary
= note: the mutable reference will refer to this temporary, not the original `const` item
note: `const` item defined here
--> $DIR/E0017.rs:2:1
--> $DIR/E0017.rs:4:1
|
LL | const C: i32 = 2;
| ^^^^^^^^^^^^
@ -76,13 +60,7 @@ error[E0764]: mutable references are not allowed in the final value of statics
LL | static CONST_REF: &'static mut i32 = &mut C;
| ^^^^^^
error[E0764]: mutable references are not allowed in the final value of statics
--> $DIR/E0017.rs:15:52
|
LL | static STATIC_MUT_REF: &'static mut i32 = unsafe { &mut M };
| ^^^^^^
error: aborting due to 3 previous errors; 3 warnings emitted
error: aborting due to 6 previous errors; 3 warnings emitted
Some errors have detailed explanations: E0596, E0658, E0764.
Some errors have detailed explanations: E0596, E0764.
For more information about an error, try `rustc --explain E0596`.

View File

@ -3,9 +3,7 @@ const C: i32 = 2;
const CR: &'static mut i32 = &mut C; //~ ERROR mutable references are not allowed
//~| WARN taking a mutable
static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR cannot borrow
//~| ERROR E0658
//~| ERROR mutable references are not allowed
static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR E0658
static CONST_REF: &'static mut i32 = &mut C; //~ ERROR mutable references are not allowed
//~| WARN taking a mutable

View File

@ -19,7 +19,7 @@ error[E0764]: mutable references are not allowed in the final value of constants
LL | const CR: &'static mut i32 = &mut C;
| ^^^^^^
error[E0658]: mutation through a reference is not allowed in statics
error[E0658]: mutable references are not allowed in statics
--> $DIR/E0388.rs:6:39
|
LL | static STATIC_REF: &'static mut i32 = &mut X;
@ -29,20 +29,8 @@ LL | static STATIC_REF: &'static mut i32 = &mut X;
= help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
error[E0764]: mutable references are not allowed in the final value of statics
--> $DIR/E0388.rs:6:39
|
LL | static STATIC_REF: &'static mut i32 = &mut X;
| ^^^^^^
error[E0596]: cannot borrow immutable static item `X` as mutable
--> $DIR/E0388.rs:6:39
|
LL | static STATIC_REF: &'static mut i32 = &mut X;
| ^^^^^^ cannot borrow as mutable
warning: taking a mutable reference to a `const` item
--> $DIR/E0388.rs:10:38
--> $DIR/E0388.rs:8:38
|
LL | static CONST_REF: &'static mut i32 = &mut C;
| ^^^^^^
@ -56,12 +44,12 @@ LL | const C: i32 = 2;
| ^^^^^^^^^^^^
error[E0764]: mutable references are not allowed in the final value of statics
--> $DIR/E0388.rs:10:38
--> $DIR/E0388.rs:8:38
|
LL | static CONST_REF: &'static mut i32 = &mut C;
| ^^^^^^
error: aborting due to 5 previous errors; 2 warnings emitted
error: aborting due to 3 previous errors; 2 warnings emitted
Some errors have detailed explanations: E0596, E0658, E0764.
For more information about an error, try `rustc --explain E0596`.
Some errors have detailed explanations: E0658, E0764.
For more information about an error, try `rustc --explain E0658`.