Skip to content

addr_of! grants mutable access, maybe? #90413

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

Merged
merged 2 commits into from
Nov 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions compiler/rustc_const_eval/src/transform/check_consts/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,10 @@ where
}
}

fn address_of_allows_mutation(&self, mt: mir::Mutability, place: mir::Place<'tcx>) -> bool {
match mt {
mir::Mutability::Mut => true,
mir::Mutability::Not => self.shared_borrow_allows_mutation(place),
}
fn address_of_allows_mutation(&self, _mt: mir::Mutability, _place: mir::Place<'tcx>) -> bool {
// Exact set of permissions granted by AddressOf is undecided. Conservatively assume that
// it might allow mutation until resolution of #56604.
true
}

fn ref_allows_mutation(&self, kind: mir::BorrowKind, place: mir::Place<'tcx>) -> bool {
Expand All @@ -110,6 +109,15 @@ where
}
}

/// `&` only allow mutation if the borrowed place is `!Freeze`.
///
/// This assumes that it is UB to take the address of a struct field whose type is
/// `Freeze`, then use pointer arithmetic to derive a pointer to a *different* field of
/// that same struct whose type is `!Freeze`. If we decide that this is not UB, we will
/// have to check the type of the borrowed **local** instead of the borrowed **place**
/// below. See [rust-lang/unsafe-code-guidelines#134].
///
/// [rust-lang/unsafe-code-guidelines#134]: https://github.com/rust-lang/unsafe-code-guidelines/issues/134
fn shared_borrow_allows_mutation(&self, place: mir::Place<'tcx>) -> bool {
!place
.ty(self.ccx.body, self.ccx.tcx)
Expand Down
132 changes: 15 additions & 117 deletions compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,55 +3,26 @@ use super::*;
use crate::{AnalysisDomain, GenKill, GenKillAnalysis};
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::*;
use rustc_middle::ty::{ParamEnv, TyCtxt};
use rustc_span::DUMMY_SP;

pub type MaybeMutBorrowedLocals<'mir, 'tcx> = MaybeBorrowedLocals<MutBorrow<'mir, 'tcx>>;

/// A dataflow analysis that tracks whether a pointer or reference could possibly exist that points
/// to a given local.
///
/// The `K` parameter determines what kind of borrows are tracked. By default,
/// `MaybeBorrowedLocals` looks for *any* borrow of a local. If you are only interested in borrows
/// that might allow mutation, use the `MaybeMutBorrowedLocals` type alias instead.
///
/// At present, this is used as a very limited form of alias analysis. For example,
/// `MaybeBorrowedLocals` is used to compute which locals are live during a yield expression for
/// immovable generators. `MaybeMutBorrowedLocals` is used during const checking to prove that a
/// local has not been mutated via indirect assignment (e.g., `*p = 42`), the side-effects of a
/// function call or inline assembly.
pub struct MaybeBorrowedLocals<K = AnyBorrow> {
kind: K,
/// immovable generators.
pub struct MaybeBorrowedLocals {
ignore_borrow_on_drop: bool,
}

impl MaybeBorrowedLocals {
/// A dataflow analysis that records whether a pointer or reference exists that may alias the
/// given local.
pub fn all_borrows() -> Self {
MaybeBorrowedLocals { kind: AnyBorrow, ignore_borrow_on_drop: false }
}
}

impl MaybeMutBorrowedLocals<'mir, 'tcx> {
/// A dataflow analysis that records whether a pointer or reference exists that may *mutably*
/// alias the given local.
///
/// This includes `&mut` and pointers derived from an `&mut`, as well as shared borrows of
/// types with interior mutability.
pub fn mut_borrows_only(
tcx: TyCtxt<'tcx>,
body: &'mir mir::Body<'tcx>,
param_env: ParamEnv<'tcx>,
) -> Self {
MaybeBorrowedLocals {
kind: MutBorrow { body, tcx, param_env },
ignore_borrow_on_drop: false,
}
MaybeBorrowedLocals { ignore_borrow_on_drop: false }
}
}

impl<K> MaybeBorrowedLocals<K> {
impl MaybeBorrowedLocals {
/// During dataflow analysis, ignore the borrow that may occur when a place is dropped.
///
/// Drop terminators may call custom drop glue (`Drop::drop`), which takes `&mut self` as a
Expand All @@ -69,21 +40,14 @@ impl<K> MaybeBorrowedLocals<K> {
MaybeBorrowedLocals { ignore_borrow_on_drop: true, ..self }
}

fn transfer_function<'a, T>(&'a self, trans: &'a mut T) -> TransferFunction<'a, T, K> {
TransferFunction {
kind: &self.kind,
trans,
ignore_borrow_on_drop: self.ignore_borrow_on_drop,
}
fn transfer_function<'a, T>(&'a self, trans: &'a mut T) -> TransferFunction<'a, T> {
TransferFunction { trans, ignore_borrow_on_drop: self.ignore_borrow_on_drop }
}
}

impl<K> AnalysisDomain<'tcx> for MaybeBorrowedLocals<K>
where
K: BorrowAnalysisKind<'tcx>,
{
impl AnalysisDomain<'tcx> for MaybeBorrowedLocals {
type Domain = BitSet<Local>;
const NAME: &'static str = K::ANALYSIS_NAME;
const NAME: &'static str = "maybe_borrowed_locals";

fn bottom_value(&self, body: &mir::Body<'tcx>) -> Self::Domain {
// bottom = unborrowed
Expand All @@ -95,10 +59,7 @@ where
}
}

impl<K> GenKillAnalysis<'tcx> for MaybeBorrowedLocals<K>
where
K: BorrowAnalysisKind<'tcx>,
{
impl GenKillAnalysis<'tcx> for MaybeBorrowedLocals {
type Idx = Local;

fn statement_effect(
Expand Down Expand Up @@ -131,16 +92,14 @@ where
}

/// A `Visitor` that defines the transfer function for `MaybeBorrowedLocals`.
struct TransferFunction<'a, T, K> {
struct TransferFunction<'a, T> {
trans: &'a mut T,
kind: &'a K,
ignore_borrow_on_drop: bool,
}

impl<T, K> Visitor<'tcx> for TransferFunction<'a, T, K>
impl<T> Visitor<'tcx> for TransferFunction<'a, T>
where
T: GenKill<Local>,
K: BorrowAnalysisKind<'tcx>,
{
fn visit_statement(&mut self, stmt: &Statement<'tcx>, location: Location) {
self.super_statement(stmt, location);
Expand All @@ -156,14 +115,14 @@ where
self.super_rvalue(rvalue, location);

match rvalue {
mir::Rvalue::AddressOf(mt, borrowed_place) => {
if !borrowed_place.is_indirect() && self.kind.in_address_of(*mt, *borrowed_place) {
mir::Rvalue::AddressOf(_mt, borrowed_place) => {
if !borrowed_place.is_indirect() {
self.trans.gen(borrowed_place.local);
}
}

mir::Rvalue::Ref(_, kind, borrowed_place) => {
if !borrowed_place.is_indirect() && self.kind.in_ref(*kind, *borrowed_place) {
mir::Rvalue::Ref(_, _kind, borrowed_place) => {
if !borrowed_place.is_indirect() {
self.trans.gen(borrowed_place.local);
}
}
Expand Down Expand Up @@ -211,64 +170,3 @@ where
}
}
}

pub struct AnyBorrow;

pub struct MutBorrow<'mir, 'tcx> {
tcx: TyCtxt<'tcx>,
body: &'mir Body<'tcx>,
param_env: ParamEnv<'tcx>,
}

impl MutBorrow<'mir, 'tcx> {
/// `&` and `&raw` only allow mutation if the borrowed place is `!Freeze`.
///
/// This assumes that it is UB to take the address of a struct field whose type is
/// `Freeze`, then use pointer arithmetic to derive a pointer to a *different* field of
/// that same struct whose type is `!Freeze`. If we decide that this is not UB, we will
/// have to check the type of the borrowed **local** instead of the borrowed **place**
/// below. See [rust-lang/unsafe-code-guidelines#134].
///
/// [rust-lang/unsafe-code-guidelines#134]: https://github.com/rust-lang/unsafe-code-guidelines/issues/134
fn shared_borrow_allows_mutation(&self, place: Place<'tcx>) -> bool {
!place.ty(self.body, self.tcx).ty.is_freeze(self.tcx.at(DUMMY_SP), self.param_env)
}
}

pub trait BorrowAnalysisKind<'tcx> {
const ANALYSIS_NAME: &'static str;

fn in_address_of(&self, mt: Mutability, place: Place<'tcx>) -> bool;
fn in_ref(&self, kind: mir::BorrowKind, place: Place<'tcx>) -> bool;
}

impl BorrowAnalysisKind<'tcx> for AnyBorrow {
const ANALYSIS_NAME: &'static str = "maybe_borrowed_locals";

fn in_ref(&self, _: mir::BorrowKind, _: Place<'_>) -> bool {
true
}
fn in_address_of(&self, _: Mutability, _: Place<'_>) -> bool {
true
}
}

impl BorrowAnalysisKind<'tcx> for MutBorrow<'mir, 'tcx> {
const ANALYSIS_NAME: &'static str = "maybe_mut_borrowed_locals";

fn in_ref(&self, kind: mir::BorrowKind, place: Place<'tcx>) -> bool {
match kind {
mir::BorrowKind::Mut { .. } => true,
mir::BorrowKind::Shared | mir::BorrowKind::Shallow | mir::BorrowKind::Unique => {
self.shared_borrow_allows_mutation(place)
}
}
}

fn in_address_of(&self, mt: Mutability, place: Place<'tcx>) -> bool {
match mt {
Mutability::Mut => true,
Mutability::Not => self.shared_borrow_allows_mutation(place),
}
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_mir_dataflow/src/impls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ mod init_locals;
mod liveness;
mod storage_liveness;

pub use self::borrowed_locals::{MaybeBorrowedLocals, MaybeMutBorrowedLocals};
pub use self::borrowed_locals::MaybeBorrowedLocals;
pub use self::init_locals::MaybeInitializedLocals;
pub use self::liveness::MaybeLiveLocals;
pub use self::storage_liveness::{MaybeRequiresStorage, MaybeStorageLive};
Expand Down
31 changes: 1 addition & 30 deletions compiler/rustc_mir_dataflow/src/rustc_peek.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ use rustc_middle::mir::{self, Body, Local, Location};
use rustc_middle::ty::{self, Ty, TyCtxt};

use crate::impls::{
DefinitelyInitializedPlaces, MaybeInitializedPlaces, MaybeLiveLocals, MaybeMutBorrowedLocals,
MaybeUninitializedPlaces,
DefinitelyInitializedPlaces, MaybeInitializedPlaces, MaybeLiveLocals, MaybeUninitializedPlaces,
};
use crate::move_paths::{HasMoveData, MoveData};
use crate::move_paths::{LookupResult, MovePathIndex};
Expand Down Expand Up @@ -62,14 +61,6 @@ impl<'tcx> MirPass<'tcx> for SanityCheck {
sanity_check_via_rustc_peek(tcx, body, &attributes, &flow_def_inits);
}

if has_rustc_mir_with(sess, &attributes, sym::rustc_peek_indirectly_mutable).is_some() {
let flow_mut_borrowed = MaybeMutBorrowedLocals::mut_borrows_only(tcx, body, param_env)
.into_engine(tcx, body)
.iterate_to_fixpoint();

sanity_check_via_rustc_peek(tcx, body, &attributes, &flow_mut_borrowed);
}

if has_rustc_mir_with(sess, &attributes, sym::rustc_peek_liveness).is_some() {
let flow_liveness = MaybeLiveLocals.into_engine(tcx, body).iterate_to_fixpoint();

Expand Down Expand Up @@ -281,26 +272,6 @@ where
}
}

impl<'tcx> RustcPeekAt<'tcx> for MaybeMutBorrowedLocals<'_, 'tcx> {
fn peek_at(
&self,
tcx: TyCtxt<'tcx>,
place: mir::Place<'tcx>,
flow_state: &BitSet<Local>,
call: PeekCall,
) {
info!(?place, "peek_at");
let Some(local) = place.as_local() else {
tcx.sess.span_err(call.span, "rustc_peek: argument was not a local");
return;
};

if !flow_state.contains(local) {
tcx.sess.span_err(call.span, "rustc_peek: bit not set");
}
}
}

impl<'tcx> RustcPeekAt<'tcx> for MaybeLiveLocals {
fn peek_at(
&self,
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,6 @@ symbols! {
rustc_partition_reused,
rustc_peek,
rustc_peek_definite_init,
rustc_peek_indirectly_mutable,
rustc_peek_liveness,
rustc_peek_maybe_init,
rustc_peek_maybe_uninit,
Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/consts/qualif-indirect-mutation-fail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#![feature(const_mut_refs)]
#![feature(const_precise_live_drops)]
#![feature(const_swap)]
#![feature(raw_ref_op)]

// Mutable borrow of a field with drop impl.
pub const fn f() {
Expand Down Expand Up @@ -42,3 +43,22 @@ pub const fn g2<T>() {
let _ = x.is_some();
let _y = x; //~ ERROR destructors cannot be evaluated
}

// Mutable raw reference to a Drop type.
pub const fn address_of_mut() {
let mut x: Option<String> = None; //~ ERROR destructors cannot be evaluated
&raw mut x;

let mut y: Option<String> = None; //~ ERROR destructors cannot be evaluated
std::ptr::addr_of_mut!(y);
}

// Const raw reference to a Drop type. Conservatively assumed to allow mutation
// until resolution of https://github.com/rust-lang/rust/issues/56604.
pub const fn address_of_const() {
let x: Option<String> = None; //~ ERROR destructors cannot be evaluated
&raw const x;

let y: Option<String> = None; //~ ERROR destructors cannot be evaluated
std::ptr::addr_of!(y);
}
36 changes: 30 additions & 6 deletions src/test/ui/consts/qualif-indirect-mutation-fail.stderr
Original file line number Diff line number Diff line change
@@ -1,33 +1,57 @@
error[E0493]: destructors cannot be evaluated at compile-time
--> $DIR/qualif-indirect-mutation-fail.rs:8:9
--> $DIR/qualif-indirect-mutation-fail.rs:9:9
|
LL | let mut a: (u32, Option<String>) = (0, None);
| ^^^^^ constant functions cannot evaluate destructors

error[E0493]: destructors cannot be evaluated at compile-time
--> $DIR/qualif-indirect-mutation-fail.rs:14:9
--> $DIR/qualif-indirect-mutation-fail.rs:15:9
|
LL | let mut x = None;
| ^^^^^ constants cannot evaluate destructors

error[E0493]: destructors cannot be evaluated at compile-time
--> $DIR/qualif-indirect-mutation-fail.rs:30:9
--> $DIR/qualif-indirect-mutation-fail.rs:31:9
|
LL | let _z = x;
| ^^ constants cannot evaluate destructors

error[E0493]: destructors cannot be evaluated at compile-time
--> $DIR/qualif-indirect-mutation-fail.rs:35:9
--> $DIR/qualif-indirect-mutation-fail.rs:36:9
|
LL | let x: Option<T> = None;
| ^ constant functions cannot evaluate destructors

error[E0493]: destructors cannot be evaluated at compile-time
--> $DIR/qualif-indirect-mutation-fail.rs:43:9
--> $DIR/qualif-indirect-mutation-fail.rs:44:9
|
LL | let _y = x;
| ^^ constant functions cannot evaluate destructors

error: aborting due to 5 previous errors
error[E0493]: destructors cannot be evaluated at compile-time
--> $DIR/qualif-indirect-mutation-fail.rs:52:9
|
LL | let mut y: Option<String> = None;
| ^^^^^ constant functions cannot evaluate destructors

error[E0493]: destructors cannot be evaluated at compile-time
--> $DIR/qualif-indirect-mutation-fail.rs:49:9
|
LL | let mut x: Option<String> = None;
| ^^^^^ constant functions cannot evaluate destructors

error[E0493]: destructors cannot be evaluated at compile-time
--> $DIR/qualif-indirect-mutation-fail.rs:62:9
|
LL | let y: Option<String> = None;
| ^ constant functions cannot evaluate destructors

error[E0493]: destructors cannot be evaluated at compile-time
--> $DIR/qualif-indirect-mutation-fail.rs:59:9
|
LL | let x: Option<String> = None;
| ^ constant functions cannot evaluate destructors

error: aborting due to 9 previous errors

For more information about this error, try `rustc --explain E0493`.
Loading