Reporting cleanup 14: log error using state

Summary:
Separate and rename error reporting functions that use the biabduction state.
No checkers should call these functions.

Reviewed By: da319

Differential Revision: D9633579

fbshipit-source-id: 884fcee66

infer/src/backend/ondemand.ml

@@ -150,8 +150,7 @@ let run_proc_analysis analyze_proc ~caller_pdesc callee_pdesc =
     summary
   in
   let log_error_and_continue exn (summary : Summary.t) kind =
-    let loc = State.get_loc () in
+    Reporting.log_error_using_state summary exn ;
-    Reporting.log_error summary ~loc exn ;
     let stats = Summary.Stats.update summary.stats ~failure_kind:kind in
     let payloads =
       let biabduction =

infer/src/backend/reporting.ml

@@ -17,11 +17,10 @@ type log_t =
   -> exn
   -> unit
 
-let log_issue_from_errlog procname ~clang_method_kind severity err_log ~loc ~node ~ltr
+let log_issue_from_errlog procname ~clang_method_kind severity err_log ~loc ~node ~session ~ltr
     ~linters_def_file ~doc_url ~access ~extras exn =
   let issue_type = (Exceptions.recognize_exception exn).name in
   if (not Config.filtering) (* no-filtering takes priority *) || issue_type.IssueType.enabled then
-    let session = (State.get_session () :> int) in
     Errlog.log_issue procname ~clang_method_kind severity err_log ~loc ~node ~session ~ltr
       ~linters_def_file ~doc_url ~access ~extras exn
 
@@ -29,12 +28,12 @@ let log_issue_from_errlog procname ~clang_method_kind severity err_log ~loc ~nod
 let log_frontend_issue procname severity errlog ~loc ~node_key ~ltr ~linters_def_file ~doc_url exn
     =
   let node = Errlog.FrontendNode {node_key} in
-  log_issue_from_errlog procname ~clang_method_kind:None severity errlog ~loc ~node ~ltr
+  log_issue_from_errlog procname ~clang_method_kind:None severity errlog ~loc ~node ~session:0 ~ltr
     ~linters_def_file ~doc_url ~access:None ~extras:None exn
 
 
-let log_issue_from_summary severity summary ~node ~loc ?ltr ?linters_def_file ?doc_url ?access
+let log_issue_from_summary severity summary ~node ~session ~loc ~ltr ?linters_def_file ?doc_url
-    ?extras exn =
+    ?access ?extras exn =
   let attrs = Summary.get_attributes summary in
   let procname = attrs.proc_name in
   let is_java_generated_method =
@@ -53,20 +52,23 @@ let log_issue_from_summary severity summary ~node ~loc ?ltr ?linters_def_file ?d
   else
     let err_log = Summary.get_err_log summary in
     let clang_method_kind = Some attrs.clang_method_kind in
-    let ltr = match ltr with None -> State.get_loc_trace () | Some ltr -> ltr in
+    log_issue_from_errlog procname ~clang_method_kind severity err_log ~loc ~node ~session ~ltr
-    let node = Errlog.BackendNode {node} in
-    log_issue_from_errlog procname ~clang_method_kind severity err_log ~loc ~node ~ltr
       ~linters_def_file ~doc_url ~access ~extras exn
 
 
-let log_issue_deprecated severity proc_name ?node ?loc ?ltr ?linters_def_file ?doc_url ?access
+let log_issue_deprecated_using_state severity proc_name ?node ?loc ?ltr ?linters_def_file ?doc_url
-    ?extras:_ exn =
+    ?access ?extras:_ exn =
   match Summary.get proc_name with
   | Some summary ->
+      let node =
         let node = match node with None -> State.get_node () | Some node -> node in
+        Errlog.BackendNode {node}
+      in
+      let session = State.get_session () in
       let loc = match loc with None -> State.get_loc () | Some loc -> loc in
