[cost] Refactor dispatch mechanism to be uniform across cost kinds

Summary: Dispatch & function call mechanism was so jumbled up together. Let's refactor it to be cleaner.

Reviewed By: skcho

Differential Revision: D24049889

fbshipit-source-id: 42a218016

infer/src/cost/cost.ml

@@ -40,18 +40,6 @@ module InstrBasicCostWithReason = struct
     For example for basic operation we set it to 1 and for function call we take it from the spec of the function.
   *)
 
-  let allocation_functions =
-    [ BuiltinDecl.__new
-    ; BuiltinDecl.__new_array
-    ; BuiltinDecl.__objc_alloc_no_fail
-    ; BuiltinDecl.malloc
-    ; BuiltinDecl.malloc_no_fail ]
-
-
-  let is_allocation_function callee_pname =
-    List.exists allocation_functions ~f:(fun f -> Procname.equal callee_pname f)
-
-
   (** The methods whose name start with one of the prefixes return an object that is owned by the
       caller. Therefore ARC will not add any objects they return into an autorelease pool. *)
   let return_object_owned_by_caller =
@@ -67,9 +55,55 @@ module InstrBasicCostWithReason = struct
          ~f:(fun {ProcAttributes.is_objc_arc_on} -> is_objc_arc_on)
 
 
+  let dispatch_operation tenv callee_pname callee_cost_opt fun_arg_list model_env ret inferbo_mem =
+    match CostModels.Call.dispatch tenv callee_pname fun_arg_list with
+    | Some model ->
+        BasicCostWithReason.of_basic_cost (model (Lazy.force model_env) ~ret inferbo_mem)
+    | None -> (
+      match callee_cost_opt with
+      | Some callee_cost ->
+          let () =
+            Logging.(debug Analysis Quiet)
+              "@.Instantiated cost : %a \n" BasicCostWithReason.pp_hum callee_cost
+          in
+          callee_cost
+      | _ ->
+          ScubaLogging.cost_log_message ~label:"unmodeled_function_operation_cost"
+            ~message:(F.asprintf "Unmodeled Function[Operation Cost] : %a" Procname.pp callee_pname) ;
+          BasicCostWithReason.one () )
+
+
+  let dispatch_autoreleasepool tenv callee_pname callee_cost_opt fun_arg_list
+      ({get_summary} as extras) model_env ((_, ret_typ) as ret) cfg loc inferbo_mem :
+      BasicCostWithReason.t =
+    match CostAutoreleaseModels.Call.dispatch tenv callee_pname fun_arg_list with
+    | Some model ->
+        let autoreleasepool_size = model get_summary (Lazy.force model_env) ~ret inferbo_mem in
+        BasicCostWithReason.of_basic_cost autoreleasepool_size
+    | None ->
+        let fun_cost =
+          if
+            is_objc_call_from_no_arc_to_arc extras cfg callee_pname
+            && Typ.is_pointer_to_objc_non_tagged_class ret_typ
+            && not (return_object_owned_by_caller callee_pname)
+          then
+            let autoreleasepool_trace =
+              Bounds.BoundTrace.of_arc_from_non_arc (Procname.to_string callee_pname) loc
+            in
+            BasicCostWithReason.one ~autoreleasepool_trace ()
+          else BasicCostWithReason.zero
+        in
+        Option.value_map ~default:fun_cost ~f:(BasicCostWithReason.plus fun_cost) callee_cost_opt
+
+
+  let dispatch_allocation tenv callee_pname callee_cost_opt : BasicCostWithReason.t =
+    CostAllocationModels.ProcName.dispatch tenv callee_pname
+    |> Option.value ~default:(Option.value callee_cost_opt ~default:BasicCostWithReason.zero)
+
+
   let get_instr_cost_record tenv extras cfg instr_node instr =
     match instr with
