bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #69113 - ecstatic-morse:unified-dataflow-borrowed, r=wesleywiser

Browse source 
Combine `HaveBeenBorrowedLocals` and `IndirectlyMutableLocals` into one dataflow analysis

This PR began as an attempt to port `HaveBeenBorrowedLocals` to the new dataflow framework (see #68241 for prior art). Along the way, I noticed that it could share most of its code with `IndirectlyMutableLocals` and then found a few bugs in the two analyses:
- Neither one marked locals as borrowed after an `Rvalue::AddressOf`.
- `IndirectlyMutableLocals` was missing a minor fix that `HaveBeenBorrowedLocals` got in #61069. This is not a problem today since it is only used during const-checking, where custom drop glue is forbidden. However, this may change some day.

I decided to combine the two analyses so that they wouldn't diverge in the future while ensuring that they remain distinct types (called `MaybeBorrowedLocals` and `MaybeMutBorrowedLocals` to be consistent with the `Maybe{Un,}InitializedPlaces` naming scheme). I fixed the bugs and switched to exhaustive matching where possible to make them less likely in the future. Finally, I added comments explaining some of the finer points of the transfer function for these analyses (see #61069 and #65006).
This commit is contained in:
bors 2020-02-19 04:57:10 +00:00
commit 3a8108d8e5
12 changed files with 314 additions and 290 deletions
Download patch file Download diff file Expand all files Collapse all files

11
src/librustc_mir/dataflow/generic/mod.rs
View file

@ -395,5 +395,16 @@ impl<T: Idx> GenKill<T> for BitSet<T> {
}
}
// For compatibility with old framework
impl<T: Idx> GenKill<T> for crate::dataflow::GenKillSet<T> {
fn gen(&mut self, elem: T) {
self.gen(elem);
}
fn kill(&mut self, elem: T) {
self.kill(elem);
}
}
#[cfg(test)]
mod tests;

312
src/librustc_mir/dataflow/impls/borrowed_locals.rs
View file

