From 9a6cafe6ecb8d6005debc4e30cc7b0fcdd4dd79f Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Wed, 29 Apr 2020 02:58:33 -0700 Subject: [PATCH] execute checkers in the order they are registered Summary: This fixes a long-standing TODO. I re-ordered the lines by: - first putting each entry on a single line (replacing "^J " -> " " in emacs) - then using emacs' M-x reverse-region - then auto-formatting again Reviewed By: skcho Differential Revision: D21257475 fbshipit-source-id: 91b780a5d --- infer/src/backend/callbacks.ml | 21 ++- infer/src/checkers/registerCheckers.ml | 190 ++++++++++++------------- 2 files changed, 104 insertions(+), 107 deletions(-) diff --git a/infer/src/backend/callbacks.ml b/infer/src/backend/callbacks.ml index 0185e8841..da057ae19 100644 --- a/infer/src/backend/callbacks.ml +++ b/infer/src/backend/callbacks.ml @@ -28,18 +28,18 @@ type file_callback = (** Place for storing issues generated at file-level analysis stage (additionally to ones generated by procedure-level callbacks which are stored in summaries) *) } -let procedure_callbacks = ref [] +let procedure_callbacks_rev = ref [] -let file_callbacks = ref [] +let file_callbacks_rev = ref [] let register_procedure_callback ~checker_name ?(dynamic_dispatch = false) language (callback : proc_callback_t) = - procedure_callbacks := - {checker_name; dynamic_dispatch; language; callback} :: !procedure_callbacks + procedure_callbacks_rev := + {checker_name; dynamic_dispatch; language; callback} :: !procedure_callbacks_rev let register_file_callback ~checker_name language (callback : file_callback_t) ~issue_dir = - file_callbacks := {checker_name; language; callback; issue_dir} :: !file_callbacks + file_callbacks_rev := {checker_name; language; callback; issue_dir} :: !file_callbacks_rev let iterate_procedure_callbacks exe_env summary = @@ -58,8 +58,8 @@ let iterate_procedure_callbacks exe_env summary = Option.value_map source_file ~default:[] ~f:SourceFiles.proc_names_of_source in let is_specialized = Procdesc.is_specialized proc_desc in - List.fold ~init:summary - ~f:(fun summary {checker_name; dynamic_dispatch; language; callback} -> + List.fold_right ~init:summary !procedure_callbacks_rev + ~f:(fun {checker_name; dynamic_dispatch; language; callback} summary -> if Language.equal language procedure_language && (dynamic_dispatch || not is_specialized) then ( PerfEvent.( log (fun logger -> @@ -70,11 +70,10 @@ let iterate_procedure_callbacks exe_env summary = PerfEvent.(log (fun logger -> log_end_event logger ())) ; summary ) else summary ) - !procedure_callbacks let iterate_file_callbacks_and_store_issues procedures exe_env source_file = - if not (List.is_empty !file_callbacks) then + if not (List.is_empty !file_callbacks_rev) then let environment = {procedures; source_file; exe_env} in let language_matches language = match procedures with @@ -83,10 +82,8 @@ let iterate_file_callbacks_and_store_issues procedures exe_env source_file = | _ -> true in - List.iter - ~f:(fun {language; callback; issue_dir} -> + List.iter (List.rev !file_callbacks_rev) ~f:(fun {language; callback; issue_dir} -> if language_matches language then ( Language.curr_language := language ; let issue_log = callback environment in IssueLog.store ~file:source_file ~entry:issue_dir issue_log ) ) - !file_callbacks diff --git a/infer/src/checkers/registerCheckers.ml b/infer/src/checkers/registerCheckers.ml index 311cfb191..385d5f83f 100644 --- a/infer/src/checkers/registerCheckers.ml +++ b/infer/src/checkers/registerCheckers.ml @@ -28,58 +28,66 @@ type callback = callback_fun * Language.t type checker = {name: string; active: bool; callbacks: callback list} let all_checkers = - (* TODO (T24393492): the checkers should run in the order from this list. - Currently, the checkers are run in the reverse order *) - [ { name= "annotation reachability" - ; active= Config.is_checker_enabled AnnotationReachability + (* The order of the list is important for those checkers that depend on other checkers having run + before them. *) + [ { name= "Self captured in block checker" + ; active= Config.is_checker_enabled SelfInBlock + ; callbacks= [(Procedure SelfInBlock.checker, Language.Clang)] } + ; { name= "Class loading analysis" + ; active= Config.is_checker_enabled ClassLoads + ; callbacks= [(Procedure ClassLoads.analyze_procedure, Language.Java)] } + ; { name= "purity" + ; active= Config.(is_checker_enabled Purity || is_checker_enabled LoopHoisting) ; callbacks= - [ (Procedure AnnotationReachability.checker, Language.Java) - ; (Procedure AnnotationReachability.checker, Language.Clang) ] } - ; { name= "biabduction" - ; active= Config.is_checker_enabled Biabduction + [(Procedure Purity.checker, Language.Java); (Procedure Purity.checker, Language.Clang)] } + ; { name= "Starvation analysis" + ; active= Config.is_checker_enabled Starvation ; callbacks= - [ (DynamicDispatch Interproc.analyze_procedure, Language.Clang) - ; (DynamicDispatch Interproc.analyze_procedure, Language.Java) ] } - ; { name= "buffer overrun analysis" + [ (Procedure Starvation.analyze_procedure, Language.Java) + ; (File {callback= Starvation.reporting; issue_dir= StarvationIssues}, Language.Java) + ; (Procedure Starvation.analyze_procedure, Language.Clang) + ; (File {callback= Starvation.reporting; issue_dir= StarvationIssues}, Language.Clang) ] } + ; { name= "loop hoisting" + ; active= Config.is_checker_enabled LoopHoisting + ; callbacks= + [(Procedure Hoisting.checker, Language.Clang); (Procedure Hoisting.checker, Language.Java)] + } + ; { name= "cost analysis" ; active= Config.( - is_checker_enabled BufferOverrun || is_checker_enabled Cost - || is_checker_enabled LoopHoisting || is_checker_enabled Purity) - ; callbacks= - [ (Procedure BufferOverrunAnalysis.do_analysis, Language.Clang) - ; (Procedure BufferOverrunAnalysis.do_analysis, Language.Java) ] } - ; { name= "buffer overrun checker" - ; active= Config.(is_checker_enabled BufferOverrun) - ; callbacks= - [ (Procedure BufferOverrunChecker.checker, Language.Clang) - ; (Procedure BufferOverrunChecker.checker, Language.Java) ] } - ; { name= "eradicate" - ; active= Config.is_checker_enabled Eradicate + is_checker_enabled Cost + || (is_checker_enabled LoopHoisting && hoisting_report_only_expensive)) + ; callbacks= [(Procedure Cost.checker, Language.Clang); (Procedure Cost.checker, Language.Java)] + } + ; { name= "uninitialized variables" + ; active= Config.is_checker_enabled Uninit + ; callbacks= [(Procedure Uninit.checker, Language.Clang)] } + ; { name= "SIOF" + ; active= Config.is_checker_enabled SIOF + ; callbacks= [(Procedure Siof.checker, Language.Clang)] } + ; { name= "litho-required-props" + ; active= Config.is_checker_enabled LithoRequiredProps + ; callbacks= [(Procedure RequiredProps.checker, Language.Java)] } + ; (* toy resource analysis to use in the infer lab, see the lab/ directory *) + { name= "resource leak" + ; active= Config.is_checker_enabled ResourceLeak ; callbacks= - [ (Procedure Eradicate.proc_callback, Language.Java) - ; (File {callback= Eradicate.file_callback; issue_dir= NullsafeFileIssues}, Language.Java) - ] } - ; { name= "fragment retains view" - ; active= Config.is_checker_enabled FragmentRetainsView + [ ( (* the checked-in version is intraprocedural, but the lab asks to make it + interprocedural later on *) + Procedure ResourceLeaks.checker + , Language.Java ) ] } + ; { name= "RacerD" + ; active= Config.is_checker_enabled RacerD ; callbacks= - [(Procedure FragmentRetainsViewChecker.callback_fragment_retains_view, Language.Java)] } - ; { name= "immutable cast" - ; active= Config.is_checker_enabled ImmutableCast - ; callbacks= [(Procedure ImmutableChecker.callback_check_immutable_cast, Language.Java)] } - ; { name= "inefficient keyset iterator" - ; active= Config.is_checker_enabled InefficientKeysetIterator - ; callbacks= [(Procedure InefficientKeysetIterator.checker, Language.Java)] } - ; { name= "liveness" - ; active= Config.is_checker_enabled Liveness - ; callbacks= [(Procedure Liveness.checker, Language.Clang)] } - ; { name= "printf args" - ; active= Config.is_checker_enabled PrintfArgs - ; callbacks= [(Procedure PrintfArgs.callback_printf_args, Language.Java)] } - ; { name= "impurity" - ; active= Config.is_checker_enabled Impurity + [ (Procedure RacerD.analyze_procedure, Language.Clang) + ; (Procedure RacerD.analyze_procedure, Language.Java) + ; (File {callback= RacerD.file_analysis; issue_dir= RacerDIssues}, Language.Clang) + ; (File {callback= RacerD.file_analysis; issue_dir= RacerDIssues}, Language.Java) ] } + ; { name= "quandary" + ; active= Config.(is_checker_enabled Quandary) ; callbacks= - [(Procedure Impurity.checker, Language.Java); (Procedure Impurity.checker, Language.Clang)] - } + [ (Procedure JavaTaintAnalysis.checker, Language.Java) + ; (Procedure ClangTaintAnalysis.checker, Language.Clang) ] } ; { name= "pulse" ; active= Config.(is_checker_enabled Pulse || is_checker_enabled Impurity) ; callbacks= @@ -87,64 +95,56 @@ let all_checkers = :: ( if Config.is_checker_enabled Impurity then [(Procedure Pulse.checker, Language.Java)] else [] ) } - ; { name= "quandary" - ; active= Config.(is_checker_enabled Quandary) + ; { name= "impurity" + ; active= Config.is_checker_enabled Impurity ; callbacks= - [ (Procedure JavaTaintAnalysis.checker, Language.Java) - ; (Procedure ClangTaintAnalysis.checker, Language.Clang) ] } - ; { name= "RacerD" - ; active= Config.is_checker_enabled RacerD + [(Procedure Impurity.checker, Language.Java); (Procedure Impurity.checker, Language.Clang)] + } + ; { name= "printf args" + ; active= Config.is_checker_enabled PrintfArgs + ; callbacks= [(Procedure PrintfArgs.callback_printf_args, Language.Java)] } + ; { name= "liveness" + ; active= Config.is_checker_enabled Liveness + ; callbacks= [(Procedure Liveness.checker, Language.Clang)] } + ; { name= "inefficient keyset iterator" + ; active= Config.is_checker_enabled InefficientKeysetIterator + ; callbacks= [(Procedure InefficientKeysetIterator.checker, Language.Java)] } + ; { name= "immutable cast" + ; active= Config.is_checker_enabled ImmutableCast + ; callbacks= [(Procedure ImmutableChecker.callback_check_immutable_cast, Language.Java)] } + ; { name= "fragment retains view" + ; active= Config.is_checker_enabled FragmentRetainsView ; callbacks= - [ (Procedure RacerD.analyze_procedure, Language.Clang) - ; (Procedure RacerD.analyze_procedure, Language.Java) - ; (File {callback= RacerD.file_analysis; issue_dir= RacerDIssues}, Language.Clang) - ; (File {callback= RacerD.file_analysis; issue_dir= RacerDIssues}, Language.Java) ] } - (* toy resource analysis to use in the infer lab, see the lab/ directory *) - ; { name= "resource leak" - ; active= Config.is_checker_enabled ResourceLeak + [(Procedure FragmentRetainsViewChecker.callback_fragment_retains_view, Language.Java)] } + ; { name= "eradicate" + ; active= Config.is_checker_enabled Eradicate ; callbacks= - [ ( (* the checked-in version is intraprocedural, but the lab asks to make it - interprocedural later on *) - Procedure ResourceLeaks.checker - , Language.Java ) ] } - ; { name= "litho-required-props" - ; active= Config.is_checker_enabled LithoRequiredProps - ; callbacks= [(Procedure RequiredProps.checker, Language.Java)] } - ; { name= "SIOF" - ; active= Config.is_checker_enabled SIOF - ; callbacks= [(Procedure Siof.checker, Language.Clang)] } - ; { name= "uninitialized variables" - ; active= Config.is_checker_enabled Uninit - ; callbacks= [(Procedure Uninit.checker, Language.Clang)] } - ; { name= "cost analysis" + [ (Procedure Eradicate.proc_callback, Language.Java) + ; (File {callback= Eradicate.file_callback; issue_dir= NullsafeFileIssues}, Language.Java) + ] } + ; { name= "buffer overrun checker" + ; active= Config.(is_checker_enabled BufferOverrun) + ; callbacks= + [ (Procedure BufferOverrunChecker.checker, Language.Clang) + ; (Procedure BufferOverrunChecker.checker, Language.Java) ] } + ; { name= "buffer overrun analysis" ; active= Config.( - is_checker_enabled Cost - || (is_checker_enabled LoopHoisting && hoisting_report_only_expensive)) - ; callbacks= [(Procedure Cost.checker, Language.Clang); (Procedure Cost.checker, Language.Java)] - } - ; { name= "loop hoisting" - ; active= Config.is_checker_enabled LoopHoisting + is_checker_enabled BufferOverrun || is_checker_enabled Cost + || is_checker_enabled LoopHoisting || is_checker_enabled Purity) ; callbacks= - [(Procedure Hoisting.checker, Language.Clang); (Procedure Hoisting.checker, Language.Java)] - } - ; { name= "Starvation analysis" - ; active= Config.is_checker_enabled Starvation + [ (Procedure BufferOverrunAnalysis.do_analysis, Language.Clang) + ; (Procedure BufferOverrunAnalysis.do_analysis, Language.Java) ] } + ; { name= "biabduction" + ; active= Config.is_checker_enabled Biabduction ; callbacks= - [ (Procedure Starvation.analyze_procedure, Language.Java) - ; (File {callback= Starvation.reporting; issue_dir= StarvationIssues}, Language.Java) - ; (Procedure Starvation.analyze_procedure, Language.Clang) - ; (File {callback= Starvation.reporting; issue_dir= StarvationIssues}, Language.Clang) ] } - ; { name= "purity" - ; active= Config.(is_checker_enabled Purity || is_checker_enabled LoopHoisting) + [ (DynamicDispatch Interproc.analyze_procedure, Language.Clang) + ; (DynamicDispatch Interproc.analyze_procedure, Language.Java) ] } + ; { name= "annotation reachability" + ; active= Config.is_checker_enabled AnnotationReachability ; callbacks= - [(Procedure Purity.checker, Language.Java); (Procedure Purity.checker, Language.Clang)] } - ; { name= "Class loading analysis" - ; active= Config.is_checker_enabled ClassLoads - ; callbacks= [(Procedure ClassLoads.analyze_procedure, Language.Java)] } - ; { name= "Self captured in block checker" - ; active= Config.is_checker_enabled SelfInBlock - ; callbacks= [(Procedure SelfInBlock.checker, Language.Clang)] } ] + [ (Procedure AnnotationReachability.checker, Language.Java) + ; (Procedure AnnotationReachability.checker, Language.Clang) ] } ] let get_active_checkers () =