diff --git a/infer/src/IR/JavaClassName.ml b/infer/src/IR/JavaClassName.ml index 088c7100e..116b4ca71 100644 --- a/infer/src/IR/JavaClassName.ml +++ b/infer/src/IR/JavaClassName.ml @@ -10,7 +10,7 @@ module F = Format module L = Logging (** invariant: if [package = Some str] then [not (String.equal str "")] *) -type t = {classname: string; package: string option} [@@deriving compare] +type t = {classname: string; package: string option} [@@deriving compare, equal] let from_string str = match String.rsplit2 str ~on:'.' with @@ -47,3 +47,6 @@ let is_external_via_config t = let pp_with_verbosity ~verbose fmt t = if verbose then pp fmt t else F.pp_print_string fmt (classname t) + + +let java_lang_integer = {classname= "Integer"; package= Some "java.lang"} diff --git a/infer/src/IR/JavaClassName.mli b/infer/src/IR/JavaClassName.mli index a096c948a..33bc55d33 100644 --- a/infer/src/IR/JavaClassName.mli +++ b/infer/src/IR/JavaClassName.mli @@ -7,7 +7,7 @@ open! IStd -type t [@@deriving compare] +type t [@@deriving compare, equal] val from_string : string -> t @@ -24,3 +24,5 @@ val classname : t -> string val is_external_via_config : t -> bool (** Considered external based on config flags. *) + +val java_lang_integer : t diff --git a/infer/src/IR/Procname.ml b/infer/src/IR/Procname.ml index 2f5cec21b..e450103c6 100644 --- a/infer/src/IR/Procname.ml +++ b/infer/src/IR/Procname.ml @@ -828,3 +828,13 @@ end module SQLiteList = SqliteUtils.MarshalledDataNOTForComparison (struct type nonrec t = t list end) + +module UnitCache = struct + let create () = + let cache = ref None in + let cache_get pname = + Option.bind !cache ~f:(fun (pname', value) -> Option.some_if (equal pname pname') value) + in + let cache_set pname value = cache := Some (pname, value) in + (cache_get, cache_set) +end diff --git a/infer/src/IR/Procname.mli b/infer/src/IR/Procname.mli index ec933fc7f..fdbfa7c2c 100644 --- a/infer/src/IR/Procname.mli +++ b/infer/src/IR/Procname.mli @@ -243,6 +243,11 @@ end module SQLiteList : SqliteUtils.Data with type t = t list +(** One-sized cache for one procedure at a time. Returns getter and setter. *) +module UnitCache : sig + val create : unit -> (t -> 'a option) * (t -> 'a -> unit) +end + val empty_block : t (** Empty block name. *) diff --git a/infer/src/bufferoverrun/bufferOverrunAnalysis.ml b/infer/src/bufferoverrun/bufferOverrunAnalysis.ml index 18ed86a61..7e098d19f 100644 --- a/infer/src/bufferoverrun/bufferOverrunAnalysis.ml +++ b/infer/src/bufferoverrun/bufferOverrunAnalysis.ml @@ -456,12 +456,7 @@ let compute_invariant_map : let cached_compute_invariant_map = - let cache = ref None in - let cache_get pname = - Option.bind !cache ~f:(fun (pname', inv_map) -> - Option.some_if (Procname.equal pname pname') inv_map ) - in - let cache_set pname inv_map = cache := Some (pname, inv_map) in + let cache_get, cache_set = Procname.UnitCache.create () in fun summary tenv integer_type_widths -> let pname = Summary.get_proc_name summary in match cache_get pname with diff --git a/infer/src/bufferoverrun/bufferOverrunUtils.ml b/infer/src/bufferoverrun/bufferOverrunUtils.ml index a85748132..14b908d75 100644 --- a/infer/src/bufferoverrun/bufferOverrunUtils.ml +++ b/infer/src/bufferoverrun/bufferOverrunUtils.ml @@ -22,10 +22,11 @@ module ModelEnv = struct ; node_hash: int ; location: Location.t ; tenv: Tenv.t - ; integer_type_widths: Typ.IntegerWidths.t } + ; integer_type_widths: Typ.IntegerWidths.t + ; get_cast_type: CastType.get_cast_type option } - let mk_model_env pname ~node_hash location tenv integer_type_widths = - {pname; node_hash; location; tenv; integer_type_widths} + let mk_model_env pname ~node_hash ?get_cast_type location tenv integer_type_widths = + {pname; node_hash; location; tenv; integer_type_widths; get_cast_type} end module Exec = struct diff --git a/infer/src/bufferoverrun/bufferOverrunUtils.mli b/infer/src/bufferoverrun/bufferOverrunUtils.mli index b90c36c88..474921da7 100644 --- a/infer/src/bufferoverrun/bufferOverrunUtils.mli +++ b/infer/src/bufferoverrun/bufferOverrunUtils.mli @@ -16,10 +16,17 @@ module ModelEnv : sig ; node_hash: int ; location: Location.t ; tenv: Tenv.t - ; integer_type_widths: Typ.IntegerWidths.t } + ; integer_type_widths: Typ.IntegerWidths.t + ; get_cast_type: CastType.get_cast_type option } val mk_model_env : - Procname.t -> node_hash:int -> Location.t -> Tenv.t -> Typ.IntegerWidths.t -> model_env + Procname.t + -> node_hash:int + -> ?get_cast_type:CastType.get_cast_type + -> Location.t + -> Tenv.t + -> Typ.IntegerWidths.t + -> model_env end module Exec : sig diff --git a/infer/src/bufferoverrun/castType.ml b/infer/src/bufferoverrun/castType.ml new file mode 100644 index 000000000..66b0a040f --- /dev/null +++ b/infer/src/bufferoverrun/castType.ml @@ -0,0 +1,103 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +open! IStd +module CFG = ProcCfg.Backward (ProcCfg.NormalOneInstrPerNode) +module F = Format + +module Loc = struct + type t = Ident of Ident.t | Pvar of Pvar.t [@@deriving compare] + + let pp f = function Ident id -> Ident.pp f id | Pvar pvar -> Pvar.pp Pp.text f pvar +end + +module Val = struct + include AbstractDomain.FiniteSet (struct + include Typ + + let pp = pp Pp.text + end) + + let is_integer_type = + let is_integer_type = function + | Typ.{desc= Tstruct (JavaClass name)} -> + JavaClassName.(equal name java_lang_integer) + | _ -> + false + in + fun x -> (not (is_empty x)) && for_all is_integer_type x +end + +module Dom = struct + include AbstractDomain.Map (Loc) (Val) + + let lookup l m = Option.value (find_opt l m) ~default:Val.bottom + + let lookup_ident id m = lookup (Loc.Ident id) m + + let lookup_pvar pvar m = lookup (Loc.Pvar pvar) m + + let cast id typ m = + let f = function None -> Some (Val.singleton typ) | Some prev -> Some (Val.add typ prev) in + update (Loc.Ident id) f m + + + let load id pvar m = add (Loc.Pvar pvar) (lookup_ident id m) m + + let store pvar id m = add (Loc.Ident id) (lookup_pvar pvar m) m +end + +module TransferFunctions = struct + module CFG = CFG + module Domain = Dom + + type extras = unit + + let exec_instr mem _pdata _node = function + | Sil.Load {id; e= Exp.Lvar pvar} -> + Dom.load id pvar mem + | Sil.Store {e1= Exp.Lvar pvar; e2= Exp.Var id} -> + Dom.store pvar id mem + | Sil.Call (_, Const (Cfun callee_pname), (Exp.Var id, _) :: (Exp.Sizeof {typ}, _) :: _, _, _) + when Procname.equal callee_pname BuiltinDecl.__cast -> + Dom.cast id typ mem + | _ -> + mem + + + let pp_session_name node f = F.fprintf f "Cast type analysis %a" CFG.Node.pp_id (CFG.Node.id node) +end + +module Analyzer = AbstractInterpreter.MakeWTO (TransferFunctions) + +let compute_invariant_map summary tenv = + let pdata = ProcData.make summary tenv () in + Analyzer.exec_pdesc ~do_narrowing:false ~initial:Dom.bottom pdata + + +type get_cast_type = Ident.t -> Val.t + +let compute_get_cast_type = + let cache_get, cache_set = Procname.UnitCache.create () in + fun {Callbacks.exe_env; summary} -> + let pname = Summary.get_proc_name summary in + let casted_types = + match cache_get pname with + | Some casted_types -> + casted_types + | None -> + let pdesc = Summary.get_proc_desc summary in + let cfg = CFG.from_pdesc pdesc in + let tenv = Exe_env.get_tenv exe_env pname in + let inv_map = compute_invariant_map summary tenv in + let exit_node_id = CFG.exit_node cfg |> CFG.Node.id in + let casted_types = + Analyzer.extract_post exit_node_id inv_map |> Option.value ~default:Dom.bottom + in + cache_set pname casted_types ; casted_types + in + fun id -> Dom.lookup_ident id casted_types diff --git a/infer/src/bufferoverrun/castType.mli b/infer/src/bufferoverrun/castType.mli new file mode 100644 index 000000000..20bfd6c68 --- /dev/null +++ b/infer/src/bufferoverrun/castType.mli @@ -0,0 +1,18 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) + +open! IStd + +module Val : sig + type t + + val is_integer_type : t -> bool +end + +type get_cast_type = Ident.t -> Val.t + +val compute_get_cast_type : Callbacks.proc_callback_args -> Ident.t -> Val.t diff --git a/infer/src/cost/cost.ml b/infer/src/cost/cost.ml index 4f98d5d2c..10e19c107 100644 --- a/infer/src/cost/cost.ml +++ b/infer/src/cost/cost.ml @@ -29,7 +29,8 @@ type extras_WorstCaseCost = { inferbo_invariant_map: BufferOverrunAnalysis.invariant_map ; integer_type_widths: Typ.IntegerWidths.t ; get_node_nb_exec: Node.id -> BasicCost.t - ; get_callee_summary_and_formals: Procname.t -> callee_summary_and_formals option } + ; get_callee_summary_and_formals: Procname.t -> callee_summary_and_formals option + ; get_cast_type: CastType.get_cast_type } let instantiate_cost integer_type_widths ~inferbo_caller_mem ~callee_pname ~callee_formals ~params ~callee_cost ~loc = @@ -58,7 +59,7 @@ module InstrBasicCost = struct List.exists allocation_functions ~f:(fun f -> Procname.equal callee_pname f) - let get_instr_cost_record tenv extras instr_node instr = + let get_instr_cost_record tenv ({get_cast_type} as extras) instr_node instr = match instr with | Sil.Call (ret, Exp.Const (Const.Cfun callee_pname), params, _, _) -> let {inferbo_invariant_map; integer_type_widths; get_callee_summary_and_formals} = extras in @@ -78,8 +79,8 @@ module InstrBasicCost = struct | Some model -> let node_hash = InstrCFG.Node.hash instr_node in let model_env = - BufferOverrunUtils.ModelEnv.mk_model_env callee_pname ~node_hash loc tenv - integer_type_widths + BufferOverrunUtils.ModelEnv.mk_model_env callee_pname ~node_hash ~get_cast_type + loc tenv integer_type_widths in CostDomain.of_operation_cost (model model_env ~ret inferbo_mem) | None -> ( @@ -314,9 +315,13 @@ let compute_get_node_nb_exec node_cfg bound_map : get_node_nb_exec = let compute_worst_case_cost tenv integer_type_widths get_callee_summary_and_formals instr_cfg_wto - inferbo_invariant_map get_node_nb_exec = + inferbo_invariant_map get_node_nb_exec get_cast_type = let extras = - {inferbo_invariant_map; integer_type_widths; get_node_nb_exec; get_callee_summary_and_formals} + { inferbo_invariant_map + ; integer_type_widths + ; get_node_nb_exec + ; get_callee_summary_and_formals + ; get_cast_type } in WorstCaseCost.compute tenv extras instr_cfg_wto @@ -329,7 +334,7 @@ let report_errors ~is_on_ui_thread proc_desc astate summary = Check.check_and_report ~is_on_ui_thread astate proc_desc summary -let checker {Callbacks.exe_env; summary} : Summary.t = +let checker ({Callbacks.exe_env; summary} as callback_args) : Summary.t = let proc_name = Summary.get_proc_name summary in let tenv = Exe_env.get_tenv exe_env proc_name in let integer_type_widths = Exe_env.get_integer_type_widths exe_env proc_name in @@ -362,6 +367,7 @@ let checker {Callbacks.exe_env; summary} : Summary.t = in let is_on_ui_thread = ConcurrencyModels.runs_on_ui_thread ~attrs_of_pname tenv proc_name in let get_node_nb_exec = compute_get_node_nb_exec node_cfg bound_map in + let get_cast_type = CastType.compute_get_cast_type callback_args in let astate = let get_callee_summary_and_formals callee_pname = Payload.read_full ~caller_summary:summary ~callee_pname @@ -371,7 +377,7 @@ let checker {Callbacks.exe_env; summary} : Summary.t = let instr_cfg = InstrCFG.from_pdesc proc_desc in let instr_cfg_wto = InstrCFG.wto instr_cfg in compute_worst_case_cost tenv integer_type_widths get_callee_summary_and_formals instr_cfg_wto - inferbo_invariant_map get_node_nb_exec + inferbo_invariant_map get_node_nb_exec get_cast_type in let () = let exit_cost_record = astate.WorstCaseCost.costs in diff --git a/infer/src/cost/costModels.ml b/infer/src/cost/costModels.ml index 56b39c285..add8f1adc 100644 --- a/infer/src/cost/costModels.ml +++ b/infer/src/cost/costModels.ml @@ -23,11 +23,18 @@ let linear = cost_of_exp ~degree_kind:Polynomials.DegreeKind.Linear let log = cost_of_exp ~degree_kind:Polynomials.DegreeKind.Log -let expensive_modeled ~of_function {pname; location} ~ret:(_, ret_typ) _ : BasicCost.t = - let callsite = CallSite.make pname location in - let path = Symb.SymbolPath.of_callsite ~ret_typ callsite in - let itv = Itv.of_modeled_path ~is_expensive:true path in - CostUtils.of_itv ~itv ~degree_kind:Polynomials.DegreeKind.Linear ~of_function location +let provider_get {pname; location; get_cast_type} ~ret:(ret_id, ret_typ) _ : BasicCost.t = + let is_integer_type = + Option.exists get_cast_type ~f:(fun get_cast_type -> + CastType.Val.is_integer_type (get_cast_type ret_id) ) + in + if is_integer_type then BasicCost.one + else + let callsite = CallSite.make pname location in + let path = Symb.SymbolPath.of_callsite ~ret_typ callsite in + let itv = Itv.of_modeled_path ~is_expensive:true path in + CostUtils.of_itv ~itv ~degree_kind:Polynomials.DegreeKind.Linear ~of_function:"Provider.get" + location module BoundsOf (Container : CostUtils.S) = struct @@ -185,9 +192,7 @@ module Call = struct &:: "substring" $ any_arg_of_typ (+PatternMatch.implements_lang "String") $+ capt_exp $+ capt_exp $--> JavaString.substring - ; +PatternMatch.implements_inject "Provider" - &:: "get" - <>--> expensive_modeled ~of_function:"Provider.get" + ; +PatternMatch.implements_inject "Provider" &:: "get" <>--> provider_get ; +PatternMatch.implements_xmob_utils "IntHashMap" &:: "" <>--> unit_cost_model ; +PatternMatch.implements_xmob_utils "IntHashMap" &:: "getElement" <>--> unit_cost_model ; +PatternMatch.implements_xmob_utils "IntHashMap" &:: "put" <>--> unit_cost_model diff --git a/infer/tests/codetoanalyze/java/performance/ProviderTest.java b/infer/tests/codetoanalyze/java/performance/ProviderTest.java new file mode 100644 index 000000000..62e58e8f9 --- /dev/null +++ b/infer/tests/codetoanalyze/java/performance/ProviderTest.java @@ -0,0 +1,27 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +import javax.inject.*; + +class ProviderTest { + /* Assume that injecting A is expensive, on the other hand injecting Integer is cheap. */ + class A {} + + @Inject private Provider mProviderA; + @Inject private Provider mProviderInteger; + + void use_provided_A(A a) {} + + void use_provided_Integer(Integer i) {} + + void expensive_get_A_constant() { + use_provided_A(mProviderA.get()); + } + + void cheap_get_Integer_linear() { + use_provided_Integer(mProviderInteger.get()); + } +} diff --git a/infer/tests/codetoanalyze/java/performance/issues.exp b/infer/tests/codetoanalyze/java/performance/issues.exp index 6bbc20e8d..d2cd6b35a 100644 --- a/infer/tests/codetoanalyze/java/performance/issues.exp +++ b/infer/tests/codetoanalyze/java/performance/issues.exp @@ -182,6 +182,7 @@ codetoanalyze/java/performance/MathTest.java, codetoanalyze.java.performance.Mat codetoanalyze/java/performance/MathTest.java, codetoanalyze.java.performance.MathTest.max2_symbolic(int,int):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 6 + 9 ⋅ (max(x, y)), O((max(x, y))), degree = 1,{max(x, y)},Loop at line 20] codetoanalyze/java/performance/MathTest.java, codetoanalyze.java.performance.MathTest.max_symbolic(int[]):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 2 + 5 ⋅ arr.length + 4 ⋅ (arr.length + 1), O(arr.length), degree = 1,{arr.length + 1},Loop at line 16,{arr.length},Loop at line 16] codetoanalyze/java/performance/PreconditionTest.java, PreconditionTest.checkNotNull_linear(java.util.ArrayList,java.lang.Object):void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 11 + 8 ⋅ list.length + 3 ⋅ (list.length + 1), O(list.length), degree = 1,{list.length + 1},Loop at line 37,{list.length},Loop at line 37] +codetoanalyze/java/performance/ProviderTest.java, ProviderTest.expensive_get_A_constant():void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 3 + (Provider.get()(expensive call).ub), O((Provider.get()(expensive call).ub)), degree = 1,{Provider.get()(expensive call).ub},Modeled call to Provider.get] codetoanalyze/java/performance/StringBuilderTest.java, StringBuilderTest.append_linear(java.lang.String):void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 25 + 5 ⋅ (s.length + 2) + 3 ⋅ (s.length + 3), O(s.length), degree = 1,{s.length + 3},call to void StringBuilderTest.new_linear(String),Loop at line 13,{s.length + 2},call to void StringBuilderTest.new_linear(String),Loop at line 13] codetoanalyze/java/performance/StringBuilderTest.java, StringBuilderTest.new_linear(java.lang.String):void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 9 + 5 ⋅ s.length + 3 ⋅ (s.length + 1), O(s.length), degree = 1,{s.length + 1},Loop at line 13,{s.length},Loop at line 13] codetoanalyze/java/performance/StringTest.java, StringTest.byte_array_constructor_linear(byte[]):void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 6 + 5 ⋅ data.length + 3 ⋅ (data.length + 1), O(data.length), degree = 1,{data.length + 1},Loop at line 55,{data.length},Loop at line 55]