From db67af3e27422de8929719560c0cb5d2607bf8a0 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 18 May 2023 15:05:50 -0700 Subject: [PATCH 1/2] Refactor transfer function to accept multiple mutations at once. Improve field-sensitivity of recursive Flowistry wrt return values. --- crates/flowistry/src/infoflow/analysis.rs | 158 +++++++++++------- crates/flowistry/src/infoflow/dependencies.rs | 9 +- crates/flowistry/src/infoflow/mutation.rs | 75 +++++---- crates/flowistry/src/infoflow/recursive.rs | 86 +++++----- .../recurse_return_field_sensitive.txt | 8 + ...ecurse_return_field_sensitive.txt.expected | 8 + .../src/focus/direct_influence.rs | 29 ++-- 7 files changed, 218 insertions(+), 155 deletions(-) create mode 100644 crates/flowistry/tests/extensions/recurse_return_field_sensitive.txt create mode 100644 crates/flowistry/tests/extensions/recurse_return_field_sensitive.txt.expected diff --git a/crates/flowistry/src/infoflow/analysis.rs b/crates/flowistry/src/infoflow/analysis.rs index 0f0e402d9..c4e69b395 100644 --- a/crates/flowistry/src/infoflow/analysis.rs +++ b/crates/flowistry/src/infoflow/analysis.rs @@ -83,37 +83,48 @@ impl<'a, 'tcx> FlowAnalysis<'a, 'tcx> { self.aliases.location_domain() } + // This function expects *ALL* the mutations that occur within a given [`Location`] at once. pub(crate) fn transfer_function( &self, state: &mut FlowDomain<'tcx>, - Mutation { - mutated, - inputs, - location, - status, - }: Mutation<'_, 'tcx>, + mutations: Vec>, + location: Location, + // Small hack for recursive Flowistry that doesn't lookup conflicts of inputs, only aliases + recursive: bool, ) { - debug!(" Applying mutation to {mutated:?} with inputs {inputs:?}"); + debug!(" Applying mutations {mutations:?}"); let location_domain = self.location_domain(); let all_aliases = &self.aliases; - let mutated_aliases = all_aliases.aliases(mutated); - trace!(" Mutated aliases: {mutated_aliases:?}"); - assert!(!mutated_aliases.is_empty()); - - // Clear sub-places of mutated place (if sound to do so) - if matches!(status, MutationStatus::Definitely) && mutated_aliases.len() == 1 { - let mutated_direct = mutated_aliases.iter().next().unwrap(); - for sub in all_aliases.children(*mutated_direct).iter() { - state.clear_row(all_aliases.normalize(*sub)); - } - } + 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 = LocationOrArgSet::new(location_domain); - input_location_deps.insert(location); + let mut input_location_deps = (0 .. mutations.len()) + .map(|_| { + let mut deps = LocationOrArgSet::new(location_domain); + deps.insert(location); + deps + }) + .collect::>(); - let add_deps = |place: Place<'tcx>, location_deps: &mut LocationOrArgSet| { - let reachable_values = all_aliases.reachable_values(place, Mutability::Not); + let add_deps = |state: &mut FlowDomain<'tcx>, + place: Place<'tcx>, + location_deps: &mut LocationOrArgSet| { + let reachable_values = if recursive { + all_aliases.aliases(place) + } else { + 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)) @@ -126,56 +137,87 @@ impl<'a, 'tcx> FlowAnalysis<'a, 'tcx> { } }; - // Add deps of mutated to include provenance of mutated pointers - add_deps(mutated, &mut input_location_deps); - - // Add deps of all inputs - for place in inputs.iter() { - add_deps(*place, &mut input_location_deps); + for (mt, deps) in mutations.iter().zip(&mut input_location_deps) { + // Add deps of all inputs + for input in &mt.inputs { + add_deps(state, *input, deps); + } } // Add control dependencies 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()) { - input_location_deps.insert(body.terminator_loc(block)); + for deps in &mut input_location_deps { + deps.insert(body.terminator_loc(block)); + } // 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() { - add_deps(discr_place, &mut input_location_deps); + for deps in &mut input_location_deps { + add_deps(state, discr_place, deps); + } } } } // Union dependencies into all conflicting places of the mutated place - let mut mutable_conflicts = all_aliases.conflicts(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::>(); - }; + let mutable_conflicts = mutations + .iter() + .map(|mt| { + let mut mutable_conflicts = if recursive { + all_aliases.aliases(mt.mutated).to_owned() + } else { + all_aliases.conflicts(mt.mutated).to_owned() + }; - debug!(" Mutated conflicting places: {mutable_conflicts:?}"); - debug!(" with deps {input_location_deps:?}"); + // 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::>(); + }; - for place in mutable_conflicts.into_iter() { - state.union_into_row(all_aliases.normalize(place), &input_location_deps); + 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)); + } + } + } + + 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!(" with deps {deps:?}"); + + for place in conflicts.into_iter() { + state.union_into_row(all_aliases.normalize(place), &deps); + } } } } @@ -209,8 +251,8 @@ impl<'a, 'tcx> Analysis<'tcx> for FlowAnalysis<'a, 'tcx> { statement: &Statement<'tcx>, location: Location, ) { - ModularMutationVisitor::new(&self.aliases, |mutation| { - self.transfer_function(state, mutation) + ModularMutationVisitor::new(&self.aliases, |_, mutations| { + self.transfer_function(state, mutations, location, false) }) .visit_statement(statement, location); } @@ -228,8 +270,8 @@ impl<'a, 'tcx> Analysis<'tcx> for FlowAnalysis<'a, 'tcx> { return; } - ModularMutationVisitor::new(&self.aliases, |mutation| { - self.transfer_function(state, mutation) + ModularMutationVisitor::new(&self.aliases, |_, mutations| { + self.transfer_function(state, mutations, location, false) }) .visit_terminator(terminator, location); } diff --git a/crates/flowistry/src/infoflow/dependencies.rs b/crates/flowistry/src/infoflow/dependencies.rs index db68e4f22..e8e06acf1 100644 --- a/crates/flowistry/src/infoflow/dependencies.rs +++ b/crates/flowistry/src/infoflow/dependencies.rs @@ -178,10 +178,11 @@ pub fn compute_dependencies<'tcx>( check(place); } } - _ => ModularMutationVisitor::new( - &results.analysis.aliases, - |Mutation { mutated, .. }| check(mutated), - ) + _ => ModularMutationVisitor::new(&results.analysis.aliases, |_, mutations| { + for Mutation { mutated, .. } in mutations { + check(mutated); + } + }) .visit_location(body, location), } } diff --git a/crates/flowistry/src/infoflow/mutation.rs b/crates/flowistry/src/infoflow/mutation.rs index 5ede06d4c..efd4c18e5 100644 --- a/crates/flowistry/src/infoflow/mutation.rs +++ b/crates/flowistry/src/infoflow/mutation.rs @@ -14,6 +14,7 @@ use crate::mir::{ }; /// Indicator of certainty about whether a place is being mutated. +#[derive(Debug)] pub enum MutationStatus { /// A place is definitely mutated, e.g. `x = y` definitely mutates `x`. Definitely, @@ -23,15 +24,13 @@ pub enum MutationStatus { } /// Information about a particular mutation. -pub struct Mutation<'a, 'tcx> { +#[derive(Debug)] +pub struct Mutation<'tcx> { /// The place that is being mutated. pub mutated: Place<'tcx>, /// The set of inputs to the mutating operation. - pub inputs: &'a [Place<'tcx>], - - /// Where the mutation is occuring. - pub location: Location, + pub inputs: Vec>, /// The certainty of whether the mutation is happening. pub status: MutationStatus, @@ -46,7 +45,7 @@ where // API design note: wcrichto tried making FnMut(...) a trait alias, but this // interacted poorly with type inference and required ModularMutationVisitor // clients to explicitly write out the type parameter of every closure argument. - F: FnMut(Mutation<'_, 'tcx>), + F: FnMut(Location, Vec>), { f: F, aliases: &'a Aliases<'a, 'tcx>, @@ -54,7 +53,7 @@ where impl<'a, 'tcx, F> ModularMutationVisitor<'a, 'tcx, F> where - F: FnMut(Mutation<'_, 'tcx>), + F: FnMut(Location, Vec>), { pub fn new(aliases: &'a Aliases<'a, 'tcx>, f: F) -> Self { ModularMutationVisitor { aliases, f } @@ -63,7 +62,7 @@ where impl<'tcx, F> Visitor<'tcx> for ModularMutationVisitor<'_, 'tcx, F> where - F: FnMut(Mutation<'_, 'tcx>), + F: FnMut(Location, Vec>), { fn visit_assign( &mut self, @@ -110,14 +109,14 @@ where (mutated.project_deeper(&[field], tcx), input_place) }); - for (mutated, input) in fields { - (self.f)(Mutation { + let mutations = fields + .map(|(mutated, input)| Mutation { mutated, - inputs: input.as_ref().map(std::slice::from_ref).unwrap_or_default(), - location, + inputs: input.into_iter().collect::>(), status: MutationStatus::Definitely, - }); - } + }) + .collect::>(); + (self.f)(location, mutations); return; } } @@ -133,16 +132,18 @@ where let fields = adt_def.all_fields().enumerate().map(|(i, field_def)| { PlaceElem::Field(FieldIdx::from_usize(i), field_def.ty(tcx, substs)) }); - for field in fields { - let mutated_field = mutated.project_deeper(&[field], tcx); - let input_field = place.project_deeper(&[field], tcx); - (self.f)(Mutation { - mutated: mutated_field, - inputs: &[input_field], - location, - status: MutationStatus::Definitely, - }); - } + let 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, + } + }) + .collect::>(); + (self.f)(location, mutations); return; } } @@ -153,12 +154,11 @@ where let mut collector = PlaceCollector::default(); collector.visit_rvalue(rvalue, location); - (self.f)(Mutation { + (self.f)(location, vec![Mutation { mutated: *mutated, - inputs: &collector.0, - location, + inputs: collector.0, status: MutationStatus::Definitely, - }); + }]); } fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { @@ -185,15 +185,17 @@ where .ty(self.aliases.body.local_decls(), tcx) .ty .is_unit(); - let empty = vec![]; - let inputs = if ret_is_unit { &empty } else { &arg_inputs }; + let inputs = if ret_is_unit { + Vec::new() + } else { + arg_inputs.clone() + }; - (self.f)(Mutation { + let mut mutations = vec![Mutation { mutated: *destination, inputs, - location, status: MutationStatus::Definitely, - }); + }]; for arg in arg_places { for arg_mut in self.aliases.reachable_values(arg, Mutability::Mut) { @@ -203,14 +205,15 @@ where continue; } - (self.f)(Mutation { + mutations.push(Mutation { mutated: *arg_mut, - inputs: &arg_inputs, - location, + inputs: arg_inputs.clone(), status: MutationStatus::Possibly, }); } } + + (self.f)(location, mutations); } _ => {} diff --git a/crates/flowistry/src/infoflow/recursive.rs b/crates/flowistry/src/infoflow/recursive.rs index 3cf65362c..e96ccf001 100644 --- a/crates/flowistry/src/infoflow/recursive.rs +++ b/crates/flowistry/src/infoflow/recursive.rs @@ -140,15 +140,14 @@ impl<'tcx> FlowAnalysis<'_, 'tcx> { let translate_child_to_parent = |child: Place<'tcx>, mutated: bool| -> Option> { - if child.local == RETURN_PLACE && child.projection.len() == 0 { - if child.ty(body.local_decls(), tcx).ty.is_unit() { - return None; - } - - return Some(*destination); + let child_ty = child.ty(body.local_decls(), tcx).ty; + if child_ty.is_unit() { + return None; } - if !child.is_arg(body) || (mutated && !child.is_indirect()) { + let can_translate = child.local == RETURN_PLACE + || (child.is_arg(body) && (!mutated || child.is_indirect())); + if !can_translate { return None; } @@ -158,10 +157,14 @@ impl<'tcx> FlowAnalysis<'_, 'tcx> { // parent_arg_projected = (*_5.0).1 // parent_arg_accessible = (*_5.0) - let parent_toplevel_arg = parent_arg_places - .iter() - .find(|(j, _)| child.local.as_usize() - 1 == *j) - .map(|(_, place)| place)?; + let parent_toplevel_arg = if child.local == RETURN_PLACE { + *destination + } else { + parent_arg_places + .iter() + .find(|(j, _)| child.local.as_usize() - 1 == *j) + .map(|(_, place)| *place)? + }; let mut projection = parent_toplevel_arg.projection.to_vec(); let mut ty = parent_toplevel_arg.ty(self.body.local_decls(), tcx); @@ -186,38 +189,39 @@ impl<'tcx> FlowAnalysis<'_, 'tcx> { Some(parent_arg_projected) }; - for (child, _) in return_state.rows() { - if let Some(parent) = translate_child_to_parent(child, true) { - let was_return = child.local == RETURN_PLACE; - // > 1 because arguments will always have their synthetic location in their dep set - let was_mutated = return_state.row_set(child).len() > 1; - if !was_mutated && !was_return { - continue; - } - - let child_deps = return_state.row_set(child); - let parent_deps = return_state - .rows() - .filter(|(_, deps)| child_deps.is_superset(deps)) - .filter_map(|(row, _)| translate_child_to_parent(row, false)) - .collect::>(); - - debug!( - "child {child:?} \n / child_deps {child_deps:?}\n-->\nparent {parent:?}\n / parent_deps {parent_deps:?}" - ); + let mutations = return_state.rows().filter_map(|(child, _)| { + let parent = translate_child_to_parent(child, true)?; - self.transfer_function(state, Mutation { - mutated: parent, - inputs: &parent_deps, - location, - status: if was_return { - MutationStatus::Definitely - } else { - MutationStatus::Possibly - }, - }); + let was_return: bool = child.local == RETURN_PLACE; + // > 1 because arguments will always have their synthetic location in their dep set + let was_mutated = return_state.row_set(child).len() > 1; + if !was_mutated && !was_return { + return None; } - } + + let child_deps = return_state.row_set(child); + let parent_deps = return_state + .rows() + .filter(|(_, deps)| child_deps.is_superset(deps)) + .filter_map(|(row, _)| translate_child_to_parent(row, false)) + .collect::>(); + + debug!( + "child {child:?} \n / child_deps {child_deps:?}\n-->\nparent {parent:?}\n / parent_deps {parent_deps:?}" + ); + + Some(Mutation { + mutated: parent, + inputs: parent_deps.clone(), + status: if was_return { + MutationStatus::Definitely + } else { + MutationStatus::Possibly + }, + }) + }).collect::>(); + + self.transfer_function(state, mutations, location, true); true } diff --git a/crates/flowistry/tests/extensions/recurse_return_field_sensitive.txt b/crates/flowistry/tests/extensions/recurse_return_field_sensitive.txt new file mode 100644 index 000000000..a8d5e1100 --- /dev/null +++ b/crates/flowistry/tests/extensions/recurse_return_field_sensitive.txt @@ -0,0 +1,8 @@ +/* recurse */ +fn mk(x: i32, y: i32) -> (i32, i32) { (x, y) } +fn main() { + let x = 0; + let y = 1; + let t = mk(x, y); + `(t.0)`; +} \ No newline at end of file diff --git a/crates/flowistry/tests/extensions/recurse_return_field_sensitive.txt.expected b/crates/flowistry/tests/extensions/recurse_return_field_sensitive.txt.expected new file mode 100644 index 000000000..2dc7c173b --- /dev/null +++ b/crates/flowistry/tests/extensions/recurse_return_field_sensitive.txt.expected @@ -0,0 +1,8 @@ +/* recurse */ +fn mk(x: i32, y: i32) -> (i32, i32) { (x, y) } +fn main() { + `[let x = 0;]` + let y = 1; + `[let t = mk(x, y);]` + `[t.0;]` +} \ No newline at end of file diff --git a/crates/flowistry_ide/src/focus/direct_influence.rs b/crates/flowistry_ide/src/focus/direct_influence.rs index 143c635fb..8cc5e8c16 100644 --- a/crates/flowistry_ide/src/focus/direct_influence.rs +++ b/crates/flowistry_ide/src/focus/direct_influence.rs @@ -14,27 +14,24 @@ 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()); - ModularMutationVisitor::new( - aliases, - |Mutation { - mutated, - inputs, - location, - .. - }| { - let mut add = |place: Place<'tcx>, mutability: Mutability| { - for alias in aliases.reachable_values(place, mutability) { - influence.insert(*alias, location); - } - }; + ModularMutationVisitor::new(aliases, |location, mutations| { + let mut add = |place: Place<'tcx>, mutability: Mutability| { + for alias in aliases.reachable_values(place, mutability) { + influence.insert(*alias, location); + } + }; + for Mutation { + mutated, inputs, .. + } in mutations + { for input in inputs { - add(*input, Mutability::Not); + add(input, Mutability::Not); } add(mutated, Mutability::Mut); - }, - ) + } + }) .visit_body(body); DirectInfluence { aliases, influence } From 09850d429ba42e2f807d9c217bcb36a27b9b2e40 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Mon, 22 May 2023 16:45:53 -0700 Subject: [PATCH 2/2] Fix field-sensitivity issue w/ private fields, update to latest rustc_plugin/utils --- crates/flowistry/Cargo.toml | 7 ++-- crates/flowistry/src/infoflow/analysis.rs | 15 +++------ crates/flowistry/src/infoflow/mutation.rs | 33 ++++++++++++++++--- crates/flowistry/src/infoflow/recursive.rs | 21 +++++++++--- .../string_drop_and_replace.txt.expected | 4 +-- .../struct_field_independence_heap.txt | 7 ++++ ...truct_field_independence_heap.txt.expected | 7 ++++ crates/flowistry_ide/Cargo.toml | 13 ++------ crates/flowistry_ifc/Cargo.toml | 10 ++---- 9 files changed, 73 insertions(+), 44 deletions(-) create mode 100644 crates/flowistry/tests/backward_slice/struct_field_independence_heap.txt create mode 100644 crates/flowistry/tests/backward_slice/struct_field_independence_heap.txt.expected diff --git a/crates/flowistry/Cargo.toml b/crates/flowistry/Cargo.toml index 768a007cc..5402f5822 100644 --- a/crates/flowistry/Cargo.toml +++ b/crates/flowistry/Cargo.toml @@ -11,7 +11,7 @@ license = "MIT" rustc_private = true [features] -test = ["rustc-utils/test"] +test = ["rustc_utils/test"] debug = ["html-escape"] [dependencies] @@ -20,14 +20,11 @@ log = "0.4" fluid-let = "1.0" cfg-if = "1.0" serde = {version = "1", features = ["derive"]} +rustc_utils = "0.6.0-nightly-2023-04-12" # For local debugging html-escape = {version = "0.2", optional = true} -[dependencies.rustc-utils] -git = "https://github.com/cognitive-engineering-lab/rustc-plugin" -tag = "nightly-2023-04-12-v0.1.4" - [dev-dependencies] # Hack based on https://github.com/rust-lang/cargo/issues/2911 flowistry = { path = ".", features = ["test"] } diff --git a/crates/flowistry/src/infoflow/analysis.rs b/crates/flowistry/src/infoflow/analysis.rs index c4e69b395..812d9ca73 100644 --- a/crates/flowistry/src/infoflow/analysis.rs +++ b/crates/flowistry/src/infoflow/analysis.rs @@ -22,6 +22,7 @@ use crate::{ impls::{LocationOrArg, LocationOrArgDomain, LocationOrArgSet}, IndexMatrix, IndexedDomain, }, + infoflow::mutation::ConflictType, mir::aliases::Aliases, }; @@ -89,8 +90,6 @@ impl<'a, 'tcx> FlowAnalysis<'a, 'tcx> { state: &mut FlowDomain<'tcx>, mutations: Vec>, location: Location, - // Small hack for recursive Flowistry that doesn't lookup conflicts of inputs, only aliases - recursive: bool, ) { debug!(" Applying mutations {mutations:?}"); let location_domain = self.location_domain(); @@ -120,11 +119,7 @@ impl<'a, 'tcx> FlowAnalysis<'a, 'tcx> { let add_deps = |state: &mut FlowDomain<'tcx>, place: Place<'tcx>, location_deps: &mut LocationOrArgSet| { - let reachable_values = if recursive { - all_aliases.aliases(place) - } else { - all_aliases.reachable_values(place, Mutability::Not) - }; + 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)) @@ -167,7 +162,7 @@ impl<'a, 'tcx> FlowAnalysis<'a, 'tcx> { let mutable_conflicts = mutations .iter() .map(|mt| { - let mut mutable_conflicts = if recursive { + 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() @@ -252,7 +247,7 @@ impl<'a, 'tcx> Analysis<'tcx> for FlowAnalysis<'a, 'tcx> { location: Location, ) { ModularMutationVisitor::new(&self.aliases, |_, mutations| { - self.transfer_function(state, mutations, location, false) + self.transfer_function(state, mutations, location) }) .visit_statement(statement, location); } @@ -271,7 +266,7 @@ impl<'a, 'tcx> Analysis<'tcx> for FlowAnalysis<'a, 'tcx> { } ModularMutationVisitor::new(&self.aliases, |_, mutations| { - self.transfer_function(state, mutations, location, false) + self.transfer_function(state, mutations, location) }) .visit_terminator(terminator, location); } diff --git a/crates/flowistry/src/infoflow/mutation.rs b/crates/flowistry/src/infoflow/mutation.rs index efd4c18e5..5e6333f30 100644 --- a/crates/flowistry/src/infoflow/mutation.rs +++ b/crates/flowistry/src/infoflow/mutation.rs @@ -6,7 +6,7 @@ use rustc_middle::{ ty::TyKind, }; use rustc_target::abi::FieldIdx; -use rustc_utils::{mir::place::PlaceCollector, OperandExt}; +use rustc_utils::{mir::place::PlaceCollector, AdtDefExt, OperandExt}; use crate::mir::{ aliases::Aliases, @@ -14,6 +14,7 @@ use crate::mir::{ }; /// Indicator of certainty about whether a place is being mutated. +/// Used to determine whether an update should be strong or weak. #[derive(Debug)] pub enum MutationStatus { /// A place is definitely mutated, e.g. `x = y` definitely mutates `x`. @@ -23,6 +24,12 @@ pub enum MutationStatus { Possibly, } +#[derive(Debug)] +pub enum ConflictType { + Include, + Exclude +} + /// Information about a particular mutation. #[derive(Debug)] pub struct Mutation<'tcx> { @@ -34,6 +41,8 @@ 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. @@ -114,6 +123,7 @@ where mutated, inputs: input.into_iter().collect::>(), status: MutationStatus::Definitely, + conflicts: ConflictType::Include }) .collect::>(); (self.f)(location, mutations); @@ -129,9 +139,12 @@ where let place_ty = place.ty(&body.local_decls, tcx).ty; if let TyKind::Adt(adt_def, substs) = place_ty.kind() { if adt_def.is_struct() { - let fields = adt_def.all_fields().enumerate().map(|(i, field_def)| { - PlaceElem::Field(FieldIdx::from_usize(i), field_def.ty(tcx, substs)) - }); + let fields = adt_def + .all_visible_fields(self.aliases.def_id, self.aliases.tcx) + .enumerate() + .map(|(i, field_def)| { + PlaceElem::Field(FieldIdx::from_usize(i), field_def.ty(tcx, substs)) + }); let mutations = fields .map(|field| { let mutated_field = mutated.project_deeper(&[field], tcx); @@ -140,8 +153,17 @@ where mutated: mutated_field, inputs: vec![input_field], status: MutationStatus::Definitely, + conflicts: ConflictType::Exclude } }) + .chain([ + Mutation { + mutated: *mutated, + inputs: vec![*place], + status: MutationStatus::Definitely, + conflicts: ConflictType::Exclude, + } + ]) .collect::>(); (self.f)(location, mutations); return; @@ -158,6 +180,7 @@ where mutated: *mutated, inputs: collector.0, status: MutationStatus::Definitely, + conflicts: ConflictType::Include }]); } @@ -195,6 +218,7 @@ where mutated: *destination, inputs, status: MutationStatus::Definitely, + conflicts: ConflictType::Include }]; for arg in arg_places { @@ -209,6 +233,7 @@ where 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 e96ccf001..906e7e73e 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::{Mutation, MutationStatus}, + mutation::{Mutation, MutationStatus, ConflictType}, FlowDomain, }, mir::utils, @@ -154,8 +154,8 @@ impl<'tcx> FlowAnalysis<'_, 'tcx> { // For example, say we're calling f(_5.0) and child = (*_1).1 where // .1 is private to parent. Then: // parent_toplevel_arg = _5.0 - // parent_arg_projected = (*_5.0).1 - // parent_arg_accessible = (*_5.0) + // child.projection = (*□).1 + // parent_arg_projected = (*_5.0) let parent_toplevel_arg = if child.local == RETURN_PLACE { *destination @@ -170,7 +170,17 @@ impl<'tcx> FlowAnalysis<'_, 'tcx> { let mut ty = parent_toplevel_arg.ty(self.body.local_decls(), tcx); let parent_param_env = tcx.param_env(self.def_id); log::debug!("Adding child {child:?} to parent {parent_toplevel_arg:?}"); - for elem in child.projection.iter() { + for elem in child.projection.iter() { + // Don't continue if we reach a private field + if let ProjectionElem::Field(field, _) = elem { + if let Some(adt_def) = ty.ty.ty_adt_def() { + let field = adt_def.all_fields().nth(field.as_usize()).unwrap(); + if !field.vis.is_accessible_from(self.def_id, self.tcx) { + break; + } + } + } + ty = ty.projection_ty_core( tcx, parent_param_env, @@ -218,10 +228,11 @@ impl<'tcx> FlowAnalysis<'_, 'tcx> { } else { MutationStatus::Possibly }, + conflicts: ConflictType::Exclude }) }).collect::>(); - self.transfer_function(state, mutations, location, true); + self.transfer_function(state, mutations, location); true } diff --git a/crates/flowistry/tests/backward_slice/string_drop_and_replace.txt.expected b/crates/flowistry/tests/backward_slice/string_drop_and_replace.txt.expected index f45072fe5..93c88d797 100644 --- a/crates/flowistry/tests/backward_slice/string_drop_and_replace.txt.expected +++ b/crates/flowistry/tests/backward_slice/string_drop_and_replace.txt.expected @@ -1,5 +1,5 @@ fn main() { - `[let mut x = String::new();]` - `[x = ]`String::new()`[;]` + let mut x = String::new(); + `[x = String::new();]` `[x;]` } \ No newline at end of file diff --git a/crates/flowistry/tests/backward_slice/struct_field_independence_heap.txt b/crates/flowistry/tests/backward_slice/struct_field_independence_heap.txt new file mode 100644 index 000000000..f3c05da2f --- /dev/null +++ b/crates/flowistry/tests/backward_slice/struct_field_independence_heap.txt @@ -0,0 +1,7 @@ +struct Point { x: String, y: String } +fn main() { + let x = String::new(); + let y = String::new(); + let p = Point { x, y }; + `(&p.x)`; +} \ No newline at end of file diff --git a/crates/flowistry/tests/backward_slice/struct_field_independence_heap.txt.expected b/crates/flowistry/tests/backward_slice/struct_field_independence_heap.txt.expected new file mode 100644 index 000000000..b14699fa9 --- /dev/null +++ b/crates/flowistry/tests/backward_slice/struct_field_independence_heap.txt.expected @@ -0,0 +1,7 @@ +struct Point { x: String, y: String } +fn main() { + `[let x = String::new();]` + let y = String::new(); + `[let p = Point { x, y };]` + `[&p.x;]` +} \ No newline at end of file diff --git a/crates/flowistry_ide/Cargo.toml b/crates/flowistry_ide/Cargo.toml index 9dfc40a8b..bcd4cec4d 100644 --- a/crates/flowistry_ide/Cargo.toml +++ b/crates/flowistry_ide/Cargo.toml @@ -24,6 +24,8 @@ serde = {version = "1", features = ["derive"]} serde_json = "1" flate2 = "1" base64 = "0.21" +rustc_utils = {version = "0.6.0-nightly-2023-04-12", features = ["serde"]} +rustc_plugin = "0.6.0-nightly-2023-04-12" # Decompose petgraph = {version = "0.6", default-features = false, optional = true} @@ -31,13 +33,4 @@ rayon = {version = "1.5", optional = true} # For binaries env_logger = {version = "0.9", default-features = false} -clap = {version = "4", default-features = false, features = ["std", "derive"]} - -[dependencies.rustc-plugin] -git = "https://github.com/cognitive-engineering-lab/rustc-plugin" -tag = "nightly-2023-04-12-v0.1.4" - -[dependencies.rustc-utils] -git = "https://github.com/cognitive-engineering-lab/rustc-plugin" -tag = "nightly-2023-04-12-v0.1.4" -features = ["serde"] \ No newline at end of file +clap = {version = "4", default-features = false, features = ["std", "derive"]} \ No newline at end of file diff --git a/crates/flowistry_ifc/Cargo.toml b/crates/flowistry_ifc/Cargo.toml index e9dab15fe..db46632de 100644 --- a/crates/flowistry_ifc/Cargo.toml +++ b/crates/flowistry_ifc/Cargo.toml @@ -13,11 +13,5 @@ env_logger = "0.9" termcolor = "1.1" anyhow = "1" log = "0.4" - -[dependencies.rustc-plugin] -git = "https://github.com/cognitive-engineering-lab/rustc-plugin" -tag = "nightly-2023-04-12-v0.1.4" - -[dependencies.rustc-utils] -git = "https://github.com/cognitive-engineering-lab/rustc-plugin" -tag = "nightly-2023-04-12-v0.1.4" \ No newline at end of file +rustc_plugin = "0.6.0-nightly-2023-04-12" +rustc_utils = "0.6.0-nightly-2023-04-12" \ No newline at end of file