diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index 18b1423bb..df1d26c09 100644 --- a/infer/src/base/IssueType.ml +++ b/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" diff --git a/infer/src/base/IssueType.mli b/infer/src/base/IssueType.mli index 57c7081a9..035c616a7 100644 --- a/infer/src/base/IssueType.mli +++ b/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 diff --git a/infer/src/concurrency/MethodMatcher.ml b/infer/src/concurrency/MethodMatcher.ml index c1baa0716..6bbfb3994 100644 --- a/infer/src/concurrency/MethodMatcher.ml +++ b/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 diff --git a/infer/src/concurrency/MethodMatcher.mli b/infer/src/concurrency/MethodMatcher.mli index 105b0907f..6dfd859f2 100644 --- a/infer/src/concurrency/MethodMatcher.mli +++ b/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)] *) diff --git a/infer/src/concurrency/StarvationModels.ml b/infer/src/concurrency/StarvationModels.ml index faeeafb33..5f838e8f7 100644 --- a/infer/src/concurrency/StarvationModels.ml +++ b/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 diff --git a/infer/src/concurrency/StarvationModels.mli b/infer/src/concurrency/StarvationModels.mli index a6782e667..7539022d2 100644 --- a/infer/src/concurrency/StarvationModels.mli +++ b/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? *) diff --git a/infer/src/concurrency/starvation.ml b/infer/src/concurrency/starvation.ml index f5ab5913f..e41dcce29 100644 --- a/infer/src/concurrency/starvation.ml +++ b/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 diff --git a/infer/src/concurrency/starvationDomain.ml b/infer/src/concurrency/starvationDomain.ml index dffd4a090..ccc477184 100644 --- a/infer/src/concurrency/starvationDomain.ml +++ b/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} diff --git a/infer/src/concurrency/starvationDomain.mli b/infer/src/concurrency/starvationDomain.mli index 83976896e..75a5b03e9 100644 --- a/infer/src/concurrency/starvationDomain.mli +++ b/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 *) diff --git a/infer/tests/codetoanalyze/java/starvation/StrictModeViolation.java b/infer/tests/codetoanalyze/java/starvation/StrictModeViolation.java new file mode 100644 index 000000000..aa8098519 --- /dev/null +++ b/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); + } +} diff --git a/infer/tests/codetoanalyze/java/starvation/issues.exp b/infer/tests/codetoanalyze/java/starvation/issues.exp index 4dd3e8c94..5b2ec3335 100644 --- a/infer/tests/codetoanalyze/java/starvation/issues.exp +++ b/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`]