[starvation] add issue type STRICT_MODE and make distinct reports on it

Summary:
Make distinct reports on strict mode violations.
For now, restrict to direct violations (UI threads calls transitively a violating method).
Will assess impact and enable indirect reports later (via locks).

Reviewed By: mbouaziz

Differential Revision: D10126780

fbshipit-source-id: 9c75930bc

infer/src/base/IssueType.ml

@@ -355,6 +355,8 @@ let starvation = from_string "STARVATION" ~hum:"UI Thread Starvation"
 
 let static_initialization_order_fiasco = from_string "STATIC_INITIALIZATION_ORDER_FIASCO"
 
+let strict_mode_violation = from_string "STRICT_MODE_VIOLATION" ~hum:"Strict Mode Violation"
+
 let symexec_memory_error =
   from_string "Symexec_memory_error" ~hum:"Symbolic Execution Memory Error"
 

infer/src/base/IssueType.mli

@@ -261,6 +261,8 @@ val starvation : t
 
 val static_initialization_order_fiasco : t
 
+val strict_mode_violation : t
+
 val symexec_memory_error : t
 
 val tainted_buffer_access : t

infer/src/concurrency/MethodMatcher.ml

@@ -64,6 +64,8 @@ let default =
 
 let of_list matchers tenv pn actuals = List.exists matchers ~f:(fun m -> m tenv pn actuals)
 
+let of_records records = List.map ~f:of_record records |> of_list
+
 let of_json top_json =
   let error json =
     L.(die UserError "Could not parse json matcher(s): %s" (Yojson.Basic.to_string json))
@@ -86,10 +88,10 @@ let of_json top_json =
       | _ ->
           error json
     in
