Skip to content

Commit 41fcbdb

Browse files
RalfJungRalfJung
committed
const_mut_refs: allow mutable refs to statics
1 parent 6cc4843 commit 41fcbdb

13 files changed

+167
-52
lines changed

‎compiler/rustc_const_eval/src/transform/check_consts/check.rs

+28-4
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
344344
visitor.visit_ty(ty);
345345
}
346346

347-
fn check_mut_borrow(&mut self, local: Local, kind: hir::BorrowKind) {
347+
fn check_mut_borrow(&mut self, place: &Place<'_>, kind: hir::BorrowKind) {
348348
match self.const_kind() {
349349
// In a const fn all borrows are transient or point to the places given via
350350
// references in the arguments (so we already checked them with
@@ -355,10 +355,19 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
355355
// to mutable memory.
356356
hir::ConstContext::ConstFn => self.check_op(ops::TransientMutBorrow(kind)),
357357
_ => {
358+
// For indirect places, we are not creating a new permanent borrow, it's just as
359+
// transient as the already existing one. For reborrowing references this is handled
360+
// at the top of `visit_rvalue`, but for raw pointers we handle it here.
361+
// Pointers/references to `static mut` and cases where the `*` is not the first
362+
// projection also end up here.
358363
// Locals with StorageDead do not live beyond the evaluation and can
359364
// thus safely be borrowed without being able to be leaked to the final
360365
// value of the constant.
361-
if self.local_has_storage_dead(local) {
366+
// Note: This is only sound if every local that has a `StorageDead` has a
367+
// `StorageDead` in every control flow path leading to a `return` terminator.
368+
// The good news is that interning will detect if any unexpected mutable
369+
// pointer slips through.
370+
if place.is_indirect() || self.local_has_storage_dead(place.local) {
362371
self.check_op(ops::TransientMutBorrow(kind));
363372
} else {
364373
self.check_op(ops::MutBorrow(kind));
@@ -460,7 +469,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
460469

461470
if !is_allowed {
462471
self.check_mut_borrow(
463-
place.local,
472+
place,
464473
if matches!(rvalue, Rvalue::Ref(..)) {
465474
hir::BorrowKind::Ref
466475
} else {
@@ -478,7 +487,14 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
478487
place.as_ref(),
479488
);
480489

481-
if borrowed_place_has_mut_interior {
490+
// If the place is indirect, this is basically a reborrow. We have a reborrow
491+
// special case above, but for raw pointers and pointers/references to `static` and
492+
// when the `*` is not the first projection, `place_as_reborrow` does not recognize
493+
// them as such, so we end up here. This should probably be considered a
494+
// `TransientCellBorrow` (we consider the equivalent mutable case a
495+
// `TransientMutBorrow`), but such reborrows got accidentally stabilized already and
496+
// it is too much of a breaking change to take back.
497+
if borrowed_place_has_mut_interior && !place.is_indirect() {
482498
match self.const_kind() {
483499
// In a const fn all borrows are transient or point to the places given via
484500
// references in the arguments (so we already checked them with
@@ -495,6 +511,8 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
495511
// final value.
496512
// Note: This is only sound if every local that has a `StorageDead` has a
497513
// `StorageDead` in every control flow path leading to a `return` terminator.
514+
// The good news is that interning will detect if any unexpected mutable
515+
// pointer slips through.
498516
if self.local_has_storage_dead(place.local) {
499517
self.check_op(ops::TransientCellBorrow);
500518
} else {
@@ -946,6 +964,12 @@ fn place_as_reborrow<'tcx>(
946964
) -> Option<PlaceRef<'tcx>> {
947965
match place.as_ref().last_projection() {
948966
Some((place_base, ProjectionElem::Deref)) => {
967+
// FIXME: why do statics and raw pointers get excluded here? This makes
968+
// some code involving mutable pointers unstable, but it is unclear
969+
// why that code is treated differently from mutable references.
970+
// Once TransientMutBorrow and TransientCellBorrow are stable,
971+
// this can probably be cleaned up without any behavioral changes.
972+
949973
// A borrow of a `static` also looks like `&(*_1)` in the MIR, but `_1` is a `const`
950974
// that points to the allocation for the static. Don't treat these as reborrows.
951975
if body.local_decls[place_base.local].is_ref_to_static() {

‎compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs

+21
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ pub trait Qualif {
7474
adt: AdtDef<'tcx>,
7575
args: GenericArgsRef<'tcx>,
7676
) -> 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;
7781
}
7882

7983
/// Constant containing interior mutability (`UnsafeCell<T>`).
@@ -103,6 +107,10 @@ impl Qualif for HasMutInterior {
103107
// It arises structurally for all other types.
104108
adt.is_unsafe_cell()
105109
}
110+
111+
fn deref_structural<'tcx>(_cx: &ConstCx<'_, 'tcx>) -> bool {
112+
false
113+
}
106114
}
107115

108116
/// Constant containing an ADT that implements `Drop`.
@@ -131,6 +139,10 @@ impl Qualif for NeedsDrop {
131139
) -> bool {
132140
adt.has_dtor(cx.tcx)
133141
}
142+
143+
fn deref_structural<'tcx>(_cx: &ConstCx<'_, 'tcx>) -> bool {
144+
false
145+
}
134146
}
135147

136148
/// Constant containing an ADT that implements non-const `Drop`.
@@ -210,6 +222,10 @@ impl Qualif for NeedsNonConstDrop {
210222
) -> bool {
211223
adt.has_non_const_dtor(cx.tcx)
212224
}
225+
226+
fn deref_structural<'tcx>(_cx: &ConstCx<'_, 'tcx>) -> bool {
227+
false
228+
}
213229
}
214230

215231
// FIXME: Use `mir::visit::Visitor` for the `in_*` functions if/when it supports early return.
@@ -303,6 +319,11 @@ where
303319
return false;
304320
}
305321

322+
if matches!(elem, ProjectionElem::Deref) && !Q::deref_structural(cx) {
323+
// We have to assume that this qualifies.
324+
return true;
325+
}
326+
306327
place = place_base;
307328
}
308329

‎tests/ui/consts/const-ref-to-static-linux-vtable.rs

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// check-pass
2+
//! This is the reduced version of the "Linux kernel vtable" use-case.
3+
#![feature(const_mut_refs, const_refs_to_static)]
4+
use std::ptr::addr_of_mut;
5+
6+
#[repr(C)]
7+
struct ThisModule(i32);
8+
9+
trait Module {
10+
const THIS_MODULE_PTR: *mut ThisModule;
11+
}
12+
13+
struct MyModule;
14+
15+
// Generated by a macro.
16+
extern "C" {
17+
static mut THIS_MODULE: ThisModule;
18+
}
19+
20+
// Generated by a macro.
21+
impl Module for MyModule {
22+
const THIS_MODULE_PTR: *mut ThisModule = unsafe { addr_of_mut!(THIS_MODULE) };
23+
}
24+
25+
struct Vtable {
26+
module: *mut ThisModule,
27+
foo_fn: fn(*mut ()) -> i32,
28+
}
29+
30+
trait Foo {
31+
type Mod: Module;
32+
33+
fn foo(&mut self) -> i32;
34+
}
35+
36+
fn generate_vtable<T: Foo>() -> &'static Vtable {
37+
&Vtable {
38+
module: T::Mod::THIS_MODULE_PTR,
39+
foo_fn: |ptr| unsafe { &mut *ptr.cast::<T>() }.foo(),
40+
}
41+
}
42+
43+
fn main() {}

‎tests/ui/consts/issue-17718-const-bad-values.rs

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ static mut S: usize = 3;
55
const C2: &'static mut usize = unsafe { &mut S };
66
//~^ ERROR: referencing statics in constants
77
//~| ERROR: referencing statics in constants
8+
//~| ERROR: mutable references are not allowed
89
//~| WARN mutable reference of mutable static is discouraged [static_mut_ref]
910

1011
fn main() {}

‎tests/ui/consts/issue-17718-const-bad-values.stderr

+11-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,17 @@ LL | const C2: &'static mut usize = unsafe { &mut S };
4444
= help: to fix this, the value can be extracted to a `const` and then used.
4545
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
4646

47-
error: aborting due to 3 previous errors; 1 warning emitted
47+
error[E0658]: mutable references are not allowed in constants
48+
--> $DIR/issue-17718-const-bad-values.rs:5:41
49+
|
50+
LL | const C2: &'static mut usize = unsafe { &mut S };
51+
| ^^^^^^
52+
|
53+
= note: see issue #57349 <https://github.com/rust-lang/rust/issues/57349> for more information
54+
= help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
55+
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
56+
57+
error: aborting due to 4 previous errors; 1 warning emitted
4858

4959
Some errors have detailed explanations: E0658, E0764.
5060
For more information about an error, try `rustc --explain E0658`.

‎tests/ui/consts/miri_unleashed/box.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ help: skipping check for `const_mut_refs` feature
1616
|
1717
LL | &mut *(Box::new(0))
1818
| ^^^^^^^^^^^^^^^^^^^
19-
help: skipping check that does not even have a feature gate
19+
help: skipping check for `const_mut_refs` feature
2020
--> $DIR/box.rs:8:5
2121
|
2222
LL | &mut *(Box::new(0))

‎tests/ui/consts/miri_unleashed/mutable_references_err.32bit.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ help: skipping check for `const_refs_to_static` feature
119119
|
120120
LL | const SUBTLE: &mut i32 = unsafe { &mut FOO };
121121
| ^^^
122-
help: skipping check that does not even have a feature gate
122+
help: skipping check for `const_mut_refs` feature
123123
--> $DIR/mutable_references_err.rs:32:35
124124
|
125125
LL | const SUBTLE: &mut i32 = unsafe { &mut FOO };

‎tests/ui/consts/miri_unleashed/mutable_references_err.64bit.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ help: skipping check for `const_refs_to_static` feature
119119
|
120120
LL | const SUBTLE: &mut i32 = unsafe { &mut FOO };
121121
| ^^^
122-
help: skipping check that does not even have a feature gate
122+
help: skipping check for `const_mut_refs` feature
123123
--> $DIR/mutable_references_err.rs:32:35
124124
|
125125
LL | const SUBTLE: &mut i32 = unsafe { &mut FOO };

‎tests/ui/consts/mut-ptr-to-static.rs

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// run-pass
2+
#![feature(const_mut_refs)]
3+
#![feature(sync_unsafe_cell)]
4+
5+
use std::cell::SyncUnsafeCell;
6+
use std::ptr;
7+
8+
#[repr(C)]
9+
struct SyncPtr {
10+
foo: *mut u32,
11+
}
12+
unsafe impl Sync for SyncPtr {}
13+
14+
static mut STATIC: u32 = 42;
15+
16+
static INTERIOR_MUTABLE_STATIC: SyncUnsafeCell<u32> = SyncUnsafeCell::new(42);
17+
18+
// A static that mutably points to STATIC.
19+
static PTR: SyncPtr = SyncPtr {
20+
foo: unsafe { ptr::addr_of_mut!(STATIC) },
21+
};
22+
static INTERIOR_MUTABLE_PTR: SyncPtr = SyncPtr {
23+
foo: ptr::addr_of!(INTERIOR_MUTABLE_STATIC) as *mut u32,
24+
};
25+
26+
fn main() {
27+
let ptr = PTR.foo;
28+
unsafe {
29+
assert_eq!(*ptr, 42);
30+
*ptr = 0;
31+
assert_eq!(*PTR.foo, 0);
32+
}
33+
34+
let ptr = INTERIOR_MUTABLE_PTR.foo;
35+
unsafe {
36+
assert_eq!(*ptr, 42);
37+
*ptr = 0;
38+
assert_eq!(*INTERIOR_MUTABLE_PTR.foo, 0);
39+
}
40+
}

‎tests/ui/error-codes/E0017.rs

-2
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ const CR: &'static mut i32 = &mut C; //~ ERROR mutable references are not allowe
66
//~| WARN taking a mutable
77

88
static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR E0658
9-
//~| ERROR cannot borrow
10-
//~| ERROR mutable references are not allowed
119

1210
static CONST_REF: &'static mut i32 = &mut C; //~ ERROR mutable references are not allowed
1311
//~| WARN taking a mutable

‎tests/ui/error-codes/E0017.stderr

+13-21
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
warning: mutable reference of mutable static is discouraged
2-
--> $DIR/E0017.rs:15:52
2+
--> $DIR/E0017.rs:13:52
33
|
44
LL | static STATIC_MUT_REF: &'static mut i32 = unsafe { &mut M };
55
| ^^^^^^ mutable reference of mutable static
@@ -34,7 +34,7 @@ error[E0764]: mutable references are not allowed in the final value of constants
3434
LL | const CR: &'static mut i32 = &mut C;
3535
| ^^^^^^
3636

37-
error[E0658]: mutation through a reference is not allowed in statics
37+
error[E0658]: mutable references are not allowed in statics
3838
--> $DIR/E0017.rs:8:39
3939
|
4040
LL | static STATIC_REF: &'static mut i32 = &mut X;
@@ -44,20 +44,8 @@ LL | static STATIC_REF: &'static mut i32 = &mut X;
4444
= help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
4545
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
4646

47-
error[E0764]: mutable references are not allowed in the final value of statics
48-
--> $DIR/E0017.rs:8:39
49-
|
50-
LL | static STATIC_REF: &'static mut i32 = &mut X;
51-
| ^^^^^^
52-
53-
error[E0596]: cannot borrow immutable static item `X` as mutable
54-
--> $DIR/E0017.rs:8:39
55-
|
56-
LL | static STATIC_REF: &'static mut i32 = &mut X;
57-
| ^^^^^^ cannot borrow as mutable
58-
5947
warning: taking a mutable reference to a `const` item
60-
--> $DIR/E0017.rs:12:38
48+
--> $DIR/E0017.rs:10:38
6149
|
6250
LL | static CONST_REF: &'static mut i32 = &mut C;
6351
| ^^^^^^
@@ -71,18 +59,22 @@ LL | const C: i32 = 2;
7159
| ^^^^^^^^^^^^
7260

7361
error[E0764]: mutable references are not allowed in the final value of statics
74-
--> $DIR/E0017.rs:12:38
62+
--> $DIR/E0017.rs:10:38
7563
|
7664
LL | static CONST_REF: &'static mut i32 = &mut C;
7765
| ^^^^^^
7866

79-
error[E0764]: mutable references are not allowed in the final value of statics
80-
--> $DIR/E0017.rs:15:52
67+
error[E0658]: mutable references are not allowed in statics
68+
--> $DIR/E0017.rs:13:52
8169
|
8270
LL | static STATIC_MUT_REF: &'static mut i32 = unsafe { &mut M };
8371
| ^^^^^^
72+
|
73+
= note: see issue #57349 <https://github.com/rust-lang/rust/issues/57349> for more information
74+
= help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
75+
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
8476

85-
error: aborting due to 6 previous errors; 3 warnings emitted
77+
error: aborting due to 4 previous errors; 3 warnings emitted
8678

87-
Some errors have detailed explanations: E0596, E0658, E0764.
88-
For more information about an error, try `rustc --explain E0596`.
79+
Some errors have detailed explanations: E0658, E0764.
80+
For more information about an error, try `rustc --explain E0658`.

‎tests/ui/error-codes/E0388.rs

+1-3
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@ const C: i32 = 2;
33

44
const CR: &'static mut i32 = &mut C; //~ ERROR mutable references are not allowed
55
//~| WARN taking a mutable
6-
static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR cannot borrow
7-
//~| ERROR E0658
8-
//~| ERROR mutable references are not allowed
6+
static STATIC_REF: &'static mut i32 = &mut X; //~ ERROR E0658
97

108
static CONST_REF: &'static mut i32 = &mut C; //~ ERROR mutable references are not allowed
119
//~| WARN taking a mutable

0 commit comments

Comments
 (0)