From c14fd5c6065655698b38478803390a620b252ee8 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 24 Aug 2023 18:18:38 -0700 Subject: [PATCH 1/2] Lazily check for conflicts --- crates/flowistry/src/infoflow/analysis.rs | 159 ++++++++---------- crates/flowistry/src/infoflow/dependencies.rs | 18 +- crates/flowistry/src/infoflow/mutation.rs | 48 +++--- crates/flowistry/src/infoflow/recursive.rs | 3 +- crates/flowistry/src/mir/aliases.rs | 3 +- .../tuple_write_whole_read_whole.txt.expected | 2 +- crates/flowistry_ifc/src/analysis.rs | 4 +- 7 files changed, 102 insertions(+), 135 deletions(-) diff --git a/crates/flowistry/src/infoflow/analysis.rs b/crates/flowistry/src/infoflow/analysis.rs index 812d9ca73..8ff5150b0 100644 --- a/crates/flowistry/src/infoflow/analysis.rs +++ b/crates/flowistry/src/infoflow/analysis.rs @@ -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, *}, @@ -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}, @@ -22,7 +23,6 @@ use crate::{ impls::{LocationOrArg, LocationOrArgDomain, LocationOrArgSet}, IndexMatrix, IndexedDomain, }, - infoflow::mutation::ConflictType, mir::aliases::Aliases, }; @@ -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, @@ -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::>(); - - let mut input_location_deps = (0 .. mutations.len()) - .map(|_| { - let mut deps = LocationOrArgSet::new(location_domain); - deps.insert(location); - deps - }) - .collect::>(); + // 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::>(); - }; - - mutable_conflicts - }) - .collect::>(); - - // 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); } } } diff --git a/crates/flowistry/src/infoflow/dependencies.rs b/crates/flowistry/src/infoflow/dependencies.rs index e8e06acf1..b8cc4d252 100644 --- a/crates/flowistry/src/infoflow/dependencies.rs +++ b/crates/flowistry/src/infoflow/dependencies.rs @@ -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); } } } diff --git a/crates/flowistry/src/infoflow/mutation.rs b/crates/flowistry/src/infoflow/mutation.rs index 1835770f9..8f8baa7c2 100644 --- a/crates/flowistry/src/infoflow/mutation.rs +++ b/crates/flowistry/src/infoflow/mutation.rs @@ -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}; @@ -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> { @@ -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. @@ -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::>(); - 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 @@ -123,7 +122,6 @@ where mutated, inputs: input.into_iter().collect::>(), status: MutationStatus::Definitely, - conflicts: ConflictType::Include, }) .collect::>(); (self.f)(location, mutations); @@ -145,7 +143,7 @@ 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); @@ -153,16 +151,17 @@ where mutated: mutated_field, inputs: vec![input_field], status: MutationStatus::Definitely, - conflicts: ConflictType::Exclude, } }) - .chain([Mutation { + .collect::>(); + + if mutations.is_empty() { + mutations.push(Mutation { mutated: *mutated, inputs: vec![*place], status: MutationStatus::Definitely, - conflicts: ConflictType::Exclude, - }]) - .collect::>(); + }); + } (self.f)(location, mutations); return; } @@ -178,7 +177,6 @@ where mutated: *mutated, inputs: collector.0, status: MutationStatus::Definitely, - conflicts: ConflictType::Include, }]); } @@ -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, }); } } diff --git a/crates/flowistry/src/infoflow/recursive.rs b/crates/flowistry/src/infoflow/recursive.rs index 646a37e9a..62be72b01 100644 --- a/crates/flowistry/src/infoflow/recursive.rs +++ b/crates/flowistry/src/infoflow/recursive.rs @@ -10,7 +10,7 @@ use super::{analysis::FlowAnalysis, BODY_STACK}; use crate::{ extensions::REACHED_LIBRARY, infoflow::{ - mutation::{ConflictType, Mutation, MutationStatus}, + mutation::{Mutation, MutationStatus}, FlowDomain, }, mir::utils, @@ -228,7 +228,6 @@ impl<'tcx> FlowAnalysis<'_, 'tcx> { } else { MutationStatus::Possibly }, - conflicts: ConflictType::Exclude }) }).collect::>(); diff --git a/crates/flowistry/src/mir/aliases.rs b/crates/flowistry/src/mir/aliases.rs index 34a3f5670..4a8d3f94d 100644 --- a/crates/flowistry/src/mir/aliases.rs +++ b/crates/flowistry/src/mir/aliases.rs @@ -539,7 +539,7 @@ impl<'a, 'tcx> Aliases<'a, 'tcx> { /// and can be used at the provided level of [`Mutability`]. /// /// For example, if `x = 0` and `y = (0, &x)`, then `reachable_values(y, Mutability::Not)` - /// is `{y, y.0, y.1, x}`. With `Mutability::Mut`, then the output is `{y, y.0, y.1}` (no `x`). + /// is `{y, x}`. With `Mutability::Mut`, then the output is `{y}` (no `x`). pub fn reachable_values( &self, place: Place<'tcx>, @@ -551,7 +551,6 @@ impl<'a, 'tcx> Aliases<'a, 'tcx> { loans .into_iter() .chain([place]) - .flat_map(|place| self.aliases(place).iter().copied()) .filter(|place| { if let Some((place, _)) = place.refs_in_projection().last() { let ty = place.ty(self.body.local_decls(), self.tcx).ty; diff --git a/crates/flowistry/tests/backward_slice/tuple_write_whole_read_whole.txt.expected b/crates/flowistry/tests/backward_slice/tuple_write_whole_read_whole.txt.expected index fe49f454c..2d1321c01 100644 --- a/crates/flowistry/tests/backward_slice/tuple_write_whole_read_whole.txt.expected +++ b/crates/flowistry/tests/backward_slice/tuple_write_whole_read_whole.txt.expected @@ -1,5 +1,5 @@ fn main() { - `[let mut x = (0, 1);]` + let mut x = (0, 1); `[x = (2, 3);]` `[x;]` } \ No newline at end of file diff --git a/crates/flowistry_ifc/src/analysis.rs b/crates/flowistry_ifc/src/analysis.rs index 4dc526e2b..e60815033 100644 --- a/crates/flowistry_ifc/src/analysis.rs +++ b/crates/flowistry_ifc/src/analysis.rs @@ -108,9 +108,9 @@ pub fn analyze(body_id: &BodyId, results: &FlowResults) -> Result { let mut errors = Vec::new(); for secure in secure_places.iter() { - let secure_deps = final_state.row_set(*secure); + let secure_deps = results.analysis.deps_for(&final_state, *secure); for insecure in insecure_places.iter() { - let insecure_deps = final_state.row_set(*insecure); + let insecure_deps = results.analysis.deps_for(&final_state, *insecure); if insecure_deps.is_superset(&secure_deps) { errors.push((secure, insecure)); } From 4d61b5242b42c5b85e6c25b7fd113777bd29caf2 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Fri, 25 Aug 2023 15:07:29 -0700 Subject: [PATCH 2/2] Factor out alias analysis from new PlaceInfo structure --- crates/flowistry/Cargo.toml | 2 +- crates/flowistry/src/infoflow/analysis.rs | 70 +-- crates/flowistry/src/infoflow/dependencies.rs | 16 +- crates/flowistry/src/infoflow/mod.rs | 8 +- crates/flowistry/src/infoflow/mutation.rs | 27 +- crates/flowistry/src/mir/aliases.rs | 500 +++++------------- crates/flowistry/src/mir/mod.rs | 1 + crates/flowistry/src/mir/placeinfo.rs | 339 ++++++++++++ .../backward_slice/tuple_write_ptr_field.txt | 7 + .../tuple_write_ptr_field.txt.expected | 7 + crates/flowistry_ide/Cargo.toml | 4 +- .../src/focus/direct_influence.rs | 19 +- crates/flowistry_ide/src/focus/mod.rs | 3 +- crates/flowistry_ifc/Cargo.toml | 4 +- 14 files changed, 571 insertions(+), 436 deletions(-) create mode 100644 crates/flowistry/src/mir/placeinfo.rs create mode 100644 crates/flowistry/tests/backward_slice/tuple_write_ptr_field.txt create mode 100644 crates/flowistry/tests/backward_slice/tuple_write_ptr_field.txt.expected diff --git a/crates/flowistry/Cargo.toml b/crates/flowistry/Cargo.toml index dc0834ceb..a2beb93d6 100644 --- a/crates/flowistry/Cargo.toml +++ b/crates/flowistry/Cargo.toml @@ -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} diff --git a/crates/flowistry/src/infoflow/analysis.rs b/crates/flowistry/src/infoflow/analysis.rs index 8ff5150b0..0d1ec3ab3 100644 --- a/crates/flowistry/src/infoflow/analysis.rs +++ b/crates/flowistry/src/infoflow/analysis.rs @@ -23,7 +23,7 @@ use crate::{ impls::{LocationOrArg, LocationOrArgDomain, LocationOrArgSet}, IndexMatrix, IndexedDomain, }, - mir::aliases::Aliases, + mir::placeinfo::PlaceInfo, }; /// Represents the information flows at a given instruction. See [`FlowResults`] for a high-level explanation of this datatype. @@ -56,7 +56,7 @@ pub struct FlowAnalysis<'a, 'tcx> { pub def_id: DefId, pub body: &'a Body<'tcx>, pub control_dependencies: ControlDependencies, - pub aliases: Aliases<'a, 'tcx>, + pub place_info: PlaceInfo<'a, 'tcx>, pub(crate) recurse_cache: RefCell>>, } @@ -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(); @@ -74,25 +74,29 @@ impl<'a, 'tcx> FlowAnalysis<'a, 'tcx> { tcx, def_id, body, - aliases, + place_info, control_dependencies, recurse_cache, } } pub fn location_domain(&self) -> &Rc { - self.aliases.location_domain() + self.place_info.location_domain() } fn influences(&self, place: Place<'tcx>) -> SmallVec<[Place<'tcx>; 8]> { - let conflicts = self.aliases.conflicts(place); + 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 - .aliases + .place_info .aliases(Place::from_ref(place_ref, self.tcx)) .iter() }); - conflicts.iter().chain(provenance).copied().collect() + conflicts.chain(provenance).copied().collect() } pub fn deps_for( @@ -102,12 +106,12 @@ impl<'a, 'tcx> FlowAnalysis<'a, 'tcx> { ) -> LocationOrArgSet { let mut deps = LocationOrArgSet::new(self.location_domain()); for subplace in self - .aliases + .place_info .reachable_values(place, Mutability::Not) .iter() .flat_map(|place| self.influences(*place)) { - deps.union(&state.row_set(self.aliases.normalize(subplace))); + deps.union(&state.row_set(self.place_info.normalize(subplace))); } deps } @@ -134,7 +138,7 @@ impl<'a, 'tcx> FlowAnalysis<'a, 'tcx> { input, target_deps: &mut LocationOrArgSet| { for relevant in self.influences(input) { - let relevant_deps = state.row_set(self.aliases.normalize(relevant)); + 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); } @@ -171,30 +175,36 @@ impl<'a, 'tcx> FlowAnalysis<'a, 'tcx> { 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 + && self.place_info.aliases(mt.mutated).len() == 1 { - for sub in self.aliases.children(mt.mutated).iter() { - state.clear_row(self.aliases.normalize(*sub)); + for sub in self.place_info.children(mt.mutated).iter() { + state.clear_row(self.place_info.normalize(*sub)); } } // Add deps of mutated to include provenance of mutated pointers add_deps(state, mt.mutated, deps); - debug!(" Mutated places: {:?}", mt.mutated); + 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::>(); + + debug!(" Mutated places: {mutable_aliases:?}"); debug!(" with deps {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); + state.union_into_row(self.place_info.normalize(*alias), deps); } } } @@ -210,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); } } } @@ -229,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); @@ -248,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); diff --git a/crates/flowistry/src/infoflow/dependencies.rs b/crates/flowistry/src/infoflow/dependencies.rs index b8cc4d252..812b81324 100644 --- a/crates/flowistry/src/infoflow/dependencies.rs +++ b/crates/flowistry/src/infoflow/dependencies.rs @@ -17,7 +17,7 @@ use crate::{ RefSet, }, infoflow::mutation::Mutation, - mir::aliases::Aliases, + mir::placeinfo::PlaceInfo, }; /// Which way to look for dependencies @@ -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)) @@ -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:?}"); @@ -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> { - 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 @@ -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(); @@ -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); } diff --git a/crates/flowistry/src/infoflow/mod.rs b/crates/flowistry/src/infoflow/mod.rs index 0beebd731..e4480b37b 100644 --- a/crates/flowistry/src/infoflow/mod.rs +++ b/crates/flowistry/src/infoflow/mod.rs @@ -15,7 +15,7 @@ pub use self::{ analysis::{FlowAnalysis, FlowDomain}, dependencies::{compute_dependencies, compute_dependency_spans, Direction}, }; -use crate::mir::{aliases::Aliases, engine}; +use crate::mir::{engine, placeinfo::PlaceInfo}; mod analysis; mod dependencies; @@ -91,15 +91,15 @@ pub fn compute_flow<'a, 'tcx>( debug!("{}", body_with_facts.body.to_string(tcx).unwrap()); let def_id = tcx.hir().body_owner_def_id(body_id).to_def_id(); - let aliases = Aliases::build(tcx, def_id, body_with_facts); - let location_domain = aliases.location_domain().clone(); + let place_info = PlaceInfo::build(tcx, def_id, body_with_facts); + let location_domain = place_info.location_domain().clone(); let body = &body_with_facts.body; let results = { block_timer!("Flow"); - let analysis = FlowAnalysis::new(tcx, def_id, body, aliases); + let analysis = FlowAnalysis::new(tcx, def_id, body, place_info); engine::iterate_to_fixpoint(tcx, body, location_domain, analysis) // analysis.into_engine(tcx, body).iterate_to_fixpoint() }; diff --git a/crates/flowistry/src/infoflow/mutation.rs b/crates/flowistry/src/infoflow/mutation.rs index 8f8baa7c2..55cfd6f47 100644 --- a/crates/flowistry/src/infoflow/mutation.rs +++ b/crates/flowistry/src/infoflow/mutation.rs @@ -9,7 +9,7 @@ use rustc_target::abi::FieldIdx; use rustc_utils::{mir::place::PlaceCollector, AdtDefExt, OperandExt}; use crate::mir::{ - aliases::Aliases, + placeinfo::PlaceInfo, utils::{self, AsyncHack}, }; @@ -49,15 +49,15 @@ where F: FnMut(Location, Vec>), { f: F, - aliases: &'a Aliases<'a, 'tcx>, + place_info: &'a PlaceInfo<'a, 'tcx>, } impl<'a, 'tcx, F> ModularMutationVisitor<'a, 'tcx, F> where F: FnMut(Location, Vec>), { - pub fn new(aliases: &'a Aliases<'a, 'tcx>, f: F) -> Self { - ModularMutationVisitor { aliases, f } + pub fn new(place_info: &'a PlaceInfo<'a, 'tcx>, f: F) -> Self { + ModularMutationVisitor { place_info, f } } } @@ -72,8 +72,8 @@ where location: Location, ) { debug!("Checking {location:?}: {mutated:?} = {rvalue:?}"); - let body = self.aliases.body; - let tcx = self.aliases.tcx; + let body = self.place_info.body; + let tcx = self.place_info.tcx; match rvalue { // In the case of _1 = aggregate { field1: op1, field2: op2, ... }, @@ -138,7 +138,7 @@ where if let TyKind::Adt(adt_def, substs) = place_ty.kind() { if adt_def.is_struct() { let fields = adt_def - .all_visible_fields(self.aliases.def_id, self.aliases.tcx) + .all_visible_fields(self.place_info.def_id, self.place_info.tcx) .enumerate() .map(|(i, field_def)| { PlaceElem::Field(FieldIdx::from_usize(i), field_def.ty(tcx, substs)) @@ -182,7 +182,7 @@ where fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { debug!("Checking {location:?}: {:?}", terminator.kind); - let tcx = self.aliases.tcx; + let tcx = self.place_info.tcx; match &terminator.kind { TerminatorKind::Call { @@ -191,8 +191,11 @@ where destination, .. } => { - let async_hack = - AsyncHack::new(self.aliases.tcx, self.aliases.body, self.aliases.def_id); + let async_hack = AsyncHack::new( + self.place_info.tcx, + self.place_info.body, + self.place_info.def_id, + ); let arg_places = utils::arg_places(args) .into_iter() .map(|(_, place)| place) @@ -201,7 +204,7 @@ where let arg_inputs = arg_places.clone(); let ret_is_unit = destination - .ty(self.aliases.body.local_decls(), tcx) + .ty(self.place_info.body.local_decls(), tcx) .ty .is_unit(); let inputs = if ret_is_unit { @@ -217,7 +220,7 @@ where }]; for arg in arg_places { - for arg_mut in self.aliases.reachable_values(arg, Mutability::Mut) { + for arg_mut in self.place_info.reachable_values(arg, Mutability::Mut) { mutations.push(Mutation { mutated: *arg_mut, inputs: arg_inputs.clone(), diff --git a/crates/flowistry/src/mir/aliases.rs b/crates/flowistry/src/mir/aliases.rs index 4a8d3f94d..e4a0e7708 100644 --- a/crates/flowistry/src/mir/aliases.rs +++ b/crates/flowistry/src/mir/aliases.rs @@ -1,6 +1,6 @@ //! Alias analysis to determine the points-to set of a reference. -use std::{hash::Hash, ops::ControlFlow, rc::Rc, time::Instant}; +use std::{hash::Hash, time::Instant}; use log::{debug, info}; use rustc_borrowck::consumers::BodyWithBorrowckFacts; @@ -15,32 +15,15 @@ use rustc_index::{ vec::IndexVec, }; use rustc_middle::{ - mir::{ - visit::{PlaceContext, Visitor}, - *, - }, - ty::{ - Region, RegionKind, RegionVid, Ty, TyCtxt, TyKind, TypeAndMut, TypeSuperVisitable, - TypeVisitor, - }, -}; -use rustc_target::abi::FieldIdx; -use rustc_utils::{ - block_timer, - cache::{Cache, CopyCache}, - timer::elapsed, - MutabilityExt, PlaceExt, + mir::{visit::Visitor, *}, + ty::{Region, RegionKind, RegionVid, Ty, TyCtxt, TyKind, TypeAndMut}, }; +use rustc_utils::{mir::place::UNKNOWN_REGION, timer::elapsed, PlaceExt}; use crate::{ - extensions::{is_extension_active, MutabilityMode, PointerMode}, - indexed::{ - impls::{ - build_location_arg_domain, LocationOrArgDomain, LocationOrArgIndex, PlaceSet, - }, - ToIndex, - }, - mir::utils::{self, AsyncHack}, + extensions::{is_extension_active, PointerMode}, + indexed::impls::PlaceSet, + mir::utils::AsyncHack, }; #[derive(Default)] @@ -67,100 +50,14 @@ impl<'tcx> Visitor<'tcx> for GatherBorrows<'tcx> { } } -struct FindPlaces<'a, 'tcx> { - tcx: TyCtxt<'tcx>, - body: &'a Body<'tcx>, - def_id: DefId, - places: Vec>, -} - -impl<'tcx> Visitor<'tcx> for FindPlaces<'_, 'tcx> { - // this is needed for eval? not sure why locals wouldn't show up in the body as places, - // maybe optimized out or something - fn visit_local_decl(&mut self, local: Local, _local_decl: &LocalDecl<'tcx>) { - self.places.push(Place::from_local(local, self.tcx)); - } - - fn visit_place( - &mut self, - place: &Place<'tcx>, - _context: PlaceContext, - _location: Location, - ) { - self.places.push(*place); - } - - fn visit_assign( - &mut self, - place: &Place<'tcx>, - rvalue: &Rvalue<'tcx>, - location: Location, - ) { - self.super_assign(place, rvalue, location); - - let is_borrow = matches!(rvalue, Rvalue::Ref(..)); - if is_borrow { - self.places.push(self.tcx.mk_place_deref(*place)); - } - - // See PlaceCollector for where this matters - if let Rvalue::Aggregate(box AggregateKind::Adt(def_id, idx, substs, _, _), _) = - rvalue - { - let adt_def = self.tcx.adt_def(*def_id); - let variant = adt_def.variant(*idx); - let places = variant.fields.iter().enumerate().map(|(i, field)| { - let mut projection = place.projection.to_vec(); - projection.push(ProjectionElem::Field( - FieldIdx::from_usize(i), - field.ty(self.tcx, substs), - )); - Place::make(place.local, &projection, self.tcx) - }); - self.places.extend(places); - } - } - - fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { - self.super_terminator(terminator, location); - - match &terminator.kind { - TerminatorKind::Call { args, .. } => { - let arg_places = utils::arg_places(args); - let arg_mut_ptrs = - utils::arg_mut_ptrs(&arg_places, self.tcx, self.body, self.def_id); - self - .places - .extend(arg_mut_ptrs.into_iter().map(|(_, place)| place)); - } - - _ => {} - } - } -} - type LoanSet<'tcx> = HashSet<(Place<'tcx>, Mutability)>; type LoanMap<'tcx> = HashMap>; -/// Used to describe aliases of owned and raw pointers. -pub const UNKNOWN_REGION: RegionVid = RegionVid::MAX; - /// Data structure for computing and storing aliases. pub struct Aliases<'a, 'tcx> { - // Compiler data - pub tcx: TyCtxt<'tcx>, - pub body: &'a Body<'tcx>, - pub def_id: DefId, - location_domain: Rc, - - // Core computed data structure - loans: LoanMap<'tcx>, - - // Caching for derived analysis - normalized_cache: CopyCache, Place<'tcx>>, - aliases_cache: Cache, PlaceSet<'tcx>>, - conflicts_cache: Cache, PlaceSet<'tcx>>, - reachable_cache: Cache<(Place<'tcx>, Mutability), PlaceSet<'tcx>>, + tcx: TyCtxt<'tcx>, + body: &'a Body<'tcx>, + pub(super) loans: LoanMap<'tcx>, } rustc_index::newtype_index! { @@ -169,6 +66,19 @@ rustc_index::newtype_index! { } impl<'a, 'tcx> Aliases<'a, 'tcx> { + pub fn build( + tcx: TyCtxt<'tcx>, + def_id: DefId, + body_with_facts: &'a BodyWithBorrowckFacts<'tcx>, + ) -> Self { + let loans = Self::compute_loans(tcx, def_id, body_with_facts); + Aliases { + tcx, + body: &body_with_facts.body, + loans, + } + } + fn compute_loans( tcx: TyCtxt<'tcx>, def_id: DefId, @@ -407,189 +317,55 @@ impl<'a, 'tcx> Aliases<'a, 'tcx> { contains } - pub fn build( - tcx: TyCtxt<'tcx>, - def_id: DefId, - body_with_facts: &'a BodyWithBorrowckFacts<'tcx>, - ) -> Self { - block_timer!("aliases"); - let body = &body_with_facts.body; - let location_domain = build_location_arg_domain(body); - - let loans = Self::compute_loans(tcx, def_id, body_with_facts); - debug!("Loans: {loans:?}"); + pub fn aliases(&self, place: Place<'tcx>) -> PlaceSet<'tcx> { + let mut aliases = HashSet::default(); + aliases.insert(place); - Aliases { - loans, - tcx, - body, - def_id, - location_domain, - aliases_cache: Cache::default(), - normalized_cache: CopyCache::default(), - conflicts_cache: Cache::default(), - reachable_cache: Cache::default(), + // Places with no derefs, or derefs from arguments, have no aliases + if place.is_direct(self.body) { + return aliases; } - } - /// Normalizes a place via [`PlaceExt::normalize`] (cached). - pub fn normalize(&self, place: Place<'tcx>) -> Place<'tcx> { - self - .normalized_cache - .get(place, |place| place.normalize(self.tcx, self.def_id)) - } + // place = after[*ptr] + let (ptr, after) = place.refs_in_projection().last().unwrap(); - /// Computes the aliases of a place (cached). - /// - /// Note that an alias is NOT guaranteed to be of the same type as `place`! - pub fn aliases(&self, place: Place<'tcx>) -> &PlaceSet<'tcx> { - // note: important that aliases are computed on the unnormalized place - // which contains region information - self.aliases_cache.get(self.normalize(place), move |_| { - let mut aliases = HashSet::default(); - aliases.insert(place); - - // Places with no derefs, or derefs from arguments, have no aliases - if place.is_direct(self.body) { + // ptr : &'region orig_ty + let ptr_ty = ptr.ty(self.body.local_decls(), self.tcx).ty; + let (region, orig_ty) = match ptr_ty.kind() { + _ if ptr_ty.is_box() => (UNKNOWN_REGION, ptr_ty.boxed_ty()), + TyKind::RawPtr(TypeAndMut { ty, .. }) => (UNKNOWN_REGION, *ty), + TyKind::Ref(Region(Interned(RegionKind::ReVar(region), _)), ty, _) => { + (*region, *ty) + } + _ => { return aliases; } - - // place = after[*ptr] - let (ptr, after) = place.refs_in_projection().last().unwrap(); - - // ptr : &'region orig_ty - let ptr_ty = ptr.ty(self.body.local_decls(), self.tcx).ty; - let (region, orig_ty) = match ptr_ty.kind() { - _ if ptr_ty.is_box() => (UNKNOWN_REGION, ptr_ty.boxed_ty()), - TyKind::RawPtr(TypeAndMut { ty, .. }) => (UNKNOWN_REGION, *ty), - TyKind::Ref(Region(Interned(RegionKind::ReVar(region), _)), ty, _) => { - (*region, *ty) - } - _ => { - return aliases; - } - }; - - // For each p ∈ loans('region), - // if p : orig_ty then add: after[p] - // else add: p - let region_loans = self - .loans - .get(®ion) - .map(|loans| loans.iter()) - .into_iter() - .flatten(); - let region_aliases = region_loans.map(|(loan, _)| { - let loan_ty = loan.ty(self.body.local_decls(), self.tcx).ty; - if orig_ty == loan_ty { - let mut projection = loan.projection.to_vec(); - projection.extend(after.iter().copied()); - Place::make(loan.local, &projection, self.tcx) - } else { - *loan - } - }); - - aliases.extend(region_aliases); - log::trace!("Aliases for place {place:?} are {aliases:?}"); - - aliases - }) - } - - /// Returns all reachable fields of `place` without going through references. - pub fn children(&self, place: Place<'tcx>) -> PlaceSet<'tcx> { - PlaceSet::from_iter(place.interior_places(self.tcx, self.body, self.def_id)) - } - - /// Returns all places that conflict with `place`, i.e. that a mutation to `place` - /// would also be a mutation to the conflicting place. - pub fn conflicts(&self, place: Place<'tcx>) -> &PlaceSet<'tcx> { - self.conflicts_cache.get(place, |place| { - self - .aliases(place) - .iter() - .flat_map(|alias| { - let children = self.children(*alias); - let parents = alias - .iter_projections() - .take_while(|(_, elem)| !matches!(elem, PlaceElem::Deref)) - .map(|(place_ref, _)| Place::from_ref(place_ref, self.tcx)); - children.into_iter().chain(parents) - }) - .collect() - }) - } - - fn collect_loans(&self, ty: Ty<'tcx>, mutability: Mutability) -> PlaceSet<'tcx> { - let mut collector = LoanCollector { - aliases: self, - unknown_region: self - .tcx - .mk_region_from_kind(RegionKind::ReVar(UNKNOWN_REGION)), - target_mutability: mutability, - stack: vec![], - loans: PlaceSet::default(), }; - collector.visit_ty(ty); - collector.loans - } - /// Returns all [direct](PlaceExt::is_direct) places that are reachable from `place` - /// and can be used at the provided level of [`Mutability`]. - /// - /// For example, if `x = 0` and `y = (0, &x)`, then `reachable_values(y, Mutability::Not)` - /// is `{y, x}`. With `Mutability::Mut`, then the output is `{y}` (no `x`). - pub fn reachable_values( - &self, - place: Place<'tcx>, - mutability: Mutability, - ) -> &PlaceSet<'tcx> { - self.reachable_cache.get((place, mutability), |_| { - let ty = place.ty(self.body.local_decls(), self.tcx).ty; - let loans = self.collect_loans(ty, mutability); - loans - .into_iter() - .chain([place]) - .filter(|place| { - if let Some((place, _)) = place.refs_in_projection().last() { - let ty = place.ty(self.body.local_decls(), self.tcx).ty; - if ty.is_box() || ty.is_unsafe_ptr() { - return true; - } - } - place.is_direct(self.body) - }) - .collect() - }) - } + // For each p ∈ loans('region), + // if p : orig_ty then add: after[p] + // else add: p + let region_loans = self + .loans + .get(®ion) + .map(|loans| loans.iter()) + .into_iter() + .flatten(); + let region_aliases = region_loans.map(|(loan, _)| { + let loan_ty = loan.ty(self.body.local_decls(), self.tcx).ty; + if orig_ty == loan_ty { + let mut projection = loan.projection.to_vec(); + projection.extend(after.iter().copied()); + Place::make(loan.local, &projection, self.tcx) + } else { + *loan + } + }); - /// Returns all [direct](PlaceExt::is_direct) places reachable from arguments - /// to the current body. - pub fn all_args( - &'a self, - ) -> impl Iterator, LocationOrArgIndex)> + 'a { - self.body.args_iter().flat_map(|local| { - let location = local.to_index(&self.location_domain); - let place = Place::from_local(local, self.tcx); - let ptrs = place - .interior_pointers(self.tcx, self.body, self.def_id) - .into_values() - .flat_map(|ptrs| { - ptrs - .into_iter() - .filter(|(ptr, _)| ptr.projection.len() <= 2) - .map(|(ptr, _)| self.tcx.mk_place_deref(ptr)) - }); - ptrs - .chain([place]) - .flat_map(|place| place.interior_places(self.tcx, self.body, self.def_id)) - .map(move |place| (place, location)) - }) - } + aliases.extend(region_aliases); + log::trace!("Aliases for place {place:?} are {aliases:?}"); - pub fn location_domain(&self) -> &Rc { - &self.location_domain + aliases } } @@ -626,100 +402,88 @@ fn generate_conservative_constraints<'tcx>( .collect::>() } -// TODO: this visitor shares some structure with the PlaceCollector in mir utils. -// Can we consolidate these? -struct LoanCollector<'a, 'tcx> { - aliases: &'a Aliases<'a, 'tcx>, - unknown_region: Region<'tcx>, - target_mutability: Mutability, - stack: Vec, - loans: PlaceSet<'tcx>, -} +#[cfg(test)] +mod test { + use rustc_utils::{ + hashset, + test_utils::{compare_sets, Placer}, + }; -impl<'tcx> TypeVisitor> for LoanCollector<'_, 'tcx> { - type BreakTy = (); + use super::*; + use crate::test_utils; - fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow { - match ty.kind() { - TyKind::Ref(_, _, mutability) => { - self.stack.push(*mutability); - ty.super_visit_with(self); - self.stack.pop(); - return ControlFlow::Break(()); - } - _ if ty.is_box() || ty.is_unsafe_ptr() => { - self.visit_region(self.unknown_region); - } - _ => {} - }; + fn alias_harness( + input: &str, + f: impl for<'tcx> FnOnce(TyCtxt<'tcx>, &Body<'tcx>, Aliases<'_, 'tcx>) + Send, + ) { + test_utils::compile_body(input, |tcx, body_id, body_with_facts| { + let body = &body_with_facts.body; + let def_id = tcx.hir().body_owner_def_id(body_id); + let aliases = Aliases::build(tcx, def_id.to_def_id(), body_with_facts); - ty.super_visit_with(self); - ControlFlow::Continue(()) + f(tcx, body, aliases) + }); } - fn visit_region(&mut self, region: Region<'tcx>) -> ControlFlow { - let region = match region.kind() { - RegionKind::ReVar(region) => region, - RegionKind::ReStatic => RegionVid::from_usize(0), - // TODO: do we need to handle bound regions? - // e.g. shows up with closures, for<'a> ... - RegionKind::ReErased | RegionKind::ReLateBound(_, _) => { - return ControlFlow::Continue(()); - } - _ => unreachable!("{region:?}"), - }; - if let Some(loans) = self.aliases.loans.get(®ion) { - let under_immut_ref = self.stack.iter().any(|m| *m == Mutability::Not); - let ignore_mut = - is_extension_active(|mode| mode.mutability_mode == MutabilityMode::IgnoreMut); - self - .loans - .extend(loans.iter().filter_map(|(place, mutability)| { - if ignore_mut { - return Some(place); - } - let loan_mutability = if under_immut_ref { - Mutability::Not - } else { - *mutability - }; - self - .target_mutability - .is_permissive_as(loan_mutability) - .then_some(place) - })) + #[test] + fn test_aliases_basic() { + let input = r#" + fn main() { + fn foo<'a, 'b>(x: &'a i32, y: &'b i32) -> &'a i32 { x } + + let a = 1; + let b = 2; + let c = &a; + let d = &b; + let e = foo(c, d); } + "#; + alias_harness(input, |tcx, body, aliases| { + let p = Placer::new(tcx, body); + let d_deref = p.local("d").deref().mk(); + let e_deref = p.local("e").deref().mk(); + + // `*e` aliases only `a` (not `b`) because of the lifetime constraints on `foo` + compare_sets( + aliases.aliases(e_deref), + hashset! { p.local("a").mk(), e_deref }, + ); - ControlFlow::Continue(()) + // `*e` aliases only `b` because nothing might relate it to `a` + compare_sets( + aliases.aliases(d_deref), + hashset! { p.local("b").mk(), d_deref }, + ); + }); } -} - -#[cfg(test)] -mod test { - use rustc_utils::{BodyExt, PlaceExt}; - - use super::*; - use crate::test_utils; #[test] - fn test_sccs() { + fn test_aliases_projection() { let input = r#" - fn main() { - let mut x = 1; - let y = &mut x; - *y; - } +fn main() { + let a = vec![0]; + let b = a.get(0).unwrap(); + + let c = (0, 0); + let d = &c.1; +} "#; - test_utils::compile_body(input, |tcx, body_id, body_with_facts| { - let body = &body_with_facts.body; - let def_id = tcx.hir().body_owner_def_id(body_id); - let aliases = Aliases::build(tcx, def_id.to_def_id(), body_with_facts); - let name_map = body.debug_info_name_map(); + alias_harness(input, |tcx, body, aliases| { + let p = Placer::new(tcx, body); + let b_deref = p.local("b").deref().mk(); + let d_deref = p.local("d").deref().mk(); + + // `*b` only aliases `a` because we don't have a projection for `a` + compare_sets( + aliases.aliases(b_deref), + hashset! { p.local("a").mk(), b_deref }, + ); - let x = Place::from_local(name_map["x"], tcx); - let y = Place::from_local(name_map["y"], tcx); - let y_deref = tcx.mk_place_deref(y); - assert!(aliases.aliases(y_deref).contains(&x)); - }) + // `*d` aliases `c.1` because we know the projection from the source + compare_sets( + aliases.aliases(d_deref), + hashset! { p.local("c").field(1).mk(), d_deref }, + ); + }); } } diff --git a/crates/flowistry/src/mir/mod.rs b/crates/flowistry/src/mir/mod.rs index 090fdab3f..07cbd3c2f 100644 --- a/crates/flowistry/src/mir/mod.rs +++ b/crates/flowistry/src/mir/mod.rs @@ -2,4 +2,5 @@ pub mod aliases; pub mod engine; +pub mod placeinfo; pub mod utils; diff --git a/crates/flowistry/src/mir/placeinfo.rs b/crates/flowistry/src/mir/placeinfo.rs new file mode 100644 index 000000000..b546c4c51 --- /dev/null +++ b/crates/flowistry/src/mir/placeinfo.rs @@ -0,0 +1,339 @@ +//! Utilities for analyzing places: children, aliases, etc. + +use std::{ops::ControlFlow, rc::Rc}; + +use rustc_borrowck::consumers::BodyWithBorrowckFacts; +use rustc_hir::def_id::DefId; +use rustc_middle::{ + mir::*, + ty::{ + Region, RegionKind, RegionVid, Ty, TyCtxt, TyKind, TypeSuperVisitable, TypeVisitor, + }, +}; +use rustc_utils::{ + block_timer, + cache::{Cache, CopyCache}, + mir::place::UNKNOWN_REGION, + MutabilityExt, PlaceExt, +}; + +use super::aliases::Aliases; +use crate::{ + extensions::{is_extension_active, MutabilityMode}, + indexed::{ + impls::{ + build_location_arg_domain, LocationOrArgDomain, LocationOrArgIndex, PlaceSet, + }, + ToIndex, + }, +}; + +/// Utilities for analyzing places: children, aliases, etc. +pub struct PlaceInfo<'a, 'tcx> { + pub tcx: TyCtxt<'tcx>, + pub body: &'a Body<'tcx>, + pub def_id: DefId, + location_domain: Rc, + + // Core computed data structure + aliases: Aliases<'a, 'tcx>, + + // Caching for derived analysis + normalized_cache: CopyCache, Place<'tcx>>, + aliases_cache: Cache, PlaceSet<'tcx>>, + conflicts_cache: Cache, PlaceSet<'tcx>>, + reachable_cache: Cache<(Place<'tcx>, Mutability), PlaceSet<'tcx>>, +} + +impl<'a, 'tcx> PlaceInfo<'a, 'tcx> { + pub fn build( + tcx: TyCtxt<'tcx>, + def_id: DefId, + body_with_facts: &'a BodyWithBorrowckFacts<'tcx>, + ) -> Self { + block_timer!("aliases"); + let body = &body_with_facts.body; + let location_domain = build_location_arg_domain(body); + let aliases = Aliases::build(tcx, def_id, body_with_facts); + + PlaceInfo { + aliases, + tcx, + body, + def_id, + location_domain, + aliases_cache: Cache::default(), + normalized_cache: CopyCache::default(), + conflicts_cache: Cache::default(), + reachable_cache: Cache::default(), + } + } + + /// Normalizes a place via [`PlaceExt::normalize`] (cached). + /// + /// See the `PlaceExt` documentation for details on how normalization works. + pub fn normalize(&self, place: Place<'tcx>) -> Place<'tcx> { + self + .normalized_cache + .get(place, |place| place.normalize(self.tcx, self.def_id)) + } + + /// Computes the aliases of a place (cached). + /// + /// For example, if `x = &y`, then `*x` aliases `y`. + /// Note that an alias is NOT guaranteed to be of the same type as `place`! + pub fn aliases(&self, place: Place<'tcx>) -> &PlaceSet<'tcx> { + // note: important that aliases are computed on the unnormalized place + // which contains region information + self + .aliases_cache + .get(self.normalize(place), move |_| self.aliases.aliases(place)) + } + + /// Returns all reachable fields of `place` without going through references. + /// + /// For example, if `x = (0, 1)` then `children(x) = {x, x.0, x.1}`. + pub fn children(&self, place: Place<'tcx>) -> PlaceSet<'tcx> { + PlaceSet::from_iter(place.interior_places(self.tcx, self.body, self.def_id)) + } + + /// Returns all places that conflict with `place`, i.e. that a mutation to `place` + /// would also be a mutation to the conflicting place. + /// + /// For example, if `x = ((0, 1), 2)` then `conflicts(x.0) = {x, x.0, x.0.0, x.0.1}`, but not `x.1`. + pub fn conflicts(&self, place: Place<'tcx>) -> &PlaceSet<'tcx> { + self.conflicts_cache.get(place, |place| { + let children = self.children(place); + let parents = place + .iter_projections() + .take_while(|(_, elem)| !matches!(elem, PlaceElem::Deref)) + .map(|(place_ref, _)| Place::from_ref(place_ref, self.tcx)); + children.into_iter().chain(parents).collect() + }) + } + + /// Returns all [direct](PlaceExt::is_direct) places that are reachable from `place` + /// and can be used at the provided level of [`Mutability`] (cached). + /// + /// For example, if `x = 0` and `y = (0, &x)`, then `reachable_values(y, Mutability::Not)` + /// is `{y, x}`. With `Mutability::Mut`, then the output is `{y}` (no `x`). + pub fn reachable_values( + &self, + place: Place<'tcx>, + mutability: Mutability, + ) -> &PlaceSet<'tcx> { + self.reachable_cache.get((place, mutability), |_| { + let ty = place.ty(self.body.local_decls(), self.tcx).ty; + let loans = self.collect_loans(ty, mutability); + loans + .into_iter() + .chain([place]) + .filter(|place| { + if let Some((place, _)) = place.refs_in_projection().last() { + let ty = place.ty(self.body.local_decls(), self.tcx).ty; + if ty.is_box() || ty.is_unsafe_ptr() { + return true; + } + } + place.is_direct(self.body) + }) + .collect() + }) + } + + fn collect_loans(&self, ty: Ty<'tcx>, mutability: Mutability) -> PlaceSet<'tcx> { + let mut collector = LoanCollector { + aliases: &self.aliases, + unknown_region: self + .tcx + .mk_region_from_kind(RegionKind::ReVar(UNKNOWN_REGION)), + target_mutability: mutability, + stack: vec![], + loans: PlaceSet::default(), + }; + collector.visit_ty(ty); + collector.loans + } + + /// Returns all [direct](PlaceExt::is_direct) places reachable from arguments + /// to the current body. + pub fn all_args( + &'a self, + ) -> impl Iterator, LocationOrArgIndex)> + 'a { + self.body.args_iter().flat_map(|local| { + let location = local.to_index(&self.location_domain); + let place = Place::from_local(local, self.tcx); + let ptrs = place + .interior_pointers(self.tcx, self.body, self.def_id) + .into_values() + .flat_map(|ptrs| { + ptrs + .into_iter() + .filter(|(ptr, _)| ptr.projection.len() <= 2) + .map(|(ptr, _)| self.tcx.mk_place_deref(ptr)) + }); + ptrs + .chain([place]) + .flat_map(|place| place.interior_places(self.tcx, self.body, self.def_id)) + .map(move |place| (place, location)) + }) + } + + pub fn location_domain(&self) -> &Rc { + &self.location_domain + } +} + +// TODO: this visitor shares some structure with the PlaceCollector in mir utils. +// Can we consolidate these? +struct LoanCollector<'a, 'tcx> { + aliases: &'a Aliases<'a, 'tcx>, + unknown_region: Region<'tcx>, + target_mutability: Mutability, + stack: Vec, + loans: PlaceSet<'tcx>, +} + +impl<'tcx> TypeVisitor> for LoanCollector<'_, 'tcx> { + type BreakTy = (); + + fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow { + match ty.kind() { + TyKind::Ref(_, _, mutability) => { + self.stack.push(*mutability); + ty.super_visit_with(self); + self.stack.pop(); + return ControlFlow::Break(()); + } + _ if ty.is_box() || ty.is_unsafe_ptr() => { + self.visit_region(self.unknown_region); + } + _ => {} + }; + + ty.super_visit_with(self); + ControlFlow::Continue(()) + } + + fn visit_region(&mut self, region: Region<'tcx>) -> ControlFlow { + let region = match region.kind() { + RegionKind::ReVar(region) => region, + RegionKind::ReStatic => RegionVid::from_usize(0), + // TODO: do we need to handle bound regions? + // e.g. shows up with closures, for<'a> ... + RegionKind::ReErased | RegionKind::ReLateBound(_, _) => { + return ControlFlow::Continue(()); + } + _ => unreachable!("{region:?}"), + }; + if let Some(loans) = self.aliases.loans.get(®ion) { + let under_immut_ref = self.stack.iter().any(|m| *m == Mutability::Not); + let ignore_mut = + is_extension_active(|mode| mode.mutability_mode == MutabilityMode::IgnoreMut); + self + .loans + .extend(loans.iter().filter_map(|(place, mutability)| { + if ignore_mut { + return Some(place); + } + let loan_mutability = if under_immut_ref { + Mutability::Not + } else { + *mutability + }; + self + .target_mutability + .is_permissive_as(loan_mutability) + .then_some(place) + })) + } + + ControlFlow::Continue(()) + } +} + +#[cfg(test)] +mod test { + use rustc_utils::{ + hashset, + test_utils::{compare_sets, Placer}, + }; + + use super::*; + use crate::test_utils; + + fn placeinfo_harness( + input: &str, + f: impl for<'tcx> FnOnce(TyCtxt<'tcx>, &Body<'tcx>, PlaceInfo<'_, 'tcx>) + Send, + ) { + test_utils::compile_body(input, |tcx, body_id, body_with_facts| { + let body = &body_with_facts.body; + let def_id = tcx.hir().body_owner_def_id(body_id); + let place_info = PlaceInfo::build(tcx, def_id.to_def_id(), body_with_facts); + + f(tcx, body, place_info) + }); + } + + #[test] + fn test_placeinfo_basic() { + let input = r#" +fn main() { + let a = 0; + let mut b = 1; + let c = ((0, &a), &mut b); + let d = 0; + let e = &d; + let f = &e; +} + "#; + placeinfo_harness(input, |tcx, body, place_info| { + let p = Placer::new(tcx, body); + let c = p.local("c"); + compare_sets(place_info.children(c.mk()), hashset! { + c.mk(), + c.field(0).mk(), + c.field(0).field(0).mk(), + c.field(0).field(1).mk(), + c.field(1).mk(), + }); + + compare_sets(place_info.conflicts(c.field(0).mk()), &hashset! { + c.mk(), + c.field(0).mk(), + c.field(0).field(0).mk(), + c.field(0).field(1).mk(), + // c.field(1) not part of the set + }); + + // a and b are reachable at immut-level + compare_sets( + place_info.reachable_values(c.mk(), Mutability::Not), + &hashset! { + c.mk(), + p.local("a").mk(), + p.local("b").mk() + }, + ); + + // only b is reachable at mut-level + compare_sets( + place_info.reachable_values(c.mk(), Mutability::Mut), + &hashset! { + c.mk(), + p.local("b").mk() + }, + ); + + // handles transitive references + compare_sets( + place_info.reachable_values(p.local("f").mk(), Mutability::Not), + &hashset! { + p.local("f").mk(), + p.local("e").mk(), + p.local("d").mk() + }, + ) + }); + } +} diff --git a/crates/flowistry/tests/backward_slice/tuple_write_ptr_field.txt b/crates/flowistry/tests/backward_slice/tuple_write_ptr_field.txt new file mode 100644 index 000000000..3a3a57e5a --- /dev/null +++ b/crates/flowistry/tests/backward_slice/tuple_write_ptr_field.txt @@ -0,0 +1,7 @@ +fn main() { + let mut a = 0; + let b = (1, &mut a); + *b.1 += 1; + let c = b; + `(c)`; +} \ No newline at end of file diff --git a/crates/flowistry/tests/backward_slice/tuple_write_ptr_field.txt.expected b/crates/flowistry/tests/backward_slice/tuple_write_ptr_field.txt.expected new file mode 100644 index 000000000..40c12c765 --- /dev/null +++ b/crates/flowistry/tests/backward_slice/tuple_write_ptr_field.txt.expected @@ -0,0 +1,7 @@ +fn main() { + `[let mut a = 0;]` + `[let b = (1, &mut a);]` + `[*b.1 += 1;]` + `[let c = b;]` + `[c;]` +} \ No newline at end of file diff --git a/crates/flowistry_ide/Cargo.toml b/crates/flowistry_ide/Cargo.toml index aa5d1f5e9..e535412bc 100644 --- a/crates/flowistry_ide/Cargo.toml +++ b/crates/flowistry_ide/Cargo.toml @@ -24,8 +24,8 @@ serde = {version = "1", features = ["derive"]} serde_json = "1" flate2 = "1" base64 = "0.21" -rustc_utils = {version = "0.6.2-nightly-2023-04-12", features = ["serde"]} -rustc_plugin = "0.6.2-nightly-2023-04-12" +rustc_utils = {version = "0.6.3-nightly-2023-04-12", features = ["serde"]} +rustc_plugin = "0.6.3-nightly-2023-04-12" # Decompose petgraph = {version = "0.6", default-features = false, optional = true} diff --git a/crates/flowistry_ide/src/focus/direct_influence.rs b/crates/flowistry_ide/src/focus/direct_influence.rs index 8cc5e8c16..ce9ffc58e 100644 --- a/crates/flowistry_ide/src/focus/direct_influence.rs +++ b/crates/flowistry_ide/src/focus/direct_influence.rs @@ -1,22 +1,22 @@ use flowistry::{ indexed::{impls::LocationOrArg, IndexMatrix}, infoflow::mutation::{ModularMutationVisitor, Mutation}, - mir::aliases::Aliases, + mir::placeinfo::PlaceInfo, }; use rustc_middle::mir::{visit::Visitor, Body, Mutability, Place}; pub struct DirectInfluence<'a, 'tcx> { - aliases: &'a Aliases<'a, 'tcx>, + place_info: &'a PlaceInfo<'a, 'tcx>, influence: IndexMatrix, LocationOrArg>, } impl<'a, 'tcx> DirectInfluence<'a, 'tcx> { - pub fn build(body: &Body<'tcx>, aliases: &'a Aliases<'a, 'tcx>) -> Self { - let mut influence = IndexMatrix::new(aliases.location_domain()); + pub fn build(body: &Body<'tcx>, place_info: &'a PlaceInfo<'a, 'tcx>) -> Self { + let mut influence = IndexMatrix::new(place_info.location_domain()); - ModularMutationVisitor::new(aliases, |location, mutations| { + ModularMutationVisitor::new(place_info, |location, mutations| { let mut add = |place: Place<'tcx>, mutability: Mutability| { - for alias in aliases.reachable_values(place, mutability) { + for alias in place_info.reachable_values(place, mutability) { influence.insert(*alias, location); } }; @@ -34,11 +34,14 @@ impl<'a, 'tcx> DirectInfluence<'a, 'tcx> { }) .visit_body(body); - DirectInfluence { aliases, influence } + DirectInfluence { + place_info, + influence, + } } pub fn lookup(&self, target: Place<'tcx>) -> Vec { - let aliases = self.aliases.reachable_values(target, Mutability::Not); + let aliases = self.place_info.reachable_values(target, Mutability::Not); aliases .iter() .flat_map(|target_alias| { diff --git a/crates/flowistry_ide/src/focus/mod.rs b/crates/flowistry_ide/src/focus/mod.rs index e8004b81b..2bf5e9a45 100644 --- a/crates/flowistry_ide/src/focus/mod.rs +++ b/crates/flowistry_ide/src/focus/mod.rs @@ -65,7 +65,8 @@ pub fn focus(tcx: TyCtxt, body_id: BodyId) -> Result { let relevant = infoflow::compute_dependency_spans(results, targets, Direction::Both, &spanner); - let direct = direct_influence::DirectInfluence::build(body, &results.analysis.aliases); + let direct = + direct_influence::DirectInfluence::build(body, &results.analysis.place_info); let slices = grouped_spans .iter() diff --git a/crates/flowistry_ifc/Cargo.toml b/crates/flowistry_ifc/Cargo.toml index fdcb17d76..0e4aa328d 100644 --- a/crates/flowistry_ifc/Cargo.toml +++ b/crates/flowistry_ifc/Cargo.toml @@ -13,5 +13,5 @@ env_logger = "0.9" termcolor = "1.1" anyhow = "1" log = "0.4" -rustc_plugin = "0.6.2-nightly-2023-04-12" -rustc_utils = "0.6.2-nightly-2023-04-12" \ No newline at end of file +rustc_plugin = "0.6.3-nightly-2023-04-12" +rustc_utils = "0.6.3-nightly-2023-04-12" \ No newline at end of file