From a4013bdbe99afcd6c469cb0f8c174859aea0c9b0 Mon Sep 17 00:00:00 2001 From: Phoebe Nichols Date: Fri, 28 Jun 2019 02:02:15 -0700 Subject: [PATCH] Remove analyze_ondemand from Ondemand.callbacks Summary: Currently, `Callbacks.analyze_procedures` creates a function to call the method `Callbacks.iterate_procedure_callbacks`. This is supplied as an argument to functions in `ondemand.ml`, so that it can be invoked. This is done to avoid a cyclic dependancy. This diff moves the functions that `ondemand.ml` needs to call into `ondemand.ml`, avoiding the need to supply them as arguments. Reviewed By: ngorogiannis Differential Revision: D16028836 fbshipit-source-id: 16ae27a3e --- infer/src/backend/InferAnalyze.ml | 4 +- infer/src/backend/callbacks.ml | 62 ------------------------ infer/src/backend/callbacks.mli | 8 ++-- infer/src/backend/ondemand.ml | 78 ++++++++++++++++++++++++++----- infer/src/backend/ondemand.mli | 17 ++++--- infer/src/unit/TaintTests.ml | 6 +-- 6 files changed, 83 insertions(+), 92 deletions(-) diff --git a/infer/src/backend/InferAnalyze.ml b/infer/src/backend/InferAnalyze.ml index da5a7ae2e..3e860bf36 100644 --- a/infer/src/backend/InferAnalyze.ml +++ b/infer/src/backend/InferAnalyze.ml @@ -22,7 +22,7 @@ let analyze_target : TaskScheduler.target Tasks.doer = if Topl.is_active () then DB.Results_dir.init (Topl.sourcefile ()) ; DB.Results_dir.init source_file ; L.task_progress SourceFile.pp source_file ~f:(fun () -> - Callbacks.analyze_file exe_env source_file ; + Ondemand.analyze_file exe_env source_file ; if Topl.is_active () && Config.debug_mode then Dotty.print_icfg_dotty (Topl.sourcefile ()) (Topl.cfg ()) ; if Config.write_html then Printer.write_all_html_files source_file ) @@ -38,7 +38,7 @@ let analyze_target : TaskScheduler.target Tasks.doer = L.log_task "Analysing block of %d procs, starting with %a@." per_procedure_logging_granularity Typ.Procname.pp proc_name ; procs_left := per_procedure_logging_granularity ) ; - Callbacks.analyze_proc_name exe_env proc_name + Ondemand.analyze_proc_name_toplevel exe_env proc_name in fun target -> if Config.memcached then Memcached.connect () ; diff --git a/infer/src/backend/callbacks.ml b/infer/src/backend/callbacks.ml index 9743d90f9..f0b28798c 100644 --- a/infer/src/backend/callbacks.ml +++ b/infer/src/backend/callbacks.ml @@ -6,7 +6,6 @@ *) open! IStd -module F = Format (** Module to register and invoke callbacks *) @@ -102,64 +101,3 @@ let iterate_cluster_callbacks all_procs exe_env source_file = List.iter ~f:(fun {language; callback} -> if language_matches language then callback environment) !cluster_callbacks - - -let dump_duplicate_procs source_file procs = - let duplicate_procs = - List.filter_map procs ~f:(fun pname -> - match Attributes.load pname with - | Some - { is_defined= - true - (* likely not needed: if [pname] is part of [procs] then it *is* defined, so we - expect the attribute to be defined too *) - ; translation_unit - ; loc } - when (* defined in another file *) - (not (SourceFile.equal source_file translation_unit)) - && (* really defined in that file and not in an include *) - SourceFile.equal translation_unit loc.file -> - Some (pname, translation_unit) - | _ -> - None ) - in - let output_to_file duplicate_procs = - Out_channel.with_file (Config.results_dir ^/ Config.duplicates_filename) - ~append:true ~perm:0o666 ~f:(fun outc -> - let fmt = F.formatter_of_out_channel outc in - List.iter duplicate_procs ~f:(fun (pname, source_captured) -> - F.fprintf fmt "DUPLICATE_SYMBOLS source:%a source_captured:%a pname:%a@\n" - SourceFile.pp source_file SourceFile.pp source_captured Typ.Procname.pp pname ) ; - F.pp_print_flush fmt () ) - in - if not (List.is_empty duplicate_procs) then output_to_file duplicate_procs - - -let create_perf_stats_report source_file = - PerfStats.register_report PerfStats.TimeAndMemory (PerfStats.Backend source_file) ; - PerfStats.get_reporter (PerfStats.Backend source_file) () - - -let analyze_procedures exe_env procs_to_analyze source_file_opt = - let saved_language = !Language.curr_language in - Option.iter source_file_opt ~f:(fun source_file -> - if Config.dump_duplicate_symbols then dump_duplicate_procs source_file procs_to_analyze ) ; - let analyze_ondemand summary proc_desc = iterate_procedure_callbacks exe_env summary proc_desc in - Ondemand.set_callbacks {Ondemand.exe_env; analyze_ondemand} ; - let analyze_proc_name pname = ignore (Ondemand.analyze_proc_name pname : Summary.t option) in - List.iter ~f:analyze_proc_name procs_to_analyze ; - Option.iter source_file_opt ~f:(fun source_file -> - iterate_cluster_callbacks procs_to_analyze exe_env source_file ; - create_perf_stats_report source_file ) ; - Ondemand.unset_callbacks () ; - Language.curr_language := saved_language - - -(** Invoke all procedure and cluster callbacks on a given environment. *) -let analyze_file (exe_env : Exe_env.t) source_file = - let procs_to_analyze = SourceFiles.proc_names_of_source source_file in - analyze_procedures exe_env procs_to_analyze (Some source_file) - - -(** Invoke procedure callbacks on a given environment. *) -let analyze_proc_name (exe_env : Exe_env.t) proc_name = analyze_procedures exe_env [proc_name] None diff --git a/infer/src/backend/callbacks.mli b/infer/src/backend/callbacks.mli index f0d284bed..0ad7d241e 100644 --- a/infer/src/backend/callbacks.mli +++ b/infer/src/backend/callbacks.mli @@ -36,8 +36,8 @@ val register_procedure_callback : val register_cluster_callback : name:string -> Language.t -> cluster_callback_t -> unit (** register a cluster callback *) -val analyze_file : Exe_env.t -> SourceFile.t -> unit -(** Invoke all the registered callbacks on the given file. *) +val iterate_procedure_callbacks : Exe_env.t -> Summary.t -> Procdesc.t -> Summary.t +(** Invoke all registered procedure callbacks on the given procedure. *) -val analyze_proc_name : Exe_env.t -> Typ.Procname.t -> unit -(** Invoke all the registered callbacks on the given procedure. *) +val iterate_cluster_callbacks : Typ.Procname.t sexp_list -> Exe_env.t -> SourceFile.t -> unit +(** Invoke all registered cluster callbacks on a cluster of procedures. *) diff --git a/infer/src/backend/ondemand.ml b/infer/src/backend/ondemand.ml index 916de9729..a8b91c2c4 100644 --- a/infer/src/backend/ondemand.ml +++ b/infer/src/backend/ondemand.ml @@ -12,17 +12,13 @@ open! IStd module L = Logging module F = Format -type analyze_ondemand = Summary.t -> Procdesc.t -> Summary.t - -type callbacks = {exe_env: Exe_env.t; analyze_ondemand: analyze_ondemand} - -let callbacks_ref = ref None +let exe_env_ref = ref None let cached_results = lazy (Typ.Procname.Hash.create 128) -let set_callbacks (callbacks : callbacks) = callbacks_ref := Some callbacks +let set_exe_env (env : Exe_env.t) = exe_env_ref := Some env -let unset_callbacks () = callbacks_ref := None +let unset_exe_env () = exe_env_ref := None (* always incremented before use *) let nesting = ref (-1) @@ -195,7 +191,7 @@ let run_proc_analysis analyze_proc ~caller_pdesc callee_pdesc = match exn with | SymOp.Analysis_failure_exe kind -> (* in production mode, log the timeout/crash and continue with the summary we had before - the failure occurred *) + the failure occurred *) log_error_and_continue exn initial_summary kind | _ -> (* this happens with assert false or some other unrecognized exception *) @@ -215,8 +211,44 @@ let run_proc_analysis analyze_proc ~caller_pdesc callee_pdesc = summary +let dump_duplicate_procs source_file procs = + let duplicate_procs = + List.filter_map procs ~f:(fun pname -> + match Attributes.load pname with + | Some + { is_defined= + true + (* likely not needed: if [pname] is part of [procs] then it *is* defined, so we + expect the attribute to be defined too *) + ; translation_unit + ; loc } + when (* defined in another file *) + (not (SourceFile.equal source_file translation_unit)) + && (* really defined in that file and not in an include *) + SourceFile.equal translation_unit loc.file -> + Some (pname, translation_unit) + | _ -> + None ) + in + let output_to_file duplicate_procs = + Out_channel.with_file (Config.results_dir ^/ Config.duplicates_filename) + ~append:true ~perm:0o666 ~f:(fun outc -> + let fmt = F.formatter_of_out_channel outc in + List.iter duplicate_procs ~f:(fun (pname, source_captured) -> + F.fprintf fmt "DUPLICATE_SYMBOLS source:%a source_captured:%a pname:%a@\n" + SourceFile.pp source_file SourceFile.pp source_captured Typ.Procname.pp pname ) ; + F.pp_print_flush fmt () ) + in + if not (List.is_empty duplicate_procs) then output_to_file duplicate_procs + + +let create_perf_stats_report source_file = + PerfStats.register_report PerfStats.TimeAndMemory (PerfStats.Backend source_file) ; + PerfStats.get_reporter (PerfStats.Backend source_file) () + + let analyze_proc ?caller_pdesc callee_pdesc = - let callbacks = Option.value_exn !callbacks_ref in + let exe_env = Option.value_exn !exe_env_ref in (* wrap [callbacks.analyze_ondemand] to update the status bar *) let analyze_proc summary pdesc = let proc_name = Procdesc.get_proc_name callee_pdesc in @@ -231,8 +263,8 @@ let analyze_proc ?caller_pdesc callee_pdesc = in current_taskbar_status := Some (t0, status) ; !ProcessPoolState.update_status t0 status ; - let summary = callbacks.analyze_ondemand summary pdesc in - if Topl.is_active () then Topl.add_errors callbacks.exe_env summary ; + let summary = Callbacks.iterate_procedure_callbacks exe_env summary pdesc in + if Topl.is_active () then Topl.add_errors exe_env summary ; summary in Some (run_proc_analysis analyze_proc ~caller_pdesc callee_pdesc) @@ -312,3 +344,27 @@ let analyze_proc_name ?caller_pdesc callee_pname = let clear_cache () = Typ.Procname.Hash.clear (Lazy.force cached_results) + +let analyze_procedures exe_env procs_to_analyze source_file_opt = + let saved_language = !Language.curr_language in + Option.iter source_file_opt ~f:(fun source_file -> + if Config.dump_duplicate_symbols then dump_duplicate_procs source_file procs_to_analyze ) ; + set_exe_env exe_env ; + let analyze_proc_name_call pname = ignore (analyze_proc_name pname : Summary.t option) in + List.iter ~f:analyze_proc_name_call procs_to_analyze ; + Option.iter source_file_opt ~f:(fun source_file -> + Callbacks.iterate_cluster_callbacks procs_to_analyze exe_env source_file ; + create_perf_stats_report source_file ) ; + unset_exe_env () ; + Language.curr_language := saved_language + + +(** Invoke all procedure and cluster callbacks on a given environment. *) +let analyze_file (exe_env : Exe_env.t) source_file = + let procs_to_analyze = SourceFiles.proc_names_of_source source_file in + analyze_procedures exe_env procs_to_analyze (Some source_file) + + +(** Invoke procedure callbacks on a given environment. *) +let analyze_proc_name_toplevel (exe_env : Exe_env.t) proc_name = + analyze_procedures exe_env [proc_name] None diff --git a/infer/src/backend/ondemand.mli b/infer/src/backend/ondemand.mli index 2c2e60bea..d37c59393 100644 --- a/infer/src/backend/ondemand.mli +++ b/infer/src/backend/ondemand.mli @@ -9,10 +9,6 @@ open! IStd (** Module for on-demand analysis. *) -type analyze_ondemand = Summary.t -> Procdesc.t -> Summary.t - -type callbacks = {exe_env: Exe_env.t; analyze_ondemand: analyze_ondemand} - val get_proc_desc : Typ.Procname.t -> Procdesc.t option (** Find a proc desc for the procedure, perhaps loading it from disk. *) @@ -24,11 +20,14 @@ val analyze_proc_name : ?caller_pdesc:Procdesc.t -> Typ.Procname.t -> Summary.t (** [analyze_proc_name ~caller_pdesc proc_name] performs an on-demand analysis of proc_name triggered during the analysis of caller_pdesc *) -val set_callbacks : callbacks -> unit -(** Set the callbacks used to perform on-demand analysis. *) - -val unset_callbacks : unit -> unit -(** Unset the callbacks used to perform on-demand analysis. *) +val set_exe_env : Exe_env.t -> unit +(** Set the execution enviroment used during on-demand analysis. *) val clear_cache : unit -> unit (** empty the cache of ondemand results *) + +val analyze_file : Exe_env.t -> SourceFile.t -> unit +(** Invoke all the callbacks registered in {!Callbacks} on the given file. *) + +val analyze_proc_name_toplevel : Exe_env.t -> Typ.Procname.t -> unit +(** Invoke all the callbacks registered in {!Callbacks} on the given procedure. *) diff --git a/infer/src/unit/TaintTests.ml b/infer/src/unit/TaintTests.ml index b71156e1d..7b1c29e63 100644 --- a/infer/src/unit/TaintTests.ml +++ b/infer/src/unit/TaintTests.ml @@ -115,10 +115,8 @@ let tests = make_load_fld ~rhs_typ:(Typ.mk Tvoid) lhs_id_str fld_str (Exp.Var (ident_of_str root_str)) in let assert_empty = invariant "{ }" in - (* hack: register an empty analyze_ondemand to prevent a crash because the callback is unset *) - let analyze_ondemand summary _ = summary in - let callbacks = {Ondemand.exe_env= Exe_env.mk (); analyze_ondemand} in - Ondemand.set_callbacks callbacks ; + let exe_env = Exe_env.mk () in + Ondemand.set_exe_env exe_env ; let test_list = [ ("source recorded", [assign_to_source "ret_id"; invariant "{ ret_id$0* => (SOURCE -> ?) }"]) ; ("non-source not recorded", [assign_to_non_source "ret_id"; assert_empty])