Skip to content

Commit 60ba5fa

Browse files
stroxlerfacebook-github-bot
authored andcommitted
Rewire interprocedural to be one-Analysis [1] update public interface
Summary: **About the stack** After some discussion, all the pysa stakeholders are agreed that we don't have any expectation of needing to run multiple concurrent analyses and share data across them. As a result, we can significantly simplify the framework by restricting to one analysis at a time. Code simplification is an important long-term goal, but our immediate concern is really functionality: by restricting the framework to run on one analysis at a time, we can make it possible for us to put callbacks into the analysis which the "plumbing" code, such as callgraph construction, can use to customize framework behavior for each analysis; this isn't possible to do cleanly in a multi-analysis framework because it's not clear which analysis would call the shots. This change is needed to unblock full feature parity of interprocedural infer vs ordinary infer, because we need the ability to exclude some code from the analysis based on commandline flags that don't line up with taint. **About this diff** It's going to take some time to fully simplify the existing multi-analysis code. This diff is a minimal initial change to rewire the public functions from `interproceduralAnalysis.ml` while making it trivial to review the changes; the interfaces are clean but the code is now a little messy because to preserve the existing logic I'm often doing a "fold" over just one analysis. I'll stack changes on top to both simplify the interproceduralResult types and get rid of some of the accumulate boilerplate still left over because we previously had to fold over analyses. Reviewed By: dkgi Differential Revision: D28886238 fbshipit-source-id: d82d3c422a71c8ce0a260289e0ab36d67e60d356
1 parent 8a355b9 commit 60ba5fa

File tree

10 files changed

+45
-133
lines changed

10 files changed

+45
-133
lines changed

source/command/analyzeCommand.ml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ let run_analysis
177177
Scheduler.with_scheduler ~configuration ~f:(fun scheduler ->
178178
Interprocedural.Analysis.initialize_configuration
179179
~static_analysis_configuration
180-
[analysis_kind];
180+
analysis_kind;
181181
let cached_environment =
182182
if use_cache then Service.StaticAnalysis.Cache.load_environment ~configuration else None
183183
in
@@ -234,7 +234,7 @@ let run_analysis
234234
in
235235
Service.StaticAnalysis.analyze
236236
~scheduler
237-
~analysis_kind
237+
~analysis:analysis_kind
238238
~static_analysis_configuration
239239
~filename_lookup
240240
~environment:(Analysis.TypeEnvironment.read_only environment)

source/interprocedural/interproceduralAnalysis.ml

Lines changed: 20 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,18 @@ open Statement
1212
module Kind = AnalysisKind
1313
module Result = InterproceduralResult
1414

15-
let initialize_configuration kinds ~static_analysis_configuration =
16-
let initialize_kind kind =
17-
let (Result.Analysis { analysis; _ }) = Result.get_abstract_analysis kind in
18-
let module Analysis = (val analysis) in
19-
Analysis.initialize_configuration ~static_analysis_configuration
20-
in
21-
List.iter kinds ~f:initialize_kind
15+
let initialize_configuration kind ~static_analysis_configuration =
16+
let (Result.Analysis { analysis; _ }) = Result.get_abstract_analysis kind in
17+
let module Analysis = (val analysis) in
18+
Analysis.initialize_configuration ~static_analysis_configuration
2219

2320

2421
type initialize_result = {
2522
initial_models: InterproceduralResult.model_t Callable.Map.t;
2623
skip_overrides: Ast.Reference.Set.t;
2724
}
2825

29-
let initialize_models kinds ~scheduler ~static_analysis_configuration ~environment ~functions ~stubs
30-
=
26+
let initialize_models kind ~scheduler ~static_analysis_configuration ~environment ~functions ~stubs =
3127
let initialize_each
3228
{ initial_models = models; skip_overrides }
3329
(Result.Analysis { kind; analysis })
@@ -57,10 +53,7 @@ let initialize_models kinds ~scheduler ~static_analysis_configuration ~environme
5753
}
5854
in
5955
let accumulate model kind = initialize_each model (Result.get_abstract_analysis kind) in
60-
List.fold
61-
kinds
62-
~init:{ initial_models = Callable.Map.empty; skip_overrides = Reference.Set.empty }
63-
~f:accumulate
56+
accumulate { initial_models = Callable.Map.empty; skip_overrides = Reference.Set.empty } kind
6457

6558

6659
let record_initial_models ~functions ~stubs models =
@@ -103,7 +96,7 @@ let get_empty_model (type a) (kind : < model : a ; .. > Result.storable_kind) :
10396
Analysis.empty_model
10497

10598