@ -1,102 +1,274 @@
pub use super::*;
use crate::dataflow::{BitDenotation, GenKillSet};
use crate::dataflow::generic::{AnalysisDomain, GenKill, GenKillAnalysis};
use rustc::mir::visit::Visitor;
use rustc::mir::*;
use rustc::ty::{ParamEnv, TyCtxt};
use rustc_span::DUMMY_SP;
/// This calculates if any part of a MIR local could have previously been borrowed.
/// This means that once a local has been borrowed, its bit will be set
/// from that point and onwards, until we see a StorageDead statement for the local,
/// at which points there is no memory associated with the local, so it cannot be borrowed.
/// This is used to compute which locals are live during a yield expression for
/// immovable generators.
#[derive(Copy, Clone)]
pub struct HaveBeenBorrowedLocals<'a, 'tcx> {
body: &'a Body<'tcx>,
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,
ignore_borrow_on_drop: bool,
}
impl<'a, 'tcx> HaveBeenBorrowedLocals<'a, 'tcx> {
pub fn new(body: &'a Body<'tcx>) -> Self {
HaveBeenBorrowedLocals { body }
}
pub fn body(&self) -> &Body<'tcx> {
self.body
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<'a, 'tcx> BitDenotation<'tcx> for HaveBeenBorrowedLocals<'a, 'tcx> {
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> {
/// 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
/// parameter. In the general case, a drop impl could launder that reference into the
/// surrounding environment through a raw pointer, thus creating a valid `*mut` pointing to the
/// dropped local. We are not yet willing to declare this particular case UB, so we must treat
/// all dropped locals as mutably borrowed for now. See discussion on [#61069].
///
/// In some contexts, we know that this borrow will never occur. For example, during
/// const-eval, custom drop glue cannot be run. Code that calls this should document the
/// assumptions that justify ignoring `Drop` terminators in this way.
///
/// [#61069]: https://github.com/rust-lang/rust/pull/61069
pub fn unsound_ignore_borrow_on_drop(self) -> Self {
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,
}
}
}
impl<K> AnalysisDomain<'tcx> for MaybeBorrowedLocals<K>
where
K: BorrowAnalysisKind<'tcx>,
{
type Idx = Local;
fn name() -> &'static str {
"has_been_borrowed_locals"
}
fn bits_per_block(&self) -> usize {
self.body.local_decls.len()
const NAME: &'static str = K::ANALYSIS_NAME;
fn bits_per_block(&self, body: &mir::Body<'tcx>) -> usize {
body.local_decls().len()
}
fn start_block_effect(&self, _on_entry: &mut BitSet<Local>) {
// Nothing is borrowed on function entry
fn initialize_start_block(&self, _: &mir::Body<'tcx>, _: &mut BitSet<Self::Idx>) {
// No locals are aliased on function entry
}
}
fn statement_effect(&self, trans: &mut GenKillSet<Local>, loc: Location) {
let stmt = &self.body[loc.block].statements[loc.statement_index];
BorrowedLocalsVisitor { trans }.visit_statement(stmt, loc);
// StorageDead invalidates all borrows and raw pointers to a local
match stmt.kind {
StatementKind::StorageDead(l) => trans.kill(l),
_ => (),
}
}
fn terminator_effect(&self, trans: &mut GenKillSet<Local>, loc: Location) {
let terminator = self.body[loc.block].terminator();
BorrowedLocalsVisitor { trans }.visit_terminator(terminator, loc);
match &terminator.kind {
// Drop terminators borrows the location
TerminatorKind::Drop { location, .. }
| TerminatorKind::DropAndReplace { location, .. } => {
if let Some(local) = find_local(location) {
trans.gen(local);
}
}
_ => (),
}
}
fn propagate_call_return(
impl<K> GenKillAnalysis<'tcx> for MaybeBorrowedLocals<K>
where
K: BorrowAnalysisKind<'tcx>,
{
fn statement_effect(
&self,
_in_out: &mut BitSet<Local>,
_call_bb: mir::BasicBlock,
_dest_bb: mir::BasicBlock,
trans: &mut impl GenKill<Self::Idx>,
statement: &mir::Statement<'tcx>,
location: Location,
) {
self.transfer_function(trans).visit_statement(statement, location);
}
fn terminator_effect(
&self,
trans: &mut impl GenKill<Self::Idx>,
terminator: &mir::Terminator<'tcx>,
location: Location,
) {
self.transfer_function(trans).visit_terminator(terminator, location);
}
fn call_return_effect(
&self,
_trans: &mut impl GenKill<Self::Idx>,
_block: mir::BasicBlock,
_func: &mir::Operand<'tcx>,
_args: &[mir::Operand<'tcx>],
_dest_place: &mir::Place<'tcx>,
) {
// Nothing to do when a call returns successfully
}
}
impl<'a, 'tcx> BottomValue for HaveBeenBorrowedLocals<'a, 'tcx> {
impl<K> BottomValue for MaybeBorrowedLocals<K> {
// bottom = unborrowed
const BOTTOM_VALUE: bool = false;
}
struct BorrowedLocalsVisitor<'gk> {
trans: &'gk mut GenKillSet<Local>,
/// A `Visitor` that defines the transfer function for `MaybeBorrowedLocals`.
struct TransferFunction<'a, T, K> {
trans: &'a mut T,
kind: &'a K,
ignore_borrow_on_drop: bool,
}
fn find_local(place: &Place<'_>) -> Option<Local> {
if !place.is_indirect() { Some(place.local) } else { None }
}
impl<T, K> Visitor<'tcx> for TransferFunction<'a, T, K>
where
T: GenKill<Local>,
K: BorrowAnalysisKind<'tcx>,
{
fn visit_statement(&mut self, stmt: &Statement<'tcx>, location: Location) {
self.super_statement(stmt, location);
impl<'tcx> Visitor<'tcx> for BorrowedLocalsVisitor<'_> {
fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) {
if let Rvalue::Ref(_, _, ref place) = *rvalue {
if let Some(local) = find_local(place) {
self.trans.gen(local);
}
// When we reach a `StorageDead` statement, we can assume that any pointers to this memory
// are now invalid.
if let StatementKind::StorageDead(local) = stmt.kind {
self.trans.kill(local);
}
}
self.super_rvalue(rvalue, location)
fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'tcx>, location: Location) {
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) {
self.trans.gen(borrowed_place.local);
}
}
mir::Rvalue::Ref(_, kind, borrowed_place) => {
if !borrowed_place.is_indirect() && self.kind.in_ref(*kind, borrowed_place) {
self.trans.gen(borrowed_place.local);
}
}
mir::Rvalue::Cast(..)
| mir::Rvalue::Use(..)
| mir::Rvalue::Repeat(..)
| mir::Rvalue::Len(..)
| mir::Rvalue::BinaryOp(..)
| mir::Rvalue::CheckedBinaryOp(..)
| mir::Rvalue::NullaryOp(..)
| mir::Rvalue::UnaryOp(..)
| mir::Rvalue::Discriminant(..)
| mir::Rvalue::Aggregate(..) => {}
}
}
fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) {
self.super_terminator(terminator, location);
match terminator.kind {
mir::TerminatorKind::Drop { location: dropped_place, .. }
| mir::TerminatorKind::DropAndReplace { location: dropped_place, .. } => {
// See documentation for `unsound_ignore_borrow_on_drop` for an explanation.
if !self.ignore_borrow_on_drop {
self.trans.gen(dropped_place.local);
}
}
TerminatorKind::Abort
| TerminatorKind::Assert { .. }
| TerminatorKind::Call { .. }
| TerminatorKind::FalseEdges { .. }
| TerminatorKind::FalseUnwind { .. }
| TerminatorKind::GeneratorDrop
| TerminatorKind::Goto { .. }
| TerminatorKind::Resume
| TerminatorKind::Return
| TerminatorKind::SwitchInt { .. }
| TerminatorKind::Unreachable
| TerminatorKind::Yield { .. } => {}
}
}
}
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, self.param_env, DUMMY_SP)
}
}
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),
}
}
}

