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

Allow references to interior mutable data behind a feature gate #80418

Merged
merged 13 commits into from
Jan 4, 2021
Merged
4 changes: 2 additions & 2 deletions compiler/rustc_error_codes/src/error_codes/E0492.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Erroneous code example:
use std::sync::atomic::AtomicUsize;

const A: AtomicUsize = AtomicUsize::new(0);
static B: &'static AtomicUsize = &A;
const B: &'static AtomicUsize = &A;
// error: cannot borrow a constant which may contain interior mutability,
// create a static instead
```
Expand All @@ -18,7 +18,7 @@ can't be changed via a shared `&` pointer, but interior mutability would allow
it. That is, a constant value could be mutated. On the other hand, a `static` is
explicitly a single memory location, which can be mutated at will.

So, in order to solve this error, either use statics which are `Sync`:
So, in order to solve this error, use statics which are `Sync`:

```
use std::sync::atomic::AtomicUsize;
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,9 @@ declare_features! (
/// Allows const generics to have default values (e.g. `struct Foo<const N: usize = 3>(...);`).
(active, const_generics_defaults, "1.51.0", Some(44580), None),

/// Allows references to types with interior mutability within constants
(active, const_refs_to_cell, "1.51.0", Some(80384), None),

// -------------------------------------------------------------------------
// feature-group-end: actual feature gates
// -------------------------------------------------------------------------
Expand Down
53 changes: 49 additions & 4 deletions compiler/rustc_mir/src/transform/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,16 +209,61 @@ impl NonConstOp for LiveDrop {
}

#[derive(Debug)]
/// A borrow of a type that contains an `UnsafeCell` somewhere. The borrow never escapes to
/// the final value of the constant.
pub struct TransientCellBorrow;
impl NonConstOp for TransientCellBorrow {
fn status_in_item(&self, _: &ConstCx<'_, '_>) -> Status {
Status::Unstable(sym::const_refs_to_cell)
}
fn importance(&self) -> DiagnosticImportance {
// The cases that cannot possibly work will already emit a `CellBorrow`, so we should
// not additionally emit a feature gate error if activating the feature gate won't work.
DiagnosticImportance::Secondary
}
fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> DiagnosticBuilder<'tcx> {
feature_err(
&ccx.tcx.sess.parse_sess,
sym::const_refs_to_cell,
span,
"cannot borrow here, since the borrowed element may contain interior mutability",
)
}
}

#[derive(Debug)]
/// A borrow of a type that contains an `UnsafeCell` somewhere. The borrow might escape to
/// the final value of the constant, and thus we cannot allow this (for now). We may allow
/// it in the future for static items.
pub struct CellBorrow;
impl NonConstOp for CellBorrow {
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> DiagnosticBuilder<'tcx> {
struct_span_err!(
let mut err = struct_span_err!(
ccx.tcx.sess,
span,
E0492,
"cannot borrow a constant which may contain \
interior mutability, create a static instead"
)
"{}s cannot refer to interior mutable data",
ccx.const_kind(),
);
err.span_label(
span,
format!("this borrow of an interior mutable value may end up in the final value"),
);
if let hir::ConstContext::Static(_) = ccx.const_kind() {
err.help(
"to fix this, the value can be extracted to a separate \
`static` item and then referenced",
);
}
if ccx.tcx.sess.teach(&err.get_code().unwrap()) {
err.note(
"A constant containing interior mutable data behind a reference can allow you
to modify that data. This would make multiple uses of a constant to be able to
see different values and allow circumventing the `Send` and `Sync` requirements
for shared mutable data, which is unsound.",
);
}
err
}
}