-    | Sil.Call (((_, ret_typ) as ret), Exp.Const (Const.Cfun callee_pname), params, location, _)
+    | Sil.Call (ret, Exp.Const (Const.Cfun callee_pname), params, location, _)
       when Config.inclusive_cost -> (
         let { inferbo_invariant_map
             ; integer_type_widths
@@ -91,57 +125,31 @@ module InstrBasicCostWithReason = struct
              BufferOverrunUtils.ModelEnv.mk_model_env callee_pname ~node_hash location tenv
                integer_type_widths inferbo_get_summary )
         in
-        let cost =
+        let get_callee_cost_opt kind inferbo_mem =
-          match inferbo_mem_opt with
+          match (get_summary callee_pname, get_formals callee_pname) with
-          | None ->
+          | Some {CostDomain.post= callee_cost_record}, Some callee_formals ->
-              CostDomain.unit_cost_atomic_operation
+              CostDomain.find_opt kind callee_cost_record
-          | Some inferbo_mem -> (
+              |> Option.map ~f:(fun callee_cost ->
-            match CostModels.Call.dispatch tenv callee_pname fun_arg_list with
+                     instantiate_cost integer_type_widths ~inferbo_caller_mem:inferbo_mem
-            | Some model ->
+                       ~callee_pname ~callee_formals ~params ~callee_cost ~loc:location )
-                CostDomain.of_operation_cost (model (Lazy.force model_env) ~ret inferbo_mem)
+          | _ ->
-            | None -> (
+              None
-              match (get_summary callee_pname, get_formals callee_pname) with
-              | Some {CostDomain.post= callee_cost_record}, Some callee_formals -> (
-                  let instantiated_cost =
-                    CostDomain.map callee_cost_record ~f:(fun callee_cost ->
-                        instantiate_cost integer_type_widths ~inferbo_caller_mem:inferbo_mem
-                          ~callee_pname ~callee_formals ~params ~callee_cost ~loc:location )
-                  in
-                  match CostDomain.get_operation_cost callee_cost_record with
-                  | {cost; top_pname_opt= None} when BasicCost.is_top cost ->
-                      CostDomain.add_top_pname_opt CostKind.OperationCost instantiated_cost
-                        (Some callee_pname)
-                  | _ ->
-                      instantiated_cost )
-              | _, _ ->
-                  ScubaLogging.cost_log_message ~label:"unmodeled_function_cost_analysis"
-                    ~message:
-                      (F.asprintf "Unmodeled Function[Cost Analysis] : %a" Procname.pp callee_pname) ;
-                  CostDomain.unit_cost_atomic_operation ) )
-        in
-        let cost =
-          if is_allocation_function callee_pname then
-            CostDomain.plus CostDomain.unit_cost_allocation cost
-          else cost
         in
-        match
+        match inferbo_mem_opt with
-          (inferbo_mem_opt, CostAutoreleaseModels.Call.dispatch tenv callee_pname fun_arg_list)
+        | None ->
-        with
+            CostDomain.unit_cost_atomic_operation
-        | Some inferbo_mem, Some model ->
+        | Some inferbo_mem ->
-            let autoreleasepool_size = model get_summary (Lazy.force model_env) ~ret inferbo_mem in
+            CostDomain.construct ~f:(fun kind ->
-            CostDomain.plus_autoreleasepool_size autoreleasepool_size cost
+                let callee_cost_opt = get_callee_cost_opt kind inferbo_mem in
-        | _, _ ->
+                match kind with
-            if
+                | OperationCost ->
-              is_objc_call_from_no_arc_to_arc extras cfg callee_pname
+                    dispatch_operation tenv callee_pname callee_cost_opt fun_arg_list model_env ret
-              && Typ.is_pointer_to_objc_non_tagged_class ret_typ
+                      inferbo_mem
-              && not (return_object_owned_by_caller callee_pname)
+                | AllocationCost ->
-            then
+                    dispatch_allocation tenv callee_pname callee_cost_opt
-              let autoreleasepool_trace =
+                | AutoreleasepoolSize ->
-                Bounds.BoundTrace.of_arc_from_non_arc (Procname.to_string callee_pname) location
+                    dispatch_autoreleasepool tenv callee_pname callee_cost_opt fun_arg_list extras
-              in
+                      model_env ret cfg location inferbo_mem ) )
-              CostDomain.plus cost
-                (CostDomain.unit_cost_autoreleasepool_size ~autoreleasepool_trace)
-            else cost )
     | Sil.Call (_, Exp.Const (Const.Cfun _), _, _, _) ->
         CostDomain.zero_record
     | Sil.Load {id= lhs_id} when Ident.is_none lhs_id ->
@@ -175,7 +183,7 @@ module InstrBasicCostWithReason = struct
     let cost = get_instr_cost_record tenv extras cfg instr_node instr in
     let operation_cost = CostDomain.get_operation_cost cost in
     let log_msg top_or_bottom =
-      Logging.d_printfln_escaped "Statement cost became %s at %a (%a)." top_or_bottom
+      Logging.d_printfln_escaped "Statement's operation cost became %s at %a (%a)." top_or_bottom
         InstrCFG.Node.pp_id (InstrCFG.Node.id instr_node)
         (Sil.pp_instr ~print_types:false Pp.text)
         instr

infer/src/cost/costAllocationModels.ml

