unify handling of many special cases in callExpr_trans

Summary:
1. Attempt to simplify callExpr_trans by merging multiple if-else branches into one in `CTrans_utils.builtin_trans`. Not every special case works that way, so some of them are still there.
2. On top of that, make type of parameters in `CTrans_models` functions `Procname.t` instead `Procname.t option`. This triggered some more changes to `callExpr_trans`.
3. Semi-randomly reorder instructions in `callExpr_trans`

This is mainly an attempt to clean up the code so any suggestions how to make it better are welcome.

Reviewed By: jvillard

Differential Revision: D3586419

fbshipit-source-id: aef8580

infer/src/clang/cTrans.ml

@@ -817,13 +817,7 @@ struct
     (* claim priority if no ancestors has claimed priority before *)
     let context_callee = { context with CContext.is_callee_expression = true } in
     let trans_state_callee = { trans_state_pri with context = context_callee; succ_nodes = [] } in
-    let is_call_to_block = objc_exp_of_type_block fun_exp_stmt in
     let res_trans_callee = instruction trans_state_callee fun_exp_stmt in
-    (* As we may have nodes coming from different parameters we need to  *)
-    (* call instruction for each parameter and collect the results       *)
-    (* afterwards. The 'instructions' function does not do that          *)
-    let trans_state_param =
-      { trans_state_pri with succ_nodes = []; var_exp_typ = None  } in
     let (sil_fe, _) = extract_exp_from_list res_trans_callee.exps
         "WARNING: The translation of fun_exp did not return an expression.\
          Returning -1. NEED TO BE FIXED" in
@@ -832,66 +826,64 @@ struct
       | Sil.Const (Const.Cfun pn) ->
           Some pn
       | _ -> None (* function pointer *) in
+    (* we cannot translate the arguments of __builtin_object_size because preprocessing copies
+       them verbatim from a call to a different function, and they might be side-effecting *)
     let should_translate_args =
-      match callee_pname_opt with
-      | Some pn ->
-          (* we cannot translate the arguments of this builtin because preprocessing copies them *)
-          (* verbatim from a call to a different function, and they might be side-effecting *)
-          (Procname.to_string pn) <> CFrontend_config.builtin_object_size
-      | _ -> true in
+      not (Option.map_default CTrans_models.is_builtin_object_size false callee_pname_opt) in
     let params_stmt = if should_translate_args then params_stmt
       else [] in