Expand Down
50 changes: 49 additions & 1 deletion compiler/rustc_mir/src/transform/check_consts/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use rustc_errors::{struct_span_err, Applicability, Diagnostic, ErrorReported};
use rustc_hir::def_id::DefId;
use rustc_hir::{self as hir, HirId, LangItem};
use rustc_index::bit_set::BitSet;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::traits::{ImplSource, Obligation, ObligationCause};
use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
Expand Down Expand Up @@ -188,6 +189,9 @@ pub struct Validator<'mir, 'tcx> {
/// The span of the current statement.
span: Span,

/// A set that stores for each local whether it has a `StorageDead` for it somewhere.
local_has_storage_dead: Option<BitSet<Local>>,

error_emitted: Option<ErrorReported>,
secondary_errors: Vec<Diagnostic>,
}
Expand All @@ -206,6 +210,7 @@ impl Validator<'mir, 'tcx> {
span: ccx.body.span,
ccx,
qualifs: Default::default(),
local_has_storage_dead: None,
error_emitted: None,
secondary_errors: Vec::new(),
}
Expand Down Expand Up @@ -282,6 +287,27 @@ impl Validator<'mir, 'tcx> {
}
}

fn local_has_storage_dead(&mut self, local: Local) -> bool {
let ccx = self.ccx;
self.local_has_storage_dead
.get_or_insert_with(|| {
struct StorageDeads {
locals: BitSet<Local>,
}
impl Visitor<'tcx> for StorageDeads {
fn visit_statement(&mut self, stmt: &Statement<'tcx>, _: Location) {
if let StatementKind::StorageDead(l) = stmt.kind {
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
self.locals.insert(l);
}
}
}
let mut v = StorageDeads { locals: BitSet::new_empty(ccx.body.local_decls.len()) };
v.visit_body(ccx.body);
v.locals
})
.contains(local)
}

pub fn qualifs_in_return_place(&mut self) -> ConstQualifs {
self.qualifs.in_return_place(self.ccx, self.error_emitted)
}
Expand Down Expand Up @@ -556,7 +582,29 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> {
);