106-
let get_obscure_models analyses =
99+
let get_obscure_models analysis =
107100
let get_analysis_specific_obscure_model map abstract_analysis =
108101
let (Result.Analysis { kind; analysis }) = abstract_analysis in
109102
let module Analysis = (val analysis) in
@@ -113,7 +106,7 @@ let get_obscure_models analyses =
113106
(Result.Pkg { kind = ModelPart kind; value = obscure_model })
114107
map
115108
in
116-
let models = List.fold ~f:get_analysis_specific_obscure_model ~init:Kind.Map.empty analyses in
109+
let models = get_analysis_specific_obscure_model Kind.Map.empty analysis in
117110
Result.{ models; is_obscure = true }
118111

119112

@@ -289,7 +282,7 @@ let widen_if_necessary step callable ~old_model ~new_model result =
289282

290283
let analyze_define
291284
step
292-
analyses
285+
analysis
293286
callable
294287
environment
295288
qualifier
@@ -329,12 +322,7 @@ let analyze_define
329322
let akind, model, result = analyze analysis in
330323
Result.Kind.Map.add akind model models, Result.Kind.Map.add akind result results
331324
in
332-
(* Run all the analyses *)
333-
try
334-
let init = Result.Kind.Map.(empty, empty) in
335-
let models, results = List.fold ~f:accumulate analyses ~init in
336-
models, results
337-
with
325+
try accumulate Result.Kind.Map.(empty, empty) analysis with
338326
| Analysis.ClassHierarchy.Untracked annotation ->
339327
Log.log
340328
~section:`Info
@@ -407,7 +395,7 @@ let callables_to_dump =
407395
ref Callable.Set.empty
408396

409397

410-
let analyze_callable analyses step callable environment =
398+
let analyze_callable analysis step callable environment =
411399
let resolution = Analysis.TypeEnvironment.ReadOnly.global_resolution environment in
412400
let () =
413401
(* Verify invariants *)
@@ -446,13 +434,13 @@ let analyze_callable analyses step callable environment =
446434
Fixpoint.
447435
{
448436
is_partial = false;
449-
model = get_obscure_models analyses;
437+
model = get_obscure_models analysis;
450438
result = Result.empty_result;
451439
}
452440
| Some (qualifier, ({ Node.value; _ } as define)) ->
453441
if Define.dump value then
454442
callables_to_dump := Callable.Set.add callable !callables_to_dump;
455-
analyze_define step analyses callable environment qualifier define )
443+
analyze_define step analysis callable environment qualifier define )
456444
| #Callable.override_target as callable -> analyze_overrides step callable
457445
| #Callable.object_target as path ->
458446
Format.asprintf "Found object %a in fixpoint analysis" Callable.pp path |> failwith
@@ -470,11 +458,11 @@ type result = {
470458
}
471459

472460
(* Called on a worker with a set of functions to analyze. *)
473-
let one_analysis_pass ~analyses ~step ~environment ~callables =
474-
let analyses = List.map ~f:Result.get_abstract_analysis analyses in
461+
let one_analysis_pass ~analysis ~step ~environment ~callables =
462+
let analysis = Result.get_abstract_analysis analysis in
475463
let analyze_and_cache expensive_callables callable =
476464
let timer = Timer.start () in
477-
let result = analyze_callable analyses step callable environment in
465+
let result = analyze_callable analysis step callable environment in
478466
Fixpoint.add_state step callable result;
479467
(* Log outliers. *)
480468
if Timer.stop_in_ms timer > 500 then begin
@@ -564,7 +552,7 @@ let compute_callables_to_reanalyze
564552
let compute_fixpoint
565553
~scheduler
566554
~environment
567-
~analyses
555+
~analysis
568556
~dependencies
569557
~filtered_callables
570558
~all_callables
@@ -635,7 +623,7 @@ let compute_fixpoint
635623
Scheduler.map_reduce
636624
scheduler
637625
~policy:(Scheduler.Policy.legacy_fixed_chunk_size 1000)
638-
~map:(fun _ callables -> one_analysis_pass ~analyses ~step ~environment ~callables)
626+
~map:(fun _ callables -> one_analysis_pass ~analysis ~step ~environment ~callables)
639627
~initial:
640628
{
641629
callables_processed = 0;
@@ -722,7 +710,7 @@ let report_results
722710
~static_analysis_configuration
723711
~environment
724712
~filename_lookup
725-
~analyses
713+
~analysis
726714
~callables
727715
~skipped_overrides
728716
~fixpoint_timer
@@ -740,5 +728,4 @@ let report_results
740728
~fixpoint_timer
741729
~fixpoint_iterations
742730
in
743-
let analyses = List.map ~f:Result.get_abstract_analysis analyses in
744-
analyses |> List.concat_map ~f:report_analysis
731+
Result.get_abstract_analysis analysis |> report_analysis

source/interprocedural/interproceduralAnalysis.mli

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@ type initialize_result = {
1313
}
1414

1515
val initialize_configuration
16-
: Kind.abstract list ->
16+
: Kind.abstract ->
1717
static_analysis_configuration:Configuration.StaticAnalysis.t ->
1818
unit
1919

20-
(* Calls init on all specified analyses to get initial models *)
20+
(* Calls init on specified analysis to get initial models *)
2121
val initialize_models
22-
: Kind.abstract list ->
22+
: Kind.abstract ->
2323
scheduler:Scheduler.t ->
2424
static_analysis_configuration:Configuration.StaticAnalysis.t ->
2525
environment:Analysis.TypeEnvironment.ReadOnly.t ->
@@ -45,7 +45,7 @@ type result = {
4545
}
4646

4747
val one_analysis_pass
48-
: analyses:Kind.abstract list ->
48+
: analysis:Kind.abstract ->
4949
step:Fixpoint.step ->
5050
environment:Analysis.TypeEnvironment.ReadOnly.t ->
5151
callables:Callable.t list ->
@@ -55,7 +55,7 @@ val one_analysis_pass
5555
val compute_fixpoint
5656
: scheduler:Scheduler.t ->
5757
environment:Analysis.TypeEnvironment.ReadOnly.t ->
58-
analyses:Kind.abstract list ->
58+
analysis:Kind.abstract ->
5959
dependencies:DependencyGraph.t ->
6060
filtered_callables:Callable.Set.t ->
6161
all_callables:Callable.t list ->
@@ -69,7 +69,7 @@ val report_results
6969
static_analysis_configuration:Configuration.StaticAnalysis.t ->
7070
environment:Analysis.TypeEnvironment.ReadOnly.t ->
7171
filename_lookup:(Ast.Reference.t -> string option) ->
72-
analyses:Kind.abstract list ->
72+
analysis:Kind.abstract ->
7373
callables:Callable.Set.t ->
7474
skipped_overrides:Ast.Reference.t list ->
7575
fixpoint_timer:Timer.t ->

source/interprocedural/test/interproceduralAnalysisTest.ml

Lines changed: 5 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -106,76 +106,7 @@ module AnalysisA = ResultA.Register (struct
106106
callables |> Callable.Set.elements |> List.map ~f:get_model
107107
end)
108108

109-
module ResultB = Interprocedural.Result.Make (struct
110-
type result = int
111-
112-
type call_model = string [@@deriving show]
113-
114-
let name = "analysisB"
115-
116-
let empty_model = "empty"
117-
118-
let obscure_model = "obscure"
119-
120-
let join ~iteration:_ a b = a ^ b
121-
122-
let widen ~iteration ~previous ~next = join ~iteration previous next
123-
124-
let reached_fixpoint ~iteration:_ ~previous ~next = String.(next <= previous)
125-
126-
let strip_for_callsite model = model
127-
end)
128-
129-
module AnalysisB = ResultB.Register (struct
130-
let initialize_configuration ~static_analysis_configuration:_ = ()
131-
132-
let initialize_models
133-
~scheduler:_
134-
~static_analysis_configuration:_
135-
~environment:_
136-
~functions:_
137-
~stubs:_
138-
=
139-
{ Result.initial_models = Callable.Map.empty; skip_overrides = Reference.Set.empty }
140-
141-
142-
let analyze ~environment:_ ~callable:_ ~qualifier:_ ~define:_ ~existing:_ = 7, "B"
143-
144-
let report
145-
~scheduler:_
146-
~static_analysis_configuration:_
147-
~environment:_
148-
~filename_lookup:_
149-
~callables
150-
~skipped_overrides:_
151-
~fixpoint_timer:_
152-
~fixpoint_iterations:_
153-
=
154-
let get_model callable : Yojson.Safe.json =
155-
let model =
156-
Fixpoint.get_model callable
157-
>>= Result.get_model ResultB.kind
158-
>>| (fun r -> `String r)
159-
|> Option.value ~default:`Null
160-
in
161-
let result =
162-
Fixpoint.get_result callable
163-
|> Result.get_result ResultB.kind
164-
>>| (fun r -> `Int r)
165-
|> Option.value ~default:`Null
166-
in
167-
`Assoc
168-
[
169-
"analysis", `String ResultB.name;
170-
"callable", `String (Callable.show callable);
171-
"model", model;
172-
"result", result;
173-
]
174-
in
175-
callables |> Callable.Set.elements |> List.map ~f:get_model
176-
end)
177-
178-
let analyses = [AnalysisA.abstract_kind; AnalysisB.abstract_kind]
109+
let analysis = AnalysisA.abstract_kind
179110

