Check configurations for @AnalyzeAllOverrides when invalidating the c…

…ache

Summary:
We should invalidate the cache if detecting a change in analysis mode
`AnalyzeALlOverrides`. This diff adds an integration test for this situation.

Reviewed By: alexkassil

Differential Revision: D50291405

fbshipit-source-id: 3368267dfda81d2f5bd1972b66a5d14ad9a36a60

source/interprocedural_analyses/taint/cache.ml

@@ -63,6 +63,7 @@ end
 module AnalysisSetup = struct
   type t = {
     maximum_overrides: int option;
+    analyze_all_overrides_targets: Interprocedural.Target.Set.t;
     attribute_targets: Interprocedural.Target.Set.t;
     skip_analysis_targets: Interprocedural.Target.Set.t;
     skip_overrides_targets: Ast.Reference.SerializableSet.t;
@@ -452,6 +453,7 @@ let save_shared_memory ~configuration =
 
 let save
     ~maximum_overrides
+    ~analyze_all_overrides_targets
     ~attribute_targets
     ~skip_analysis_targets
     ~skip_overrides_targets
@@ -475,6 +477,7 @@ let save
       PreviousAnalysisSetupSharedMemory.save_to_cache
         {
           AnalysisSetup.maximum_overrides;
+          analyze_all_overrides_targets;
           attribute_targets;
           skip_analysis_targets;
           skip_overrides_targets;
@@ -565,25 +568,35 @@ module OverrideGraphSharedMemory = struct
   let is_reusable
       ~skip_overrides_targets
       ~maximum_overrides
+      ~analyze_all_overrides_targets
       {
         AnalysisSetup.maximum_overrides = previous_maximum_overrides;
+        analyze_all_overrides_targets = previous_analyze_all_overrides_targets;
         skip_overrides_targets = previous_skip_overrides_targets;
         _;
       }
     =
-    let no_change_in_skip_overrides =
-      Ast.Reference.SerializableSet.equal previous_skip_overrides_targets skip_overrides_targets
-    in
     let no_change_in_maximum_overrides =
       Option.equal Int.equal maximum_overrides previous_maximum_overrides
     in
-    no_change_in_skip_overrides && no_change_in_maximum_overrides
+    let no_change_in_analyze_all_overrides_targets =
+      Interprocedural.Target.Set.equal
+        previous_analyze_all_overrides_targets
+        analyze_all_overrides_targets
+    in
+    let no_change_in_skip_overrides =
+      Ast.Reference.SerializableSet.equal previous_skip_overrides_targets skip_overrides_targets
+    in
+    no_change_in_skip_overrides
+    && no_change_in_analyze_all_overrides_targets
+    && no_change_in_maximum_overrides
 
 
   let load_or_compute_if_unloadable
-      ~skip_overrides_targets
       ~previous_analysis_setup:{ AnalysisSetup.skipped_overrides; _ }
+      ~skip_overrides_targets
       ~maximum_overrides
+      ~analyze_all_overrides_targets
       entry_status
       compute_value
     =
@@ -602,7 +615,7 @@ module OverrideGraphSharedMemory = struct
             ~usage:SaveLoadSharedMemory.Usage.Used
             entry_status )
     | Error error ->
-        ( compute_value ~skip_overrides_targets ~maximum_overrides (),
+        ( compute_value ~skip_overrides_targets ~maximum_overrides ~analyze_all_overrides_targets (),
           EntryStatus.add ~name:Entry.OverrideGraph ~usage:error entry_status )
 
 
@@ -617,21 +630,27 @@ module OverrideGraphSharedMemory = struct
   let load_or_compute_if_stale_or_unloadable
       ~skip_overrides_targets
       ~maximum_overrides
+      ~analyze_all_overrides_targets
       ({ status; _ } as cache)
       compute_value
     =
     match status with
     | Loaded ({ previous_analysis_setup; entry_status } as loaded) ->
         let reusable =
-          is_reusable ~skip_overrides_targets ~maximum_overrides previous_analysis_setup
+          is_reusable
+            ~skip_overrides_targets
+            ~analyze_all_overrides_targets
+            ~maximum_overrides
+            previous_analysis_setup
         in
         if reusable then
           let () = Log.info "Trying to reuse the override graph from the cache." in
           let value, entry_status =
             load_or_compute_if_unloadable
-              ~skip_overrides_targets
               ~previous_analysis_setup
+              ~skip_overrides_targets
               ~maximum_overrides
+              ~analyze_all_overrides_targets
               entry_status
               compute_value
           in
@@ -646,8 +665,15 @@ module OverrideGraphSharedMemory = struct
               ~usage:(SaveLoadSharedMemory.Usage.Unused Stale)
               cache
           in
-          compute_value ~skip_overrides_targets ~maximum_overrides (), cache
-    | _ -> compute_value ~skip_overrides_targets ~maximum_overrides (), cache
+          ( compute_value
+              ~skip_overrides_targets
+              ~maximum_overrides
+              ~analyze_all_overrides_targets
+              (),
+            cache )
+    | _ ->
+        ( compute_value ~skip_overrides_targets ~maximum_overrides ~analyze_all_overrides_targets (),
+          cache )
 end
 
 let override_graph = OverrideGraphSharedMemory.load_or_compute_if_stale_or_unloadable

source/interprocedural_analyses/taint/cache.mli

@@ -22,6 +22,7 @@ val try_load
 
 val save
   :  maximum_overrides:int option ->
+  analyze_all_overrides_targets:Interprocedural.Target.Set.t ->
   attribute_targets:Interprocedural.Target.Set.t ->
   skip_analysis_targets:Interprocedural.Target.Set.t ->
   skip_overrides_targets:Ast.Reference.SerializableSet.t ->
@@ -53,9 +54,11 @@ val metadata_to_json : t -> Yojson.Safe.t
 val override_graph
   :  skip_overrides_targets:Ast.Reference.SerializableSet.t ->
   maximum_overrides:int option ->
+  analyze_all_overrides_targets:Interprocedural.Target.Set.t ->
   t ->
   (skip_overrides_targets:Ast.Reference.SerializableSet.t ->
   maximum_overrides:int option ->
+  analyze_all_overrides_targets:Interprocedural.Target.Set.t ->
   unit ->
   Interprocedural.OverrideGraph.whole_program_overrides) ->
   Interprocedural.OverrideGraph.whole_program_overrides * t

source/interprocedural_analyses/taint/taintAnalysis.ml

@@ -511,8 +511,9 @@ let run_taint_analysis
     Cache.override_graph
       ~skip_overrides_targets
       ~maximum_overrides
+      ~analyze_all_overrides_targets
       cache
-      (fun ~skip_overrides_targets ~maximum_overrides () ->
+      (fun ~skip_overrides_targets ~maximum_overrides ~analyze_all_overrides_targets () ->
         Interprocedural.OverrideGraph.build_whole_program_overrides
           ~static_analysis_configuration
           ~scheduler
@@ -610,6 +611,7 @@ let run_taint_analysis
       ~attribute_targets
       ~skip_analysis_targets
       ~skip_overrides_targets
+      ~analyze_all_overrides_targets
       ~skipped_overrides
       ~override_graph_shared_memory
       ~initial_callables

stubs/integration_test/fixture_source/integration_test/cache.py

@@ -41,6 +41,7 @@ class AnotherBase:
     def method(self, x):
         pass
 
+
 class AnotherOverride(AnotherBase):
     def method(self, x):
         sink(x)
@@ -49,6 +50,7 @@ def method(self, x):
 def test_overrides_cap(instance: AnotherBase):
     instance.method(source())
 
+
 def test_skip_analysis():
     sink(source())
 
@@ -59,3 +61,27 @@ class Token:
 
 def test_attribute(token: Token) -> None:
     sink(token.token)
+
+
+class YetAnotherBase:
+    def method(self, x):
+        pass
+
+
+class YetAnotherOverride1(YetAnotherBase):
+    def method(self, x):
+        sink(x)
+
+
+class YetAnotherOverride2(YetAnotherBase):
+    def method(self, x):
+        pass
+
+
+class YetAnotherOverride3(YetAnotherBase):
+    def method(self, x):
+        pass
+
+
+def test_analyze_all_overrides(b: YetAnotherBase):
+    b.method(source())

stubs/integration_test/result.json

@@ -488,11 +488,11 @@
     "column": 20,
     "define": "integration_test.cache.test_overrides_cap",
     "description": "Possible shell injection [5001]: Data from [UserControlled] source(s) may reach [RemoteCodeExecution] sink(s)",
-    "line": 50,
+    "line": 51,
     "name": "Possible shell injection",
     "path": "fixture_source/integration_test/cache.py",
     "stop_column": 28,
-    "stop_line": 50
+    "stop_line": 51
   },
   {
     "code": 5001,
@@ -1934,5 +1934,16 @@
     "path": "fixture_source/integration_test/top_level.py",
     "stop_column": 23,
     "stop_line": 15
+  },
+  {
+      "code": 5001,
+      "column": 13,
+      "define": "integration_test.cache.test_analyze_all_overrides",
+      "description": "Possible shell injection [5001]: Data from [UserControlled] source(s) may reach [RemoteCodeExecution] sink(s)",
+      "line": 87,
+      "name": "Possible shell injection",
+      "path": "fixture_source/integration_test/cache.py",
+      "stop_column": 21,
+      "stop_line": 87
   }
 ]

stubs/integration_test/run_cache_test.py

@@ -589,7 +589,70 @@ def run_test_changed_decorators(self) -> None:
         _exit_or_continue(returncode, self.exit_on_error)
 
     @save_restore_cache
-    def run_test_changed_overrides(self) -> None:
+    def run_test_changed_analyze_all_overrides(self) -> None:
+        """
+        Run Pysa after removing a @AnalyzeAllOverrides model to test cache invalidation.
+        Pysa should detect that the override graph has changed and fall back
+        to doing a clean run.
+        """
+        # Remove a test taint file
+        test_model_path = Path("test_taint/analyze_all_overrides.pysa")
+        # Save contents for cleanup phase
+        original_content = open(test_model_path).read()
+        try:
+            test_model_path.unlink()
+        except FileNotFoundError:
+            LOG.warning(f"Could not remove up {test_model_path.absolute()}.")
+            pass
+
+        LOG.info("Testing cache invalidation after changes in @AnalyzeAllOverrides:")
+        pysa_command = _pysa_command(
+            self.typeshed_path, self.cache_path, self.save_results_to, use_cache=True
+        )
+        expected_cache_usage = {
+            "shared_memory_status": {
+                "Loaded": {
+                    "CallGraph": "(Unused Stale)",
+                    "ClassHierarchyGraph": "Used",
+                    "ClassIntervalGraph": "Used",
+                    "GlobalConstants": "Used",
+                    "InitialCallables": "Used",
+                    "PreviousAnalysisSetup": "Used",
+                    "OverrideGraph": "(Unused Stale)",
+                    "TypeEnvironment": "Used",
+                }
+            },
+            "save_cache": True,
+        }
+
+        # Expected should have one less issue due to skipping overrides
+        issue = {
+            "code": 5001,
+            "column": 13,
+            "define": "integration_test.cache.test_analyze_all_overrides",
+            "description": "Possible shell injection [5001]: Data from [UserControlled] source(s) may reach [RemoteCodeExecution] sink(s)",
+            "line": 87,
+            "name": "Possible shell injection",
+            "path": "fixture_source/integration_test/cache.py",
+            "stop_column": 21,
+            "stop_line": 87
+        }
+        new_expected = copy.deepcopy(self.expected)
+        new_expected.remove(issue)
+        returncode = _run_and_check_output(
+            pysa_command,
+            new_expected,
+            self.save_results_to,
+            expected_cache_usage,
+        )
+
+        # Restore the original model file
+        open(test_model_path, "w").write(original_content)
+
+        _exit_or_continue(returncode, self.exit_on_error)
+
+    @save_restore_cache
+    def run_test_changed_skip_overrides(self) -> None:
         """
         Run Pysa after removing a @SkipOverrides model to test cache invalidation.
         Pysa should detect that the override graph has changed and fall back
@@ -686,11 +749,11 @@ def run_test_changed_overrides_cap(self) -> None:
             "column": 20,
             "define": "integration_test.cache.test_overrides_cap",
             "description": "Possible shell injection [5001]: Data from [UserControlled] source(s) may reach [RemoteCodeExecution] sink(s)",
-            "line": 50,
+            "line": 51,
             "name": "Possible shell injection",
             "path": "fixture_source/integration_test/cache.py",
             "stop_column": 28,
-            "stop_line": 50,
+            "stop_line": 51,
         }
         new_expected = copy.deepcopy(self.expected)
         new_expected.remove(issue)
@@ -744,11 +807,11 @@ def run_test_changed_skip_analysis(self) -> None:
             "column": 9,
             "define": "integration_test.cache.test_skip_analysis",
             "description": "Possible shell injection [5001]: Data from [UserControlled] source(s) may reach [RemoteCodeExecution] sink(s)",
-            "line": 53,
+            "line": 55,
             "name": "Possible shell injection",
             "path": "fixture_source/integration_test/cache.py",
             "stop_column": 17,
-            "stop_line": 53,
+            "stop_line": 55,
         }
         returncode = _run_and_check_output(
             pysa_command,
@@ -792,11 +855,11 @@ def run_test_changed_definitions(self) -> None:
             "column": 9,
             "define": "integration_test.cache.new_definition",
             "description": "Possible shell injection [5001]: Data from [UserControlled] source(s) may reach [RemoteCodeExecution] sink(s)",
-            "line": 64,
+            "line": 90,
             "name": "Possible shell injection",
             "path": "fixture_source/integration_test/cache.py",
             "stop_column": 17,
-            "stop_line": 64,
+            "stop_line": 90,
         }
         returncode = _run_and_check_output(
             pysa_command,
@@ -851,11 +914,11 @@ def run_test_changed_attribute_targets(self) -> None:
             "column": 9,
             "define": "integration_test.cache.test_attribute",
             "description": "Possible shell injection [5001]: Data from [UserControlled] source(s) may reach [RemoteCodeExecution] sink(s)",
-            "line": 61,
+            "line": 63,
             "name": "Possible shell injection",
             "path": "fixture_source/integration_test/cache.py",
             "stop_column": 20,
-            "stop_line": 61,
+            "stop_line": 63,
         }
         returncode = _run_and_check_output(
             pysa_command,
@@ -910,7 +973,8 @@ def run_tests(exit_on_error: bool) -> None:
         test_class.run_test_changed_source_files()
         test_class.run_test_changed_definitions()
         test_class.run_test_changed_decorators()
-        test_class.run_test_changed_overrides()
+        test_class.run_test_changed_skip_overrides()
+        test_class.run_test_changed_analyze_all_overrides()
         test_class.run_test_changed_overrides_cap()
         test_class.run_test_changed_skip_analysis()
         test_class.run_test_changed_attribute_targets()

stubs/integration_test/test_taint/analyze_all_overrides.pysa

@@ -0,0 +1,2 @@
+@AnalyzeAllOverrides
+def integration_test.cache.YetAnotherBase.method(): ...

stubs/integration_test/test_taint/taint.config

@@ -7,5 +7,8 @@
       "sinks": [ "ReturnedToUser" ],
       "message_format": "Data from [{$sources}] source(s) may reach [{$sinks}] sink(s)"
     }
-  ]
+  ],
+  "options": {
+    "maximum_overrides_to_analyze": 3
+  }
 }