+    (* As we may have nodes coming from different parameters we need to  *)
+    (* call instruction for each parameter and collect the results       *)
+    (* afterwards. The 'instructions' function does not do that          *)
+    let trans_state_param =
+      { trans_state_pri with succ_nodes = []; var_exp_typ = None  } in
     let result_trans_subexprs =
       let instruction' = exec_with_self_exception (exec_with_glvalue_as_reference instruction) in
       let res_trans_p = IList.map (instruction' trans_state_param) params_stmt in
       res_trans_callee :: res_trans_p in
-    let sil_fe, is_cf_retain_release = CTrans_models.builtin_predefined_model fun_exp_stmt sil_fe in
-    if CTrans_models.is_assert_log sil_fe then
-      CTrans_utils.trans_assertion sil_loc context trans_state.succ_nodes
-    else
-      let act_params =
-        let params = IList.tl (collect_exprs result_trans_subexprs) in
-        if IList.length params = IList.length params_stmt then
-          params
-        else (Printing.log_err
-                "WARNING: stmt_list and res_trans_par.exps must have same size. \
-                 NEED TO BE FIXED\n\n";
-              fix_param_exps_mismatch params_stmt params) in
-      let act_params = if is_cf_retain_release then
-          (Sil.Const (Const.Cint IntLit.one), Typ.Tint Typ.IBool) :: act_params
-        else act_params in
-      match
-        CTrans_utils.builtin_trans trans_state_pri sil_loc si function_type callee_pname_opt with
-      | Some builtin -> builtin
-      | None ->
-          (* Translate call to __builtin_expect as the first argument *)
-          (* for simpler symbolic execution *)
-          match callee_pname_opt, result_trans_subexprs with
-          | Some pname, [_; fst_arg_res; _]
-            when Procname.to_string pname = CFrontend_config.builtin_expect ->
-              fst_arg_res
+    match Option.map_default (CTrans_utils.builtin_trans trans_state_pri sil_loc si function_type
+                                result_trans_subexprs) None callee_pname_opt with
+    | Some builtin -> builtin
+    | None ->
+        let callee_pname_opt', is_cf_retain_release =
+          CTrans_models.builtin_predefined_model fun_exp_stmt callee_pname_opt in
+        let act_params =
+          let params = IList.tl (collect_exprs result_trans_subexprs) in
+          if IList.length params = IList.length params_stmt then
+            params
+          else (Printing.log_err
+                  "WARNING: stmt_list and res_trans_par.exps must have same size. \
+                   NEED TO BE FIXED\n\n";
+                fix_param_exps_mismatch params_stmt params) in
+        let act_params = if is_cf_retain_release then
+            (Sil.Const (Const.Cint IntLit.one), Typ.Tint Typ.IBool) :: act_params
+          else act_params in
+        let sil_fe' = match callee_pname_opt' with
+          | Some pn -> Sil.Const (Const.Cfun pn)
+          | _ -> sil_fe in
+        let res_trans_call =
+          let cast_trans_fun = cast_trans context act_params sil_loc function_type in
+          match Option.map_default cast_trans_fun None callee_pname_opt' with
+          | Some (instr, cast_exp) ->
+              { empty_res_trans with
+                instrs = [instr];
+                exps = [(cast_exp, function_type)]; }
           | _ ->
-              let res_trans_call =
-                match cast_trans context act_params sil_loc callee_pname_opt function_type with
-                | Some (instr, cast_exp) ->
-                    { empty_res_trans with
-                      instrs = [instr];
-                      exps = [(cast_exp, function_type)]; }
-                | None ->
-                    let call_flags =
-                      { CallFlags.default with CallFlags.cf_is_objc_block = is_call_to_block; } in
-                    create_call_instr trans_state function_type sil_fe act_params sil_loc
-                      call_flags ~is_objc_method:false in
-              let nname = "Call "^(Sil.exp_to_string sil_fe) in
-              let all_res_trans = result_trans_subexprs @ [res_trans_call] in
-              let res_trans_to_parent = PriorityNode.compute_results_to_parent trans_state_pri
-                  sil_loc nname si all_res_trans in
-              (match callee_pname_opt with
-               | Some callee_pname ->
-                   if not (Builtin.is_registered callee_pname) then
-                     Cg.add_edge context.CContext.cg procname callee_pname
-               | None -> ());
-              { res_trans_to_parent with exps = res_trans_call.exps }
+              let is_call_to_block = objc_exp_of_type_block fun_exp_stmt in
+              let call_flags =
+                { CallFlags.default with CallFlags.cf_is_objc_block = is_call_to_block; } in
+              create_call_instr trans_state function_type sil_fe' act_params sil_loc
+                call_flags ~is_objc_method:false in
+        let nname = "Call "^(Sil.exp_to_string sil_fe') in
+        let all_res_trans = result_trans_subexprs @ [res_trans_call] in
+        let res_trans_to_parent = PriorityNode.compute_results_to_parent trans_state_pri
+            sil_loc nname si all_res_trans in
+        let add_cg_edge callee_pname =
+          if not (Builtin.is_registered callee_pname) then
+            Cg.add_edge context.CContext.cg procname callee_pname
+        in
+        Option.may add_cg_edge callee_pname_opt';
+        { res_trans_to_parent with exps = res_trans_call.exps }
 
   and cxx_method_construct_call_trans trans_state_pri result_trans_callee params_stmt
       si function_type is_cpp_call_virtual =
@@ -902,38 +894,36 @@ struct
     (* first for method address, second for 'this' expression *)
     assert ((IList.length result_trans_callee.exps) = 2);
     let (sil_method, _) = IList.hd result_trans_callee.exps in
-    let callee_pname = match sil_method with
+    let callee_pname =
+      match sil_method with
       | Sil.Const (Const.Cfun pn) -> pn
       | _ -> assert false (* method pointer not implemented, this shouldn't happen *) in
-    if CTrans_models.is_assert_log sil_method then
-      CTrans_utils.trans_assertion sil_loc context trans_state_pri.succ_nodes
-    else
-      (* As we may have nodes coming from different parameters we need to  *)
-      (* call instruction for each parameter and collect the results       *)
-      (* afterwards. The 'instructions' function does not do that          *)
+    (* As we may have nodes coming from different parameters we need to  *)
+    (* call instruction for each parameter and collect the results       *)
+    (* afterwards. The 'instructions' function does not do that          *)
+    let result_trans_subexprs =
       let trans_state_param =
         { trans_state_pri with succ_nodes = []; var_exp_typ = None } in
-      let result_trans_subexprs =
-        let instruction' = exec_with_self_exception (exec_with_glvalue_as_reference instruction) in
-        let res_trans_p = IList.map (instruction' trans_state_param) params_stmt in
-        result_trans_callee :: res_trans_p in
-      (* first expr is method address, rest are params including 'this' parameter *)
-      let actual_params = IList.tl (collect_exprs result_trans_subexprs) in
-      let call_flags = {
-        CallFlags.cf_virtual = is_cpp_call_virtual;
-        CallFlags.cf_interface = false;
-        CallFlags.cf_noreturn = false;
-        CallFlags.cf_is_objc_block = false;
-        CallFlags.cf_targets = [];
-      } in
-      let res_trans_call = create_call_instr trans_state_pri function_type sil_method actual_params
-          sil_loc call_flags ~is_objc_method:false in
-      let nname = "Call " ^ (Sil.exp_to_string sil_method) in
-      let all_res_trans = result_trans_subexprs @ [res_trans_call] in
-      let result_trans_to_parent =
-        PriorityNode.compute_results_to_parent trans_state_pri sil_loc nname si all_res_trans in
-      Cg.add_edge context.CContext.cg procname callee_pname;
-      { result_trans_to_parent with exps = res_trans_call.exps }
+      let instruction' = exec_with_self_exception (exec_with_glvalue_as_reference instruction) in
+      let res_trans_p = IList.map (instruction' trans_state_param) params_stmt in
+      result_trans_callee :: res_trans_p in
+    (* first expr is method address, rest are params including 'this' parameter *)
+    let actual_params = IList.tl (collect_exprs result_trans_subexprs) in
+    match cxx_method_builtin_trans trans_state_pri sil_loc callee_pname with
+    | Some builtin -> builtin
+    | _ ->
+        let call_flags = {
+          CallFlags.default with
+          CallFlags.cf_virtual = is_cpp_call_virtual;
+        } in
+        let res_trans_call = create_call_instr trans_state_pri function_type sil_method
+            actual_params sil_loc call_flags ~is_objc_method:false in
+        let nname = "Call " ^ (Sil.exp_to_string sil_method) in
+        let all_res_trans = result_trans_subexprs @ [res_trans_call] in
+        let result_trans_to_parent =
+          PriorityNode.compute_results_to_parent trans_state_pri sil_loc nname si all_res_trans in
+        Cg.add_edge context.CContext.cg procname callee_pname;
+        { result_trans_to_parent with exps = res_trans_call.exps }
 
 
   and cxxMemberCallExpr_trans trans_state si stmt_list expr_info =
@@ -1011,7 +1001,7 @@ struct
       | _ -> None
       (* assertions *)
     else if CTrans_models.is_handleFailureInMethod selector then
-      Some (CTrans_utils.trans_assertion sil_loc context trans_state.succ_nodes)
+      Some (CTrans_utils.trans_assertion trans_state sil_loc)
     else None
 
 

infer/src/clang/cTrans_models.ml

@@ -12,29 +12,26 @@ open! Utils
 open CFrontend_utils
 open Objc_models
 
-let is_cf_non_null_alloc funct =
-  match funct with
-  | Some procname ->
-      Procname.to_string procname = CFrontend_config.cf_non_null_alloc
-  | None -> false
-
-let is_alloc funct =
-  match funct with
-  | Some procname ->
-      Procname.to_string procname = CFrontend_config.cf_alloc
-  | None -> false
-
-let is_alloc_model typ funct =
-  match funct with
-  | Some procname ->
-      if Specs.summary_exists procname then false
-      else
-        let funct = Procname.to_string procname in
-        (* if (Core_foundation_model.is_core_lib_create typ funct) then
-           print_endline ("\nCore Foundation create not modelled "
-           ^(Typ.to_string typ)^" "^(funct));*)
-        Core_foundation_model.is_core_lib_create typ funct
-  | None -> false
+let is_cf_non_null_alloc pname =
+  Procname.to_string pname = CFrontend_config.cf_non_null_alloc
+
+let is_alloc pname =
+  Procname.to_string pname = CFrontend_config.cf_alloc
+
+let is_alloc_model typ pname =
+  if Specs.summary_exists pname then false
+  else
+    let funct = Procname.to_string pname in
+    (* if (Core_foundation_model.is_core_lib_create typ funct) then
+       print_endline ("\nCore Foundation create not modelled "
+       ^(Typ.to_string typ)^" "^(funct));*)
+    Core_foundation_model.is_core_lib_create typ funct
+
+let is_builtin_expect pname =
+  Procname.to_string pname = CFrontend_config.builtin_expect
+
+let is_builtin_object_size pname =
+  (Procname.to_string pname) == CFrontend_config.builtin_object_size
 
 let rec get_func_type_from_stmt stmt =
   match stmt with
@@ -45,10 +42,12 @@ let rec get_func_type_from_stmt stmt =
       | stmt:: _ -> get_func_type_from_stmt stmt
       | [] -> None
 
-let is_retain_predefined_model typ funct =
+let is_retain_predefined_model typ pname =
+  let funct = Procname.to_string pname in
   Core_foundation_model.is_core_lib_retain typ funct
 
-let is_release_predefined_model typ funct =
+let is_release_predefined_model typ pname =
+  let funct = Procname.to_string pname in
   Core_foundation_model.is_core_lib_release typ funct ||
   Core_foundation_model.is_core_graphics_release typ funct
 
@@ -91,36 +90,33 @@ let is_retain_or_release funct =
   is_release_method funct ||
   is_autorelease_method funct
 
-let is_toll_free_bridging pn_opt =
-  match pn_opt with
-  | Some pn ->
-      let funct = (Procname.to_string pn) in
-      funct = CFrontend_config.cf_bridging_release ||
-      funct = CFrontend_config.cf_bridging_retain ||
-      funct = CFrontend_config.cf_autorelease ||
-      funct = CFrontend_config.ns_make_collectable
-  | None -> false
+let is_toll_free_bridging pn =
+  let funct = (Procname.to_string pn) in
+  funct = CFrontend_config.cf_bridging_release ||
+  funct = CFrontend_config.cf_bridging_retain ||
+  funct = CFrontend_config.cf_autorelease ||
+  funct = CFrontend_config.ns_make_collectable
 
 (** If the function is a builtin model, return the model, otherwise return the function *)
-let builtin_predefined_model fun_stmt sil_fe =
+let builtin_predefined_model fun_stmt pname_opt =
   match get_func_type_from_stmt fun_stmt with
   | Some typ ->
       let typ = Ast_utils.string_of_type_ptr typ in
-      (match sil_fe with
-       | Sil.Const (Const.Cfun pn) when Specs.summary_exists pn -> sil_fe, false
-       | Sil.Const (Const.Cfun pn) when is_retain_predefined_model typ (Procname.to_string pn) ->
-           Sil.Const (Const.Cfun ModelBuiltins.__objc_retain_cf) , true
-       | Sil.Const (Const.Cfun pn) when is_release_predefined_model typ (Procname.to_string pn) ->
-           Sil.Const (Const.Cfun ModelBuiltins.__objc_release_cf), true
-       | _ -> sil_fe, false)
-  | _ -> sil_fe, false
+      (match pname_opt with
+       | Some pn when Specs.summary_exists pn -> pname_opt, false
+       | Some pn when is_retain_predefined_model typ pn ->
+           Some ModelBuiltins.__objc_retain_cf, true
+       | Some pn when is_release_predefined_model typ pn ->
+           Some ModelBuiltins.__objc_release_cf, true
+       | _ -> pname_opt, false)
+  | _ -> pname_opt, false
 
 (** If the function is a builtin model, return the model, otherwise return the function *)
-let is_assert_log sil_fe =
-  match sil_fe with
-  | Sil.Const (Const.Cfun (Procname.ObjC_Cpp _ as pn)) ->
-      is_assert_log_method (Procname.to_string pn)
-  | Sil.Const (Const.Cfun (Procname.C _ as pn)) -> is_assert_log_s (Procname.to_string pn)
+let is_assert_log pname =
+  match pname with
+  | Procname.ObjC_Cpp _ ->
+      is_assert_log_method (Procname.to_string pname)
+  | Procname.C _ -> is_assert_log_s (Procname.to_string pname)
   | _ -> false
 
 

infer/src/clang/cTrans_models.mli

@@ -9,23 +9,27 @@
 
 open! Utils
 
-val is_cf_non_null_alloc : Procname.t option -> bool
+val is_cf_non_null_alloc : Procname.t -> bool
 
-val is_alloc : Procname.t option -> bool
+val is_alloc : Procname.t -> bool
 
-val is_alloc_model : Typ.t -> Procname.t option -> bool
+val is_alloc_model : Typ.t -> Procname.t -> bool
+
+val is_builtin_expect : Procname.t -> bool
+
+val is_builtin_object_size : Procname.t -> bool
 
 val is_objc_memory_model_controlled : string -> bool
 
-val builtin_predefined_model : Clang_ast_t.stmt -> Sil.exp -> Sil.exp * bool
+val builtin_predefined_model : Clang_ast_t.stmt -> Procname.t option -> Procname.t option * bool
 
-val is_assert_log : Sil.exp -> bool
+val is_assert_log : Procname.t -> bool
 
 val is_handleFailureInMethod : string -> bool
 
 val is_modeled_builtin : string -> bool
 
-val is_toll_free_bridging : Procname.t option -> bool
+val is_toll_free_bridging : Procname.t -> bool
 
 val get_predefined_model_method_signature : string -> string ->
   (string -> string -> Procname.objc_cpp_method_kind -> Procname.t) ->

infer/src/clang/cTrans_utils.ml

@@ -379,22 +379,14 @@ let create_cast_instrs context exp cast_from_typ cast_to_typ sil_loc =
     Sil.Call ([ret_id], Sil.Const (Const.Cfun pname), args, sil_loc, CallFlags.default) in
   (stmt_call, Sil.Var ret_id)
 
-let cast_trans context exps sil_loc callee_pname_opt function_type =
-  if CTrans_models.is_toll_free_bridging callee_pname_opt then
+let cast_trans context exps sil_loc function_type pname =
+  if CTrans_models.is_toll_free_bridging pname then
     match exps with
     | [exp, typ] ->
         Some (create_cast_instrs context exp typ function_type sil_loc)
     | _ -> assert false
   else None
 
-let builtin_trans trans_state loc stmt_info function_type callee_pname_opt =
-  if CTrans_models.is_cf_non_null_alloc callee_pname_opt ||
-     CTrans_models.is_alloc_model function_type callee_pname_opt then
-    Some (alloc_trans trans_state loc stmt_info function_type true callee_pname_opt)
-  else if CTrans_models.is_alloc callee_pname_opt then
-    Some (alloc_trans trans_state loc stmt_info function_type false None)
-  else None
-
 let dereference_var_sil (exp, typ) sil_loc =
   let id = Ident.create_fresh Ident.knormal in
   let sil_instr = Sil.Letderef (id, exp, typ, sil_loc) in
@@ -462,10 +454,36 @@ let trans_assume_false sil_loc context succ_nodes =
   Cfg.Node.set_succs_exn context.CContext.cfg prune_node succ_nodes [];
   { empty_res_trans with root_nodes = [prune_node]; leaf_nodes = [prune_node] }
 
-let trans_assertion sil_loc context succ_nodes =
+let trans_assertion trans_state sil_loc =
+  let context = trans_state.context in
   if Config.report_custom_error then
     trans_assertion_failure sil_loc context
-  else trans_assume_false sil_loc context succ_nodes
+  else trans_assume_false sil_loc context trans_state.succ_nodes
+
+let trans_builtin_expect params_trans_res =
+  (* Translate call to __builtin_expect as the first argument *)
+  (* for simpler symbolic execution *)
+  match params_trans_res with
+  | [_; fst_arg_res; _] -> Some fst_arg_res
+  | _ -> None
+
+let builtin_trans trans_state loc stmt_info function_type params_trans_res pname =
+  if CTrans_models.is_cf_non_null_alloc pname ||
+     CTrans_models.is_alloc_model function_type pname then
+    Some (alloc_trans trans_state loc stmt_info function_type true (Some pname))
+  else if CTrans_models.is_alloc pname then
+    Some (alloc_trans trans_state loc stmt_info function_type false None)
+  else if CTrans_models.is_assert_log pname then
+    Some (trans_assertion trans_state loc)
+  else if CTrans_models.is_builtin_expect pname then
+    trans_builtin_expect params_trans_res
+  else None
+
+let cxx_method_builtin_trans trans_state loc pname =
+  if CTrans_models.is_assert_log pname then
+    Some (trans_assertion trans_state loc)
+  else
+    None
 
 let define_condition_side_effects e_cond instrs_cond sil_loc =
   let (e', typ) = extract_exp_from_list e_cond "\nWARNING: Missing expression in IfStmt. Need to be fixed\n" in

infer/src/clang/cTrans_utils.mli

@@ -86,7 +86,7 @@ val cast_operation :
   trans_state -> Clang_ast_t.cast_kind -> (Sil.exp * Typ.t) list -> Typ.t -> Location.t ->
   bool -> Sil.instr list * (Sil.exp * Typ.t)
 
-val trans_assertion: Location.t -> CContext.t -> Cfg.Node.t list ->  trans_result
+val trans_assertion: trans_state -> Location.t ->  trans_result
 
 val is_owning_method : Clang_ast_t.stmt -> bool
 
@@ -99,7 +99,10 @@ val contains_opaque_value_expr : Clang_ast_t.stmt -> bool
 val get_decl_ref_info : Clang_ast_t.stmt -> Clang_ast_t.decl_ref
 
 val builtin_trans : trans_state -> Location.t -> Clang_ast_t.stmt_info ->
-  Typ.t -> Procname.t option -> trans_result option
+  Typ.t -> trans_result list -> Procname.t -> trans_result option
+
+val cxx_method_builtin_trans : trans_state -> Location.t -> Procname.t ->
+  trans_result option
 
 val alloc_trans :
   trans_state -> Location.t -> Clang_ast_t.stmt_info -> Typ.t -> bool ->
@@ -111,7 +114,7 @@ val new_or_alloc_trans : trans_state -> Location.t -> Clang_ast_t.stmt_info ->
 val cpp_new_trans : trans_state -> Location.t -> Typ.t -> Sil.exp option -> trans_result
 
 val cast_trans :
-  CContext.t -> (Sil.exp * Typ.t) list -> Location.t -> Procname.t option -> Typ.t ->
+  CContext.t -> (Sil.exp * Typ.t) list -> Location.t -> Typ.t -> Procname.t ->
   (Sil.instr * Sil.exp) option
 
 val dereference_var_sil : Sil.exp * Typ.t -> Location.t -> Sil.instr list * Sil.exp