136
src/librustc_mir/dataflow/impls/indirect_mutation.rs
View file

@ -1,136 +0,0 @@
use rustc::mir::visit::Visitor;
use rustc::mir::{self, Local, Location};
use rustc::ty::{self, TyCtxt};
use rustc_index::bit_set::BitSet;
use rustc_span::DUMMY_SP;
use crate::dataflow::{self, GenKillSet};
/// Whether a borrow to a `Local` has been created that could allow that `Local` to be mutated
/// indirectly. This could either be a mutable reference (`&mut`) or a shared borrow if the type of
/// that `Local` allows interior mutability. Operations that can mutate local's indirectly include:
/// assignments through a pointer (`*p = 42`), function calls, drop terminators and inline assembly.
///
/// If this returns false for a `Local` at a given statement (or terminator), that `Local` could
/// not possibly have been mutated indirectly prior to that statement.
#[derive(Copy, Clone)]
pub struct IndirectlyMutableLocals<'mir, 'tcx> {
body: &'mir mir::Body<'tcx>,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
}
impl<'mir, 'tcx> IndirectlyMutableLocals<'mir, 'tcx> {
pub fn new(
tcx: TyCtxt<'tcx>,
body: &'mir mir::Body<'tcx>,
param_env: ty::ParamEnv<'tcx>,
) -> Self {
IndirectlyMutableLocals { body, tcx, param_env }
}
fn transfer_function<'a>(
&self,
trans: &'a mut GenKillSet<Local>,
) -> TransferFunction<'a, 'mir, 'tcx> {
TransferFunction { body: self.body, tcx: self.tcx, param_env: self.param_env, trans }
}
}
impl<'mir, 'tcx> dataflow::BitDenotation<'tcx> for IndirectlyMutableLocals<'mir, 'tcx> {
type Idx = Local;
fn name() -> &'static str {
"mut_borrowed_locals"
}
fn bits_per_block(&self) -> usize {
self.body.local_decls.len()
}
fn start_block_effect(&self, _entry_set: &mut BitSet<Local>) {
// Nothing is borrowed on function entry
}
fn statement_effect(&self, trans: &mut GenKillSet<Local>, loc: Location) {
let stmt = &self.body[loc.block].statements[loc.statement_index];
self.transfer_function(trans).visit_statement(stmt, loc);
}
fn terminator_effect(&self, trans: &mut GenKillSet<Local>, loc: Location) {
let terminator = self.body[loc.block].terminator();
self.transfer_function(trans).visit_terminator(terminator, loc);
}
fn propagate_call_return(
&self,
_in_out: &mut BitSet<Local>,
_call_bb: mir::BasicBlock,
_dest_bb: mir::BasicBlock,
_dest_place: &mir::Place<'tcx>,
) {
// Nothing to do when a call returns successfully
}
}
impl<'mir, 'tcx> dataflow::BottomValue for IndirectlyMutableLocals<'mir, 'tcx> {
// bottom = unborrowed
const BOTTOM_VALUE: bool = false;
}
/// A `Visitor` that defines the transfer function for `IndirectlyMutableLocals`.
struct TransferFunction<'a, 'mir, 'tcx> {
trans: &'a mut GenKillSet<Local>,
body: &'mir mir::Body<'tcx>,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
}
impl<'tcx> TransferFunction<'_, '_, 'tcx> {
/// Returns `true` if this borrow would allow mutation of the `borrowed_place`.
fn borrow_allows_mutation(
&self,
kind: mir::BorrowKind,
borrowed_place: &mir::Place<'tcx>,
) -> bool {
match kind {
mir::BorrowKind::Mut { .. } => true,
mir::BorrowKind::Shared | mir::BorrowKind::Shallow | mir::BorrowKind::Unique => {
!borrowed_place.ty(self.body, self.tcx).ty.is_freeze(
self.tcx,
self.param_env,
DUMMY_SP,
)
}
}
}
}
impl<'tcx> Visitor<'tcx> for TransferFunction<'_, '_, 'tcx> {
fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'tcx>, location: Location) {
if let mir::Rvalue::Ref(_, kind, ref borrowed_place) = *rvalue {
if self.borrow_allows_mutation(kind, borrowed_place) {
if !borrowed_place.is_indirect() {
self.trans.gen(borrowed_place.local);
}
}
}
self.super_rvalue(rvalue, location);
}
fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) {
// This method purposely does nothing except call `super_terminator`. It exists solely to
// document the subtleties around drop terminators.
self.super_terminator(terminator, location);
if let mir::TerminatorKind::Drop { location: _, .. }
| mir::TerminatorKind::DropAndReplace { location: _, .. } = &terminator.kind
{
// Although drop terminators mutably borrow the location being dropped, that borrow
// cannot live beyond the drop terminator because the dropped location is invalidated.
}
}
}

