Remove MaybeMutBorrowedLocals

This commit is contained in:
Tomasz Miąsko 2021-10-29 00:00:00 +00:00
parent 473eaa42e9
commit b285e0c5d8
7 changed files with 26 additions and 207 deletions

View File

@ -110,6 +110,15 @@ where
}
}
/// `&` 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: mir::Place<'tcx>) -> bool {
!place
.ty(self.ccx.body, self.ccx.tcx)

View File

@ -3,25 +3,14 @@ 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,
}
@ -29,29 +18,11 @@ 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 }
MaybeBorrowedLocals { 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,
}
}
}
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
@ -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
@ -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(
@ -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);
@ -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);
}
}
@ -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),
}
}
}

View File

@ -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};

View File

@ -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};
@ -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();
@ -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,

View File

@ -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,

View File

@ -1,48 +0,0 @@
// compile-flags: -Zunleash-the-miri-inside-of-you
// This test demonstrates a shortcoming of the `MaybeMutBorrowedLocals` analysis. It does not
// handle code that takes a reference to one field of a struct, then use pointer arithmetic to
// transform it to another field of that same struct that may have interior mutability. For now,
// this is UB, but this may change in the future. See [rust-lang/unsafe-code-guidelines#134].
//
// [rust-lang/unsafe-code-guidelines#134]: https://github.com/rust-lang/unsafe-code-guidelines/issues/134
#![feature(core_intrinsics, rustc_attrs, const_raw_ptr_deref)]
use std::cell::UnsafeCell;
use std::intrinsics::rustc_peek;
#[repr(C)]
struct PartialInteriorMut {
zst: [i32; 0],
cell: UnsafeCell<i32>,
}
#[rustc_mir(rustc_peek_indirectly_mutable,stop_after_dataflow)]
const BOO: i32 = {
let x = PartialInteriorMut {
zst: [],
cell: UnsafeCell::new(0),
};
let p_zst: *const _ = &x.zst ; // Doesn't cause `x` to get marked as indirectly mutable.
let rmut_cell = unsafe {
// Take advantage of the fact that `zst` and `cell` are at the same location in memory.
// This trick would work with any size type if miri implemented `ptr::offset`.
let p_cell = p_zst as *const UnsafeCell<i32>;
let pmut_cell = (*p_cell).get();
&mut *pmut_cell
};
*rmut_cell = 42; // Mutates `x` indirectly even though `x` is not marked indirectly mutable!!!
let val = *rmut_cell;
rustc_peek(x); //~ ERROR rustc_peek: bit not set
val
};
fn main() {
println!("{}", BOO);
}

View File

@ -1,10 +0,0 @@
error: rustc_peek: bit not set
--> $DIR/indirect-mutation-offset.rs:41:5
|
LL | rustc_peek(x);
| ^^^^^^^^^^^^^
error: stop_after_dataflow ended compilation
error: aborting due to 2 previous errors