-    (match json with `Assoc fields -> parse_fields fields default | _ -> error json) |> of_record
+    match json with `Assoc fields -> parse_fields fields default | _ -> error json
   in
   match top_json with
   | `List matchers_json ->
-      List.map matchers_json ~f:make_matcher_from_json |> of_list
+      List.map matchers_json ~f:make_matcher_from_json |> of_records
   | _ ->
       error top_json

infer/src/concurrency/MethodMatcher.mli

@@ -50,3 +50,6 @@ val of_json : Yojson.Basic.json -> t
 
 val of_list : t list -> t
 (** Or combinator *)
+
+val of_records : record list -> t
+(** shorthand for [of_list (List.map ~f:of_record r)] *)

infer/src/concurrency/StarvationModels.ml

@@ -89,11 +89,58 @@ let empty_or_excessive_timeout actuals =
       false
 
 
+let standard_matchers =
+  let open MethodMatcher in
+  let open StarvationDomain.Event in
+  let high_sev =
+    [ {default with classname= "java.lang.Thread"; methods= ["sleep"]}
+    ; { default with
+        classname= "java.lang.Object"; methods= ["wait"]; actuals_pred= empty_or_excessive_timeout
+      }
+    ; { default with
+        classname= "java.util.concurrent.CountDownLatch"
+      ; methods= ["await"]
+      ; actuals_pred= empty_or_excessive_timeout }
+      (* an IBinder.transact call is an RPC.  If the 4th argument (5th counting `this` as the first)
+         is int-zero then a reply is expected and returned from the remote process, thus potentially
+         blocking.  If the 4th argument is anything else, we assume a one-way call which doesn't block. *)
+    ; { default with
+        classname= "android.os.IBinder"
+      ; methods= ["transact"]
+      ; actuals_pred=
+          (fun actuals ->
+            List.nth actuals 4 |> Option.value_map ~default:false ~f:HilExp.is_int_zero ) }
+    ; { default with
+        classname= "android.accounts.AccountManager"
+      ; methods= ["setUserData"]
+      ; search_superclasses= false } ]
+  in
+  let low_sev =
+    [ { default with
+        classname= "java.util.concurrent.Future"
+      ; methods= ["get"]
+      ; actuals_pred= empty_or_excessive_timeout
+      ; search_superclasses= false }
+    ; { default with
+        classname= "android.os.AsyncTask"
+      ; methods= ["get"]
+      ; actuals_pred= empty_or_excessive_timeout } ]
+  in
+  let high_sev_matcher = List.map high_sev ~f:of_record |> of_list in
+  let low_sev_matcher = List.map low_sev ~f:of_record |> of_list in
+  [(high_sev_matcher, High); (low_sev_matcher, Low)]
+
+
+(* sort from High to Low *)
+let may_block tenv pn actuals =
+  List.find_map standard_matchers ~f:(fun (matcher, sev) ->
+      Option.some_if (matcher tenv pn actuals) sev )
+
+
 (* selection is a bit arbitrary as some would be generated anyway if not here; no harm though *)
 (* some, like [file], need manual addition due to our lack of handling dynamic dispatch *)
 let strict_mode_matcher =
   let open MethodMatcher in
-  let open StarvationDomain.Event in
   (* NB [default] searches superclasses too.  Most of the classes below are final and we don't
      really want to search superclasses for those that aren't, so for performance, disable that *)
   let dont_search_superclasses = {default with search_superclasses= false} in
@@ -137,57 +184,8 @@ let strict_mode_matcher =
           ; "setReadOnly"
           ; "setWritable" ] } ]
   in
-  let matcher = of_list (List.map matcher_records ~f:of_record) in
-  (matcher, High)
+  of_records matcher_records
 
 
-let standard_matchers =
-  let open MethodMatcher in
-  let open StarvationDomain.Event in
-  let high_sev =
-    [ {default with classname= "java.lang.Thread"; methods= ["sleep"]}
-    ; { default with
-        classname= "java.lang.Object"; methods= ["wait"]; actuals_pred= empty_or_excessive_timeout
-      }
-    ; { default with
-        classname= "java.util.concurrent.CountDownLatch"
-      ; methods= ["await"]
-      ; actuals_pred= empty_or_excessive_timeout }
-      (* an IBinder.transact call is an RPC.  If the 4th argument (5th counting `this` as the first)
-         is int-zero then a reply is expected and returned from the remote process, thus potentially
-         blocking.  If the 4th argument is anything else, we assume a one-way call which doesn't block. *)
-    ; { default with
-        classname= "android.os.IBinder"
-      ; methods= ["transact"]
-      ; actuals_pred=
-          (fun actuals ->
-            List.nth actuals 4 |> Option.value_map ~default:false ~f:HilExp.is_int_zero ) }
-    ; { default with
-        classname= "android.accounts.AccountManager"
-      ; methods= ["setUserData"]
-      ; search_superclasses= false } ]
-  in
-  let low_sev =
-    [ { default with
-        classname= "java.util.concurrent.Future"
-      ; methods= ["get"]
-      ; actuals_pred= empty_or_excessive_timeout
-      ; search_superclasses= false }
-    ; { default with
-        classname= "android.os.AsyncTask"
-      ; methods= ["get"]
-      ; actuals_pred= empty_or_excessive_timeout } ]
-  in
-  let high_sev_matcher = List.map high_sev ~f:of_record |> of_list in
-  let low_sev_matcher = List.map low_sev ~f:of_record |> of_list in
-  [(high_sev_matcher, High); (low_sev_matcher, Low)]
-
-
-(* sort from High to Low *)
-let may_block =
-  let matchers =
-    if Config.starvation_strict_mode then strict_mode_matcher :: standard_matchers
-    else standard_matchers
-  in
-  fun tenv pn actuals ->
-    List.find_map matchers ~f:(fun (matcher, sev) -> Option.some_if (matcher tenv pn actuals) sev)
+let is_strict_mode_violation tenv pn actuals =
+  Config.starvation_strict_mode && strict_mode_matcher tenv pn actuals

infer/src/concurrency/StarvationModels.mli

@@ -11,9 +11,11 @@ val may_block :
   Tenv.t -> Typ.Procname.t -> HilExp.t list -> StarvationDomain.Event.severity_t option
 (** is the method call potentially blocking, given the actuals passed? *)
 
+val is_strict_mode_violation : Tenv.t -> Typ.Procname.t -> HilExp.t list -> bool
+
 val is_synchronized_library_call : Tenv.t -> Typ.Procname.t -> bool
 (** does the method call lock-then-unlock the underlying object?
     legacy Java containers like Vector do this, and can interact with explicit locking *)
 
 val should_skip_analysis : Tenv.t -> Typ.Procname.t -> HilExp.t list -> bool
-(** should we go avoid analyzing a library method (eg in guava) to avoid FPs? *)
+(** should we treat a method call as skip (eg library methods in guava) to avoid FPs? *)

infer/src/concurrency/starvation.ml

@@ -9,6 +9,8 @@ module F = Format
 module L = Logging
 module MF = MarkupFormatter
 
+let pname_pp = MF.wrap_monospaced Typ.Procname.pp
+
 let debug fmt = L.(debug Analysis Verbose fmt)
 
 let is_on_ui_thread pn =
@@ -90,23 +92,25 @@ module TransferFunctions (CFG : ProcCfg.S) = struct
     in
     match instr with
     | Call (_, Direct callee, actuals, _, loc) -> (
-      match get_lock callee actuals with
-      | Lock ->
-          do_lock actuals loc astate
-      | Unlock ->
-          do_unlock actuals astate
-      | LockedIfTrue ->
-          astate
-      | NoEffect when should_skip_analysis tenv callee actuals ->
-          astate
-      | NoEffect when is_synchronized_library_call tenv callee ->
-          (* model a synchronized call without visible internal behaviour *)
-          do_lock actuals loc astate |> do_unlock actuals
-      | NoEffect when is_on_ui_thread callee ->
-          let explanation = F.asprintf "it calls %a" (MF.wrap_monospaced Typ.Procname.pp) callee in
-          Domain.set_on_ui_thread astate loc explanation
-      | NoEffect -> (
-          let caller = Procdesc.get_proc_name pdesc in
+        let caller = Procdesc.get_proc_name pdesc in
+        match get_lock callee actuals with
+        | Lock ->
+            do_lock actuals loc astate
+        | Unlock ->
+            do_unlock actuals astate
+        | LockedIfTrue ->
+            astate
+        | NoEffect when should_skip_analysis tenv callee actuals ->
+            astate
+        | NoEffect when is_synchronized_library_call tenv callee ->
+            (* model a synchronized call without visible internal behaviour *)
+            do_lock actuals loc astate |> do_unlock actuals
+        | NoEffect when is_on_ui_thread callee ->
+            let explanation = F.asprintf "it calls %a" pname_pp callee in
+            Domain.set_on_ui_thread astate loc explanation
+        | NoEffect when StarvationModels.is_strict_mode_violation tenv callee actuals ->
+            Domain.strict_mode_call ~caller ~callee loc astate
+        | NoEffect -> (
           match may_block tenv callee actuals with
           | Some sev ->
               Domain.blocking_call ~caller ~callee sev loc astate
@@ -184,77 +188,115 @@ module ReportMap : sig
 
   val empty : t
 
-  val add_deadlock : Tenv.t -> Procdesc.t -> Location.t -> Errlog.loc_trace -> string -> t -> t
+  type report_add_t = Typ.Procname.t -> Location.t -> Errlog.loc_trace -> string -> t -> t
 
-  val add_starvation :
-       Tenv.t
-    -> StarvationDomain.Event.severity_t
-    -> Procdesc.t
-    -> Location.t
-    -> Errlog.loc_trace
-    -> string
-    -> t
-    -> t
+  val add_deadlock : report_add_t
 
-  val log : t -> unit
-end = struct
-  type starvation_t = StarvationDomain.Event.severity_t
+  val add_starvation : StarvationDomain.Event.severity_t -> report_add_t
 
-  type deadlock_t = int
+  val add_strict_mode_volation : report_add_t
+
+  val log : Tenv.t -> Procdesc.t -> t -> unit
+end = struct
+  type problem =
+    | Starvation of StarvationDomain.Event.severity_t
+    | Deadlock of int
+    | StrictModeViolation of int
 
-  type 'weight_t report_t =
-    {weight: 'weight_t; pname: Typ.Procname.t; ltr: Errlog.loc_trace; message: string}
+  type report_t = {problem: problem; pname: Typ.Procname.t; ltr: Errlog.loc_trace; message: string}
 
   module LocMap = PrettyPrintable.MakePPMap (Location)
 
-  type t = (deadlock_t report_t list * starvation_t report_t list) LocMap.t
+  type t = report_t list LocMap.t
+
+  type report_add_t = Typ.Procname.t -> Location.t -> Errlog.loc_trace -> string -> t -> t
 
   let empty : t = LocMap.empty
 
-  let add_deadlock tenv pdesc loc ltr message (map : t) =
-    let pname = Procdesc.get_proc_name pdesc in
-    if Reporting.is_suppressed tenv pdesc IssueType.deadlock ~field_name:None then map
-    else
-      let rep = {weight= -List.length ltr; pname; ltr; message} in
-      let deadlocks, starvations = try LocMap.find loc map with Caml.Not_found -> ([], []) in
-      let new_reports = (rep :: deadlocks, starvations) in
-      LocMap.add loc new_reports map
+  let add loc report map =
+    let reports = try LocMap.find loc map with Caml.Not_found -> [] in
+    let new_reports = report :: reports in
+    LocMap.add loc new_reports map
 
 
-  let add_starvation tenv sev pdesc loc ltr message map =
-    let pname = Procdesc.get_proc_name pdesc in
-    if Reporting.is_suppressed tenv pdesc IssueType.starvation ~field_name:None then map
-    else
-      let rep = {weight= sev; pname; ltr; message} in
-      let deadlocks, starvations = try LocMap.find loc map with Caml.Not_found -> ([], []) in
-      let new_reports = (deadlocks, rep :: starvations) in
-      LocMap.add loc new_reports map
+  let add_deadlock pname loc ltr message (map : t) =
+    let report = {problem= Deadlock (-List.length ltr); pname; ltr; message} in
+    add loc report map
+
 
+  let add_starvation sev pname loc ltr message map =
+    let report = {pname; problem= Starvation sev; ltr; message} in
+    add loc report map
 
-  let log map =
-    let log_report issuetype loc {pname; ltr; message} =
-      Reporting.log_issue_external pname Exceptions.Error ~loc ~ltr issuetype message
+
+  let add_strict_mode_volation pname loc ltr message (map : t) =
+    let report = {problem= StrictModeViolation (-List.length ltr); pname; ltr; message} in
+    add loc report map
+
+
+  let log tenv pdesc map =
+    let log_report loc {problem; pname; ltr; message} =
+      let issue_type =
+        match problem with
+        | Deadlock _ ->
+            IssueType.deadlock
+        | Starvation _ ->
+            IssueType.starvation
+        | StrictModeViolation _ ->
+            IssueType.strict_mode_violation
+      in
+      Reporting.log_issue_external pname Exceptions.Error ~loc ~ltr issue_type message
     in
-    let mk_deduped_report num_of_reports ({message} as report) =
+    let mk_deduped_report reports ({message} as report) =
       { report with
         message=
           Printf.sprintf "%s %d more report(s) on the same line suppressed." message
-            (num_of_reports - 1) }
+            (List.length reports - 1) }
     in
-    let log_loc_reports issuetype compare loc = function
+    let log_reports compare loc = function
       | [] ->
           ()
-      | [report] ->
-          log_report issuetype loc report
+      | [(_, report, _)] ->
+          log_report loc report
       | reports ->
-          List.max_elt ~compare:(fun {weight} {weight= weight'} -> compare weight weight') reports
-          |> Option.iter ~f:(fun rep ->
-                 mk_deduped_report (List.length reports) rep |> log_report issuetype loc )
+          List.max_elt ~compare reports
+          |> Option.iter ~f:(fun (_, rep, _) -> mk_deduped_report reports rep |> log_report loc)
     in
-    let log_location loc (deadlocks, starvations) =
-      log_loc_reports IssueType.deadlock Int.compare loc deadlocks ;
-      log_loc_reports IssueType.starvation StarvationDomain.Event.compare_severity_t loc
-        starvations
+    let filter_map_deadlock report =
+      if Reporting.is_suppressed tenv pdesc IssueType.deadlock then None
+      else
+        match report with
+        | {problem= Deadlock len} ->
+            Some (len, report, IssueType.deadlock)
+        | _ ->
+            None
+    in
+    let filter_map_starvation report =
+      if Reporting.is_suppressed tenv pdesc IssueType.starvation then None
+      else
+        match report with
+        | {problem= Starvation sev} ->
+            Some (sev, report, IssueType.starvation)
+        | _ ->
+            None
+    in
+    let filter_map_strict_mode_violation report =
+      if Reporting.is_suppressed tenv pdesc IssueType.deadlock then None
+      else
+        match report with
+        | {problem= StrictModeViolation len} ->
+            Some (len, report, IssueType.strict_mode_violation)
+        | _ ->
+            None
+    in
+    let compare_reports weight_compare (w, _, _) (w', _, _) = weight_compare w w' in
+    let log_location loc problems =
+      let deadlocks = List.filter_map problems ~f:filter_map_deadlock in
+      log_reports (compare_reports Int.compare) loc deadlocks ;
+      let starvations = List.filter_map problems ~f:filter_map_starvation in
+      log_reports (compare_reports StarvationDomain.Event.compare_severity_t) loc starvations ;
+      let strict_mode_violations = List.filter_map problems ~f:filter_map_strict_mode_violation in
+      log_reports (compare_reports Int.compare) loc strict_mode_violations
     in
     LocMap.iter log_location map
 end
@@ -262,7 +304,7 @@ end
 let should_report_deadlock_on_current_proc current_elem endpoint_elem =
   let open StarvationDomain in
   match (current_elem.Order.elem.first, current_elem.Order.elem.eventually.elem) with
-  | _, MayBlock _ ->
+  | _, (MayBlock _ | StrictModeCall _) ->
       (* should never happen *)
       L.die InternalError "Deadlock cannot occur without two lock events: %a" Order.pp current_elem
   | ((Var.LogicalVar _, _), []), _ ->
@@ -322,12 +364,10 @@ let fold_reportable_summaries (tenv, current_pdesc) clazz ~init ~f =
          once, as opposed to twice with all other deadlock pairs. *)
 let report_deadlocks env {StarvationDomain.order; ui} report_map' =
   let open StarvationDomain in
-  let tenv, current_pdesc = env in
+  let _, current_pdesc = env in
   let current_pname = Procdesc.get_proc_name current_pdesc in
   let pp_acquire fmt (lock, loc, pname) =
-    F.fprintf fmt "%a (%a in %a)" Lock.pp lock Location.pp loc
-      (MF.wrap_monospaced Typ.Procname.pp)
-      pname
+    F.fprintf fmt "%a (%a in %a)" Lock.pp lock Location.pp loc pname_pp pname
   in
   let pp_thread fmt
       ( pname
@@ -356,22 +396,20 @@ let report_deadlocks env {StarvationDomain.order; ui} report_map' =
           let error_message =
             Format.asprintf
               "Potential deadlock.@.Trace 1 (starts at %a) %a.@.Trace 2 (starts at %a), %a."
-              (MF.wrap_monospaced Typ.Procname.pp)
-              current_pname pp_thread (current_pname, current_elem)
-              (MF.wrap_monospaced Typ.Procname.pp)
+              pname_pp current_pname pp_thread (current_pname, current_elem) pname_pp
               endpoint_pname pp_thread (endpoint_pname, elem)
           in
           let first_trace = Order.make_trace ~header:"[Trace 1] " current_pname current_elem in
           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 tenv current_pdesc loc ltr error_message report_map
+          ReportMap.add_deadlock current_pname loc ltr error_message report_map
       | _, _ ->
           report_map
   in
   let report_on_current_elem elem report_map =
     match elem.Order.elem.eventually.elem with
-    | MayBlock _ ->
+    | MayBlock _ | StrictModeCall _ ->
         report_map
     | LockAcquire endpoint_lock ->
         Lock.owner_class endpoint_lock
@@ -380,11 +418,9 @@ let report_deadlocks env {StarvationDomain.order; ui} report_map' =
                    and retrieve all the summaries of the methods of that class *)
                (* for each summary related to the endpoint, analyse and report on its pairs *)
                fold_reportable_summaries env endpoint_class ~init:report_map
-                 ~f:(fun acc (endp_pname, endpoint_summary) ->
-                   let endp_order = endpoint_summary.order in
-                   let endp_ui = endpoint_summary.ui in
+                 ~f:(fun acc (endpoint_pname, {order= endp_order; ui= endp_ui}) ->
                    if UIThreadDomain.is_empty ui || UIThreadDomain.is_empty endp_ui then
-                     OrderDomain.fold (report_endpoint_elem elem endp_pname) endp_order acc
+                     OrderDomain.fold (report_endpoint_elem elem endpoint_pname) endp_order acc
                    else acc ) )
   in
   OrderDomain.fold report_on_current_elem order report_map'
@@ -392,7 +428,7 @@ let report_deadlocks env {StarvationDomain.order; ui} report_map' =
 
 let report_starvation env {StarvationDomain.events; ui} report_map' =
   let open StarvationDomain in
-  let tenv, current_pdesc = env in
+  let _, current_pdesc = env in
   let current_pname = Procdesc.get_proc_name current_pdesc in
   let report_remote_block ui_explain event current_lock endpoint_pname endpoint_elem report_map =
     let lock = endpoint_elem.Order.elem.first in
@@ -402,8 +438,7 @@ let report_starvation env {StarvationDomain.events; ui} report_map' =
           Format.asprintf
             "Method %a runs on UI thread (because %a) and %a, which may be held by another thread \
              which %s."
-            (MF.wrap_monospaced Typ.Procname.pp)
-            current_pname UIThreadExplanationDomain.pp ui_explain Lock.pp lock block_descr
+            pname_pp current_pname UIThreadExplanationDomain.pp ui_explain Lock.pp lock block_descr
         in
         let first_trace = Event.make_trace ~header:"[Trace 1] " current_pname event in
         let second_trace = Order.make_trace ~header:"[Trace 2] " endpoint_pname endpoint_elem in
@@ -413,7 +448,7 @@ let report_starvation env {StarvationDomain.events; ui} report_map' =
         in
         let ltr = first_trace @ second_trace @ ui_trace in
         let loc = Event.get_loc event in
-        ReportMap.add_starvation tenv sev current_pdesc loc ltr error_message report_map
+        ReportMap.add_starvation sev current_pname loc ltr error_message report_map
     | _ ->
         report_map
   in
@@ -421,8 +456,21 @@ let report_starvation env {StarvationDomain.events; ui} report_map' =
     match event.Event.elem with
     | MayBlock (_, sev) ->
         let error_message =
-          Format.asprintf "Method %a runs on UI thread (because %a), and may block; %a."
-            (MF.wrap_monospaced Typ.Procname.pp)
+          Format.asprintf "Method %a runs on UI thread (because %a), and may block; %a." pname_pp
+            current_pname UIThreadExplanationDomain.pp ui_explain Event.pp event
+        in
+        let loc = Event.get_loc event in
+        let trace = Event.make_trace current_pname event in
+        let ui_trace =
+          UIThreadExplanationDomain.make_trace ~header:"[Trace on UI thread] " current_pname
+            ui_explain
+        in
+        let ltr = trace @ ui_trace in
+        ReportMap.add_starvation sev current_pname loc ltr error_message report_map
+    | StrictModeCall _ ->
+        let error_message =
+          Format.asprintf
+            "Method %a runs on UI thread (because %a), and may violate Strict Mode; %a." pname_pp
             current_pname UIThreadExplanationDomain.pp ui_explain Event.pp event
         in
         let loc = Event.get_loc event in
@@ -432,7 +480,7 @@ let report_starvation env {StarvationDomain.events; ui} report_map' =
             ui_explain
         in
         let ltr = trace @ ui_trace in
-        ReportMap.add_starvation tenv sev current_pdesc loc ltr error_message report_map
+        ReportMap.add_strict_mode_volation current_pname loc ltr error_message report_map
     | LockAcquire endpoint_lock ->
         Lock.owner_class endpoint_lock
         |> Option.value_map ~default:report_map ~f:(fun endpoint_class ->
@@ -456,13 +504,13 @@ let report_starvation env {StarvationDomain.events; ui} report_map' =
 
 
 let reporting {Callbacks.procedures; source_file} =
-  let report_procedure ((_, proc_desc) as env) =
+  let report_procedure ((tenv, proc_desc) as env) =
     die_if_not_java proc_desc ;
     if should_report proc_desc then
       Payload.read proc_desc (Procdesc.get_proc_name proc_desc)
       |> Option.iter ~f:(fun summary ->
              report_deadlocks env summary ReportMap.empty
-             |> report_starvation env summary |> ReportMap.log )
+             |> report_starvation env summary |> ReportMap.log tenv proc_desc )
   in
   List.iter procedures ~f:report_procedure ;
   IssueLog.store Config.starvation_issues_dir_name source_file

infer/src/concurrency/starvationDomain.ml

@@ -8,6 +8,8 @@ open! IStd
 module F = Format
 module MF = MarkupFormatter
 
+let pname_pp = MF.wrap_monospaced Typ.Procname.pp
+
 module Lock = struct
   type t = AccessPath.t
 
@@ -52,7 +54,11 @@ end
 module Event = struct
   type severity_t = Low | Medium | High [@@deriving compare]
 
-  type event_t = LockAcquire of Lock.t | MayBlock of (string * severity_t) [@@deriving compare]
+  type event_t =
+    | LockAcquire of Lock.t
+    | MayBlock of (string * severity_t)
+    | StrictModeCall of string
+  [@@deriving compare]
 
   include ExplicitTrace.MakeTraceElem (struct
     type t = event_t [@@deriving compare]
@@ -62,21 +68,22 @@ module Event = struct
           Lock.pp fmt lock
       | MayBlock (msg, _) ->
           F.pp_print_string fmt msg
+      | StrictModeCall msg ->
+          F.pp_print_string fmt msg
   end)
 
   let make_acquire lock loc = make (LockAcquire lock) loc
 
   let make_blocking_call ~caller ~callee sev loc =
-    let descr =
-      F.asprintf "calls %a from %a"
-        (MF.wrap_monospaced Typ.Procname.pp)
-        callee
-        (MF.wrap_monospaced Typ.Procname.pp)
-        caller
-    in
+    let descr = F.asprintf "calls %a from %a" pname_pp callee pname_pp caller in
     make (MayBlock (descr, sev)) loc
 
 
+  let make_strict_mode_call ~caller ~callee loc =
+    let descr = F.asprintf "calls %a from %a" pname_pp callee pname_pp caller in
+    make (StrictModeCall descr) loc
+
+
   let make_trace ?(header = "") pname elem =
     let trace = make_loc_trace elem in
     let trace_descr = Format.asprintf "%s%a" header (MF.wrap_monospaced Typ.Procname.pp) pname in
@@ -260,6 +267,12 @@ let blocking_call ~caller ~callee sev loc ({lock_state; events; order} as astate
     events= EventDomain.add new_event events; order= add_order_pairs lock_state new_event order }
 
 
+let strict_mode_call ~caller ~callee loc ({lock_state; events; order} as astate) =
+  let new_event = Event.make_strict_mode_call ~caller ~callee loc in
+  { astate with
+    events= EventDomain.add new_event events; order= add_order_pairs lock_state new_event order }
+
+
 let release ({lock_state} as astate) lock =
   {astate with lock_state= LockState.release lock lock_state}
 

infer/src/concurrency/starvationDomain.mli

@@ -25,7 +25,11 @@ end
 module Event : sig
   type severity_t = Low | Medium | High [@@deriving compare]
 
-  type event_t = LockAcquire of Lock.t | MayBlock of (string * severity_t) [@@deriving compare]
+  type event_t =
+    | LockAcquire of Lock.t
+    | MayBlock of (string * severity_t)
+    | StrictModeCall of string
+  [@@deriving compare]
 
   include ExplicitTrace.TraceElem with type elem_t = event_t
 
@@ -85,6 +89,9 @@ val blocking_call :
   -> astate
   -> astate
 
+val strict_mode_call :
+  caller:Typ.Procname.t -> callee:Typ.Procname.t -> Location.t -> astate -> astate
+
 val set_on_ui_thread : astate -> Location.t -> string -> astate
 (** set the property "runs on UI thread" to true by attaching the given explanation string as to
     why this method is thought to do so *)

infer/tests/codetoanalyze/java/starvation/StrictModeViolation.java

@@ -0,0 +1,41 @@
+/*
+ * 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 android.support.annotation.UiThread;
+import java.io.File;
+import java.io.IOException;
+
+class StrictModeViolation {
+  File f;
+
+  @UiThread
+  void violateStrictModeBad() throws IOException {
+    f.canRead();
+    f.canWrite();
+    f.createNewFile();
+    f.createTempFile("a", "b");
+    f.delete();
+    f.getCanonicalPath();
+    f.getFreeSpace();
+    f.getTotalSpace();
+    f.getUsableSpace();
+    f.isDirectory();
+    f.isFile();
+    f.isHidden();
+    f.lastModified();
+    f.length();
+    f.list();
+    f.listFiles();
+    f.mkdir();
+    f.renameTo(f);
+    f.setExecutable(true);
+    f.setLastModified(1L);
+    f.setReadable(true);
+    f.setReadOnly();
+    f.setWritable(true);
+  }
+}

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

@@ -45,6 +45,29 @@ codetoanalyze/java/starvation/PubPriv.java, PubPriv.callOneWayBad():void, 49, DE
 codetoanalyze/java/starvation/PubPriv.java, PubPriv.transactBad():void, 21, STARVATION, no_bucket, ERROR, [`void PubPriv.transactBad()`,Method call: `void PubPriv.doTransactOk()`,calls `boolean Binder.transact(int,Parcel,Parcel,int)` from `void PubPriv.doTransactOk()`,[Trace on UI thread] `void PubPriv.transactBad()`,Method call: `void PubPriv.doTransactOk()`,`void PubPriv.doTransactOk()` is annotated `UiThread`]
 codetoanalyze/java/starvation/ServiceOnUIThread.java, ServiceOnUIThread.onBind(android.content.Intent):android.os.IBinder, 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()`,[Trace on UI thread] `IBinder ServiceOnUIThread.onBind(Intent)`,`IBinder ServiceOnUIThread.onBind(Intent)` is a standard UI-thread method]
 codetoanalyze/java/starvation/StaticLock.java, StaticLock.lockOtherClassOneWayBad():void, 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/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 17, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.canRead()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 18, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.canWrite()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 19, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.createNewFile()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 20, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `File File.createTempFile(String,String)` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 21, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.delete()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 22, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `String File.getCanonicalPath()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 23, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `long File.getFreeSpace()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 24, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `long File.getTotalSpace()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 25, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `long File.getUsableSpace()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 26, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.isDirectory()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 27, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.isFile()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 28, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.isHidden()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 29, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `long File.lastModified()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 30, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `long File.length()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 31, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `java.lang.String[] File.list()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 32, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `java.io.File[] File.listFiles()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 33, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.mkdir()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 34, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.renameTo(File)` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 35, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.setExecutable(boolean)` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 36, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.setLastModified(long)` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 37, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.setReadable(boolean)` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 38, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.setReadOnly()` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
+codetoanalyze/java/starvation/StrictModeViolation.java, StrictModeViolation.violateStrictModeBad():void, 39, STRICT_MODE_VIOLATION, no_bucket, ERROR, [`void StrictModeViolation.violateStrictModeBad()`,calls `boolean File.setWritable(boolean)` from `void StrictModeViolation.violateStrictModeBad()`,[Trace on UI thread] `void StrictModeViolation.violateStrictModeBad()`,`void StrictModeViolation.violateStrictModeBad()` is annotated `UiThread`]
 codetoanalyze/java/starvation/SuppLint.java, SuppLint.onUiThreadBad():void, 25, STARVATION, no_bucket, ERROR, [`void SuppLint.onUiThreadBad()`,calls `Object Future.get()` from `void SuppLint.onUiThreadBad()`,[Trace on UI thread] `void SuppLint.onUiThreadBad()`,`void SuppLint.onUiThreadBad()` is annotated `UiThread`]
 codetoanalyze/java/starvation/ThreadSleep.java, ThreadSleep.indirectSleepOnUIThreadBad():void, 24, STARVATION, no_bucket, ERROR, [[Trace 1] `void ThreadSleep.indirectSleepOnUIThreadBad()`,locks `this.ThreadSleep.lock` in class `ThreadSleep*`,[Trace 2] `void ThreadSleep.lockAndSleepOnNonUIThread()`,locks `this.ThreadSleep.lock` in class `ThreadSleep*`,Method call: `void ThreadSleep.sleepOnAnyThreadOk()`,calls `void Thread.sleep(long)` from `void ThreadSleep.sleepOnAnyThreadOk()`,[Trace 1 on UI thread] `void ThreadSleep.indirectSleepOnUIThreadBad()`,`void ThreadSleep.indirectSleepOnUIThreadBad()` is annotated `UiThread`]
 codetoanalyze/java/starvation/ThreadSleep.java, ThreadSleep.sleepOnUIThreadBad():void, 17, STARVATION, no_bucket, ERROR, [`void ThreadSleep.sleepOnUIThreadBad()`,calls `void Thread.sleep(long)` from `void ThreadSleep.sleepOnUIThreadBad()`,[Trace on UI thread] `void ThreadSleep.sleepOnUIThreadBad()`,`void ThreadSleep.sleepOnUIThreadBad()` is annotated `UiThread`]