2
src/librustc_mir/dataflow/impls/mod.rs
View file

@ -20,11 +20,9 @@ use super::drop_flag_effects_for_location;
use super::on_lookup_result_bits;
mod borrowed_locals;
mod indirect_mutation;
mod storage_liveness;
pub use self::borrowed_locals::*;
pub use self::indirect_mutation::IndirectlyMutableLocals;
pub use self::storage_liveness::*;
pub(super) mod borrows;

46
src/librustc_mir/dataflow/impls/storage_liveness.rs
View file

@ -1,8 +1,8 @@
pub use super::*;
use crate::dataflow::generic::{Results, ResultsRefCursor};
use crate::dataflow::BitDenotation;
use crate::dataflow::HaveBeenBorrowedLocals;
use crate::dataflow::{DataflowResults, DataflowResultsCursor, DataflowResultsRefCursor};
use crate::dataflow::MaybeBorrowedLocals;
use rustc::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor};
use rustc::mir::*;
use std::cell::RefCell;
@ -69,22 +69,23 @@ impl<'a, 'tcx> BottomValue for MaybeStorageLive<'a, 'tcx> {
const BOTTOM_VALUE: bool = false;
}
type BorrowedLocalsResults<'a, 'tcx> = ResultsRefCursor<'a, 'a, 'tcx, MaybeBorrowedLocals>;
/// Dataflow analysis that determines whether each local requires storage at a
/// given location; i.e. whether its storage can go away without being observed.
pub struct RequiresStorage<'mir, 'tcx> {
body: ReadOnlyBodyAndCache<'mir, 'tcx>,
borrowed_locals:
RefCell<DataflowResultsRefCursor<'mir, 'tcx, HaveBeenBorrowedLocals<'mir, 'tcx>>>,
borrowed_locals: RefCell<BorrowedLocalsResults<'mir, 'tcx>>,
}
impl<'mir, 'tcx: 'mir> RequiresStorage<'mir, 'tcx> {
pub fn new(
body: ReadOnlyBodyAndCache<'mir, 'tcx>,
borrowed_locals: &'mir DataflowResults<'tcx, HaveBeenBorrowedLocals<'mir, 'tcx>>,
borrowed_locals: &'mir Results<'tcx, MaybeBorrowedLocals>,
) -> Self {
RequiresStorage {
body,
borrowed_locals: RefCell::new(DataflowResultsCursor::new(borrowed_locals, *body)),
borrowed_locals: RefCell::new(ResultsRefCursor::new(*body, borrowed_locals)),
}
}
@ -111,11 +112,12 @@ impl<'mir, 'tcx> BitDenotation<'tcx> for RequiresStorage<'mir, 'tcx> {
}
fn before_statement_effect(&self, sets: &mut GenKillSet<Self::Idx>, loc: Location) {
// If we borrow or assign to a place then it needs storage for that
// statement.
self.check_for_borrow(sets, loc);
let stmt = &self.body[loc.block].statements[loc.statement_index];
// If a place is borrowed in a statement, it needs storage for that statement.
self.borrowed_locals.borrow().analysis().statement_effect(sets, stmt, loc);
// If a place is assigned to in a statement, it needs storage for that statement.
match stmt.kind {
StatementKind::StorageDead(l) => sets.kill(l),
StatementKind::Assign(box (ref place, _))
@ -138,12 +140,13 @@ impl<'mir, 'tcx> BitDenotation<'tcx> for RequiresStorage<'mir, 'tcx> {
}
fn before_terminator_effect(&self, sets: &mut GenKillSet<Local>, loc: Location) {
self.check_for_borrow(sets, loc);
let terminator = self.body[loc.block].terminator();
if let TerminatorKind::Call { destination: Some((Place { local, .. }, _)), .. } =
self.body[loc.block].terminator().kind
{
sets.gen(local);
// If a place is borrowed in a terminator, it needs storage for that terminator.
self.borrowed_locals.borrow().analysis().terminator_effect(sets, terminator, loc);
if let TerminatorKind::Call { destination: Some((place, _)), .. } = terminator.kind {
sets.gen(place.local);
}
}
@ -179,14 +182,6 @@ impl<'mir, 'tcx> RequiresStorage<'mir, 'tcx> {
let mut visitor = MoveVisitor { sets, borrowed_locals: &self.borrowed_locals };
visitor.visit_location(self.body, loc);
}
/// Gen locals that are newly borrowed. This includes borrowing any part of
/// a local (we rely on this behavior of `HaveBeenBorrowedLocals`).
fn check_for_borrow(&self, sets: &mut GenKillSet<Local>, loc: Location) {
let mut borrowed_locals = self.borrowed_locals.borrow_mut();
borrowed_locals.seek(loc);
borrowed_locals.each_gen_bit(|l| sets.gen(l));
}
}
impl<'mir, 'tcx> BottomValue for RequiresStorage<'mir, 'tcx> {
@ -195,8 +190,7 @@ impl<'mir, 'tcx> BottomValue for RequiresStorage<'mir, 'tcx> {
}
struct MoveVisitor<'a, 'mir, 'tcx> {
borrowed_locals:
&'a RefCell<DataflowResultsRefCursor<'mir, 'tcx, HaveBeenBorrowedLocals<'mir, 'tcx>>>,
borrowed_locals: &'a RefCell<BorrowedLocalsResults<'mir, 'tcx>>,
sets: &'a mut GenKillSet<Local>,
}
@ -204,7 +198,7 @@ impl<'a, 'mir: 'a, 'tcx> Visitor<'tcx> for MoveVisitor<'a, 'mir, 'tcx> {
fn visit_local(&mut self, local: &Local, context: PlaceContext, loc: Location) {
if PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) == context {
let mut borrowed_locals = self.borrowed_locals.borrow_mut();
borrowed_locals.seek(loc);
borrowed_locals.seek_before(loc);
if !borrowed_locals.contains(*local) {
self.sets.kill(*local);
}

3
src/librustc_mir/dataflow/mod.rs
View file

@ -23,8 +23,7 @@ pub(crate) use self::drop_flag_effects::*;
pub use self::impls::borrows::Borrows;
pub use self::impls::DefinitelyInitializedPlaces;
pub use self::impls::EverInitializedPlaces;
pub use self::impls::HaveBeenBorrowedLocals;
pub use self::impls::IndirectlyMutableLocals;
pub use self::impls::{MaybeBorrowedLocals, MaybeMutBorrowedLocals};
pub use self::impls::{MaybeInitializedPlaces, MaybeUninitializedPlaces};
pub use self::impls::{MaybeStorageLive, RequiresStorage};

2
src/librustc_mir/transform/check_consts/resolver.rs
View file

@ -15,7 +15,7 @@ use crate::dataflow::{self as old_dataflow, generic as dataflow};
/// `FlowSensitiveAnalysis`.
///
/// This transfer does nothing when encountering an indirect assignment. Consumers should rely on
/// the `IndirectlyMutableLocals` dataflow pass to see if a `Local` may have become qualified via
/// the `MaybeMutBorrowedLocals` dataflow pass to see if a `Local` may have become qualified via
/// an indirect assignment or function call.
struct TransferFunction<'a, 'mir, 'tcx, Q> {
item: &'a Item<'mir, 'tcx>,

37
src/librustc_mir/transform/check_consts/validation.rs
View file

@ -16,17 +16,19 @@ use rustc_span::Span;
use std::borrow::Cow;
use std::ops::Deref;
use self::old_dataflow::IndirectlyMutableLocals;
use super::ops::{self, NonConstOp};
use super::qualifs::{self, HasMutInterior, NeedsDrop};
use super::resolver::FlowSensitiveAnalysis;
use super::{is_lang_panic_fn, ConstKind, Item, Qualif};
use crate::const_eval::{is_const_fn, is_unstable_const_fn};
use crate::dataflow::{self as old_dataflow, generic as dataflow};
use dataflow::Analysis;
use crate::dataflow::generic::{self as dataflow, Analysis};
use crate::dataflow::MaybeMutBorrowedLocals;
// We are using `MaybeMutBorrowedLocals` as a proxy for whether an item may have been mutated
// through a pointer prior to the given point. This is okay even though `MaybeMutBorrowedLocals`
// kills locals upon `StorageDead` because a local will never be used after a `StorageDead`.
pub type IndirectlyMutableResults<'mir, 'tcx> =
old_dataflow::DataflowResultsCursor<'mir, 'tcx, IndirectlyMutableLocals<'mir, 'tcx>>;
dataflow::ResultsCursor<'mir, 'tcx, MaybeMutBorrowedLocals<'mir, 'tcx>>;
struct QualifCursor<'a, 'mir, 'tcx, Q: Qualif> {
cursor: dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'a, 'mir, 'tcx, Q>>,
@ -59,7 +61,7 @@ pub struct Qualifs<'a, 'mir, 'tcx> {
impl Qualifs<'a, 'mir, 'tcx> {
fn indirectly_mutable(&mut self, local: Local, location: Location) -> bool {
self.indirectly_mutable.seek(location);
self.indirectly_mutable.seek_before(location);
self.indirectly_mutable.get().contains(local)
}
@ -135,22 +137,21 @@ impl Deref for Validator<'_, 'mir, 'tcx> {
impl Validator<'a, 'mir, 'tcx> {
pub fn new(item: &'a Item<'mir, 'tcx>) -> Self {
let Item { tcx, body, def_id, param_env, .. } = *item;
let needs_drop = QualifCursor::new(NeedsDrop, item);
let has_mut_interior = QualifCursor::new(HasMutInterior, item);
let dead_unwinds = BitSet::new_empty(item.body.basic_blocks().len());
let indirectly_mutable = old_dataflow::do_dataflow(
item.tcx,
&*item.body,
item.def_id,
&item.tcx.get_attrs(item.def_id),
&dead_unwinds,
old_dataflow::IndirectlyMutableLocals::new(item.tcx, *item.body, item.param_env),
|_, local| old_dataflow::DebugFormatted::new(&local),
);
let indirectly_mutable =
old_dataflow::DataflowResultsCursor::new(indirectly_mutable, *item.body);
// We can use `unsound_ignore_borrow_on_drop` here because custom drop impls are not
// allowed in a const.
//
// FIXME(ecstaticmorse): Someday we want to allow custom drop impls. How do we do this
// without breaking stable code?
let indirectly_mutable = MaybeMutBorrowedLocals::mut_borrows_only(tcx, *body, param_env)
.unsound_ignore_borrow_on_drop()
.into_engine(tcx, *body, def_id)
.iterate_to_fixpoint()
.into_results_cursor(*body);
let qualifs = Qualifs { needs_drop, has_mut_interior, indirectly_mutable };

20
src/librustc_mir/transform/generator.rs
View file

@ -49,9 +49,10 @@
//! For generators with state 1 (returned) and state 2 (poisoned) it does nothing.
//! Otherwise it drops all the values in scope at the last suspension point.
use crate::dataflow::generic::{Analysis, ResultsCursor};
use crate::dataflow::{do_dataflow, DataflowResultsCursor, DebugFormatted};
use crate::dataflow::{DataflowResults, DataflowResultsConsumer, FlowAtLocation};
use crate::dataflow::{HaveBeenBorrowedLocals, MaybeStorageLive, RequiresStorage};
use crate::dataflow::{MaybeBorrowedLocals, MaybeStorageLive, RequiresStorage};
use crate::transform::no_landing_pads::no_landing_pads;
use crate::transform::simplify;
use crate::transform::{MirPass, MirSource};
@ -471,17 +472,10 @@ fn locals_live_across_suspend_points(
// Calculate the MIR locals which have been previously
// borrowed (even if they are still active).
let borrowed_locals_analysis = HaveBeenBorrowedLocals::new(body_ref);
let borrowed_locals_results = do_dataflow(
tcx,
body_ref,
def_id,
&[],
&dead_unwinds,
borrowed_locals_analysis,
|bd, p| DebugFormatted::new(&bd.body().local_decls[p]),
);
let mut borrowed_locals_cursor = DataflowResultsCursor::new(&borrowed_locals_results, body_ref);
let borrowed_locals_results =
MaybeBorrowedLocals::all_borrows().into_engine(tcx, body_ref, def_id).iterate_to_fixpoint();
let mut borrowed_locals_cursor = ResultsCursor::new(body_ref, &borrowed_locals_results);
// Calculate the MIR locals that we actually need to keep storage around
// for.
@ -521,7 +515,7 @@ fn locals_live_across_suspend_points(
// If a borrow is converted to a raw reference, we must also assume that it lives
// forever. Note that the final liveness is still bounded by the storage liveness
// of the local, which happens using the `intersect` operation below.
borrowed_locals_cursor.seek(loc);
borrowed_locals_cursor.seek_before(loc);
liveness.outs[block].union(borrowed_locals_cursor.get());
}

26
src/librustc_mir/transform/rustc_peek.rs
View file

@ -12,9 +12,8 @@ use rustc_index::bit_set::BitSet;
use crate::dataflow::generic::{Analysis, Results, ResultsCursor};
use crate::dataflow::move_paths::{HasMoveData, MoveData};
use crate::dataflow::move_paths::{LookupResult, MovePathIndex};
use crate::dataflow::IndirectlyMutableLocals;
use crate::dataflow::MaybeMutBorrowedLocals;
use crate::dataflow::MoveDataParamEnv;
use crate::dataflow::{do_dataflow, DebugFormatted};
use crate::dataflow::{
DefinitelyInitializedPlaces, MaybeInitializedPlaces, MaybeUninitializedPlaces,
};
@ -24,7 +23,6 @@ pub struct SanityCheck;
impl<'tcx> MirPass<'tcx> for SanityCheck {
fn run_pass(&self, tcx: TyCtxt<'tcx>, src: MirSource<'tcx>, body: &mut BodyAndCache<'tcx>) {
use crate::dataflow::has_rustc_mir_with;
let def_id = src.def_id();
if !tcx.has_attr(def_id, sym::rustc_mir) {
debug!("skipping rustc_peek::SanityCheck on {}", tcx.def_path_str(def_id));
@ -37,7 +35,6 @@ impl<'tcx> MirPass<'tcx> for SanityCheck {
let param_env = tcx.param_env(def_id);
let move_data = MoveData::gather_moves(body, tcx, param_env).unwrap();
let mdpe = MoveDataParamEnv { move_data: move_data, param_env: param_env };
let dead_unwinds = BitSet::new_empty(body.basic_blocks().len());
let flow_inits = MaybeInitializedPlaces::new(tcx, body, &mdpe)
.into_engine(tcx, body, def_id)
@ -48,15 +45,9 @@ impl<'tcx> MirPass<'tcx> for SanityCheck {
let flow_def_inits = DefinitelyInitializedPlaces::new(tcx, body, &mdpe)
.into_engine(tcx, body, def_id)
.iterate_to_fixpoint();
let _flow_indirectly_mut = do_dataflow(
tcx,
body,
def_id,
&attributes,
&dead_unwinds,
IndirectlyMutableLocals::new(tcx, body, param_env),
|_, i| DebugFormatted::new(&i),
);
let flow_mut_borrowed = MaybeMutBorrowedLocals::mut_borrows_only(tcx, body, param_env)
.into_engine(tcx, body, def_id)
.iterate_to_fixpoint();
if has_rustc_mir_with(&attributes, sym::rustc_peek_maybe_init).is_some() {
sanity_check_via_rustc_peek(tcx, body, def_id, &attributes, &flow_inits);
@ -67,12 +58,9 @@ impl<'tcx> MirPass<'tcx> for SanityCheck {
if has_rustc_mir_with(&attributes, sym::rustc_peek_definite_init).is_some() {
sanity_check_via_rustc_peek(tcx, body, def_id, &attributes, &flow_def_inits);
}
// FIXME: Uncomment these as analyses are migrated to the new framework
/*
if has_rustc_mir_with(&attributes, sym::rustc_peek_indirectly_mutable).is_some() {
sanity_check_via_rustc_peek(tcx, body, def_id, &attributes, &flow_indirectly_mut);
sanity_check_via_rustc_peek(tcx, body, def_id, &attributes, &flow_mut_borrowed);
}
*/
if has_rustc_mir_with(&attributes, sym::stop_after_dataflow).is_some() {
tcx.sess.fatal("stop_after_dataflow ended compilation");
}
@ -276,8 +264,7 @@ where
}
}
/* FIXME: Add this back once `IndirectlyMutableLocals` uses the new dataflow framework.
impl<'tcx> RustcPeekAt<'tcx> for IndirectlyMutableLocals<'_, 'tcx> {
impl<'tcx> RustcPeekAt<'tcx> for MaybeMutBorrowedLocals<'_, 'tcx> {
fn peek_at(
&self,
tcx: TyCtxt<'tcx>,
@ -298,4 +285,3 @@ impl<'tcx> RustcPeekAt<'tcx> for IndirectlyMutableLocals<'_, 'tcx> {
}
}
}
*/

7
src/test/ui/mir-dataflow/indirect-mutation-offset.rs
View file

@ -1,6 +1,11 @@
// compile-flags: -Zunleash-the-miri-inside-of-you
// ignore-test Temporarily ignored while this analysis is migrated to the new framework.
// 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)]

2
src/test/ui/mir-dataflow/indirect-mutation-offset.stderr
View file

@ -1,5 +1,5 @@
error: rustc_peek: bit not set
--> $DIR/indirect-mutation-offset.rs:34:14
--> $DIR/indirect-mutation-offset.rs:41:14
|
LL | unsafe { rustc_peek(x) };
| ^^^^^^^^^^^^^