-      log_issue_from_summary severity summary ~node ~loc ?ltr ?linters_def_file ?doc_url ?access
+      let ltr = match ltr with None -> State.get_loc_trace () | Some ltr -> ltr in
-        exn
+      log_issue_from_summary severity summary ~node ~session ~loc ~ltr ?linters_def_file ?doc_url
+        ?access exn
   | None ->
       L.(die InternalError)
         "Trying to report error on procedure %a, but cannot because no summary exists for this \
@@ -74,11 +76,10 @@ let log_issue_deprecated severity proc_name ?node ?loc ?ltr ?linters_def_file ?d
         Typ.Procname.pp proc_name Typ.Procname.pp proc_name
 
 
-let log_issue_from_summary_simplified severity summary ~loc ?ltr ?linters_def_file ?doc_url ?access
+let log_issue_from_summary_simplified severity summary ~loc ?(ltr = []) ?linters_def_file ?doc_url
-    ?extras exn =
+    ?access ?extras exn =
-  let node = State.get_node () in
+  log_issue_from_summary severity summary ~node:Errlog.UnknownNode ~session:0 ~loc ~ltr
-  log_issue_from_summary severity summary ~node ~loc ?ltr ?linters_def_file ?doc_url ?access
+    ?linters_def_file ?doc_url ?access ?extras exn
-    ?extras exn
 
 
 let log_error = log_issue_from_summary_simplified Exceptions.Error
@@ -89,10 +90,19 @@ let log_issue_external procname severity ~loc ~ltr ?access issue_type error_mess
   let exn = Exceptions.Checkers (issue_type, Localise.verbatim_desc error_message) in
   let errlog = IssueLog.get_errlog procname in
   let node = Errlog.UnknownNode in
-  log_issue_from_errlog procname ~clang_method_kind:None severity errlog ~loc ~node ~ltr
+  log_issue_from_errlog procname ~clang_method_kind:None severity errlog ~loc ~node ~session:0 ~ltr
     ~linters_def_file:None ~doc_url:None ~access ~extras:None exn
 
 
+let log_error_using_state summary exn =
+  let node = Errlog.BackendNode {node= State.get_node ()} in
+  let session = State.get_session () in
+  let loc = State.get_loc () in
+  let ltr = State.get_loc_trace () in
+  log_issue_from_summary Exceptions.Error summary ~node ~session ~loc ~ltr ?linters_def_file:None
+    ?doc_url:None ?access:None exn
+
+
 let is_suppressed ?(field_name = None) tenv proc_desc kind =
   let lookup = Tenv.lookup tenv in
   let proc_attributes = Procdesc.get_attributes proc_desc in

infer/src/backend/reporting.mli

@@ -18,9 +18,9 @@ type log_t =
   -> exn
   -> unit
 
-val log_issue_deprecated :
+val log_issue_deprecated_using_state :
   Exceptions.severity -> Typ.Procname.t -> ?node:Procdesc.Node.t -> ?loc:Location.t -> log_t