180111
let assert_report ~expected report =
181112
let json_printer jsons = String.concat ~sep:"\n" (List.map ~f:Yojson.Safe.to_string jsons) in
@@ -197,15 +128,14 @@ let test_unknown_function_analysis context =
197128
|> TypeAnalysis.TypeEnvironment.read_only
198129
in
199130
let step = Fixpoint.{ epoch = 1; iteration = 0 } in
200-
let _ = Analysis.one_analysis_pass ~step ~analyses ~environment ~callables:targets in
131+
let _ = Analysis.one_analysis_pass ~step ~analysis ~environment ~callables:targets in
201132
(* Make sure obscure models are correctly handled *)
202133
let check_obscure_model target =
203134
match Fixpoint.get_model target with
204135
| None ->
205136
Format.sprintf "no model stored for target %s" (Callable.show target) |> assert_failure
206137
| Some models ->
207-
assert_equal (Result.get_model ResultA.kind models) (Some ResultA.obscure_model);
208-
assert_equal (Result.get_model ResultB.kind models) (Some ResultB.obscure_model)
138+
assert_equal (Result.get_model ResultA.kind models) (Some ResultA.obscure_model)
209139
in
210140
List.iter ~f:check_obscure_model targets;
211141
(* Make sure result extraction works (this verifies a lot of the type magic) *)
@@ -215,7 +145,7 @@ let test_unknown_function_analysis context =
215145
~scheduler:(Test.mock_scheduler ())
216146
~static_analysis_configuration
217147
~environment
218-
~analyses
148+
~analysis
219149
~filename_lookup:(fun _ -> None)
220150
~callables:(targets |> Callable.Set.of_list)
221151
~skipped_overrides:[]
@@ -228,8 +158,6 @@ let test_unknown_function_analysis context =
228158
[
229159
{| {"analysis":"analysisA","callable":"fun_a (fun)","model":-1,"result":null} |};
230160
{| {"analysis":"analysisA","callable":"fun_b (fun)","model":-1,"result":null} |};
231-
{| {"analysis":"analysisB","callable":"fun_a (fun)","model":"obscure","result":null} |};
232-
{| {"analysis":"analysisB","callable":"fun_b (fun)","model":"obscure","result":null} |};
233161
]
234162

