Skip to content

Commit

Permalink
Merge pull request #76 from willcrichton/lazy-places
Browse files Browse the repository at this point in the history
Lazily check for conflicts
  • Loading branch information
willcrichton authored Aug 25, 2023
2 parents 77ae052 + 4d61b52 commit ada4490
Show file tree
Hide file tree
Showing 17 changed files with 652 additions and 550 deletions.
2 changes: 1 addition & 1 deletion crates/flowistry/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ log = "0.4"
fluid-let = "1.0"
cfg-if = "1.0"
serde = {version = "1", features = ["derive"]}
rustc_utils = "0.6.2-nightly-2023-04-12"
rustc_utils = "0.6.3-nightly-2023-04-12"

# For local debugging
html-escape = {version = "0.2", optional = true}
Expand Down
189 changes: 91 additions & 98 deletions crates/flowistry/src/infoflow/analysis.rs
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,8 +23,7 @@ use crate::{
impls::{LocationOrArg, LocationOrArgDomain, LocationOrArgSet},
IndexMatrix, IndexedDomain,
},
infoflow::mutation::ConflictType,
mir::aliases::Aliases,
mir::placeinfo::PlaceInfo,
};

/// Represents the information flows at a given instruction. See [`FlowResults`] for a high-level explanation of this datatype.
Expand Down Expand Up @@ -56,7 +56,7 @@ pub struct FlowAnalysis<'a, 'tcx> {
pub def_id: DefId,
pub body: &'a Body<'tcx>,
pub control_dependencies: ControlDependencies<BasicBlock>,
pub aliases: Aliases<'a, 'tcx>,
pub place_info: PlaceInfo<'a, 'tcx>,
pub(crate) recurse_cache: RefCell<HashMap<BodyId, FlowResults<'a, 'tcx>>>,
}

Expand All @@ -65,7 +65,7 @@ impl<'a, 'tcx> FlowAnalysis<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
def_id: DefId,
body: &'a Body<'tcx>,
aliases: Aliases<'a, 'tcx>,
place_info: PlaceInfo<'a, 'tcx>,
) -> Self {
let recurse_cache = RefCell::new(HashMap::default());
let control_dependencies = body.control_dependencies();
Expand All @@ -74,14 +74,46 @@ impl<'a, 'tcx> FlowAnalysis<'a, 'tcx> {
tcx,
def_id,
body,
aliases,
place_info,
control_dependencies,
recurse_cache,
}
}

pub fn location_domain(&self) -> &Rc<LocationOrArgDomain> {
self.aliases.location_domain()
self.place_info.location_domain()
}

fn influences(&self, place: Place<'tcx>) -> SmallVec<[Place<'tcx>; 8]> {
let conflicts = self
.place_info
.aliases(place)
.iter()
.flat_map(|alias| self.place_info.conflicts(*alias));
let provenance = place.refs_in_projection().flat_map(|(place_ref, _)| {
self
.place_info
.aliases(Place::from_ref(place_ref, self.tcx))
.iter()
});
conflicts.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
.place_info
.reachable_values(place, Mutability::Not)
.iter()
.flat_map(|place| self.influences(*place))
{
deps.union(&state.row_set(self.place_info.normalize(subplace)));
}
deps
}

// This function expects *ALL* the mutations that occur within a given [`Location`] at once.
Expand All @@ -94,124 +126,85 @@ 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.place_info.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.place_info.aliases(mt.mutated).len() == 1
{
for sub in self.place_info.children(mt.mutated).iter() {
state.clear_row(self.place_info.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:?}");
let mutable_aliases = self
.place_info
.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
})
.collect::<SmallVec<[_; 8]>>();

debug!(" Mutated places: {mutable_aliases:?}");
debug!(" with deps {deps:?}");

for place in conflicts.into_iter() {
state.union_into_row(all_aliases.normalize(place), &deps);
for alias in mutable_aliases {
state.union_into_row(self.place_info.normalize(*alias), deps);
}
}
}
Expand All @@ -227,13 +220,13 @@ impl<'a, 'tcx> AnalysisDomain<'tcx> for FlowAnalysis<'a, 'tcx> {
}

