Skip to content

Commit

Permalink
Lazily check for conflicts
Browse files Browse the repository at this point in the history
  • Loading branch information
@willcrichton
willcrichton committed Aug 25, 2023
1 parent 77ae052 commit c14fd5c
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 135 deletions.
159 changes: 71 additions & 88 deletions crates/flowistry/src/infoflow/analysis.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{cell::RefCell, rc::Rc};

use log::{debug, trace};
use rustc_data_structures::fx::{FxHashMap as HashMap, FxHashSet as HashSet};
use rustc_data_structures::fx::FxHashMap as HashMap;
use rustc_hir::{def_id::DefId, BodyId};
use rustc_middle::{
mir::{visit::Visitor, *},
Expand All @@ -11,6 +11,7 @@ use rustc_mir_dataflow::{Analysis, AnalysisDomain, Forward};
use rustc_utils::{
mir::control_dependencies::ControlDependencies, BodyExt, OperandExt, PlaceExt,
};
use smallvec::SmallVec;

use super::{
mutation::{ModularMutationVisitor, Mutation, MutationStatus},
Expand All @@ -22,7 +23,6 @@ use crate::{
impls::{LocationOrArg, LocationOrArgDomain, LocationOrArgSet},
IndexMatrix, IndexedDomain,
},
infoflow::mutation::ConflictType,
mir::aliases::Aliases,
};

Expand Down Expand Up @@ -84,6 +84,34 @@ impl<'a, 'tcx> FlowAnalysis<'a, 'tcx> {
self.aliases.location_domain()
}

fn influences(&self, place: Place<'tcx>) -> SmallVec<[Place<'tcx>; 8]> {
let conflicts = self.aliases.conflicts(place);
let provenance = place.refs_in_projection().flat_map(|(place_ref, _)| {
self
.aliases
.aliases(Place::from_ref(place_ref, self.tcx))
.iter()
});
conflicts.iter().chain(provenance).copied().collect()
}

pub fn deps_for(
&self,
state: &FlowDomain<'tcx>,
place: Place<'tcx>,
) -> LocationOrArgSet {
let mut deps = LocationOrArgSet::new(self.location_domain());
for subplace in self
.aliases
.reachable_values(place, Mutability::Not)
.iter()
.flat_map(|place| self.influences(*place))
{
deps.union(&state.row_set(self.aliases.normalize(subplace)));
}
deps
}

// This function expects *ALL* the mutations that occur within a given [`Location`] at once.
pub(crate) fn transfer_function(
&self,
Expand All @@ -94,124 +122,79 @@ impl<'a, 'tcx> FlowAnalysis<'a, 'tcx> {
debug!(" Applying mutations {mutations:?}");
let location_domain = self.location_domain();

let all_aliases = &self.aliases;
let all_mutated_aliases = mutations
.iter()
.map(|mt| {
let mutated_aliases = all_aliases.aliases(mt.mutated);
assert!(!mutated_aliases.is_empty());
trace!(
" Mutated aliases for {:?}: {mutated_aliases:?}",
mt.mutated
);
mutated_aliases
})
.collect::<Vec<_>>();

let mut input_location_deps = (0 .. mutations.len())
.map(|_| {
let mut deps = LocationOrArgSet::new(location_domain);
deps.insert(location);
deps
})
.collect::<Vec<_>>();
// Initialize dependencies to include current location of mutation.
let mut all_deps = {
let mut deps = LocationOrArgSet::new(location_domain);
deps.insert(location);
vec![deps; mutations.len()]
};

let add_deps = |state: &mut FlowDomain<'tcx>,
place: Place<'tcx>,
location_deps: &mut LocationOrArgSet| {
let reachable_values = all_aliases.reachable_values(place, Mutability::Not);
let provenance = place.refs_in_projection().flat_map(|(place_ref, _)| {
all_aliases
.aliases(Place::from_ref(place_ref, self.tcx))
.iter()
});
for relevant in reachable_values.iter().chain(provenance) {
let deps = state.row_set(all_aliases.normalize(*relevant));
trace!(" For relevant {relevant:?} for input {place:?} adding deps {deps:?}");
location_deps.union(&deps);
// Add every influence on `input` to `deps`.
let add_deps = |state: &FlowDomain<'tcx>,
input,
target_deps: &mut LocationOrArgSet| {
for relevant in self.influences(input) {
let relevant_deps = state.row_set(self.aliases.normalize(relevant));
trace!(" For relevant {relevant:?} for input {input:?} adding deps {relevant_deps:?}");
target_deps.union(&relevant_deps);
}
};

for (mt, deps) in mutations.iter().zip(&mut input_location_deps) {
// Add deps of all inputs
// Register every explicitly provided input as an input.
for (mt, deps) in mutations.iter().zip(&mut all_deps) {
for input in &mt.inputs {
add_deps(state, *input, deps);
}
}

// Add control dependencies
// Add location of every control dependency.
let controlled_by = self.control_dependencies.dependent_on(location.block);
let body = self.body;
for block in controlled_by.into_iter().flat_map(|set| set.iter()) {
for deps in &mut input_location_deps {
for deps in &mut all_deps {
deps.insert(body.terminator_loc(block));
}

// Include dependencies of the switch's operand
// Include dependencies of the switch's operand.
let terminator = body.basic_blocks[block].terminator();
if let TerminatorKind::SwitchInt { discr, .. } = &terminator.kind {
if let Some(discr_place) = discr.as_place() {
for deps in &mut input_location_deps {
for deps in &mut all_deps {
add_deps(state, discr_place, deps);
}
}
}
}

// Union dependencies into all conflicting places of the mutated place
let mutable_conflicts = mutations
.iter()
.map(|mt| {
let mut mutable_conflicts = if matches!(mt.conflicts, ConflictType::Exclude) {
all_aliases.aliases(mt.mutated).to_owned()
} else {
all_aliases.conflicts(mt.mutated).to_owned()
};

// Remove any conflicts that aren't actually mutable, e.g. if x : &T ends up
// as an alias of y: &mut T. See test function_lifetime_alias_mut for an example.
let ignore_mut =
is_extension_active(|mode| mode.mutability_mode == MutabilityMode::IgnoreMut);
if !ignore_mut {
let body = self.body;
let tcx = self.tcx;
mutable_conflicts = mutable_conflicts
.iter()
.filter(|place| {
place.iter_projections().all(|(sub_place, _)| {
let ty = sub_place.ty(body.local_decls(), tcx).ty;
!matches!(ty.ref_mutability(), Some(Mutability::Not))
})
})
.copied()
.collect::<HashSet<_>>();
};

mutable_conflicts
})
.collect::<Vec<_>>();

// Clear sub-places of mutated place (if sound to do so)
for (mt, aliases) in mutations.iter().zip(&all_mutated_aliases) {
if matches!(mt.status, MutationStatus::Definitely) && aliases.len() == 1 {
let mutated_direct = aliases.iter().next().unwrap();
for sub in all_aliases.children(*mutated_direct).iter() {
state.clear_row(all_aliases.normalize(*sub));
let ignore_mut =
is_extension_active(|mode| mode.mutability_mode == MutabilityMode::IgnoreMut);
for (mt, deps) in mutations.iter().zip(&mut all_deps) {
// Clear sub-places of mutated place (if sound to do so)
if matches!(mt.status, MutationStatus::Definitely)
&& self.aliases.aliases(mt.mutated).len() == 1
{
for sub in self.aliases.children(mt.mutated).iter() {
state.clear_row(self.aliases.normalize(*sub));
}
}
}

for (mt, deps) in mutations.iter().zip(&mut input_location_deps) {
// Add deps of mutated to include provenance of mutated pointers
add_deps(state, mt.mutated, deps);
}

for (conflicts, deps) in mutable_conflicts.into_iter().zip(input_location_deps) {
debug!(" Mutated conflicting places: {conflicts:?}");
debug!(" Mutated places: {:?}", mt.mutated);
debug!(" with deps {deps:?}");

for place in conflicts.into_iter() {
state.union_into_row(all_aliases.normalize(place), &deps);
let mutable_aliases = self.aliases.aliases(mt.mutated).iter().filter(|alias| {
// Remove any conflicts that aren't actually mutable, e.g. if x : &T ends up
// as an alias of y: &mut T. See test function_lifetime_alias_mut for an example.
let has_immut = alias.iter_projections().any(|(sub_place, _)| {
let ty = sub_place.ty(body.local_decls(), self.tcx).ty;
matches!(ty.ref_mutability(), Some(Mutability::Not))
});
!has_immut || ignore_mut
});
for alias in mutable_aliases {
state.union_into_row(self.aliases.normalize(*alias), deps);
}
}
}
Expand Down
18 changes: 7 additions & 11 deletions crates/flowistry/src/infoflow/dependencies.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,17 +191,13 @@ pub fn compute_dependencies<'tcx>(
let backward = || {
for (targets, outputs) in iter::zip(&all_targets, &mut *outputs.borrow_mut()) {
for (place, location) in targets {
for value in results
.analysis
.aliases
.reachable_values(*place, Mutability::Not)
{
match location {
LocationOrArg::Arg(..) => outputs.insert(location),
LocationOrArg::Location(location) => {
let deps = deps(results.state_at(*location), aliases, *value);
outputs.union(&deps);
}
match location {
LocationOrArg::Arg(..) => outputs.insert(location),
LocationOrArg::Location(location) => {
let deps = results
.analysis
.deps_for(results.state_at(*location), *place);
outputs.union(&deps);
}
}
}
Expand Down
48 changes: 19 additions & 29 deletions crates/flowistry/src/infoflow/mutation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use log::debug;
use rustc_middle::{
mir::{visit::Visitor, *},
ty::TyKind,
ty::{AdtKind, TyKind},
};
use rustc_target::abi::FieldIdx;
use rustc_utils::{mir::place::PlaceCollector, AdtDefExt, OperandExt};
Expand All @@ -24,12 +24,6 @@ pub enum MutationStatus {
Possibly,
}

#[derive(Debug)]
pub enum ConflictType {
Include,
Exclude,
}

/// Information about a particular mutation.
#[derive(Debug)]
pub struct Mutation<'tcx> {
Expand All @@ -41,8 +35,6 @@ pub struct Mutation<'tcx> {

/// The certainty of whether the mutation is happening.
pub status: MutationStatus,

pub conflicts: ConflictType,
}

/// MIR visitor that invokes a callback for every [`Mutation`] in the visited object.
Expand Down Expand Up @@ -88,24 +80,31 @@ where
// then destructure this into a series of mutations like
// _1.field1 = op1, _1.field2 = op2, and so on.
Rvalue::Aggregate(agg_kind, ops) => {
let tys = match &**agg_kind {
let info = match &**agg_kind {
AggregateKind::Adt(def_id, idx, substs, _, _) => {
let adt_def = tcx.adt_def(*def_id);
let variant = adt_def.variant(*idx);
let mutated = match adt_def.adt_kind() {
AdtKind::Enum => mutated.project_deeper(
&[ProjectionElem::Downcast(Some(variant.name), *idx)],
tcx,
),
AdtKind::Struct | AdtKind::Union => *mutated,
};
let fields = variant.fields.iter();
let tys = fields
.map(|field| field.ty(tcx, substs))
.collect::<Vec<_>>();
Some(tys)
Some((mutated, tys))
}
AggregateKind::Tuple => {
let ty = rvalue.ty(body.local_decls(), tcx);
Some(ty.tuple_fields().to_vec())
Some((*mutated, ty.tuple_fields().to_vec()))
}
_ => None,
};

if let Some(tys) = tys {
if let Some((mutated, tys)) = info {
if tys.len() > 0 {
let fields =
tys
Expand All @@ -123,7 +122,6 @@ where
mutated,
inputs: input.into_iter().collect::<Vec<_>>(),
status: MutationStatus::Definitely,
conflicts: ConflictType::Include,
})
.collect::<Vec<_>>();
(self.f)(location, mutations);
Expand All @@ -145,24 +143,25 @@ where
.map(|(i, field_def)| {
PlaceElem::Field(FieldIdx::from_usize(i), field_def.ty(tcx, substs))
});
let mutations = fields
let mut mutations = fields
.map(|field| {
let mutated_field = mutated.project_deeper(&[field], tcx);
let input_field = place.project_deeper(&[field], tcx);
Mutation {
mutated: mutated_field,
inputs: vec![input_field],
status: MutationStatus::Definitely,
conflicts: ConflictType::Exclude,
}
})
.chain([Mutation {
.collect::<Vec<_>>();

if mutations.is_empty() {
mutations.push(Mutation {
mutated: *mutated,
inputs: vec![*place],
status: MutationStatus::Definitely,
conflicts: ConflictType::Exclude,
}])
.collect::<Vec<_>>();
});
}
(self.f)(location, mutations);
return;
}
Expand All @@ -178,7 +177,6 @@ where
mutated: *mutated,
inputs: collector.0,
status: MutationStatus::Definitely,
conflicts: ConflictType::Include,
}]);
}

Expand Down Expand Up @@ -216,22 +214,14 @@ where
mutated: *destination,
inputs,
status: MutationStatus::Definitely,
conflicts: ConflictType::Include,
}];

for arg in arg_places {
for arg_mut in self.aliases.reachable_values(arg, Mutability::Mut) {
// The argument itself can never be modified in a caller-visible way,
// because it's either getting moved or copied.
if arg == *arg_mut {
continue;
}

mutations.push(Mutation {
mutated: *arg_mut,
inputs: arg_inputs.clone(),
status: MutationStatus::Possibly,
conflicts: ConflictType::Include,
});
}
}
Expand Down
Loading

0 comments on commit c14fd5c

Please sign in to comment.