if borrowed_place_has_mut_interior {
self.check_op(ops::CellBorrow);
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
// TransientCellBorrow/CellBorrow as appropriate).
// The borrow checker guarantees that no new non-transient borrows are created.
// NOTE: Once we have heap allocations during CTFE we need to figure out
// how to prevent `const fn` to create long-lived allocations that point
// to (interior) mutable memory.
hir::ConstContext::ConstFn => self.check_op(ops::TransientCellBorrow),
_ => {
// Locals with StorageDead are definitely not part of the final constant value, and
// it is thus inherently safe to permit such locals to have their
// address taken as we can't end up with a reference to them in the
// 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.
if self.local_has_storage_dead(place.local) {
self.check_op(ops::TransientCellBorrow);
} else {
self.check_op(ops::CellBorrow);
}
}
}
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ symbols! {
const_ptr,
const_raw_ptr_deref,
const_raw_ptr_to_usize_cast,
const_refs_to_cell,
const_slice_ptr,
const_trait_bound_opt_out,
const_trait_impl,
Expand Down
22 changes: 17 additions & 5 deletions src/test/ui/consts/const-address-of-interior-mut.stderr
Original file line number Diff line number Diff line change
@@ -1,27 +1,39 @@
error[E0492]: cannot borrow a constant which may contain interior mutability, create a static instead
error[E0658]: cannot borrow here, since the borrowed element may contain interior mutability
--> $DIR/const-address-of-interior-mut.rs:5:39
|
LL | const A: () = { let x = Cell::new(2); &raw const x; };
| ^^^^^^^^^^^^
|
= note: see issue #80384 <https://github.com/rust-lang/rust/issues/80384> for more information
= help: add `#![feature(const_refs_to_cell)]` to the crate attributes to enable

error[E0492]: cannot borrow a constant which may contain interior mutability, create a static instead
error[E0658]: cannot borrow here, since the borrowed element may contain interior mutability
--> $DIR/const-address-of-interior-mut.rs:7:40
|
LL | static B: () = { let x = Cell::new(2); &raw const x; };
| ^^^^^^^^^^^^
|
= note: see issue #80384 <https://github.com/rust-lang/rust/issues/80384> for more information
= help: add `#![feature(const_refs_to_cell)]` to the crate attributes to enable

error[E0492]: cannot borrow a constant which may contain interior mutability, create a static instead
error[E0658]: cannot borrow here, since the borrowed element may contain interior mutability
--> $DIR/const-address-of-interior-mut.rs:9:44
|
LL | static mut C: () = { let x = Cell::new(2); &raw const x; };
| ^^^^^^^^^^^^
|
= note: see issue #80384 <https://github.com/rust-lang/rust/issues/80384> for more information
= help: add `#![feature(const_refs_to_cell)]` to the crate attributes to enable

error[E0492]: cannot borrow a constant which may contain interior mutability, create a static instead
error[E0658]: cannot borrow here, since the borrowed element may contain interior mutability
--> $DIR/const-address-of-interior-mut.rs:13:13
|
LL | let y = &raw const x;
| ^^^^^^^^^^^^
|
= note: see issue #80384 <https://github.com/rust-lang/rust/issues/80384> for more information
= help: add `#![feature(const_refs_to_cell)]` to the crate attributes to enable

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0492`.
For more information about this error, try `rustc --explain E0658`.
2 changes: 1 addition & 1 deletion src/test/ui/consts/const-multi-ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const _: i32 = {

const _: std::cell::Cell<i32> = {
let mut a = std::cell::Cell::new(5);
let p = &a; //~ ERROR cannot borrow a constant which may contain interior mutability
let p = &a; //~ ERROR borrowed element may contain interior mutability

let reborrow = {p};
let pp = &reborrow;
Expand Down
9 changes: 6 additions & 3 deletions src/test/ui/consts/const-multi-ref.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ error[E0764]: mutable references are not allowed in constants
LL | let p = &mut a;
| ^^^^^^ `&mut` is only allowed in `const fn`

error[E0492]: cannot borrow a constant which may contain interior mutability, create a static instead
error[E0658]: cannot borrow here, since the borrowed element may contain interior mutability
--> $DIR/const-multi-ref.rs:16:13
|
LL | let p = &a;
| ^^
|
= note: see issue #80384 <https://github.com/rust-lang/rust/issues/80384> for more information
= help: add `#![feature(const_refs_to_cell)]` to the crate attributes to enable

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0492, E0764.
For more information about an error, try `rustc --explain E0492`.
Some errors have detailed explanations: E0658, E0764.
For more information about an error, try `rustc --explain E0658`.
2 changes: 1 addition & 1 deletion src/test/ui/consts/partial_qualif.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::cell::Cell;
const FOO: &(Cell<usize>, bool) = {
let mut a = (Cell::new(0), false);
a.1 = true; // sets `qualif(a)` to `qualif(a) | qualif(true)`
&{a} //~ ERROR cannot borrow a constant which may contain interior mutability
&{a} //~ ERROR cannot refer to interior mutable
};

fn main() {}
4 changes: 2 additions & 2 deletions src/test/ui/consts/partial_qualif.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0492]: cannot borrow a constant which may contain interior mutability, create a static instead
error[E0492]: constants cannot refer to interior mutable data
--> $DIR/partial_qualif.rs:6:5
|
LL | &{a}
| ^^^^
| ^^^^ this borrow of an interior mutable value may end up in the final value

error: aborting due to previous error

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/consts/qualif_overwrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::cell::Cell;
const FOO: &Option<Cell<usize>> = {
let mut a = Some(Cell::new(0));
a = None; // sets `qualif(a)` to `qualif(a) | qualif(None)`
&{a} //~ ERROR cannot borrow a constant which may contain interior mutability
&{a} //~ ERROR cannot refer to interior mutable
};