-(** Report an issue in the given procedure.
+(** Report an issue in the given procedure using biabduction state (DO NOT USE ELSEWHERE).
     DEPRECATED as it can create race conditions between checkers.
     Use log_error/warning instead *)
 
@@ -43,6 +43,9 @@ val log_error : Summary.t -> loc:Location.t -> log_t
 val log_warning : Summary.t -> loc:Location.t -> log_t
 (** Add an warning to the given summary. *)
 
+val log_error_using_state : Summary.t -> exn -> unit
+(** Add an error to the given summary using biabduction state (DO NOT USE ELSEWHERE). *)
+
 val log_issue_external :
      Typ.Procname.t
   -> Exceptions.severity

infer/src/biabduction/Abs.ml

@@ -1123,7 +1123,7 @@ let check_junk pname tenv prop =
             let report_leak () =
               if not report_and_continue then raise exn
               else (
-                Reporting.log_issue_deprecated Exceptions.Error pname exn ;
+                Reporting.log_issue_deprecated_using_state Exceptions.Error pname exn ;
                 leaks_reported := alloc_attribute :: !leaks_reported )
             in
             if not ignore_leak then report_leak () ;

infer/src/biabduction/Prover.ml

@@ -2609,7 +2609,7 @@ let check_implication_base pname tenv check_frame_empty calc_missing prop1 prop2
       L.d_strln ("WARNING: footprint failed to find MISSING because: " ^ s) ;
       None
   | Exceptions.Abduction_case_not_implemented _ as exn ->
-      Reporting.log_issue_deprecated Exceptions.Error pname exn ;
+      Reporting.log_issue_deprecated_using_state Exceptions.Error pname exn ;
       None
 
 

infer/src/biabduction/Rearrange.ml

@@ -59,7 +59,7 @@ let check_bad_index tenv pname p len index loc =
         Exceptions.Array_out_of_bounds_l1
           (Errdesc.explain_array_access pname tenv deref_str p loc, __POS__)
       in
-      Reporting.log_issue_deprecated Exceptions.Warning pname exn
+      Reporting.log_issue_deprecated_using_state Exceptions.Warning pname exn
     else if len_is_constant then
       let deref_str = Localise.deref_str_array_bound len_const_opt index_const_opt in
       let desc = Errdesc.explain_array_access pname tenv deref_str p loc in
@@ -67,7 +67,7 @@ let check_bad_index tenv pname p len index loc =
         if index_has_bounds () then Exceptions.Array_out_of_bounds_l2 (desc, __POS__)
         else Exceptions.Array_out_of_bounds_l3 (desc, __POS__)
       in
-      Reporting.log_issue_deprecated Exceptions.Warning pname exn
+      Reporting.log_issue_deprecated_using_state Exceptions.Warning pname exn
 
 
 (** Perform bounds checking *)
@@ -895,7 +895,7 @@ let add_guarded_by_constraints tenv prop lexp pdesc =
       let loc = State.get_loc () in
       let err_desc = Localise.desc_unsafe_guarded_by_access accessed_fld guarded_by_str loc in
       let exn = Exceptions.Unsafe_guarded_by_access (err_desc, __POS__) in
-      Reporting.log_issue_deprecated Exceptions.Error pname exn
+      Reporting.log_issue_deprecated_using_state Exceptions.Error pname exn
     in
     let rec is_read_write_lock typ =
       let str_is_read_write_lock str =
@@ -1341,7 +1341,7 @@ let check_type_size tenv pname prop texp off typ_from_instr =
           Exceptions.Pointer_size_mismatch
             (Errdesc.explain_dereference pname tenv deref_str prop loc, __POS__)
         in
-        Reporting.log_issue_deprecated Exceptions.Warning pname exn
+        Reporting.log_issue_deprecated_using_state Exceptions.Warning pname exn
   | None ->
       L.d_str "texp: " ; Sil.d_texp_full texp ; L.d_ln ()
 

infer/src/biabduction/RetainCycles.ml

@@ -242,8 +242,7 @@ let report_cycle tenv summary prop =
     RetainCyclesType.Set.iter
       (fun cycle ->
         let exn = exn_retain_cycle tenv cycle in
-        let loc = State.get_loc () in
+        Reporting.log_error_using_state summary exn )
-        Reporting.log_error summary ~loc exn )
       cycles ;
     (* we report the retain cycles above but need to raise an exception as well to stop the analysis *)
     raise (Exceptions.Dummy_exception (Localise.verbatim_desc "retain cycle found")) )

infer/src/biabduction/SymExec.ml

@@ -356,7 +356,7 @@ let check_inherently_dangerous_function caller_pname callee_pname =
       Exceptions.Inherently_dangerous_function
         (Localise.desc_inherently_dangerous_function callee_pname)
     in
-    Reporting.log_issue_deprecated Exceptions.Warning caller_pname exn
+    Reporting.log_issue_deprecated_using_state Exceptions.Warning caller_pname exn
 
 
 let reason_to_skip ~callee_desc : string option =
@@ -418,7 +418,7 @@ let check_arith_norm_exp tenv pname exp prop =
   | Some (Attribute.Div0 div), prop' ->
       let desc = Errdesc.explain_divide_by_zero tenv div (State.get_node ()) (State.get_loc ()) in
       let exn = Exceptions.Divide_by_zero (desc, __POS__) in