fn initialize_start_block(&self, _body: &Body<'tcx>, state: &mut Self::Domain) {
for (arg, loc) in self.aliases.all_args() {
for place in self.aliases.conflicts(arg) {
for (arg, loc) in self.place_info.all_args() {
for place in self.place_info.conflicts(arg) {
debug!(
"arg={arg:?} / place={place:?} / loc={:?}",
self.location_domain().value(loc)
);
state.insert(self.aliases.normalize(*place), loc);
state.insert(self.place_info.normalize(*place), loc);
}
}
}
Expand All @@ -246,7 +239,7 @@ impl<'a, 'tcx> Analysis<'tcx> for FlowAnalysis<'a, 'tcx> {
statement: &Statement<'tcx>,
location: Location,
) {
ModularMutationVisitor::new(&self.aliases, |_, mutations| {
ModularMutationVisitor::new(&self.place_info, |_, mutations| {
self.transfer_function(state, mutations, location)
})
.visit_statement(statement, location);
Expand All @@ -265,7 +258,7 @@ impl<'a, 'tcx> Analysis<'tcx> for FlowAnalysis<'a, 'tcx> {
return;
}

ModularMutationVisitor::new(&self.aliases, |_, mutations| {
ModularMutationVisitor::new(&self.place_info, |_, mutations| {
self.transfer_function(state, mutations, location)
})
.visit_terminator(terminator, location);
Expand Down
34 changes: 15 additions & 19 deletions crates/flowistry/src/infoflow/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::{
RefSet,
},
infoflow::mutation::Mutation,
mir::aliases::Aliases,
mir::placeinfo::PlaceInfo,
};

/// Which way to look for dependencies
Expand All @@ -43,12 +43,12 @@ impl TargetDeps {
targets: &[(Place<'tcx>, LocationOrArg)],
results: &FlowResults<'_, 'tcx>,
) -> Self {
let aliases = &results.analysis.aliases;
let place_info = &results.analysis.place_info;
let location_domain = results.analysis.location_domain();
// let mut backward = LocationSet::new(location_domain);

let expanded_targets = targets.iter().flat_map(|(place, location)| {
aliases
place_info
.reachable_values(*place, Mutability::Not)
.iter()
.map(move |reachable| (*reachable, *location))
Expand All @@ -65,7 +65,7 @@ impl TargetDeps {

let mut forward = LocationOrArgSet::new(location_domain);
forward.insert_all();
for conflict in aliases.children(aliases.normalize(place)) {
for conflict in place_info.children(place_info.normalize(place)) {
// conflict should already be normalized because the input to aliases.children is normalized
let deps = state.row_set(conflict);
trace!("place={place:?}, conflict={conflict:?}, deps={deps:?}");
Expand All @@ -87,10 +87,10 @@ impl TargetDeps {

pub fn deps<'a, 'tcx>(
state: &'a FlowDomain<'tcx>,
aliases: &'a Aliases<'a, 'tcx>,
place_info: &'a PlaceInfo<'a, 'tcx>,
place: Place<'tcx>,
) -> LocationOrArgSet<RefSet<'a, LocationOrArg>> {
state.row_set(aliases.normalize(place))
state.row_set(place_info.normalize(place))
}

/// Computes the dependencies of a place $p$ at a location $\ell$ in a given
Expand All @@ -111,7 +111,7 @@ pub fn compute_dependencies<'tcx>(
log::info!("Computing dependencies for {} targets", all_targets.len());
debug!("all_targets={all_targets:#?}");

let aliases = &results.analysis.aliases;
let aliases = &results.analysis.place_info;
let body = results.analysis.body;
let location_domain = results.analysis.location_domain();

Expand Down Expand Up @@ -178,7 +178,7 @@ pub fn compute_dependencies<'tcx>(
check(place);
}
}
_ => ModularMutationVisitor::new(&results.analysis.aliases, |_, mutations| {
_ => ModularMutationVisitor::new(&results.analysis.place_info, |_, mutations| {
for Mutation { mutated, .. } in mutations {
check(mutated);
}
Expand All @@ -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
Loading

0 comments on commit ada4490

Please sign in to comment.