const_mut_refs: allow mutable refs to statics

RalfJung · RalfJung · commit 81f35683af41 · 2024-02-11T15:56:54.000+01:00
diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs
@@ -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));
@@ -460,7 +469,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 +487,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 +511,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 {
@@ -946,6 +964,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() {
diff --git a/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs b/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs
@@ -74,6 +74,10 @@ 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.
+    fn deref_structural<'tcx>(cx: &ConstCx<'_, 'tcx>) -> bool;
 }
 
 /// Constant containing interior mutability (`UnsafeCell<T>`).
@@ -103,6 +107,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 +139,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 +222,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 +319,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;
     }
 
diff --git a/tests/ui/consts/const-ref-to-static-linux-vtable.rs b/tests/ui/consts/const-ref-to-static-linux-vtable.rs
@@ -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() {}
diff --git a/tests/ui/consts/issue-17718-const-bad-values.rs b/tests/ui/consts/issue-17718-const-bad-values.rs
@@ -5,6 +5,7 @@ static mut S: usize = 3;
 const C2: &'static mut usize = unsafe { &mut S };
 //~^ ERROR: referencing statics in constants
 //~| ERROR: referencing statics in constants
+//~| ERROR: mutable references are not allowed
 //~| WARN mutable reference of mutable static is discouraged [static_mut_ref]
 
 fn main() {}
diff --git a/tests/ui/consts/issue-17718-const-bad-values.stderr b/tests/ui/consts/issue-17718-const-bad-values.stderr
@@ -44,7 +44,17 @@ LL | const C2: &'static mut usize = unsafe { &mut S };
    = help: to fix this, the value can be extracted to a `const` and then used.
    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
 
-error: aborting due to 3 previous errors; 1 warning emitted
+error[E0658]: mutable references are not allowed in constants
+  --> $DIR/issue-17718-const-bad-values.rs:5: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 4 previous errors; 1 warning emitted
 
 Some errors have detailed explanations: E0658, E0764.
 For more information about an error, try `rustc --explain E0658`.
diff --git a/tests/ui/consts/miri_unleashed/box.stderr b/tests/ui/consts/miri_unleashed/box.stderr
@@ -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))
diff --git a/tests/ui/consts/miri_unleashed/mutable_references_err.64bit.stderr b/tests/ui/consts/miri_unleashed/mutable_references_err.64bit.stderr
@@ -119,7 +119,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 };
diff --git a/tests/ui/consts/mut-ptr-to-static.rs b/tests/ui/consts/mut-ptr-to-static.rs
@@ -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);
+    }
+}
diff --git a/tests/ui/error-codes/E0017.rs b/tests/ui/error-codes/E0017.rs
@@ -6,8 +6,6 @@ const CR: &'static mut i32 = &mut C; //~ ERROR mutable references are not allowe
 //~| WARN taking a mutable
 
 static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR E0658
-//~| ERROR cannot borrow
-//~| ERROR mutable references are not allowed
 
 static CONST_REF: &'static mut i32 = &mut C; //~ ERROR mutable references are not allowed
 //~| WARN taking a mutable
diff --git a/tests/ui/error-codes/E0017.stderr b/tests/ui/error-codes/E0017.stderr
@@ -1,5 +1,5 @@
 warning: mutable reference of mutable static is discouraged
-  --> $DIR/E0017.rs:15:52
+  --> $DIR/E0017.rs:13:52
    |
 LL | static STATIC_MUT_REF: &'static mut i32 = unsafe { &mut M };
    |                                                    ^^^^^^ mutable reference of mutable static
@@ -34,7 +34,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/E0017.rs:8:39
    |
 LL | static STATIC_REF: &'static mut i32 = &mut X;
@@ -44,20 +44,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/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
-   |
-LL | static STATIC_REF: &'static mut i32 = &mut X;
-   |                                       ^^^^^^ cannot borrow as mutable
-
 warning: taking a mutable reference to a `const` item
-  --> $DIR/E0017.rs:12:38
+  --> $DIR/E0017.rs:10:38
    |
 LL | static CONST_REF: &'static mut i32 = &mut C;
    |                                      ^^^^^^
@@ -71,18 +59,22 @@ LL | const C: i32 = 2;
    | ^^^^^^^^^^^^
 
 error[E0764]: mutable references are not allowed in the final value of statics
-  --> $DIR/E0017.rs:12:38
+  --> $DIR/E0017.rs:10:38
    |
 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
+error[E0658]: mutable references are not allowed in statics
+  --> $DIR/E0017.rs:13:52
    |
 LL | static STATIC_MUT_REF: &'static mut i32 = unsafe { &mut M };
    |                                                    ^^^^^^