-      Reporting.log_issue_deprecated Exceptions.Warning pname exn ;
+      Reporting.log_issue_deprecated_using_state Exceptions.Warning pname exn ;
       (Prop.exp_normalize_prop tenv prop exp, prop')
   | Some (Attribute.UminusUnsigned (e, typ)), prop' ->
       let desc =
@@ -426,7 +426,7 @@ let check_arith_norm_exp tenv pname exp prop =
           (State.get_loc ())
       in
       let exn = Exceptions.Unary_minus_applied_to_unsigned_expression (desc, __POS__) in
-      Reporting.log_issue_deprecated Exceptions.Warning pname exn ;
+      Reporting.log_issue_deprecated_using_state Exceptions.Warning pname exn ;
       (Prop.exp_normalize_prop tenv prop exp, prop')
   | None, prop' ->
       (Prop.exp_normalize_prop tenv prop exp, prop')
@@ -484,7 +484,7 @@ let check_already_dereferenced tenv pname cond prop =
           (State.get_loc ())
       in
       let exn = Exceptions.Null_test_after_dereference (desc, __POS__) in
-      Reporting.log_issue_deprecated Exceptions.Warning pname exn
+      Reporting.log_issue_deprecated_using_state Exceptions.Warning pname exn
   | None ->
       ()
 
@@ -1237,7 +1237,7 @@ let rec sym_exec exe_env tenv current_pdesc instr_ (prop_ : Prop.normal Prop.t)
       callee_pname ret_annots loc ret_id_typ ret_typ actual_args =
     let skip_res () =
       let exn = Exceptions.Skip_function (Localise.desc_skip_function callee_pname) in
-      Reporting.log_issue_deprecated Exceptions.Info current_pname exn ;
+      Reporting.log_issue_deprecated_using_state Exceptions.Info current_pname exn ;
       L.d_strln
         (F.sprintf "Skipping function '%s': %s" (Typ.Procname.to_string callee_pname) reason) ;
       Tabulation.log_call_trace ~caller_name:current_pname ~callee_name:callee_pname
@@ -1304,7 +1304,7 @@ let rec sym_exec exe_env tenv current_pdesc instr_ (prop_ : Prop.normal Prop.t)
               let exn =
                 Exceptions.Condition_always_true_false (desc, not (IntLit.iszero i), __POS__)
               in
-              Reporting.log_issue_deprecated Exceptions.Warning current_pname exn
+              Reporting.log_issue_deprecated_using_state Exceptions.Warning current_pname exn
           | _ ->
               ()
       in

infer/src/biabduction/Tabulation.ml

@@ -469,7 +469,7 @@ let check_path_errors_in_post tenv caller_pname post post_path =
           in
           State.set_path new_path path_pos_opt ;
           let exn = Exceptions.Divide_by_zero (desc, __POS__) in
-          Reporting.log_issue_deprecated Exceptions.Warning caller_pname exn )
+          Reporting.log_issue_deprecated_using_state Exceptions.Warning caller_pname exn )
     | _ ->
         ()
   in
@@ -1198,7 +1198,7 @@ let exe_spec exe_env tenv ret_id (n, nspecs) caller_pdesc callee_pname loc prop
           missing_sigma_objc_class callee_summary ) ;
       let log_check_exn check =
         let exn = get_check_exn tenv check callee_pname loc __POS__ in
-        Reporting.log_issue_deprecated Exceptions.Warning caller_pname exn
+        Reporting.log_issue_deprecated_using_state Exceptions.Warning caller_pname exn
       in
       let do_split () =
         process_splitting actual_pre sub1 sub2 frame missing_pi missing_sigma frame_fld missing_fld

infer/src/biabduction/interproc.ml

@@ -418,7 +418,7 @@ let forward_tabulate summary exe_env tenv proc_cfg wl =
     L.d_strln "SIL INSTR:" ;
     Procdesc.Node.d_instrs ~sub_instrs:true (State.get_instr ()) curr_node ;
     L.d_ln () ;
-    Reporting.log_issue_deprecated Exceptions.Error pname exn ;
+    Reporting.log_issue_deprecated_using_state Exceptions.Error pname exn ;
     State.mark_instr_fail exn
   in
   let exe_iter f pathset =
