[starvation] implement SuppressLint

Summary: Create mechanism for suppressing starvation reports.  To do that, refactor and expose a Checkers function.

Reviewed By: mbouaziz

Differential Revision: D8259583

fbshipit-source-id: f5b5a63

infer/src/absint/Checkers.ml

@@ -9,65 +9,14 @@ open! IStd
 
 (** Module for user-defined checkers. *)
 
-(** State that persists in the .specs files. *)
 module ST = struct
   let report_error tenv proc_name proc_desc kind loc ?(field_name= None) ?(origin_loc= None)
       ?(exception_kind= fun k d -> Exceptions.Checkers (k, d)) ?(always_report= false) description =
-    let lookup = Tenv.lookup tenv in
     let localized_description =
       Localise.custom_desc description [("always_report", string_of_bool always_report)]
     in
     let exn = exception_kind kind localized_description in
-    let proc_attributes = Summary.pdesc_resolve_attributes proc_desc in
-    (* Errors can be suppressed with annotations. An error of kind CHECKER_ERROR_NAME can be
-       suppressed with the following annotations:
-       - @android.annotation.SuppressLint("checker-error-name")
-       - @some.PrefixErrorName
-       where the kind matching is case - insensitive and ignores '-' and '_' characters. *)
-    let suppressed =
-      let annotation_matches (a: Annot.t) =
-        let normalize str = Str.global_replace (Str.regexp "[_-]") "" (String.lowercase str) in
-        let drop_prefix str = Str.replace_first (Str.regexp "^[A-Za-z]+_") "" str in
-        let normalized_equal s1 s2 = String.equal (normalize s1) (normalize s2) in
-        let is_parameter_suppressed =
-          String.is_suffix a.class_name ~suffix:Annotations.suppress_lint
-          && List.mem ~equal:normalized_equal a.parameters kind.IssueType.unique_id
-        in
-        let is_annotation_suppressed =
-          String.is_suffix
-            ~suffix:(normalize (drop_prefix kind.IssueType.unique_id))
-            (normalize a.class_name)
-        in
-        is_parameter_suppressed || is_annotation_suppressed
-      in
-      let is_method_suppressed =
-        Annotations.ma_has_annotation_with proc_attributes.ProcAttributes.method_annotation
-          annotation_matches
-      in
-      let is_field_suppressed =
-        match (field_name, PatternMatch.get_this_type proc_attributes) with
-        | Some field_name, Some t -> (
-          match Typ.Struct.get_field_type_and_annotation ~lookup field_name t with
-          | Some (_, ia) ->
-              Annotations.ia_has_annotation_with ia annotation_matches
-          | None ->
-              false )
-        | _ ->
-            false
-      in
-      let is_class_suppressed =
-        match PatternMatch.get_this_type proc_attributes with
-        | Some t -> (
-          match PatternMatch.type_get_annotation tenv t with
-          | Some ia ->
-              Annotations.ia_has_annotation_with ia annotation_matches
-          | None ->
-              false )
-        | None ->
-            false
-      in
-      is_method_suppressed || is_field_suppressed || is_class_suppressed
-    in
+    let suppressed = Reporting.is_suppressed tenv proc_desc kind ~field_name in
     let trace =
       let origin_elements =
         match origin_loc with

infer/src/absint/Checkers.mli

@@ -9,7 +9,6 @@ open! IStd
 
 (** Module for user-defined checkers. *)
 
-(** State that persists in the .specs files. *)
 module ST : sig
   val report_error :
     Tenv.t -> Typ.Procname.t -> Procdesc.t -> IssueType.t -> Location.t

infer/src/backend/reporting.ml

@@ -98,3 +98,55 @@ let log_issue_external procname ?clang_method_kind err_kind ?loc ?node_id ?sessi
   let errlog = IssueLog.get_errlog procname in
   log_issue_from_errlog procname ?clang_method_kind err_kind errlog ?loc ?node_id ?session ?ltr
     ?linters_def_file ?doc_url ?access exn
