From c3d37fd53faa4eef3d1cf9a7c0d0ebd8f13fdd3a Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Mon, 7 Aug 2023 15:51:35 -0700 Subject: [PATCH 01/10] Update to latest rustc_plugin, bump to 0.5.37 --- crates/flowistry/Cargo.toml | 4 ++-- crates/flowistry_ide/Cargo.toml | 8 ++++---- crates/flowistry_ifc/Cargo.toml | 8 ++++---- crates/flowistry_ifc_traits/Cargo.toml | 2 +- ide/package.json | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/flowistry/Cargo.toml b/crates/flowistry/Cargo.toml index 53b41a703e..ac7723e4a5 100644 --- a/crates/flowistry/Cargo.toml +++ b/crates/flowistry/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flowistry" -version = "0.5.36" +version = "0.5.37" edition = "2021" authors = ["Will Crichton "] description = "Modular information flow analysis" @@ -20,7 +20,7 @@ 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" +rustc_utils = "0.6.1-nightly-2023-04-12" # For local debugging html-escape = {version = "0.2", optional = true} diff --git a/crates/flowistry_ide/Cargo.toml b/crates/flowistry_ide/Cargo.toml index 40c7db5076..9681193702 100644 --- a/crates/flowistry_ide/Cargo.toml +++ b/crates/flowistry_ide/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flowistry_ide" -version = "0.5.36" +version = "0.5.37" edition = "2021" authors = ["Will Crichton "] description = "Information Flow in the IDE for Rust" @@ -14,7 +14,7 @@ rustc_private = true decompose = ["petgraph", "rayon"] [dependencies] -flowistry = {version = "0.5.36", path = "../flowistry"} +flowistry = {version = "0.5.37", path = "../flowistry"} anyhow = "1" log = "0.4" fluid-let = "1.0" @@ -24,8 +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" +rustc_utils = {version = "0.6.1-nightly-2023-04-12", features = ["serde"]} +rustc_plugin = "0.6.1-nightly-2023-04-12" # Decompose petgraph = {version = "0.6", default-features = false, optional = true} diff --git a/crates/flowistry_ifc/Cargo.toml b/crates/flowistry_ifc/Cargo.toml index 78ca44ad96..6031ac91e8 100644 --- a/crates/flowistry_ifc/Cargo.toml +++ b/crates/flowistry_ifc/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flowistry_ifc" -version = "0.5.36" +version = "0.5.37" edition = "2021" publish = false @@ -8,10 +8,10 @@ publish = false rustc_private = true [dependencies] -flowistry = {version = "0.5.36", path = "../flowistry"} +flowistry = {version = "0.5.37", path = "../flowistry"} env_logger = "0.9" termcolor = "1.1" anyhow = "1" log = "0.4" -rustc_plugin = "0.6.0-nightly-2023-04-12" -rustc_utils = "0.6.0-nightly-2023-04-12" \ No newline at end of file +rustc_plugin = "0.6.1-nightly-2023-04-12" +rustc_utils = "0.6.1-nightly-2023-04-12" \ No newline at end of file diff --git a/crates/flowistry_ifc_traits/Cargo.toml b/crates/flowistry_ifc_traits/Cargo.toml index 25c929bcf0..b61426e760 100644 --- a/crates/flowistry_ifc_traits/Cargo.toml +++ b/crates/flowistry_ifc_traits/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flowistry_ifc_traits" -version = "0.5.36" +version = "0.5.37" edition = "2021" publish = false diff --git a/ide/package.json b/ide/package.json index 3dea046bcd..8b5ba10958 100644 --- a/ide/package.json +++ b/ide/package.json @@ -12,7 +12,7 @@ "type": "git" }, "description": "Information Flow in the IDE for Rust", - "version": "0.5.36", + "version": "0.5.37", "engines": { "vscode": "^1.54.0" }, From 3f6cb7f3dcc1cfcc6a372907d87201ab288f13ae Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Mon, 7 Aug 2023 22:00:10 -0700 Subject: [PATCH 02/10] Fix WASM issue, update to 0.5.38 --- crates/flowistry/Cargo.toml | 4 ++-- crates/flowistry_ide/Cargo.toml | 8 ++++---- crates/flowistry_ifc/Cargo.toml | 8 ++++---- crates/flowistry_ifc_traits/Cargo.toml | 2 +- ide/package.json | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/flowistry/Cargo.toml b/crates/flowistry/Cargo.toml index ac7723e4a5..dc0834cebf 100644 --- a/crates/flowistry/Cargo.toml +++ b/crates/flowistry/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flowistry" -version = "0.5.37" +version = "0.5.38" edition = "2021" authors = ["Will Crichton "] description = "Modular information flow analysis" @@ -20,7 +20,7 @@ log = "0.4" fluid-let = "1.0" cfg-if = "1.0" serde = {version = "1", features = ["derive"]} -rustc_utils = "0.6.1-nightly-2023-04-12" +rustc_utils = "0.6.2-nightly-2023-04-12" # For local debugging html-escape = {version = "0.2", optional = true} diff --git a/crates/flowistry_ide/Cargo.toml b/crates/flowistry_ide/Cargo.toml index 9681193702..aa5d1f5e97 100644 --- a/crates/flowistry_ide/Cargo.toml +++ b/crates/flowistry_ide/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flowistry_ide" -version = "0.5.37" +version = "0.5.38" edition = "2021" authors = ["Will Crichton "] description = "Information Flow in the IDE for Rust" @@ -14,7 +14,7 @@ rustc_private = true decompose = ["petgraph", "rayon"] [dependencies] -flowistry = {version = "0.5.37", path = "../flowistry"} +flowistry = {version = "0.5.38", path = "../flowistry"} anyhow = "1" log = "0.4" fluid-let = "1.0" @@ -24,8 +24,8 @@ serde = {version = "1", features = ["derive"]} serde_json = "1" flate2 = "1" base64 = "0.21" -rustc_utils = {version = "0.6.1-nightly-2023-04-12", features = ["serde"]} -rustc_plugin = "0.6.1-nightly-2023-04-12" +rustc_utils = {version = "0.6.2-nightly-2023-04-12", features = ["serde"]} +rustc_plugin = "0.6.2-nightly-2023-04-12" # Decompose petgraph = {version = "0.6", default-features = false, optional = true} diff --git a/crates/flowistry_ifc/Cargo.toml b/crates/flowistry_ifc/Cargo.toml index 6031ac91e8..fdcb17d763 100644 --- a/crates/flowistry_ifc/Cargo.toml +++ b/crates/flowistry_ifc/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flowistry_ifc" -version = "0.5.37" +version = "0.5.38" edition = "2021" publish = false @@ -8,10 +8,10 @@ publish = false rustc_private = true [dependencies] -flowistry = {version = "0.5.37", path = "../flowistry"} +flowistry = {version = "0.5.38", path = "../flowistry"} env_logger = "0.9" termcolor = "1.1" anyhow = "1" log = "0.4" -rustc_plugin = "0.6.1-nightly-2023-04-12" -rustc_utils = "0.6.1-nightly-2023-04-12" \ No newline at end of file +rustc_plugin = "0.6.2-nightly-2023-04-12" +rustc_utils = "0.6.2-nightly-2023-04-12" \ No newline at end of file diff --git a/crates/flowistry_ifc_traits/Cargo.toml b/crates/flowistry_ifc_traits/Cargo.toml index b61426e760..a4b35cbed3 100644 --- a/crates/flowistry_ifc_traits/Cargo.toml +++ b/crates/flowistry_ifc_traits/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flowistry_ifc_traits" -version = "0.5.37" +version = "0.5.38" edition = "2021" publish = false diff --git a/ide/package.json b/ide/package.json index 8b5ba10958..aedf61e982 100644 --- a/ide/package.json +++ b/ide/package.json @@ -12,7 +12,7 @@ "type": "git" }, "description": "Information Flow in the IDE for Rust", - "version": "0.5.37", + "version": "0.5.38", "engines": { "vscode": "^1.54.0" }, From 260141804971dd49ab23aee37eedc20459f106b3 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 24 Aug 2023 16:07:35 -0700 Subject: [PATCH 03/10] Formatting --- crates/flowistry/src/infoflow/mutation.rs | 26 ++++++++++------------ crates/flowistry/src/infoflow/recursive.rs | 4 ++-- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/crates/flowistry/src/infoflow/mutation.rs b/crates/flowistry/src/infoflow/mutation.rs index 5e6333f30f..1835770f99 100644 --- a/crates/flowistry/src/infoflow/mutation.rs +++ b/crates/flowistry/src/infoflow/mutation.rs @@ -27,7 +27,7 @@ pub enum MutationStatus { #[derive(Debug)] pub enum ConflictType { Include, - Exclude + Exclude, } /// Information about a particular mutation. @@ -123,7 +123,7 @@ where mutated, inputs: input.into_iter().collect::>(), status: MutationStatus::Definitely, - conflicts: ConflictType::Include + conflicts: ConflictType::Include, }) .collect::>(); (self.f)(location, mutations); @@ -153,17 +153,15 @@ 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, } - ]) + }) + .chain([Mutation { + mutated: *mutated, + inputs: vec![*place], + status: MutationStatus::Definitely, + conflicts: ConflictType::Exclude, + }]) .collect::>(); (self.f)(location, mutations); return; @@ -180,7 +178,7 @@ where mutated: *mutated, inputs: collector.0, status: MutationStatus::Definitely, - conflicts: ConflictType::Include + conflicts: ConflictType::Include, }]); } @@ -218,7 +216,7 @@ where mutated: *destination, inputs, status: MutationStatus::Definitely, - conflicts: ConflictType::Include + conflicts: ConflictType::Include, }]; for arg in arg_places { @@ -233,7 +231,7 @@ where mutated: *arg_mut, inputs: arg_inputs.clone(), status: MutationStatus::Possibly, - conflicts: ConflictType::Include + conflicts: ConflictType::Include, }); } } diff --git a/crates/flowistry/src/infoflow/recursive.rs b/crates/flowistry/src/infoflow/recursive.rs index 906e7e73e0..646a37e9a0 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, ConflictType}, + mutation::{ConflictType, Mutation, MutationStatus}, FlowDomain, }, mir::utils, @@ -170,7 +170,7 @@ 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() { From 77ae052dfe7edeba4235890eff1f407180251e53 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 24 Aug 2023 16:09:25 -0700 Subject: [PATCH 04/10] Make benchmarks work on macOS --- crates/flowistry/benches/main.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/flowistry/benches/main.rs b/crates/flowistry/benches/main.rs index 51be5d238f..fe083adf5d 100644 --- a/crates/flowistry/benches/main.rs +++ b/crates/flowistry/benches/main.rs @@ -5,7 +5,7 @@ extern crate rustc_driver; extern crate rustc_hir; extern crate rustc_interface; extern crate rustc_middle; -use std::process::Command; +use std::{env::consts::DLL_SUFFIX, process::Command}; use anyhow::{Context, Result}; use criterion::{ @@ -132,7 +132,7 @@ fn criterion_benchmark(c: &mut Criterion) { let test_dir = curr_dir.join("../../../crates/flowistry/benches/tests"); // The shared object for the bench_utils crate should also be in deps/ - let bench_crate_pattern = curr_dir.join("*libbench_utils*.so"); + let bench_crate_pattern = curr_dir.join(format!("*libbench_utils*{}", DLL_SUFFIX)); let print_sysroot = Command::new("rustc") .args(&["--print", "sysroot"]) @@ -144,7 +144,12 @@ fn criterion_benchmark(c: &mut Criterion) { // Find bench_utils .so file let shared_object = glob(bench_crate_pattern.to_str().unwrap())? .nth(0) - .context("Failed to find bench_utils shared object")??; + .with_context(|| { + format!( + "Failed to find bench_utils shared object in dir {}", + curr_dir.display() + ) + })??; let mut run_bench = |test: (&str, &str)| { // Stress types correspond to bench files within ./tests/ From c14fd5c6065655698b38478803390a620b252ee8 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 24 Aug 2023 18:18:38 -0700 Subject: [PATCH 05/10] 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 812d9ca734..8ff5150b0b 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 e8e06acf10..b8cc4d2525 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 1835770f99..8f8baa7c20 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 646a37e9a0..62be72b01c 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 34a3f56708..4a8d3f94d1 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 fe49f454cd..2d1321c01f 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 4dc526e2b4..e608150339 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 06/10] 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 dc0834cebf..a2beb93d68 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 8ff5150b0b..0d1ec3ab3b 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 b8cc4d2525..812b813241 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 0beebd7317..e4480b37b7 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 8f8baa7c20..55cfd6f47a 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 4a8d3f94d1..e4a0e77089 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 090fdab3f4..07cbd3c2fe 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 0000000000..b546c4c513 --- /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 0000000000..3a3a57e5a7 --- /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 0000000000..40c12c765e --- /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 aa5d1f5e97..e535412bc0 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 8cc5e8c161..ce9ffc58e4 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 e8004b81ba..2bf5e9a452 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 fdcb17d763..0e4aa328d4 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 From 6fa1606d25653ccd4b42d8134817a43c1d6ce13e Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Fri, 25 Aug 2023 19:32:36 -0700 Subject: [PATCH 07/10] Change CharPos to use line/number encoding for compatibility w/ VSCode on Window --- Cargo.toml | 6 +- crates/flowistry/src/test_utils.rs | 17 +++--- crates/flowistry_ide/src/focus/mod.rs | 2 + crates/flowistry_ide/src/plugin.rs | 43 ++++++++++--- crates/flowistry_ide/src/spans.rs | 6 +- ide/src/decorations.ts | 52 ++++++++-------- ide/src/focus.ts | 29 +++++---- ide/src/range.ts | 83 +++++++++++++++++--------- ide/src/tests/commands/util/helpers.ts | 4 +- 9 files changed, 155 insertions(+), 87 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 042adeb9d7..df9c712c7b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,4 +3,8 @@ members = ["crates/*"] exclude = ["ide/src/tests/mock_project"] [profile.bench] -debug = true \ No newline at end of file +debug = true + +[patch.crates-io] +rustc_plugin = { git = "https://github.com/cognitive-engineering-lab/rustc_plugin", branch = "charpos-fix" } +rustc_utils = { git = "https://github.com/cognitive-engineering-lab/rustc_plugin", branch = "charpos-fix" } \ No newline at end of file diff --git a/crates/flowistry/src/test_utils.rs b/crates/flowistry/src/test_utils.rs index dad04a165f..574517d3e7 100644 --- a/crates/flowistry/src/test_utils.rs +++ b/crates/flowistry/src/test_utils.rs @@ -14,7 +14,7 @@ pub use rustc_utils::test_utils::{compare_ranges, fmt_ranges, parse_ranges}; use rustc_utils::{ mir::borrowck_facts, source_map::{ - range::{ByteRange, ToSpan}, + range::{ByteRange, CharPos, ToSpan}, spanner::Spanner, }, test_utils, @@ -57,15 +57,18 @@ pub fn bless( [("`[", char_range.start), ("]`", char_range.end)] }) .collect::>(); - delims.sort_by_key(|(_, i)| i.0); + delims.sort_by_key(|(_, i)| (i.line, i.column)); let mut output = String::new(); - for (i, g) in contents.chars().enumerate() { - while delims.len() > 0 && delims[0].1 .0 == i { - let (delim, _) = delims.remove(0); - output.push_str(delim); + for (line, line_str) in contents.lines().enumerate() { + for (column, chr) in line_str.chars().enumerate() { + while delims.len() > 0 && delims[0].1 == (CharPos { line, column }) { + let (delim, _) = delims.remove(0); + output.push_str(delim); + } + output.push(chr); } - output.push(g); + output.push('\n'); } fs::write(path.with_extension("txt.expected"), output)?; diff --git a/crates/flowistry_ide/src/focus/mod.rs b/crates/flowistry_ide/src/focus/mod.rs index 2bf5e9a452..bfa4367717 100644 --- a/crates/flowistry_ide/src/focus/mod.rs +++ b/crates/flowistry_ide/src/focus/mod.rs @@ -93,6 +93,8 @@ pub fn focus(tcx: TyCtxt, body_id: BodyId) -> Result { .collect::>() }; + log::debug!("{:#?}", to_ranges(slice.clone())); + Some(PlaceInfo { range: CharRange::from_span(mir_span.span(), source_map).ok()?, ranges: to_ranges(vec![mir_span.span()]), diff --git a/crates/flowistry_ide/src/plugin.rs b/crates/flowistry_ide/src/plugin.rs index 3775b58373..c228bf5e73 100644 --- a/crates/flowistry_ide/src/plugin.rs +++ b/crates/flowistry_ide/src/plugin.rs @@ -53,7 +53,8 @@ enum FlowistryCommand { Focus { file: String, - pos: usize, + pos_line: usize, + pos_column: usize, }, Decompose { @@ -63,8 +64,10 @@ enum FlowistryCommand { Playground { file: String, - start: usize, - end: usize, + start_line: usize, + start_column: usize, + end_line: usize, + end_column: usize, }, Preload, @@ -141,11 +144,22 @@ impl RustcPlugin for FlowistryPlugin { match plugin_args.command { Spans { file, .. } => postprocess(crate::spans::spans(&compiler_args, file)), Playground { - file, start, end, .. + file, + start_line, + start_column, + end_line, + end_column, + .. } => { let compute_target = || CharRange { - start: CharPos(start), - end: CharPos(end), + start: CharPos { + line: start_line, + column: start_column, + }, + end: CharPos { + line: end_line, + column: end_column, + }, filename: Filename::intern(&file), }; postprocess(run( @@ -154,13 +168,23 @@ impl RustcPlugin for FlowistryPlugin { &compiler_args, )) } - Focus { file, pos, .. } => { + Focus { + file, + pos_line, + pos_column, + .. + } => { let compute_target = || { + let cpos = CharPos { + line: pos_line, + column: pos_column, + }; let range = CharRange { - start: CharPos(pos), - end: CharPos(pos), + start: cpos, + end: cpos, filename: Filename::intern(&file), }; + debug!("eyo WTF {range:?} {file}"); FunctionIdentifier::Range(range) }; postprocess(run(crate::focus::focus, compute_target, &compiler_args)) @@ -312,6 +336,7 @@ impl T> rustc_driver::Callbacks let mut analysis = self.analysis.take().unwrap(); self.output = Some((|| { let target = (self.compute_target.take().unwrap())().to_span(tcx)?; + debug!("target span: {target:?}"); let mut bodies = find_enclosing_bodies(tcx, target); let body = bodies.next().context("Selection did not map to a body")?; analysis.analyze(tcx, body) diff --git a/crates/flowistry_ide/src/spans.rs b/crates/flowistry_ide/src/spans.rs index 4c33c58413..aa2a82e2df 100644 --- a/crates/flowistry_ide/src/spans.rs +++ b/crates/flowistry_ide/src/spans.rs @@ -1,5 +1,5 @@ use rustc_utils::source_map::{ - filename::Filename, find_bodies::find_bodies, range::ByteRange, + filename::Filename, find_bodies::find_bodies, range::CharRange, }; use serde::Serialize; @@ -7,7 +7,7 @@ use crate::plugin::{FlowistryError, FlowistryResult}; #[derive(Serialize)] pub struct SpansOutput { - spans: Vec, + spans: Vec, } unsafe impl Send for SpansOutput {} @@ -37,7 +37,7 @@ impl rustc_driver::Callbacks for Callbacks { .filter(|span| { source_map.lookup_source_file(span.lo()).name_hash == source_file.name_hash }) - .filter_map(|span| ByteRange::from_span(span, source_map).ok()) + .filter_map(|span| CharRange::from_span(span, source_map).ok()) .collect::>(); Ok(SpansOutput { spans }) })()); diff --git a/ide/src/decorations.ts b/ide/src/decorations.ts index 8d404f1765..8c99517e32 100644 --- a/ide/src/decorations.ts +++ b/ide/src/decorations.ts @@ -1,7 +1,13 @@ import _ from "lodash"; import * as vscode from "vscode"; -import { Range, to_vsc_range } from "./range"; +import { + Interval, + Range, + interval_to_range, + range_to_interval, + to_vsc_range, +} from "./range"; export let highlight_type = vscode.window.createTextEditorDecorationType({ backgroundColor: new vscode.ThemeColor("editor.symbolHighlightBackground"), @@ -33,38 +39,36 @@ export let select_type = vscode.window.createTextEditorDecorationType({ }, }); -export let invert_ranges = (container: Range, pieces: Range[]): Range[] => { +export let invert_ranges = ( + container: Range, + pieces: Range[], + doc: vscode.TextDocument +): Range[] => { + let icontainer = range_to_interval(container, doc); + let ipieces = pieces.map((piece) => range_to_interval(piece, doc)); + let filename = container.filename; - let pieces_sorted = _.sortBy(pieces, (r) => r.start).filter( - (r) => - container.start <= r.start && r.end <= container.end + let pieces_sorted = _.sortBy(ipieces, (r) => r[0]).filter( + (r) => icontainer[0] <= r[0] && r[1] <= icontainer[1] ); - let new_ranges: Range[] = []; - let start = container.start; + let new_ranges: Interval[] = []; + let start = icontainer[0]; pieces_sorted.forEach((r) => { - if (r.start < start) { - start = Math.max(r.end, start); + if (r[0] < start) { + start = Math.max(r[1], start); return; } - let end = r.start; - new_ranges.push({ - start: start, - end: end, - filename, - }); + let end = r[0]; + new_ranges.push([start, end]); - start = Math.max(start, r.end); + start = Math.max(start, r[1]); }); - new_ranges.push({ - start: start, - end: container.end, - filename, - }); + new_ranges.push([start, icontainer[1]]); - return new_ranges; + return new_ranges.map((intvl) => interval_to_range(intvl, filename, doc)); }; export let highlight_slice = ( @@ -76,7 +80,7 @@ export let highlight_slice = ( ) => { highlight_ranges(seeds, editor, select_type); let hide_ranges = containers - .map((container) => invert_ranges(container, slice)) + .map((container) => invert_ranges(container, slice, editor.document)) .flat(); highlight_ranges(hide_ranges, editor, hide_type); highlight_ranges(slice, editor, slice_type); @@ -90,7 +94,7 @@ export function highlight_ranges( ) { editor.setDecorations( type, - ranges.map((range) => to_vsc_range(range, editor.document)) + ranges.map((range) => to_vsc_range(range)) ); } diff --git a/ide/src/focus.ts b/ide/src/focus.ts index 325aab270d..1d9cade51b 100644 --- a/ide/src/focus.ts +++ b/ide/src/focus.ts @@ -29,11 +29,12 @@ class FocusBodyState { focus: Focus; places: RangeTree; - constructor(focus: Focus) { + constructor(focus: Focus, doc: vscode.TextDocument) { this.mark = null; this.focus = focus; this.places = new RangeTree( - focus.place_info.map((info) => ({ range: info.range, value: info })) + focus.place_info.map((info) => ({ range: info.range, value: info })), + doc ); } @@ -44,15 +45,17 @@ class FocusBodyState { let cmd = [ "focus", doc.fileName, - doc.offsetAt(selection.anchor).toString(), + selection.anchor.line.toString(), + selection.anchor.character.toString(), ]; let focus_res = await globals.call_flowistry(cmd); + console.log(focus_res); if (!is_ok(focus_res)) { return focus_res; } - return ok(new FocusBodyState(focus_res.value)); + return ok(new FocusBodyState(focus_res.value, doc)); }; private find_slice_at_selection = ( @@ -60,7 +63,6 @@ class FocusBodyState { doc: vscode.TextDocument ): { seeds: Range[]; slice: Range[]; direct_influence: Range[] } => { let query = this.places.selection_to_interval( - doc, this.mark || editor.selection ); let result = this.places.search(query); @@ -92,7 +94,7 @@ class FocusBodyState { if (seeds.length > 0) { if (select) { editor.selections = slice.map((range) => { - let vsc_range = to_vsc_range(range, doc); + let vsc_range = to_vsc_range(range); return new vscode.Selection(vsc_range.start, vsc_range.end); }); } else { @@ -131,9 +133,10 @@ class FocusDocumentState { // vscode.TextEditor to reduce API complexity. But those references were // seemingly invalidated after changing documents, so the editor must be // passed in anew each time. - constructor(spans: Spans) { + constructor(spans: Spans, doc: vscode.TextDocument) { this.bodies = new RangeTree( - spans.spans.map((range) => ({ range, value: new Cell(null) })) + spans.spans.map((range) => ({ range, value: new Cell(null) })), + doc ); } @@ -142,11 +145,12 @@ class FocusDocumentState { ): Promise> => { let cmd = ["spans", editor.document.fileName]; let spans = await globals.call_flowistry(cmd); + console.log("spans", spans); if (!is_ok(spans)) { return spans; } - return ok(new FocusDocumentState(spans.value)); + return ok(new FocusDocumentState(spans.value, editor.document)); }; on_change_selection = async ( @@ -171,7 +175,7 @@ class FocusDocumentState { ): Promise | null> => { // Find all bodies that contain the user's selection. let result = this.bodies.search( - this.bodies.selection_to_interval(editor.document, editor.selection) + this.bodies.selection_to_interval(editor.selection) ); // If the user hasn't selected a body, then return null @@ -346,7 +350,10 @@ export class FocusMode { } }; - private handle_analysis_result = async (result: FlowistryResult, userActivated: boolean = false) => { + private handle_analysis_result = async ( + result: FlowistryResult, + userActivated: boolean = false + ) => { if (!is_ok(result)) { if (userActivated) await show_error(result); this.set_mode("error"); diff --git a/ide/src/range.ts b/ide/src/range.ts index 2d614e2abb..e96c86054f 100644 --- a/ide/src/range.ts +++ b/ide/src/range.ts @@ -2,22 +2,48 @@ import IntervalTree from "@flatten-js/interval-tree"; import _ from "lodash"; import vscode from "vscode"; +export interface CharPos { + line: number; + column: number; +} + export interface Range { - start: number; - end: number; + start: CharPos; + end: CharPos; filename: string; } -export let to_vsc_range = ( - range: Range, - doc: vscode.TextDocument -): vscode.Range => +export let to_vsc_range = (range: Range): vscode.Range => new vscode.Range( - doc.positionAt(range.start), - doc.positionAt(range.end) + range.start.line, + range.start.column, + range.end.line, + range.end.column ); -type Interval = [number, number]; +export let range_to_interval = ( + range: Range, + doc: vscode.TextDocument +): Interval => { + let vsc_range = to_vsc_range(range); + return [doc.offsetAt(vsc_range.start), doc.offsetAt(vsc_range.end)]; +}; + +export let interval_to_range = ( + interval: Interval, + filename: string, + doc: vscode.TextDocument +): Range => { + let start = doc.positionAt(interval[0]); + let end = doc.positionAt(interval[1]); + return { + start: { line: start.line, column: start.character }, + end: { line: end.line, column: end.character }, + filename, + }; +}; + +export type Interval = [number, number]; export type Ranged = { range: Range; @@ -31,27 +57,31 @@ export interface SearchResult { export class RangeTree { tree: IntervalTree; + filename: string; - constructor(entries: Ranged[] = []) { + constructor(entries: Ranged[] = [], readonly doc: vscode.TextDocument) { this.tree = new IntervalTree(); + this.filename = entries.length > 0 ? entries[0].range.filename : ""; entries.forEach(({ range, value }) => { this.insert(range, value); }); } - range_to_interval(range: Range): Interval { - return [range.start, range.end]; - } - - selection_to_interval( - doc: vscode.TextDocument, - selection: vscode.Selection - ): Interval { - return [doc.offsetAt(selection.start), doc.offsetAt(selection.end)]; + selection_to_interval(selection: vscode.Selection): Interval { + return [ + this.doc.offsetAt(selection.start), + this.doc.offsetAt(selection.end), + ]; } insert(range: Range, data: T) { - this.tree.insert(this.range_to_interval(range), data); + console.log( + "Inserting range", + range, + "at interval", + range_to_interval(range, this.doc) + ); + this.tree.insert(range_to_interval(range, this.doc), data); } search(query: Interval): SearchResult { @@ -70,11 +100,7 @@ export class RangeTree { }; result.forEach(([interval, value]) => { - let range = { - start: interval[0], - end: interval[1], - filename: "", - }; + let range = interval_to_range(interval, this.filename, this.doc); if (is_contained(interval, query)) { final.contained.push({ range, value }); } else if (is_contained(query, interval)) { @@ -86,12 +112,11 @@ export class RangeTree { ["contained", "containing", "overlapping"].forEach((k) => { let final_ = final as any; - final_[k] = _.sortBy( - final_[k], - ({ range }) => range.end - range.start - ); + final_[k] = _.sortBy(final_[k], ({ range }) => range.end - range.start); }); + console.log("Querying", query, "for result", final); + return final; } } diff --git a/ide/src/tests/commands/util/helpers.ts b/ide/src/tests/commands/util/helpers.ts index 8a190686d2..0c824a1603 100644 --- a/ide/src/tests/commands/util/helpers.ts +++ b/ide/src/tests/commands/util/helpers.ts @@ -119,9 +119,7 @@ export const get_command_selections = async ( const unique_ranges = _.uniqWith(output_data.ranges, _.isEqual); const sorted_ranges = _.sortBy(unique_ranges, (range) => [range.start]); - const vscode_ranges = sorted_ranges.map((range) => - to_vsc_range(range, vscode.window.activeTextEditor?.document!) - ); + const vscode_ranges = sorted_ranges.map((range) => to_vsc_range(range)); const merged_ranges = merge_ranges(vscode_ranges); const expected_selections = merged_ranges.map( (range) => new vscode.Selection(range.start, range.end) From 093cf053444fecdbbaecf96b37d45ef571c01b46 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Sat, 26 Aug 2023 15:42:50 -0700 Subject: [PATCH 08/10] Update to latest rustc_plugin --- .github/workflows/release.yml | 2 +- Cargo.toml | 7 ++----- README.md | 4 ++-- crates/flowistry/Cargo.toml | 2 +- crates/flowistry/examples/example.rs | 2 +- crates/flowistry/src/indexed/mod.rs | 7 ++----- crates/flowistry/src/infoflow/analysis.rs | 18 ++++++++++-------- crates/flowistry/src/infoflow/mod.rs | 2 +- crates/flowistry/src/infoflow/recursive.rs | 2 +- crates/flowistry/src/mir/aliases.rs | 4 ++-- crates/flowistry/src/mir/engine.rs | 10 ++++++---- crates/flowistry/src/mir/placeinfo.rs | 4 +--- crates/flowistry/src/test_utils.rs | 2 +- crates/flowistry_ide/Cargo.toml | 6 +++--- crates/flowistry_ide/src/playground.rs | 5 ++--- crates/flowistry_ide/src/plugin.rs | 9 ++++----- crates/flowistry_ifc/Cargo.toml | 4 ++-- rust-toolchain.toml | 2 +- 18 files changed, 43 insertions(+), 49 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2032ce36e0..78adcb7e41 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -49,7 +49,7 @@ jobs: - uses: actions/checkout@v1 - uses: actions-rs/toolchain@v1 with: - toolchain: nightly-2023-04-12 + toolchain: nightly-2023-08-25 components: rust-src, rustc-dev, llvm-tools-preview target: ${{ matrix.target }} profile: minimal diff --git a/Cargo.toml b/Cargo.toml index df9c712c7b..c54172a45d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,10 +1,7 @@ [workspace] members = ["crates/*"] exclude = ["ide/src/tests/mock_project"] +resolver = "2" [profile.bench] -debug = true - -[patch.crates-io] -rustc_plugin = { git = "https://github.com/cognitive-engineering-lab/rustc_plugin", branch = "charpos-fix" } -rustc_utils = { git = "https://github.com/cognitive-engineering-lab/rustc_plugin", branch = "charpos-fix" } \ No newline at end of file +debug = true \ No newline at end of file diff --git a/README.md b/README.md index 4c2cd8878d..d546f09cf3 100644 --- a/README.md +++ b/README.md @@ -78,8 +78,8 @@ The documentation is published here: https://willcrichton.net/flowistry/flowistr ## Usage -Note that the latest Flowistry has a [**Maximum** Supported Rust Version](https://github.com/cognitive-engineering-lab/rustc_plugin/tree/main#maximum-supported-rust-version) of **Rust 1.69**. -Flowistry is not guaranteed to work with features implemented after 1.69. +Note that the latest Flowistry has a [**Maximum** Supported Rust Version](https://github.com/cognitive-engineering-lab/rustc_plugin/tree/main#maximum-supported-rust-version) of **Rust 1.73**. +Flowistry is not guaranteed to work with features implemented after 1.73. ### Startup diff --git a/crates/flowistry/Cargo.toml b/crates/flowistry/Cargo.toml index a2beb93d68..4aed2265da 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.3-nightly-2023-04-12" +rustc_utils = "0.7.0-nightly-2023-08-25" # For local debugging html-escape = {version = "0.2", optional = true} diff --git a/crates/flowistry/examples/example.rs b/crates/flowistry/examples/example.rs index c6362db6af..0efc3cd2fb 100644 --- a/crates/flowistry/examples/example.rs +++ b/crates/flowistry/examples/example.rs @@ -25,7 +25,7 @@ extern crate rustc_span; use std::process::Command; use flowistry::{indexed::impls::LocationOrArg, infoflow::Direction}; -use rustc_borrowck::BodyWithBorrowckFacts; +use rustc_borrowck::consumers::BodyWithBorrowckFacts; use rustc_hir::{BodyId, ItemKind}; use rustc_middle::{ mir::{Local, Place}, diff --git a/crates/flowistry/src/indexed/mod.rs b/crates/flowistry/src/indexed/mod.rs index 1b26dfe797..f3c1e56a9a 100644 --- a/crates/flowistry/src/indexed/mod.rs +++ b/crates/flowistry/src/indexed/mod.rs @@ -9,7 +9,7 @@ //! Therefore if we want to encode a set of locations (e.g. for the [`FlowDomain`](crate::infoflow::FlowDomain)), //! then we can assign each `Location` a numeric index and use a compact bit-set instead of, say, //! a hash set. Concretely, this means: -//! 1. Defining a type `LocationOrArgIndex` that implements the [`Idx`](rustc_index::vec::Idx) trait. +//! 1. Defining a type `LocationOrArgIndex` that implements the [`Idx`](rustc_index::Idx) trait. //! 2. Creating a mapping ("domain") from `Location`s to `LocationOrArgIndex`. //! 3. Constructing a `LocationSet` out of a [`BitSet`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_index/bit_set/struct.BitSet.html)``. //! @@ -35,10 +35,7 @@ use std::{ }; use rustc_data_structures::fx::FxHashMap as HashMap; -use rustc_index::{ - bit_set::BitSet, - vec::{Idx, IndexVec}, -}; +use rustc_index::{bit_set::BitSet, Idx, IndexVec}; use rustc_mir_dataflow::{fmt::DebugWithContext, JoinSemiLattice}; pub mod impls; diff --git a/crates/flowistry/src/infoflow/analysis.rs b/crates/flowistry/src/infoflow/analysis.rs index 0d1ec3ab3b..890bb3fd64 100644 --- a/crates/flowistry/src/infoflow/analysis.rs +++ b/crates/flowistry/src/infoflow/analysis.rs @@ -234,7 +234,7 @@ impl<'a, 'tcx> AnalysisDomain<'tcx> for FlowAnalysis<'a, 'tcx> { impl<'a, 'tcx> Analysis<'tcx> for FlowAnalysis<'a, 'tcx> { fn apply_statement_effect( - &self, + &mut self, state: &mut Self::Domain, statement: &Statement<'tcx>, location: Location, @@ -245,30 +245,32 @@ impl<'a, 'tcx> Analysis<'tcx> for FlowAnalysis<'a, 'tcx> { .visit_statement(statement, location); } - fn apply_terminator_effect( - &self, + fn apply_terminator_effect<'mir>( + &mut self, state: &mut Self::Domain, - terminator: &Terminator<'tcx>, + terminator: &'mir Terminator<'tcx>, location: Location, - ) { + ) -> TerminatorEdges<'mir, 'tcx> { if matches!(terminator.kind, TerminatorKind::Call { .. }) && is_extension_active(|mode| mode.context_mode == ContextMode::Recurse) && self.recurse_into_call(state, &terminator.kind, location) { - return; + return terminator.edges(); } ModularMutationVisitor::new(&self.place_info, |_, mutations| { self.transfer_function(state, mutations, location) }) .visit_terminator(terminator, location); + + terminator.edges() } fn apply_call_return_effect( - &self, + &mut self, _state: &mut Self::Domain, _block: BasicBlock, - _return_places: rustc_mir_dataflow::CallReturnPlaces<'_, 'tcx>, + _return_places: CallReturnPlaces<'_, 'tcx>, ) { } } diff --git a/crates/flowistry/src/infoflow/mod.rs b/crates/flowistry/src/infoflow/mod.rs index e4480b37b7..d18906febc 100644 --- a/crates/flowistry/src/infoflow/mod.rs +++ b/crates/flowistry/src/infoflow/mod.rs @@ -6,7 +6,7 @@ use std::cell::RefCell; use log::debug; -use rustc_borrowck::BodyWithBorrowckFacts; +use rustc_borrowck::consumers::BodyWithBorrowckFacts; use rustc_hir::BodyId; use rustc_middle::ty::TyCtxt; use rustc_utils::{block_timer, BodyExt}; diff --git a/crates/flowistry/src/infoflow/recursive.rs b/crates/flowistry/src/infoflow/recursive.rs index 62be72b01c..1a51511e38 100644 --- a/crates/flowistry/src/infoflow/recursive.rs +++ b/crates/flowistry/src/infoflow/recursive.rs @@ -1,7 +1,7 @@ use log::{debug, info}; use rustc_middle::{ mir::*, - ty::{subst::GenericArgKind, ClosureKind, TyKind}, + ty::{ClosureKind, GenericArgKind, TyKind}, }; use rustc_mir_dataflow::JoinSemiLattice; use rustc_utils::{mir::borrowck_facts::get_body_with_borrowck_facts, PlaceExt}; diff --git a/crates/flowistry/src/mir/aliases.rs b/crates/flowistry/src/mir/aliases.rs index e4a0e77089..a2b219166b 100644 --- a/crates/flowistry/src/mir/aliases.rs +++ b/crates/flowistry/src/mir/aliases.rs @@ -12,7 +12,7 @@ use rustc_data_structures::{ use rustc_hir::def_id::DefId; use rustc_index::{ bit_set::{HybridBitSet, SparseBitMatrix}, - vec::IndexVec, + IndexVec, }; use rustc_middle::{ mir::{visit::Visitor, *}, @@ -87,7 +87,7 @@ impl<'a, 'tcx> Aliases<'a, 'tcx> { let start = Instant::now(); let body = &body_with_facts.body; let static_region = RegionVid::from_usize(0); - let subset_base = &body_with_facts.input_facts.subset_base; + let subset_base = &body_with_facts.input_facts.as_ref().unwrap().subset_base; let all_pointers = body .local_decls() diff --git a/crates/flowistry/src/mir/engine.rs b/crates/flowistry/src/mir/engine.rs index 6751ce4508..bd7bc14440 100644 --- a/crates/flowistry/src/mir/engine.rs +++ b/crates/flowistry/src/mir/engine.rs @@ -15,7 +15,7 @@ use std::rc::Rc; use either::Either; use rustc_data_structures::{graph::WithSuccessors, work_queue::WorkQueue}; -use rustc_index::vec::IndexVec; +use rustc_index::IndexVec; use rustc_middle::{ mir::{traversal, Body, Location}, ty::TyCtxt, @@ -39,7 +39,7 @@ pub struct AnalysisResults<'tcx, A: Analysis<'tcx>> { impl<'tcx, A: Analysis<'tcx>> AnalysisResults<'tcx, A> { pub fn visit_reachable_with<'mir, V>(&self, body: &'mir Body<'tcx>, visitor: &mut V) where - V: ResultsVisitor<'mir, 'tcx, FlowState = A::Domain>, + V: ResultsVisitor<'mir, 'tcx, Self, FlowState = A::Domain>, { for (block, data) in traversal::reachable(body) { for statement_index in 0 ..= data.statements.len() { @@ -51,17 +51,19 @@ impl<'tcx, A: Analysis<'tcx>> AnalysisResults<'tcx, A> { let state = &self.state[loc_index]; if statement_index == 0 { - visitor.visit_block_start(state, data, block); + visitor.visit_block_start(self, state, data, block); } if statement_index == data.statements.len() { visitor.visit_terminator_after_primary_effect( + self, state, data.terminator(), location, ) } else { visitor.visit_statement_after_primary_effect( + self, state, &data.statements[statement_index], location, @@ -85,7 +87,7 @@ pub fn iterate_to_fixpoint<'tcx, A: Analysis<'tcx>>( _tcx: TyCtxt<'tcx>, body: &Body<'tcx>, location_domain: Rc, - analysis: A, + mut analysis: A, ) -> AnalysisResults<'tcx, A> { let bottom_value = analysis.bottom_value(body); diff --git a/crates/flowistry/src/mir/placeinfo.rs b/crates/flowistry/src/mir/placeinfo.rs index b546c4c513..bb09ac7211 100644 --- a/crates/flowistry/src/mir/placeinfo.rs +++ b/crates/flowistry/src/mir/placeinfo.rs @@ -144,9 +144,7 @@ impl<'a, 'tcx> PlaceInfo<'a, 'tcx> { 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)), + unknown_region: Region::new_var(self.tcx, UNKNOWN_REGION), target_mutability: mutability, stack: vec![], loans: PlaceSet::default(), diff --git a/crates/flowistry/src/test_utils.rs b/crates/flowistry/src/test_utils.rs index 574517d3e7..069ca00c53 100644 --- a/crates/flowistry/src/test_utils.rs +++ b/crates/flowistry/src/test_utils.rs @@ -5,7 +5,7 @@ use std::{fs, io, panic, path::Path}; use anyhow::Result; use fluid_let::fluid_set; use log::info; -use rustc_borrowck::BodyWithBorrowckFacts; +use rustc_borrowck::consumers::BodyWithBorrowckFacts; use rustc_data_structures::fx::FxHashSet as HashSet; use rustc_hir::BodyId; use rustc_middle::ty::TyCtxt; diff --git a/crates/flowistry_ide/Cargo.toml b/crates/flowistry_ide/Cargo.toml index e535412bc0..d72e6c91c6 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.3-nightly-2023-04-12", features = ["serde"]} -rustc_plugin = "0.6.3-nightly-2023-04-12" +rustc_utils = {version = "0.7.0-nightly-2023-08-25", features = ["serde"]} +rustc_plugin = "0.7.0-nightly-2023-08-25" # Decompose petgraph = {version = "0.6", default-features = false, optional = true} @@ -33,4 +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"]} \ No newline at end of file +clap = {version = "4.4", default-features = false, features = ["std", "derive"]} \ No newline at end of file diff --git a/crates/flowistry_ide/src/playground.rs b/crates/flowistry_ide/src/playground.rs index bdf476e183..4ef98e89d1 100644 --- a/crates/flowistry_ide/src/playground.rs +++ b/crates/flowistry_ide/src/playground.rs @@ -17,9 +17,8 @@ pub fn playground(tcx: TyCtxt, body_id: BodyId) -> Result { let body = &body_with_facts.body; debug!("{}", body.to_string(tcx).unwrap()); - let outlives = body_with_facts - .input_facts - .subset_base + let subset_base = &body_with_facts.input_facts.as_ref().unwrap().subset_base; + let outlives = subset_base .iter() .map(|(sup, sub, _)| (format!("{sup:?}"), format!("{sub:?}"))) .collect::>(); diff --git a/crates/flowistry_ide/src/plugin.rs b/crates/flowistry_ide/src/plugin.rs index c228bf5e73..0ebc7c2385 100644 --- a/crates/flowistry_ide/src/plugin.rs +++ b/crates/flowistry_ide/src/plugin.rs @@ -18,6 +18,7 @@ use rustc_hir::BodyId; use rustc_interface::interface::Result as RustcResult; use rustc_middle::ty::TyCtxt; use rustc_plugin::{CrateFilter, RustcPlugin, RustcPluginArgs, Utf8Path}; +use rustc_span::ErrorGuaranteed; use rustc_utils::{ mir::borrowck_facts, source_map::{ @@ -218,9 +219,7 @@ fn postprocess(result: FlowistryResult) -> RustcResult<()> { let result = match result { Ok(output) => Ok(output), Err(e) => match e { - FlowistryError::BuildError => { - return Err(rustc_errors::ErrorGuaranteed::unchecked_claim_error_was_emitted()); - } + FlowistryError::BuildError(e) => return Err(e), e => Err(e), }, }; @@ -249,7 +248,7 @@ pub fn run_with_callbacks( ); let compiler = rustc_driver::RunCompiler::new(&args, callbacks); - compiler.run().map_err(|_| FlowistryError::BuildError) + compiler.run().map_err(FlowistryError::BuildError) } fn run( @@ -281,7 +280,7 @@ fn run( #[derive(Debug, Serialize)] #[serde(tag = "type")] pub enum FlowistryError { - BuildError, + BuildError(#[serde(skip_serializing)] ErrorGuaranteed), AnalysisError { error: String }, FileNotFound, } diff --git a/crates/flowistry_ifc/Cargo.toml b/crates/flowistry_ifc/Cargo.toml index 0e4aa328d4..07dc260557 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.3-nightly-2023-04-12" -rustc_utils = "0.6.3-nightly-2023-04-12" \ No newline at end of file +rustc_plugin = "0.7.0-nightly-2023-08-25" +rustc_utils = "0.7.0-nightly-2023-08-25" \ No newline at end of file diff --git a/rust-toolchain.toml b/rust-toolchain.toml index a9dabd6501..d9d396bd29 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2023-04-12" +channel = "nightly-2023-08-25" components = ["rust-src", "rustc-dev", "llvm-tools-preview"] \ No newline at end of file From e4e8a5c785458630c90787e01c221636ee098f57 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Sat, 26 Aug 2023 15:47:49 -0700 Subject: [PATCH 09/10] Fix benchmark --- README.md | 2 +- crates/flowistry/benches/main.rs | 2 +- crates/flowistry_ide/README.md | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index d546f09cf3..d25fae98a0 100644 --- a/README.md +++ b/README.md @@ -182,7 +182,7 @@ If rustup fails, especially with an error like "could not rename downloaded file To solve the issue, go to the command line and run: ``` -rustup toolchain install nightly-2023-04-12 -c rust-src -c rustc-dev -c llvm-tools-preview +rustup toolchain install nightly-2023-08-25 -c rust-src -c rustc-dev -c llvm-tools-preview ``` Then go back to VSCode and click "Continue" to let Flowistry continue installing. diff --git a/crates/flowistry/benches/main.rs b/crates/flowistry/benches/main.rs index fe083adf5d..5a1cb5d319 100644 --- a/crates/flowistry/benches/main.rs +++ b/crates/flowistry/benches/main.rs @@ -13,7 +13,7 @@ use criterion::{ }; use flowistry::infoflow::Direction; use glob::glob; -use rustc_borrowck::BodyWithBorrowckFacts; +use rustc_borrowck::consumers::BodyWithBorrowckFacts; use rustc_hir::{BodyId, ItemKind}; use rustc_middle::{ mir::{Location, Place}, diff --git a/crates/flowistry_ide/README.md b/crates/flowistry_ide/README.md index 4ff6855686..1f6775d965 100644 --- a/crates/flowistry_ide/README.md +++ b/crates/flowistry_ide/README.md @@ -108,9 +108,9 @@ If rustup fails, especially with an error like "could not rename downloaded file To solve the issue, go to the command line and run: ``` -rustup toolchain install nightly-2023-04-12 -c rust-src -c rustc-dev -c llvm-tools-preview +rustup toolchain install nightly-2023-08-25 -c rust-src -c rustc-dev -c llvm-tools-preview ``` -> Note: double check the value of "channel" in `rust-toolchain.toml` if `nightly-2023-04-12` is no longer correct. +> Note: double check the value of "channel" in `rust-toolchain.toml` if `nightly-2023-08-25` is no longer correct. Then go back to VSCode and click "Continue" to let Flowistry continue installing. From 2a2d981e6b5e888dfed574d949e021054d242fd1 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Sat, 26 Aug 2023 18:48:50 -0700 Subject: [PATCH 10/10] Bump to 0.5.39 --- crates/flowistry/Cargo.toml | 2 +- crates/flowistry_ide/Cargo.toml | 4 ++-- crates/flowistry_ifc/Cargo.toml | 4 ++-- crates/flowistry_ifc_traits/Cargo.toml | 2 +- ide/package.json | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/flowistry/Cargo.toml b/crates/flowistry/Cargo.toml index 4aed2265da..3d0150f930 100644 --- a/crates/flowistry/Cargo.toml +++ b/crates/flowistry/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flowistry" -version = "0.5.38" +version = "0.5.39" edition = "2021" authors = ["Will Crichton "] description = "Modular information flow analysis" diff --git a/crates/flowistry_ide/Cargo.toml b/crates/flowistry_ide/Cargo.toml index d72e6c91c6..b02c36b84f 100644 --- a/crates/flowistry_ide/Cargo.toml +++ b/crates/flowistry_ide/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flowistry_ide" -version = "0.5.38" +version = "0.5.39" edition = "2021" authors = ["Will Crichton "] description = "Information Flow in the IDE for Rust" @@ -14,7 +14,7 @@ rustc_private = true decompose = ["petgraph", "rayon"] [dependencies] -flowistry = {version = "0.5.38", path = "../flowistry"} +flowistry = {version = "0.5.39", path = "../flowistry"} anyhow = "1" log = "0.4" fluid-let = "1.0" diff --git a/crates/flowistry_ifc/Cargo.toml b/crates/flowistry_ifc/Cargo.toml index 07dc260557..40881483cf 100644 --- a/crates/flowistry_ifc/Cargo.toml +++ b/crates/flowistry_ifc/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flowistry_ifc" -version = "0.5.38" +version = "0.5.39" edition = "2021" publish = false @@ -8,7 +8,7 @@ publish = false rustc_private = true [dependencies] -flowistry = {version = "0.5.38", path = "../flowistry"} +flowistry = {version = "0.5.39", path = "../flowistry"} env_logger = "0.9" termcolor = "1.1" anyhow = "1" diff --git a/crates/flowistry_ifc_traits/Cargo.toml b/crates/flowistry_ifc_traits/Cargo.toml index a4b35cbed3..da32ab8661 100644 --- a/crates/flowistry_ifc_traits/Cargo.toml +++ b/crates/flowistry_ifc_traits/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flowistry_ifc_traits" -version = "0.5.38" +version = "0.5.39" edition = "2021" publish = false diff --git a/ide/package.json b/ide/package.json index aedf61e982..82737447de 100644 --- a/ide/package.json +++ b/ide/package.json @@ -12,7 +12,7 @@ "type": "git" }, "description": "Information Flow in the IDE for Rust", - "version": "0.5.38", + "version": "0.5.39", "engines": { "vscode": "^1.54.0" },