@@ -524,7 +524,7 @@ let remove_locals_formals_and_check tenv proc_cfg p =
       let dexp_opt, _ = Errdesc.vpath_find tenv p (Exp.Lvar pvar) in
       let desc = Errdesc.explain_stack_variable_address_escape loc pvar dexp_opt in
       let exn = Exceptions.Stack_variable_address_escape (desc, __POS__) in
-      Reporting.log_issue_deprecated Exceptions.Warning pname exn
+      Reporting.log_issue_deprecated_using_state Exceptions.Warning pname exn
   in
   List.iter ~f:check_pvar pvars ; p'
 
@@ -860,7 +860,9 @@ let perform_analysis_phase exe_env tenv (summary : Summary.t) (proc_cfg : ProcCf
       forward_tabulate summary exe_env tenv proc_cfg wl
     in
     let get_results (wl : Worklist.t) () =
-      State.process_execution_failures (Reporting.log_issue_deprecated Exceptions.Warning) pname ;
+      State.process_execution_failures
+        (Reporting.log_issue_deprecated_using_state Exceptions.Warning)
+        pname ;
       let results = collect_analysis_result tenv wl proc_cfg in
       let specs =
         try extract_specs tenv (ProcCfg.Exceptional.proc_desc proc_cfg) results
@@ -869,7 +871,7 @@ let perform_analysis_phase exe_env tenv (summary : Summary.t) (proc_cfg : ProcCf
             Exceptions.Internal_error
               (Localise.verbatim_desc "Leak_while_collecting_specs_after_footprint")
           in
-          Reporting.log_issue_deprecated Exceptions.Error pname exn ;
+          Reporting.log_issue_deprecated_using_state Exceptions.Error pname exn ;
           (* retuning no specs *) []
       in
       (specs, BiabductionSummary.FOOTPRINT)
@@ -1028,7 +1030,7 @@ let report_runtime_exceptions tenv pdesc summary =
       in
       let exn_desc = Localise.java_unchecked_exn_desc pname runtime_exception pre_str in
       let exn = Exceptions.Java_runtime_exception (runtime_exception, pre_str, exn_desc) in
-      Reporting.log_issue_deprecated Exceptions.Error pname exn
+      Reporting.log_issue_deprecated_using_state Exceptions.Error pname exn
   in
   List.iter ~f:report exn_preconditions
 
@@ -1041,7 +1043,7 @@ let report_custom_errors tenv summary =
       let loc = Summary.get_loc summary in
       let err_desc = Localise.desc_custom_error loc in
       let exn = Exceptions.Custom_error (custom_error, err_desc) in
-      Reporting.log_issue_deprecated Exceptions.Error pname exn
+      Reporting.log_issue_deprecated_using_state Exceptions.Error pname exn
   in
   List.iter ~f:report error_preconditions
 
@@ -1258,6 +1260,5 @@ let analyze_procedure {Callbacks.summary; proc_desc; tenv; exe_env} : Summary.t
   BuiltinDefn.init () ;
   try analyze_procedure_aux summary exe_env tenv proc_desc with exn ->
     IExn.reraise_if exn ~f:(fun () -> not (Exceptions.handle_exception exn)) ;
-    let loc = State.get_loc () in
+    Reporting.log_error_using_state summary exn ;
-    Reporting.log_error summary ~loc exn ;
     summary

infer/src/eradicate/Checkers.ml

@@ -27,5 +27,5 @@ module ST = struct
         in
         origin_elements @ [Errlog.make_trace_element 0 loc description []]
       in
-      Reporting.log_issue_deprecated severity proc_name ~loc ~ltr:trace exn
+      Reporting.log_issue_deprecated_using_state severity proc_name ~loc ~ltr:trace exn
 end

infer/tests/codetoanalyze/java/checkers/issues.exp

@@ -25,10 +25,10 @@ codetoanalyze/java/checkers/ExpensiveInheritanceExample.java, codetoanalyze.java
 codetoanalyze/java/checkers/ExpensiveInheritanceExample.java, codetoanalyze.java.checkers.ExpensiveInheritanceExample.reportsAssumingObjectOfTypeA():void, 2, CHECKERS_CALLS_EXPENSIVE_METHOD, no_bucket, ERROR, []
 codetoanalyze/java/checkers/ExpensiveInheritanceExample.java, codetoanalyze.java.checkers.ExpensiveInheritanceExample.reportsBecauseFooIsExpensiveInA(codetoanalyze.java.checkers.A):void, 1, CHECKERS_CALLS_EXPENSIVE_METHOD, no_bucket, ERROR, []
 codetoanalyze/java/checkers/ExpensiveInterfaceExample.java, codetoanalyze.java.checkers.ExpensiveInterfaceExample$ImplementsInterface.m1():void, 1, CHECKERS_CALLS_EXPENSIVE_METHOD, no_bucket, ERROR, []
-codetoanalyze/java/checkers/ExpensiveSubtypingExample.java, codetoanalyze.java.checkers.ExpensiveSubtypingExample.m3():void, 0, CHECKERS_EXPENSIVE_OVERRIDES_UNANNOTATED, no_bucket, ERROR, [return from a call to void ExpensiveSubtypingExample.m3()]
+codetoanalyze/java/checkers/ExpensiveSubtypingExample.java, codetoanalyze.java.checkers.ExpensiveSubtypingExample.m3():void, 0, CHECKERS_EXPENSIVE_OVERRIDES_UNANNOTATED, no_bucket, ERROR, []
-codetoanalyze/java/checkers/FragmentRetainsViewExample.java, codetoanalyze.java.checkers.FragmentRetainsViewExample.onDestroyView():void, 0, CHECKERS_FRAGMENT_RETAINS_VIEW, no_bucket, ERROR, [return from a call to void FragmentRetainsViewExample.onDestroyView()]
+codetoanalyze/java/checkers/FragmentRetainsViewExample.java, codetoanalyze.java.checkers.FragmentRetainsViewExample.onDestroyView():void, 0, CHECKERS_FRAGMENT_RETAINS_VIEW, no_bucket, ERROR, []
-codetoanalyze/java/checkers/FragmentRetainsViewExample.java, codetoanalyze.java.checkers.FragmentRetainsViewExample.onDestroyView():void, 0, CHECKERS_FRAGMENT_RETAINS_VIEW, no_bucket, ERROR, [return from a call to void FragmentRetainsViewExample.onDestroyView()]
+codetoanalyze/java/checkers/FragmentRetainsViewExample.java, codetoanalyze.java.checkers.FragmentRetainsViewExample.onDestroyView():void, 0, CHECKERS_FRAGMENT_RETAINS_VIEW, no_bucket, ERROR, []
-codetoanalyze/java/checkers/FragmentRetainsViewExample.java, codetoanalyze.java.checkers.FragmentRetainsViewExample.onDestroyView():void, 0, CHECKERS_FRAGMENT_RETAINS_VIEW, no_bucket, ERROR, [return from a call to void FragmentRetainsViewExample.onDestroyView()]
+codetoanalyze/java/checkers/FragmentRetainsViewExample.java, codetoanalyze.java.checkers.FragmentRetainsViewExample.onDestroyView():void, 0, CHECKERS_FRAGMENT_RETAINS_VIEW, no_bucket, ERROR, []
 codetoanalyze/java/checkers/ImmutableCast.java, codetoanalyze.java.checkers.ImmutableCast.badCast(com.google.common.collect.ImmutableList):java.util.List, 0, CHECKERS_IMMUTABLE_CAST, no_bucket, WARNING, [Method badCast(...) returns class com.google.common.collect.ImmutableList but the return type is class java.util.List. Make sure that users of this method do not try to modify the collection.]
 codetoanalyze/java/checkers/ImmutableCast.java, codetoanalyze.java.checkers.ImmutableCast.badCastFromField():java.util.List, 0, CHECKERS_IMMUTABLE_CAST, no_bucket, WARNING, [Method badCastFromField() returns class com.google.common.collect.ImmutableList but the return type is class java.util.List. Make sure that users of this method do not try to modify the collection.]
 codetoanalyze/java/checkers/NoAllocationExample.java, codetoanalyze.java.checkers.NoAllocationExample.directlyAllocatingMethod():void, 1, CHECKERS_ALLOCATES_MEMORY, no_bucket, ERROR, []