+   |
+   = 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 6 previous errors; 3 warnings emitted
+error: aborting due to 4 previous errors; 3 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`.
diff --git a/tests/ui/error-codes/E0388.rs b/tests/ui/error-codes/E0388.rs
@@ -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
diff --git a/tests/ui/error-codes/E0388.stderr b/tests/ui/error-codes/E0388.stderr
@@ -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`.

Original file line number	Diff line number	Diff line change
`@@ -74,6 +74,10 @@ pub trait Qualif {`
`74`	`74`	`adt: AdtDef<'tcx>,`
`75`	`75`	`args: GenericArgsRef<'tcx>,`
`76`	`76`	`) -> bool;`
	`77`	`+`
	`78`	+ /// Returns `true` if this `Qualif` behaves sructurally for pointers and references:
	`79`	`+ /// the pointer/reference qualifies if and only if the pointee qualifies.`
	`80`	`+ fn deref_structural<'tcx>(cx: &ConstCx<'_, 'tcx>) -> bool;`
`77`	`81`	`}`
`78`	`82`
`79`	`83`	/// Constant containing interior mutability (`UnsafeCell<T>`).
`@@ -103,6 +107,10 @@ impl Qualif for HasMutInterior {`
`103`	`107`	`// It arises structurally for all other types.`
`104`	`108`	`adt.is_unsafe_cell()`
`105`	`109`	`}`
	`110`	`+`
	`111`	`+ fn deref_structural<'tcx>(_cx: &ConstCx<'_, 'tcx>) -> bool {`
	`112`	`+ false`
	`113`	`+ }`
`106`	`114`	`}`
`107`	`115`
`108`	`116`	/// Constant containing an ADT that implements `Drop`.
`@@ -131,6 +139,10 @@ impl Qualif for NeedsDrop {`
`131`	`139`	`) -> bool {`
`132`	`140`	`adt.has_dtor(cx.tcx)`
`133`	`141`	`}`
	`142`	`+`
	`143`	`+ fn deref_structural<'tcx>(_cx: &ConstCx<'_, 'tcx>) -> bool {`
	`144`	`+ false`
	`145`	`+ }`
`134`	`146`	`}`
`135`	`147`
`136`	`148`	/// Constant containing an ADT that implements non-const `Drop`.
`@@ -210,6 +222,10 @@ impl Qualif for NeedsNonConstDrop {`
`210`	`222`	`) -> bool {`
`211`	`223`	`adt.has_non_const_dtor(cx.tcx)`
`212`	`224`	`}`
	`225`	`+`
	`226`	`+ fn deref_structural<'tcx>(_cx: &ConstCx<'_, 'tcx>) -> bool {`
	`227`	`+ false`
	`228`	`+ }`
`213`	`229`	`}`
`214`	`230`
`215`	`231`	// FIXME: Use `mir::visit::Visitor` for the `in_*` functions if/when it supports early return.
`@@ -303,6 +319,11 @@ where`
`303`	`319`	`return false;`
`304`	`320`	`}`
`305`	`321`
	`322`	`+ if matches!(elem, ProjectionElem::Deref) && !Q::deref_structural(cx) {`
	`323`	`+ // We have to assume that this qualifies.`
	`324`	`+ return true;`
	`325`	`+ }`
	`326`	`+`
`306`	`327`	`place = place_base;`
`307`	`328`	`}`
`308`	`329`
Original file line number	Diff line number	Diff line change
@@ -16,7 +16,7 @@ help: skipping check for `const_mut_refs` feature
`16`	`16`	`\|`
`17`	`17`	`LL \| &mut *(Box::new(0))`
`18`	`18`	`\| ^^^^^^^^^^^^^^^^^^^`
`19`		`-help: skipping check that does not even have a feature gate`
	`19`	+help: skipping check for `const_mut_refs` feature
`20`	`20`	`--> $DIR/box.rs:8:5`
`21`	`21`	`\|`
`22`	`22`	`LL \| &mut *(Box::new(0))`
Original file line number	Diff line number	Diff line change
@@ -119,7 +119,7 @@ help: skipping check for `const_refs_to_static` feature
`119`	`119`	`\|`
`120`	`120`	`LL \| const SUBTLE: &mut i32 = unsafe { &mut FOO };`
`121`	`121`	`\| ^^^`
`122`		`-help: skipping check that does not even have a feature gate`
	`122`	+help: skipping check for `const_mut_refs` feature
`123`	`123`	`--> $DIR/mutable_references_err.rs:32:35`
`124`	`124`	`\|`
`125`	`125`	`LL \| const SUBTLE: &mut i32 = unsafe { &mut FOO };`