+
+
+let is_suppressed tenv proc_desc kind ?(field_name= None) =
+  let lookup = Tenv.lookup tenv in
+  let proc_attributes = Summary.pdesc_resolve_attributes proc_desc in
+  (* Errors can be suppressed with annotations. An error of kind CHECKER_ERROR_NAME can be
+         suppressed with the following annotations:
+         - @android.annotation.SuppressLint("checker-error-name")
+         - @some.PrefixErrorName
+         where the kind matching is case - insensitive and ignores '-' and '_' characters. *)
+  let annotation_matches (a: Annot.t) =
+    let normalize str = Str.global_replace (Str.regexp "[_-]") "" (String.lowercase str) in
+    let drop_prefix str = Str.replace_first (Str.regexp "^[A-Za-z]+_") "" str in
+    let normalized_equal s1 s2 = String.equal (normalize s1) (normalize s2) in
+    let is_parameter_suppressed =
+      String.is_suffix a.class_name ~suffix:Annotations.suppress_lint
+      && List.mem ~equal:normalized_equal a.parameters kind.IssueType.unique_id
+    in
+    let is_annotation_suppressed =
+      String.is_suffix
+        ~suffix:(normalize (drop_prefix kind.IssueType.unique_id))
+        (normalize a.class_name)
+    in
+    is_parameter_suppressed || is_annotation_suppressed
+  in
+  let is_method_suppressed =
+    Annotations.ma_has_annotation_with proc_attributes.ProcAttributes.method_annotation
+      annotation_matches
+  in
+  let is_field_suppressed =
+    match (field_name, PatternMatch.get_this_type proc_attributes) with
+    | Some field_name, Some t -> (
+      match Typ.Struct.get_field_type_and_annotation ~lookup field_name t with
+      | Some (_, ia) ->
+          Annotations.ia_has_annotation_with ia annotation_matches
+      | None ->
+          false )
+    | _ ->
+        false
+  in
+  let is_class_suppressed =
+    match PatternMatch.get_this_type proc_attributes with
+    | Some t -> (
+      match PatternMatch.type_get_annotation tenv t with
+      | Some ia ->
+          Annotations.ia_has_annotation_with ia annotation_matches
+      | None ->
+          false )
+    | None ->
+        false
+  in
+  is_method_suppressed || is_field_suppressed || is_class_suppressed

infer/src/backend/reporting.mli

@@ -45,3 +45,7 @@ val log_issue_external :
   Typ.Procname.t -> ?clang_method_kind:ProcAttributes.clang_method_kind -> Exceptions.err_kind
   -> log_t
 (** Log an issue to the error log in [IssueLog] associated with the given procname. *)
+
+val is_suppressed :
+  Tenv.t -> Procdesc.t -> IssueType.t -> ?field_name:Typ.Fieldname.t option -> bool
+(** should an issue report be suppressed due to a [@SuppressLint("issue")] annotation? *)

infer/src/concurrency/starvation.ml

@@ -158,21 +158,29 @@ module ReportMap = struct
 
   let empty : report_t list LocMap.t = LocMap.empty
 
-  let add issue pname loc ltr message map =
-    let rep = {issue; pname; ltr; message} in
-    let preexisting = try LocMap.find loc map with Caml.Not_found -> [] in
-    LocMap.add loc (rep :: preexisting) map
+  let issue_type issue =
+    match issue with Deadlock -> IssueType.deadlock | Starvation _ -> IssueType.starvation
 
 
-  let add_deadlock pname loc ltr exn map = add Deadlock pname loc ltr exn map
+  let add tenv issue pdesc loc ltr message map =
+    let pname = Procdesc.get_proc_name pdesc in
+    let issue_type = issue_type issue in
+    if Reporting.is_suppressed tenv pdesc issue_type ~field_name:None then map
+    else
+      let rep = {issue; pname; ltr; message} in
+      let preexisting = try LocMap.find loc map with Caml.Not_found -> [] in
+      LocMap.add loc (rep :: preexisting) map
+
+
+  let add_deadlock tenv pdesc loc ltr exn map = add tenv Deadlock pdesc loc ltr exn map
+
+  let add_starvation tenv sev pdesc loc ltr exn map =
+    add tenv (Starvation sev) pdesc loc ltr exn map
 
-  let add_starvation sev pname loc ltr exn map = add (Starvation sev) pname loc ltr exn map
 
   let log map =
     let log_report loc {issue; pname; ltr; message} =
-      let issue_type =
-        match issue with Deadlock -> IssueType.deadlock | Starvation _ -> IssueType.starvation
-      in
+      let issue_type = issue_type issue in
       let exn = Exceptions.Checkers (issue_type, Localise.verbatim_desc message) in
       Reporting.log_issue_external pname Exceptions.Kerror ~loc ~ltr exn
     in
@@ -261,7 +269,7 @@ let report_deadlocks tenv current_pdesc {StarvationDomain.order; ui} report_map'
           let second_trace = Order.make_trace ~header:"[Trace 2] " endpoint_pname elem in
           let ltr = first_trace @ second_trace in
           let loc = Order.get_loc current_elem in
-          ReportMap.add_deadlock current_pname loc ltr error_message report_map
+          ReportMap.add_deadlock tenv current_pdesc loc ltr error_message report_map
       | _, _ ->
           report_map
   in