fn main() {}
4 changes: 2 additions & 2 deletions src/test/ui/consts/qualif_overwrite.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0492]: cannot borrow a constant which may contain interior mutability, create a static instead
error[E0492]: constants cannot refer to interior mutable data
--> $DIR/qualif_overwrite.rs:10:5
|
LL | &{a}
| ^^^^
| ^^^^ this borrow of an interior mutable value may end up in the final value

error: aborting due to previous error

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/consts/qualif_overwrite_2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::cell::Cell;
const FOO: &Option<Cell<usize>> = {
let mut a = (Some(Cell::new(0)),);
a.0 = None; // sets `qualif(a)` to `qualif(a) | qualif(None)`
&{a.0} //~ ERROR cannot borrow a constant which may contain interior mutability
&{a.0} //~ ERROR cannot refer to interior mutable
};

fn main() {}
4 changes: 2 additions & 2 deletions src/test/ui/consts/qualif_overwrite_2.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0492]: cannot borrow a constant which may contain interior mutability, create a static instead
error[E0492]: constants cannot refer to interior mutable data
--> $DIR/qualif_overwrite_2.rs:8:5
|
LL | &{a.0}
| ^^^^^^
| ^^^^^^ this borrow of an interior mutable value may end up in the final value

error: aborting due to previous error

Expand Down
30 changes: 23 additions & 7 deletions src/test/ui/consts/std/cell.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,31 @@
#![feature(const_refs_to_cell)]

use std::cell::*;

// not ok, because this would create a silent constant with interior mutability.
// the rules could be relaxed in the future
// not ok, because this creates a dangling pointer, just like `let x = Cell::new(42).as_ptr()` would
static FOO: Wrap<*mut u32> = Wrap(Cell::new(42).as_ptr());
//~^ ERROR cannot borrow a constant which may contain interior mutability
//~^ ERROR encountered dangling pointer
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
const FOO_CONST: Wrap<*mut u32> = Wrap(Cell::new(42).as_ptr());
//~^ ERROR encountered dangling pointer

// Ok, these are just base values and it is the `Wrap` author's job to uphold `Send` and `Sync`
// invariants, since they used `unsafe impl`.
static FOO3: Wrap<Cell<u32>> = Wrap(Cell::new(42));
// ok
const FOO3_CONST: Wrap<Cell<u32>> = Wrap(Cell::new(42));

// ok, we are referring to the memory of another static item.
static FOO4: Wrap<*mut u32> = Wrap(FOO3.0.as_ptr());

// not ok, because the `as_ptr` call takes a reference to a type with interior mutability
// which is not allowed in constants
// not ok, the use of a constant here is equivalent to an inline declaration of the value, so
// its memory will get freed before the constant is finished evaluating, thus creating a dangling
// pointer. This would happen exactly the same at runtime.
const FOO4_CONST: Wrap<*mut u32> = Wrap(FOO3_CONST.0.as_ptr());
//~^ ERROR encountered dangling pointer
oli-obk marked this conversation as resolved.
Show resolved Hide resolved

// not ok, because the `as_ptr` call takes a reference to a temporary that will get freed
// before the constant is finished evaluating.
const FOO2: *mut u32 = Cell::new(42).as_ptr();
//~^ ERROR cannot borrow a constant which may contain interior mutability
//~^ ERROR encountered dangling pointer

struct IMSafeTrustMe(UnsafeCell<u32>);
unsafe impl Send for IMSafeTrustMe {}
Expand All @@ -21,10 +34,13 @@ unsafe impl Sync for IMSafeTrustMe {}
static BAR: IMSafeTrustMe = IMSafeTrustMe(UnsafeCell::new(5));



struct Wrap<T>(T);
unsafe impl<T> Send for Wrap<T> {}
unsafe impl<T> Sync for Wrap<T> {}

static BAR_PTR: Wrap<*mut u32> = Wrap(BAR.0.get());

const fn fst_ref<T, U>(x: &(T, U)) -> &T { &x.0 }

fn main() {}
Loading