@@ -0,0 +1,21 @@
+(*
+ * 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 BasicCostWithReason = CostDomain.BasicCostWithReason
+
+module ProcName = struct
+  let dispatch : (Tenv.t, BasicCostWithReason.t, unit) ProcnameDispatcher.ProcName.dispatcher =
+    let open ProcnameDispatcher.ProcName in
+    let match_builtin builtin _ s = String.equal s (Procname.get_method builtin) in
+    make_dispatcher
+      [ +match_builtin BuiltinDecl.__new <>--> BasicCostWithReason.one ()
+      ; +match_builtin BuiltinDecl.__new_array <>--> BasicCostWithReason.one ()
+      ; +match_builtin BuiltinDecl.__objc_alloc_no_fail <>--> BasicCostWithReason.one ()
+      ; +match_builtin BuiltinDecl.malloc <>--> BasicCostWithReason.one ()
+      ; +match_builtin BuiltinDecl.malloc_no_fail <>--> BasicCostWithReason.one () ]
+end

infer/src/cost/costAllocationModels.mli

@@ -0,0 +1,13 @@
+(*
+ * 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 ProcName : sig
+  val dispatch :
+    (Tenv.t, CostDomain.BasicCostWithReason.t, unit) ProcnameDispatcher.ProcName.dispatcher
+end

infer/src/cost/costDomain.ml

@@ -21,6 +21,8 @@ module BasicCostWithReason = struct
 
   let is_top {cost} = BasicCost.is_top cost
 
+  let is_zero {cost} = BasicCost.is_zero cost
+
   let is_unreachable {cost} = BasicCost.is_unreachable cost
 
   let zero = {cost= BasicCost.zero; top_pname_opt= None}
@@ -29,8 +31,11 @@ module BasicCostWithReason = struct
     {cost= BasicCost.one ?autoreleasepool_trace (); top_pname_opt= None}
 
 
-  let subst callee_pname location record eval_sym =
+  let of_basic_cost cost = {cost; top_pname_opt= None}
-    {record with cost= BasicCost.subst callee_pname location record.cost eval_sym}
+
+  let subst callee_pname location {cost; top_pname_opt} eval_sym =
+    let cost = BasicCost.subst callee_pname location cost eval_sym in
+    if BasicCost.is_top cost then {cost; top_pname_opt= Some callee_pname} else {cost; top_pname_opt}
 
 
   (* When we fold the nodes while traversing the cfg,
@@ -63,13 +68,15 @@ module VariantCostMap = struct
   let get kind record = find_opt kind record |> Option.value ~default:BasicCostWithReason.zero
 
   let increase_by kind cost_to_add record =
-    update kind
+    if BasicCostWithReason.is_zero cost_to_add then record
-      (function
+    else
-        | None ->
+      update kind
-            Some cost_to_add
+        (function
-        | Some existing ->
+          | None ->
-            Some (BasicCostWithReason.plus cost_to_add existing) )
+              Some cost_to_add
-      record
+          | Some existing ->
+              Some (BasicCostWithReason.plus cost_to_add existing) )
+        record
 
 
   let increment ?autoreleasepool_trace kind record =
@@ -84,12 +91,6 @@ let pp_summary fmt {post} = F.fprintf fmt "@\n Post: %a @\n" VariantCostMap.pp p
 
 let get_cost_kind kind cost_record = VariantCostMap.get kind cost_record
 
-let add_top_pname_opt kind cost_record top_pname_opt =
-  VariantCostMap.update kind
-    (function Some cost_with_reason -> Some {cost_with_reason with top_pname_opt} | _ -> None)
-    cost_record
-
-
 let get_operation_cost cost_record = get_cost_kind CostKind.OperationCost cost_record
 
 let set_autoreleasepool_size_zero cost_record =
@@ -98,8 +99,17 @@ let set_autoreleasepool_size_zero cost_record =
 
 let map ~f cost_record = VariantCostMap.map f cost_record
 
+let find_opt = VariantCostMap.find_opt
+
 let zero_record = VariantCostMap.empty
 
+let construct ~f =
+  let open CostKind in
+  List.fold_left ~init:zero_record
+    ~f:(fun acc kind -> VariantCostMap.increase_by kind (f kind) acc)
+    [OperationCost; AllocationCost; AutoreleasepoolSize]
+
+
 (** If nb_exec is unreachable, we map to unreachable, not 0 *)
 let mult_by cost_record ~nb_exec = map cost_record ~f:(BasicCostWithReason.mult_unreachable nb_exec)
 
@@ -120,25 +130,4 @@ let plus cost_record1 cost_record2 =
     cost_record1 cost_record2
 
 
-let plus_autoreleasepool_size autoreleasepool_size cost =
-  VariantCostMap.update AutoreleasepoolSize
-    (function
-      | None ->
-          Some {BasicCostWithReason.cost= autoreleasepool_size; top_pname_opt= None}
-      | Some prev ->
-          Some {prev with cost= BasicCost.plus prev.cost autoreleasepool_size} )
-    cost
-
-
 let unit_cost_atomic_operation = VariantCostMap.increment CostKind.OperationCost zero_record
-
-let unit_cost_allocation = VariantCostMap.increment CostKind.AllocationCost zero_record
-
-let unit_cost_autoreleasepool_size ~autoreleasepool_trace =
-  VariantCostMap.increment ~autoreleasepool_trace CostKind.AutoreleasepoolSize zero_record
-
-
-let of_operation_cost operation_cost =
-  VariantCostMap.increase_by CostKind.OperationCost
-    {cost= operation_cost; top_pname_opt= None}
-    zero_record

infer/src/cost/costDomain.mli

@@ -31,10 +31,18 @@ module BasicCostWithReason : sig
 
   type t = {cost: BasicCost.t; top_pname_opt: Procname.t option}
 
+  val one : ?autoreleasepool_trace:Bounds.BoundTrace.t -> unit -> t
+
+  val zero : t
+
   val is_top : t -> bool
 
+  val of_basic_cost : BasicCost.t -> t
+
   val is_unreachable : t -> bool
 
+  val plus : t -> t -> t
+
   val subst : Procname.t -> Location.t -> t -> Bounds.Bound.eval_sym -> t
 
   val degree : t -> Polynomials.Degree.t option
@@ -58,13 +66,13 @@ val pp_summary : F.formatter -> summary -> unit
 
 val get_cost_kind : CostKind.t -> t -> BasicCostWithReason.t
 
-val add_top_pname_opt : CostKind.t -> t -> Procname.t option -> t
-
 val get_operation_cost : t -> BasicCostWithReason.t
 
 val set_autoreleasepool_size_zero : t -> t
 
-val map : f:(BasicCostWithReason.t -> BasicCostWithReason.t) -> t -> t
+val find_opt : CostKind.t -> t -> BasicCostWithReason.t option
+
+val construct : f:(CostKind.t -> BasicCostWithReason.t) -> t
 
 val zero_record : t
 (** Map representing cost record \{OperationCost:0; AllocationCost:0; AutoreleasepoolSize:0\} *)
@@ -75,18 +83,5 @@ val mult_by : t -> nb_exec:BasicCost.t -> t
 val plus : t -> t -> t
 (** Union of two maps where common costs are added together *)
 
-val plus_autoreleasepool_size : BasicCost.t -> t -> t
-(** Add an autoreleasepool size to the cost map *)
-
 val unit_cost_atomic_operation : t
 (** Map representing cost record \{OperationCost:1; AllocationCost:0; AutoreleasepoolSize:0\} *)
-
-val unit_cost_allocation : t
-(** Map representing cost record \{OperationCost:0; AllocationCost:1; AutoreleasepoolSize:0\} *)
-
-val unit_cost_autoreleasepool_size : autoreleasepool_trace:Bounds.BoundTrace.t -> t
-(** Map representing cost record \{OperationCost:0; AllocationCost:0; AutoreleasepoolSize:1\} *)
-
-val of_operation_cost : BasicCost.t -> t
-(** Map representing cost record \{OperationCost:operation_cost; AllocationCost:0;
-    AutoreleasepoolSize:0\} *)

infer/tests/codetoanalyze/objc/performance/cost-issues.exp

@@ -5,7 +5,7 @@ codetoanalyze/objc/performance/NSArray.m, nsarray_access_linear, 3 + 7 ⋅ array
 codetoanalyze/objc/performance/NSArray.m, nsarray_add_object_constant, 8,  OnUIThread:false, []
 codetoanalyze/objc/performance/NSArray.m, nsarray_add_objects_from_array_linear, 9 + 3 ⋅ append_array->elements.length.ub + append_array->elements.length.ub + 3 ⋅ (append_array->elements.length.ub + 1),  OnUIThread:false, [{append_array->elements.length.ub + 1},Loop,{append_array->elements.length.ub},Modeled call to NSArray.arrayByAddingObjectsFromArray:,{append_array->elements.length.ub},Loop]
 codetoanalyze/objc/performance/NSArray.m, nsarray_array_with_objects_constant, 27,  OnUIThread:false, []
-codetoanalyze/objc/performance/NSArray.m, nsarray_binary_search_log_FN, 9,  OnUIThread:false, []
+codetoanalyze/objc/performance/NSArray.m, nsarray_binary_search_log_FN, 10,  OnUIThread:false, []
 codetoanalyze/objc/performance/NSArray.m, nsarray_contains_object_linear, 3 + array->elements.length.ub,  OnUIThread:false, [{array->elements.length.ub},Modeled call to NSArray.containsObject:]
 codetoanalyze/objc/performance/NSArray.m, nsarray_count_bounded_linear, 3 + 3 ⋅ array->elements.length.ub + 3 ⋅ (array->elements.length.ub + 1),  OnUIThread:false, [{array->elements.length.ub + 1},Loop,{array->elements.length.ub},Loop]
 codetoanalyze/objc/performance/NSArray.m, nsarray_empty_array_constant, 8,  OnUIThread:false, []