@@ -305,7 +313,7 @@ let report_starvation tenv current_pdesc {StarvationDomain.events; ui} report_ma
         let second_trace = Order.make_trace ~header:"[Trace 2] " endpoint_pname endpoint_elem in
         let ltr = first_trace @ second_trace in
         let loc = Event.get_loc event in
-        ReportMap.add_starvation sev current_pname loc ltr error_message report_map
+        ReportMap.add_starvation tenv sev current_pdesc loc ltr error_message report_map
     | _ ->
         report_map
   in
@@ -319,7 +327,7 @@ let report_starvation tenv current_pdesc {StarvationDomain.events; ui} report_ma
         in
         let loc = Event.get_loc event in
         let ltr = Event.make_trace current_pname event in
-        ReportMap.add_starvation sev current_pname loc ltr error_message report_map
+        ReportMap.add_starvation tenv sev current_pdesc loc ltr error_message report_map
     | Event.LockAcquire endpoint_lock ->
         Lock.owner_class endpoint_lock
         |> Option.value_map ~default:report_map ~f:(fun endpoint_class ->

infer/tests/codetoanalyze/java/starvation/SuppLint.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2018-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import java.util.concurrent.Future;
+import java.util.concurrent.ExecutionException;
+import java.io.IOException;
+import android.support.annotation.UiThread;
+import android.annotation.SuppressLint;
+
+class SuppLint {
+  Future future;
+
+  @UiThread
+  @SuppressLint("starvation")
+  void onUiThreadSuppressed() throws InterruptedException, ExecutionException {
+    future.get();
+  }
+
+  @UiThread
+  @SuppressLint("someOtherString")
+  void onUiThreadBad() throws InterruptedException, ExecutionException {
+    future.get();
+  }
+}
+
+@SuppressLint("STARVATION")
+class SuppLintClass {
+  Future future;
+  @UiThread
+  void onUiThreadSuppressed() throws InterruptedException, ExecutionException {
+    future.get();
+  }
+}

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

@@ -27,5 +27,6 @@ codetoanalyze/java/starvation/LegacySync.java, Object LegacySync.onUiThreadOpBad
 codetoanalyze/java/starvation/ServiceOnUIThread.java, IBinder ServiceOnUIThread.onBind(Intent), 19, STARVATION, no_bucket, ERROR, [`IBinder ServiceOnUIThread.onBind(Intent)`,Method call: `void ServiceOnUIThread.transactBad()`,calls `boolean IBinder.transact(int,Parcel,Parcel,int)` from `void ServiceOnUIThread.transactBad()`]
 codetoanalyze/java/starvation/ServiceOnUIThread.java, void ServiceOnUIThread.transactBad(), 25, STARVATION, no_bucket, ERROR, [`void ServiceOnUIThread.transactBad()`,calls `boolean IBinder.transact(int,Parcel,Parcel,int)` from `void ServiceOnUIThread.transactBad()`]
 codetoanalyze/java/starvation/StaticLock.java, void StaticLock.lockOtherClassOneWayBad(), 23, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void StaticLock.lockOtherClassOneWayBad()`,locks `StaticLock$0` in class `java.lang.Class*`,locks `this` in class `StaticLock*`,[Trace 2] `void StaticLock.lockOtherClassAnotherWayNad()`,locks `this` in class `StaticLock*`,Method call: `void StaticLock.staticSynced()`,locks `StaticLock$0` in class `java.lang.Class*`]
+codetoanalyze/java/starvation/SuppLint.java, void SuppLint.onUiThreadBad(), 26, STARVATION, no_bucket, ERROR, [`void SuppLint.onUiThreadBad()`,calls `Object Future.get()` from `void SuppLint.onUiThreadBad()`]
 codetoanalyze/java/starvation/UIDeadlock.java, void UIDeadlock.onUIThreadBad(), 26, DEADLOCK, no_bucket, ERROR, [[Trace 1] `void UIDeadlock.onUIThreadBad()`,locks `this` in class `UIDeadlock*`,locks `this.UIDeadlock.lockB` in class `UIDeadlock*`,[Trace 2] `void UIDeadlock.notOnUIThreadBad()`,locks `this.UIDeadlock.lockB` in class `UIDeadlock*`,locks `this` in class `UIDeadlock*`]
 codetoanalyze/java/starvation/VisDispFrame.java, void VisDispFrame.callsGetVisibleDisplayFrameOnUiThreadBad(), 18, STARVATION, no_bucket, ERROR, [`void VisDispFrame.callsGetVisibleDisplayFrameOnUiThreadBad()`,calls `void View.getWindowVisibleDisplayFrame(Rect)` from `void VisDispFrame.callsGetVisibleDisplayFrameOnUiThreadBad()`]