From 358c8b34ac943c125da43ad82455a84f1f352de8 Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Mon, 8 Jun 2020 04:19:33 -0700 Subject: [PATCH] stop going through Exceptions for non-biabduction issues Summary: Before: to report an issue type, wrap it in an exception type then pass that around and translate it back to an issue type at some point. After: no. Except biabduction, which was built like this and uses raising of exceptions to stop the analysis too. Reviewed By: dulmarod Differential Revision: D21904589 fbshipit-source-id: cb8446f38 --- infer/src/absint/Errlog.ml | 12 ++--- infer/src/absint/Errlog.mli | 2 +- infer/src/absint/IssueToReport.ml | 22 +++++++++ infer/src/absint/IssueToReport.mli | 26 ++++++++++ infer/src/absint/Reporting.ml | 18 +++---- infer/src/absint/Reporting.mli | 9 +++- infer/src/al/ALIssues.ml | 6 ++- infer/src/biabduction/BiabductionReporting.ml | 8 ++- .../src/{absint => biabduction}/Exceptions.ml | 49 ++++--------------- .../{absint => biabduction}/Exceptions.mli | 24 +-------- infer/src/nullsafe/EradicateReporting.ml | 6 ++- 11 files changed, 95 insertions(+), 87 deletions(-) create mode 100644 infer/src/absint/IssueToReport.ml create mode 100644 infer/src/absint/IssueToReport.mli rename infer/src/{absint => biabduction}/Exceptions.ml (86%) rename infer/src/{absint => biabduction}/Exceptions.mli (84%) diff --git a/infer/src/absint/Errlog.ml b/infer/src/absint/Errlog.ml index f5b936881..488c2077a 100644 --- a/infer/src/absint/Errlog.ml +++ b/infer/src/absint/Errlog.ml @@ -199,8 +199,7 @@ let update errlog_old errlog_new = let log_issue ?severity_override err_log ~loc ~node ~session ~ltr ~linters_def_file ~doc_url ~access - ~extras checker exn = - let error = Exceptions.recognize_exception exn in + ~extras checker (error : IssueToReport.t) = if not (IssueType.checker_can_report checker error.issue_type) then L.die InternalError "Issue type \"%s\" cannot be reported by the checker \"%s\". The only checker that is \ @@ -252,10 +251,9 @@ let log_issue ?severity_override err_log ~loc ~node ~session ~ltr ~linters_def_f let err_key = {severity; issue_type= error.issue_type; err_desc= error.description} in add_issue err_log err_key (ErrDataSet.singleton err_data) in - let should_print_now = match exn with Exceptions.Internal_error _ -> true | _ -> added in - let print_now () = + if added then ( L.debug Analysis Medium "@\n%a@\n@?" - (Exceptions.pp_err ~severity_override:severity loc error.issue_type error.description + (IssueToReport.pp_err ~severity_override:severity loc error.issue_type error.description error.ocaml_pos) () ; if not (IssueType.equal_severity severity Error) then ( @@ -276,6 +274,4 @@ let log_issue ?severity_override err_log ~loc ~node ~session ~ltr ~linters_def_f L.d_info in d warn_str ; - L.d_ln () ) - in - if should_print_now then print_now () + L.d_ln () ) ) diff --git a/infer/src/absint/Errlog.mli b/infer/src/absint/Errlog.mli index f9effc105..449609f28 100644 --- a/infer/src/absint/Errlog.mli +++ b/infer/src/absint/Errlog.mli @@ -99,5 +99,5 @@ val log_issue : -> access:string option -> extras:Jsonbug_t.extra option -> Checker.t - -> exn + -> IssueToReport.t -> unit diff --git a/infer/src/absint/IssueToReport.ml b/infer/src/absint/IssueToReport.ml new file mode 100644 index 000000000..b02e75de2 --- /dev/null +++ b/infer/src/absint/IssueToReport.ml @@ -0,0 +1,22 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +open! IStd +module L = Logging +module F = Format + +type t = {issue_type: IssueType.t; description: Localise.error_desc; ocaml_pos: L.ocaml_pos option} + +(** pretty print an error *) +let pp_err ?severity_override loc issue_type desc ocaml_pos_opt fmt () = + let severity = Option.value severity_override ~default:issue_type.IssueType.default_severity in + let kind = + IssueType.string_of_severity + (if IssueType.equal_severity severity Info then Warning else severity) + in + F.fprintf fmt "%a:%d: %s: %a %a%a@\n" SourceFile.pp loc.Location.file loc.Location.line kind + IssueType.pp issue_type Localise.pp_error_desc desc L.pp_ocaml_pos_opt ocaml_pos_opt diff --git a/infer/src/absint/IssueToReport.mli b/infer/src/absint/IssueToReport.mli new file mode 100644 index 000000000..47c9c0959 --- /dev/null +++ b/infer/src/absint/IssueToReport.mli @@ -0,0 +1,26 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +open! IStd +module L = Logging + +(** An issue about to be reported to the user *) +type t = + { issue_type: IssueType.t + ; description: Localise.error_desc + ; ocaml_pos: L.ocaml_pos option (** location in the infer source code *) } + +val pp_err : + ?severity_override:IssueType.severity + -> Location.t + -> IssueType.t + -> Localise.error_desc + -> Logging.ocaml_pos option + -> Format.formatter + -> unit + -> unit +(** pretty print an error *) diff --git a/infer/src/absint/Reporting.ml b/infer/src/absint/Reporting.ml index 85a616e92..819cdd1ba 100644 --- a/infer/src/absint/Reporting.ml +++ b/infer/src/absint/Reporting.ml @@ -11,13 +11,13 @@ type log_t = ?ltr:Errlog.loc_trace -> ?extras:Jsonbug_t.extra -> Checker.t -> IssueType.t -> string -> unit let log_issue_from_errlog ?severity_override err_log ~loc ~node ~session ~ltr ~access ~extras - checker exn = - let issue_type = (Exceptions.recognize_exception exn).issue_type in + checker (issue_to_report : IssueToReport.t) = + let issue_type = issue_to_report.issue_type in if (not Config.filtering) (* no-filtering takes priority *) || issue_type.IssueType.enabled then let doc_url = issue_type.doc_url in let linters_def_file = issue_type.linters_def_file in Errlog.log_issue ?severity_override err_log ~loc ~node ~session ~ltr ~linters_def_file ~doc_url - ~access ~extras checker exn + ~access ~extras checker issue_to_report let log_frontend_issue errlog ~loc ~node_key ~ltr exn = @@ -54,15 +54,15 @@ let log_issue_from_summary ?severity_override proc_desc err_log ~node ~session ~ checker exn -let checker_exception issue_type error_message = - Exceptions.Checkers (issue_type, Localise.verbatim_desc error_message) +let mk_issue_to_report issue_type error_message = + {IssueToReport.issue_type; description= Localise.verbatim_desc error_message; ocaml_pos= None} let log_issue_from_summary_simplified ?severity_override attrs err_log ~loc ?(ltr = []) ?extras checker issue_type error_message = - let exn = checker_exception issue_type error_message in + let issue_to_report = mk_issue_to_report issue_type error_message in log_issue_from_summary ?severity_override attrs err_log ~node:Errlog.UnknownNode ~session:0 ~loc - ~ltr ?extras checker exn + ~ltr ?extras checker issue_to_report let log_issue attrs err_log ~loc ?ltr ?extras checker issue_type error_message = @@ -71,11 +71,11 @@ let log_issue attrs err_log ~loc ?ltr ?extras checker issue_type error_message = let log_issue_external procname ~issue_log ?severity_override ~loc ~ltr ?access ?extras checker issue_type error_message = - let exn = checker_exception issue_type error_message in + let issue_to_report = mk_issue_to_report issue_type error_message in let issue_log, errlog = IssueLog.get_or_add issue_log ~proc:procname in let node = Errlog.UnknownNode in log_issue_from_errlog ?severity_override errlog ~loc ~node ~session:0 ~ltr ~access ~extras checker - exn ; + issue_to_report ; issue_log diff --git a/infer/src/absint/Reporting.mli b/infer/src/absint/Reporting.mli index 680fecd5f..3f8355e5e 100644 --- a/infer/src/absint/Reporting.mli +++ b/infer/src/absint/Reporting.mli @@ -22,11 +22,16 @@ val log_issue_from_summary : -> ltr:Errlog.loc_trace -> ?extras:Jsonbug_t.extra -> Checker.t - -> exn + -> IssueToReport.t -> unit val log_frontend_issue : - Errlog.t -> loc:Location.t -> node_key:Procdesc.NodeKey.t -> ltr:Errlog.loc_trace -> exn -> unit + Errlog.t + -> loc:Location.t + -> node_key:Procdesc.NodeKey.t + -> ltr:Errlog.loc_trace + -> IssueToReport.t + -> unit (** Report a frontend issue of a given kind in the given error log. *) val log_issue : Procdesc.t -> Errlog.t -> loc:Location.t -> log_t diff --git a/infer/src/al/ALIssues.ml b/infer/src/al/ALIssues.ml index 224ff168c..20d7a06b1 100644 --- a/infer/src/al/ALIssues.ml +++ b/infer/src/al/ALIssues.ml @@ -493,7 +493,9 @@ let log_frontend_issue method_decl_opt (node : Ctl_parser_types.ast_node) (issue let err_desc = Localise.desc_frontend_warning issue_desc.description issue_desc.suggestion issue_desc.loc in - let exn = Exceptions.Frontend_warning (issue_desc.issue_type, err_desc, __POS__) in + let issue_to_report = + {IssueToReport.issue_type= issue_desc.issue_type; description= err_desc; ocaml_pos= None} + in let trace = [Errlog.make_trace_element 0 issue_desc.loc "" []] in let key_str = match node with @@ -503,7 +505,7 @@ let log_frontend_issue method_decl_opt (node : Ctl_parser_types.ast_node) (issue CAst_utils.generate_key_stmt st in let node_key = Procdesc.NodeKey.of_frontend_node_key key_str in - Reporting.log_frontend_issue errlog exn ~loc:issue_desc.loc ~ltr:trace ~node_key + Reporting.log_frontend_issue errlog issue_to_report ~loc:issue_desc.loc ~ltr:trace ~node_key let fill_issue_desc_info_and_log context ~witness ~current_node (issue_desc : CIssue.t) loc = diff --git a/infer/src/biabduction/BiabductionReporting.ml b/infer/src/biabduction/BiabductionReporting.ml index e0c07f3ce..553eb1c68 100644 --- a/infer/src/biabduction/BiabductionReporting.ml +++ b/infer/src/biabduction/BiabductionReporting.ml @@ -16,7 +16,9 @@ let log_issue_deprecated_using_state proc_desc err_log ?node ?loc ?ltr exn = let session = AnalysisState.get_session () in let loc = match loc with None -> AnalysisState.get_loc_exn () | Some loc -> loc in let ltr = match ltr with None -> State.get_loc_trace () | Some ltr -> ltr in - Reporting.log_issue_from_summary proc_desc err_log ~node ~session ~loc ~ltr Biabduction exn + let issue_to_report = Exceptions.recognize_exception exn in + Reporting.log_issue_from_summary proc_desc err_log ~node ~session ~loc ~ltr Biabduction + issue_to_report let log_issue_using_state proc_desc err_log exn = @@ -30,4 +32,6 @@ let log_issue_using_state proc_desc err_log exn = match AnalysisState.get_loc () with Some l -> l | None -> Procdesc.Node.get_loc node' in let ltr = State.get_loc_trace () in - Reporting.log_issue_from_summary proc_desc err_log ~node ~session ~loc ~ltr Biabduction exn + let issue_to_report = Exceptions.recognize_exception exn in + Reporting.log_issue_from_summary proc_desc err_log ~node ~session ~loc ~ltr Biabduction + issue_to_report diff --git a/infer/src/absint/Exceptions.ml b/infer/src/biabduction/Exceptions.ml similarity index 86% rename from infer/src/absint/Exceptions.ml rename to infer/src/biabduction/Exceptions.ml index 9b1fc5b96..1fa9cbdce 100644 --- a/infer/src/absint/Exceptions.ml +++ b/infer/src/biabduction/Exceptions.ml @@ -10,6 +10,8 @@ open! IStd module L = Logging module F = Format +(** {1 Biabduction uses exceptions to store issues in summaries} *) + exception Abduction_case_not_implemented of L.ocaml_pos exception Analysis_stops of Localise.error_desc * L.ocaml_pos option @@ -28,8 +30,6 @@ exception Biabd_use_after_free of Localise.error_desc * L.ocaml_pos exception Cannot_star of L.ocaml_pos -exception Checkers of IssueType.t * Localise.error_desc - exception Class_cast_exception of Localise.error_desc * L.ocaml_pos exception Condition_always_true_false of Localise.error_desc * bool * L.ocaml_pos @@ -51,8 +51,6 @@ exception Empty_vector_access of Localise.error_desc * L.ocaml_pos exception Field_not_null_checked of Localise.error_desc * L.ocaml_pos -exception Frontend_warning of IssueType.t * Localise.error_desc * L.ocaml_pos - exception Inherently_dangerous_function of Localise.error_desc exception Internal_error of Localise.error_desc @@ -97,12 +95,7 @@ exception Unary_minus_applied_to_unsigned_expression of Localise.error_desc * L. exception Wrong_argument_number of L.ocaml_pos -type t = - { issue_type: IssueType.t - ; description: Localise.error_desc - ; ocaml_pos: L.ocaml_pos option (** location in the infer source code *) } - -let recognize_exception exn = +let recognize_exception exn : IssueToReport.t = match exn with | Abduction_case_not_implemented ocaml_pos -> { issue_type= IssueType.abduction_case_not_implemented @@ -160,10 +153,6 @@ let recognize_exception exn = {issue_type= IssueType.empty_vector_access; description= desc; ocaml_pos= Some ocaml_pos} | Field_not_null_checked (desc, ocaml_pos) -> {issue_type= IssueType.field_not_null_checked; description= desc; ocaml_pos= Some ocaml_pos} - | Frontend_warning (issue_type, desc, ocaml_pos) -> - {issue_type; description= desc; ocaml_pos= Some ocaml_pos} - | Checkers (kind, desc) -> - {issue_type= kind; description= desc; ocaml_pos= None} | Null_dereference (desc, ocaml_pos) -> {issue_type= IssueType.null_dereference; description= desc; ocaml_pos= Some ocaml_pos} | Null_test_after_dereference (desc, ocaml_pos) -> @@ -177,18 +166,11 @@ let recognize_exception exn = | Internal_error desc -> {issue_type= IssueType.internal_error; description= desc; ocaml_pos= None} | Leak (fp_part, (user_visible, error_desc), done_array_abstraction, resource, ocaml_pos) -> - if done_array_abstraction then - { issue_type= IssueType.leak_after_array_abstraction - ; description= error_desc - ; ocaml_pos= Some ocaml_pos } - else if fp_part then - {issue_type= IssueType.leak_in_footprint; description= error_desc; ocaml_pos= Some ocaml_pos} - else if not user_visible then - { issue_type= IssueType.leak_unknown_origin - ; description= error_desc - ; ocaml_pos= Some ocaml_pos } - else - let issue_type = + let issue_type = + if done_array_abstraction then IssueType.leak_after_array_abstraction + else if fp_part then IssueType.leak_in_footprint + else if not user_visible then IssueType.leak_unknown_origin + else match resource with | PredSymb.Rmemory _ -> IssueType.memory_leak @@ -198,8 +180,8 @@ let recognize_exception exn = IssueType.resource_leak | PredSymb.Rignore -> IssueType.memory_leak - in - {issue_type; description= error_desc; ocaml_pos= Some ocaml_pos} + in + {issue_type; description= error_desc; ocaml_pos= Some ocaml_pos} | Missing_fld (fld, ocaml_pos) -> let desc = Localise.verbatim_desc (Fieldname.to_full_string fld) in {issue_type= IssueType.missing_fld; description= desc; ocaml_pos= Some ocaml_pos} @@ -262,17 +244,6 @@ let print_exception_html s exn = error.description ocaml_pos_string -(** pretty print an error *) -let pp_err ?severity_override loc issue_type desc ocaml_pos_opt fmt () = - let severity = Option.value severity_override ~default:issue_type.IssueType.default_severity in - let kind = - IssueType.string_of_severity - (if IssueType.equal_severity severity Info then Warning else severity) - in - F.fprintf fmt "%a:%d: %s: %a %a%a@\n" SourceFile.pp loc.Location.file loc.Location.line kind - IssueType.pp issue_type Localise.pp_error_desc desc L.pp_ocaml_pos_opt ocaml_pos_opt - - (** Return true if the exception is not serious and should be handled in timeout mode *) let handle_exception exn = let error = recognize_exception exn in diff --git a/infer/src/absint/Exceptions.mli b/infer/src/biabduction/Exceptions.mli similarity index 84% rename from infer/src/absint/Exceptions.mli rename to infer/src/biabduction/Exceptions.mli index 942c1a543..0087770dd 100644 --- a/infer/src/absint/Exceptions.mli +++ b/infer/src/biabduction/Exceptions.mli @@ -8,7 +8,7 @@ open! IStd -(** Functions for logging and printing exceptions *) +(** {1 Biabduction uses exceptions to store issues in summaries} *) exception Abduction_case_not_implemented of Logging.ocaml_pos @@ -28,8 +28,6 @@ exception Biabd_use_after_free of Localise.error_desc * Logging.ocaml_pos exception Cannot_star of Logging.ocaml_pos -exception Checkers of IssueType.t * Localise.error_desc - exception Class_cast_exception of Localise.error_desc * Logging.ocaml_pos exception Condition_always_true_false of Localise.error_desc * bool * Logging.ocaml_pos @@ -52,8 +50,6 @@ exception Field_not_null_checked of Localise.error_desc * Logging.ocaml_pos exception Empty_vector_access of Localise.error_desc * Logging.ocaml_pos -exception Frontend_warning of IssueType.t * Localise.error_desc * Logging.ocaml_pos - exception Inherently_dangerous_function of Localise.error_desc exception Internal_error of Localise.error_desc @@ -104,20 +100,4 @@ val handle_exception : exn -> bool val print_exception_html : string -> exn -> unit (** print a description of the exception to the html output *) -val pp_err : - ?severity_override:IssueType.severity - -> Location.t - -> IssueType.t - -> Localise.error_desc - -> Logging.ocaml_pos option - -> Format.formatter - -> unit - -> unit -(** pretty print an error *) - -type t = - { issue_type: IssueType.t - ; description: Localise.error_desc - ; ocaml_pos: Logging.ocaml_pos option (** location in the infer source code *) } - -val recognize_exception : exn -> t +val recognize_exception : exn -> IssueToReport.t diff --git a/infer/src/nullsafe/EradicateReporting.ml b/infer/src/nullsafe/EradicateReporting.ml index 9693d6ec0..cc67f5a52 100644 --- a/infer/src/nullsafe/EradicateReporting.ml +++ b/infer/src/nullsafe/EradicateReporting.ml @@ -13,10 +13,12 @@ let report_error {IntraproceduralAnalysis.proc_desc; tenv; err_log} checker kind if suppressed then Logging.debug Analysis Medium "Reporting is suppressed!@\n" else let localized_description = Localise.verbatim_desc description in - let exn = Exceptions.Checkers (kind, localized_description) in + let issue_to_report = + {IssueToReport.issue_type= kind; description= localized_description; ocaml_pos= None} + in let trace = [Errlog.make_trace_element 0 loc description []] in let node = AnalysisState.get_node_exn () in let session = AnalysisState.get_session () in Reporting.log_issue_from_summary ~severity_override:severity proc_desc err_log ~node:(BackendNode {node}) - ~session ~loc ~ltr:trace checker exn + ~session ~loc ~ltr:trace checker issue_to_report