Treat boxes as owned pointers (#154)

## What Changed? Pointers of type `Box` are no longer associated with `UNKNOWN_REGION` and thus not considered to be aliasing one another. Also gets rid of `ModelControl` arguments, since they are unused. ## Why Does It Need To? Fixes the overtaint associated with the aliasing behavior of box. ## Checklist - [x] Make `tcx` an argument to `is_direct` - [x] Above description has been filled out so that upon quash merge we have a good record of what changed. - [x] New functions, methods, types are documented. Old documentation is updated if necessary - [ ] Documentation in Notion has been updated - [x] Tests for new behaviors are provided - [ ] New test suites (if any) ave been added to the CI tests (in `.github/workflows/rust.yml`) either as compiler test or integration test. *Or* justification for their omission from CI has been provided in this PR description.
brownsys · Jun 9, 2024 · c870cef · c870cef
1 parent d76edcb
commit c870cef
Show file tree

Hide file tree

Showing 14 changed files with 394 additions and 113 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -13,22 +13,23 @@ indexical = "0.3.1"
 serde = "1.0.188"
 petgraph = { version = "0.6", features = ["serde-1"] }
 strum = { version = "0.25", features = ["derive"] }
-# rustc_utils = { version = "=0.7.4-nightly-2023-08-25", features = [
-#     "indexical",
-# ] }
-# rustc_plugin = "=0.7.4-nightly-2023-08-25"
-rustc_utils = { git = "https://github.com/JustusAdam/rustc_plugin", rev = "e990ded60afc928f76293fb9ad265c58405da1a7", features = [
+anyhow = { version = "1.0.72", features = ["backtrace"] }
+
+rustc_utils = { version = "=0.7.4-nightly-2023-08-25", features = [
     "indexical",
 ] }
-rustc_plugin = { git = "https://github.com/JustusAdam/rustc_plugin", rev = "e990ded60afc928f76293fb9ad265c58405da1a7" }
-#rustc_plugin = { path = "../rustc_plugin/crates/rustc_plugin" }
-#rustc_utils = { path = "../rustc_plugin/crates/rustc_utils", features = ["indexical"] }
-flowistry = { git = "https://github.com/brownsys/flowistry", rev = "a2ccfca2e6b5668ffd246eddc6abaf4d6e440a35", default-features = false }
-anyhow = { version = "1.0.72", features = ["backtrace"] }
+rustc_plugin = "=0.7.4-nightly-2023-08-25"
+#flowistry = { path = "../flowistry/crates/flowistry", default-features = false }
+flowistry = { git = "https://github.com/brownsys/flowistry", rev = "b9210041eb846ce88644a4a19569ed2afb26141c", default-features = false }
 
 [profile.release]
 debug = true
 
-# [replace]
+[replace]
 # "rustc_utils:0.7.4-nightly-2023-08-25" = { path = "../rustc_plugin/crates/rustc_utils" }
 # "rustc_plugin:0.7.4-nightly-2023-08-25" = { path = "../rustc_plugin/crates/rustc_plugin" }
+
+"rustc_utils:0.7.4-nightly-2023-08-25" = { git = "https://github.com/JustusAdam/rustc_plugin", rev = "aa83f5740fa7eb5b8e3e1ee417b29536e87cc864", features = [
+    "indexical",
+] }
+"rustc_plugin:0.7.4-nightly-2023-08-25" = { git = "https://github.com/JustusAdam/rustc_plugin", rev = "aa83f5740fa7eb5b8e3e1ee417b29536e87cc864" }
diff --git a/Makefile.toml b/Makefile.toml
@@ -48,8 +48,6 @@ stage them and rerun the script until no more errors occur.
 dependencies = ["format-all", "clippy-all"]
 
 [tasks.analyzer-tests]
-description = "Low-level tests for the PDG emitted by the analyzer specifically."
-command = "cargo"
 args = [
     "test",
     "-p",
@@ -63,9 +61,13 @@ args = [
     "--test",
     "new_alias_analysis_tests",
     "--test",
+    "boxes",
+    "--test",
     "async_tests",
     "--no-fail-fast",
 ]
+description = "Low-level tests for the PDG emitted by the analyzer specifically."
+command = "cargo"
 
 [tasks.policy-framework-tests]
 description = "Tests related to the correctness of the policy framework."

diff --git a/crates/flowistry_pdg_construction/src/construct.rs b/crates/flowistry_pdg_construction/src/construct.rs
@@ -714,7 +714,7 @@ impl<'tcx> GraphConstructor<'tcx> {
     ) -> Vec<DepNode<'tcx>> {
         // Include all sources of indirection (each reference in the chain) as relevant places.
         let provenance = input
-            .refs_in_projection()
+            .refs_in_projection(self.place_info.body, self.place_info.tcx)
             .map(|(place_ref, _)| Place::from_ref(place_ref, self.tcx));
         let inputs = iter::once(input).chain(provenance);
 
@@ -794,19 +794,18 @@ impl<'tcx> GraphConstructor<'tcx> {
     ) -> Vec<(Place<'tcx>, DepNode<'tcx>)> {
         // **POINTER-SENSITIVITY:**
         // If `mutated` involves indirection via dereferences, then resolve it to the direct places it could point to.
-        let aliases = self.aliases(mutated).collect_vec();
+        let aliases = self.aliases(mutated);
 
         // **FIELD-SENSITIVITY:** we do NOT deal with fields on *writes* (in this function),
         // only on *reads* (in `add_input_to_op`).
 
         // For each mutated `dst`:
         aliases
-            .iter()
             .map(|dst| {
                 // Create a destination node for (DST @ CURRENT_LOC).
                 (
-                    *dst,
-                    DepNode::new(*dst, self.make_call_string(location), self.tcx, &self.body),
+                    dst,
+                    DepNode::new(dst, self.make_call_string(location), self.tcx, &self.body),
                 )
             })
             .collect()

diff --git a/crates/flowistry_pdg_construction/src/mutation.rs b/crates/flowistry_pdg_construction/src/mutation.rs
@@ -234,7 +234,7 @@ where
             // reference, except for the provenance of reborrows.
             Rvalue::Ref(_, _, place) => {
                 let inputs = place
-                    .refs_in_projection()
+                    .refs_in_projection(self.place_info.body, self.place_info.tcx)
                     .map(|(place_ref, _)| (Place::from_ref(place_ref, tcx), None))
                     .collect::<Vec<_>>();
                 (self.f)(

diff --git a/crates/paralegal-flow/src/ann/db.rs b/crates/paralegal-flow/src/ann/db.rs
@@ -412,17 +412,7 @@ impl<'tcx> MarkerCtx<'tcx> {
             )
         };
 
-        let include_type_markers =
-            self.0.config.local_function_type_marking() || !function.def_id().is_local();
-        direct_markers.chain(
-            if include_type_markers {
-                get_type_markers()
-            } else {
-                None
-            }
-            .into_iter()
-            .flatten(),
-        )
+        direct_markers.chain(get_type_markers().into_iter().flatten())
     }
 
     /// Iterate over all discovered annotations, whether local or external
@@ -463,7 +453,7 @@ pub struct MarkerDatabase<'tcx> {
     /// Cache whether markers are reachable transitively.
     reachable_markers: Cache<FnResolution<'tcx>, Box<[Identifier]>>,
     /// Configuration options
-    config: &'static MarkerControl,
+    _config: &'static MarkerControl,
     type_markers: Cache<ty::Ty<'tcx>, Box<TypeMarkers>>,
 }
 
@@ -475,7 +465,7 @@ impl<'tcx> MarkerDatabase<'tcx> {
             local_annotations: HashMap::default(),
             external_annotations: resolve_external_markers(args, tcx),
             reachable_markers: Default::default(),
-            config: args.marker_control(),
+            _config: args.marker_control(),
             type_markers: Default::default(),
         }
     }
@@ -530,7 +520,7 @@ type RawExternalMarkers = HashMap<String, Vec<crate::ann::MarkerAnnotation>>;
 /// Given the TOML of external annotations we have parsed, resolve the paths
 /// (keys of the map) to [`DefId`]s.
 fn resolve_external_markers(opts: &Args, tcx: TyCtxt) -> ExternalMarkers {
-    if let Some(annotation_file) = opts.modelctrl().external_annotations() {
+    if let Some(annotation_file) = opts.marker_control().external_annotations() {
         let from_toml: RawExternalMarkers = toml::from_str(
             &std::fs::read_to_string(annotation_file).unwrap_or_else(|_| {
                 panic!(

diff --git a/crates/paralegal-flow/src/args.rs b/crates/paralegal-flow/src/args.rs
@@ -12,6 +12,7 @@
 use anyhow::Error;
 use clap::ValueEnum;
 use std::ffi::{OsStr, OsString};
+use std::path::PathBuf;
 
 use crate::utils::TinyBitSet;
 use crate::{num_derive, num_traits::FromPrimitive};
@@ -48,7 +49,6 @@ impl TryFrom<ClapArgs> for Args {
             target,
             abort_after_analysis,
             mut anactrl,
-            modelctrl,
             dump,
             marker_control,
             cargo_args,
@@ -98,7 +98,6 @@ impl TryFrom<ClapArgs> for Args {
             target,
             abort_after_analysis,
             anactrl: anactrl.try_into()?,
-            modelctrl,
             dump,
             build_config,
             marker_control,
@@ -133,8 +132,6 @@ pub struct Args {
     marker_control: MarkerControl,
     /// Additional arguments that control the flow analysis specifically
     anactrl: AnalysisCtrl,
-    /// Additional arguments that control the generation and composition of the model
-    modelctrl: ModelCtrl,
     /// Additional arguments that control debug output specifically
     dump: DumpArgs,
     /// Additional configuration for the build process/rustc
@@ -143,6 +140,25 @@ pub struct Args {
     cargo_args: Vec<String>,
 }
 
+impl Default for Args {
+    fn default() -> Self {
+        Self {
+            verbosity: log::LevelFilter::Info,
+            log_level_config: LogLevelConfig::Disabled,
+            result_path: PathBuf::from(paralegal_spdg::FLOW_GRAPH_OUT_NAME),
+            relaxed: false,
+            target: None,
+            abort_after_analysis: false,
+            marker_control: Default::default(),
+            anactrl: Default::default(),
+            dump: Default::default(),
+            build_config: Default::default(),
+            cargo_args: Vec::new(),
+            attach_to_debugger: None,
+        }
+    }
+}
+
 /// Arguments as exposed on the command line.
 ///
 /// You should then use `try_into` to convert this to [`Args`], the argument
@@ -185,9 +201,6 @@ pub struct ClapArgs {
     /// Additional arguments which control marker assignment and discovery
     #[clap(flatten, next_help_heading = "Marker Control")]
     marker_control: MarkerControl,
-    /// Additional arguments that control the generation and composition of the model
-    #[clap(flatten, next_help_heading = "Model Generation")]
-    modelctrl: ModelCtrl,
     /// Additional arguments that control debug args specifically
     #[clap(flatten)]
     dump: ParseableDumpArgs,
@@ -253,7 +266,7 @@ impl From<ParseableDumpArgs> for DumpArgs {
 /// cli, internally we use the snake-case version of the option as a method on
 /// this type. This is so we can rename the outer UI without breaking code or
 /// even combine options together.
-#[derive(serde::Serialize, serde::Deserialize, Clone)]
+#[derive(serde::Serialize, serde::Deserialize, Clone, Default)]
 pub struct DumpArgs(TinyBitSet);
 
 impl DumpArgs {
@@ -331,9 +344,6 @@ impl Args {
         &self.anactrl
     }
 
-    pub fn modelctrl(&self) -> &ModelCtrl {
-        &self.modelctrl
-    }
     /// the file to write results to
     pub fn result_path(&self) -> &std::path::Path {
         self.result_path.as_path()
@@ -366,8 +376,8 @@ impl Args {
     }
 }
 
-#[derive(serde::Serialize, serde::Deserialize, clap::Args)]
-pub struct ModelCtrl {
+#[derive(serde::Serialize, serde::Deserialize, clap::Args, Default)]
+pub struct MarkerControl {
     /// A JSON file from which to load additional annotations. Whereas normally
     /// annotation can only be placed on crate-local items, these can also be
     /// placed on third party items, such as functions from the stdlib.
@@ -381,30 +391,12 @@ pub struct ModelCtrl {
     external_annotations: Option<std::path::PathBuf>,
 }
 
-impl ModelCtrl {
+impl MarkerControl {
     pub fn external_annotations(&self) -> Option<&std::path::Path> {
         self.external_annotations.as_deref()
     }
 }
 
-/// Arguments which control marker assignment and discovery
-#[derive(serde::Serialize, serde::Deserialize, clap::Args)]
-pub struct MarkerControl {
-    /// Don't mark the outputs of local functions if they are of a marked type.
-    ///
-    /// Be aware that disabling this can cause unsoundness as inline
-    /// construction of such types will not be emitted into the model. A warning
-    /// is however emitted in that case.
-    #[clap(long, env = "PARALEGAL_NO_LOCAL_FUNCTION_TYPE_MARKING")]
-    no_local_function_type_marking: bool,
-}
-
-impl MarkerControl {
-    pub fn local_function_type_marking(&self) -> bool {
-        !self.no_local_function_type_marking
-    }
-}
-
 /// Arguments that control the flow analysis
 #[derive(clap::Args)]
 struct ClapAnalysisCtrl {
@@ -442,6 +434,15 @@ pub struct AnalysisCtrl {
     inlining_depth: InliningDepth,
 }
 
+impl Default for AnalysisCtrl {
+    fn default() -> Self {
+        Self {
+            analyze: Vec::new(),
+            inlining_depth: InliningDepth::Adaptive,
+        }
+    }
+}
+
 impl TryFrom<ClapAnalysisCtrl> for AnalysisCtrl {
     type Error = Error;
     fn try_from(value: ClapAnalysisCtrl) -> Result<Self, Self::Error> {