235163

@@ -262,7 +190,7 @@ let test_meta_data context =
262190
|> TypeAnalysis.TypeEnvironment.create
263191
|> TypeAnalysis.TypeEnvironment.read_only
264192
in
265-
let _ = Analysis.one_analysis_pass ~step:step1 ~analyses ~environment ~callables:targets in
193+
let _ = Analysis.one_analysis_pass ~step:step1 ~analysis ~environment ~callables:targets in
266194
(* All obscure functions should reach fixpoint in 1st step *)
267195
let () = List.iter ~f:(check_meta_data ~step:step1 ~is_partial:false) targets in
268196
()

source/interprocedural_analyses/taint/test/fixpointTest.ml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,11 @@ let assert_fixpoint ?models ~context source ~expect:{ iterations = expect_iterat
2626
|> DependencyGraph.union overrides
2727
|> DependencyGraph.reverse
2828
in
29-
let analyses = [TaintAnalysis.abstract_kind] in
3029
let iterations =
3130
Analysis.compute_fixpoint
3231
~scheduler
3332
~environment
34-
~analyses
33+
~analysis:TaintAnalysis.abstract_kind
3534
~dependencies
3635
~filtered_callables:Callable.Set.empty
3736
~all_callables:callables_to_analyze

source/interprocedural_analyses/taint/test/integrationTest.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ let test_integration path context =
109109
Analysis.compute_fixpoint
110110
~scheduler:(Test.mock_scheduler ())
111111
~environment
112-
~analyses:[TaintAnalysis.abstract_kind]
112+
~analysis:TaintAnalysis.abstract_kind
113113
~dependencies
114114
~filtered_callables:Callable.Set.empty
115115
~all_callables:callables_to_analyze

source/interprocedural_analyses/taint/test/missingFlowsTest.ml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,11 @@ let assert_fixpoint
3636
|> DependencyGraph.union overrides
3737
|> DependencyGraph.reverse
3838
in
39-
let analyses = [TaintAnalysis.abstract_kind] in
4039
let iterations =
4140
Analysis.compute_fixpoint
4241
~scheduler
4342
~environment
44-
~analyses
43+
~analysis:TaintAnalysis.abstract_kind
4544
~dependencies
4645
~filtered_callables:Callable.Set.empty
4746
~all_callables:callables_to_analyze

0 commit comments

Comments
 (0)