[Infer][C frontend] Fixing double translation of builtin args

Summary: @publicThe first argument of builtin calls in C gets translated twice, which is bad if the argument is a side-effecting expression like a function call.

Test Plan: Attached test previously reported a memory leak because the translation introduces an extra call to malloc(), now reports nothing.

infer/models/c/src/infer_builtins.c

@@ -87,3 +87,8 @@ long infer__builtin_expect(long e, long x) {
     return e;
   }
 }
+
+void *infer__builtin___memset_chk(void *dest, int val, unsigned long len, unsigned long dstlen) {
+  INFER_EXCLUDE_CONDITION(dstlen < len);
+  return dest;
+}

infer/src/backend/symExec.ml

@@ -2157,9 +2157,6 @@ module ModelBuiltins = struct
       Sil.Set (Sil.Lvar Sil.global_error, Sil.Tvoid, Sil.Const (Sil.Cstr error_str), loc) in
     sym_exec_generated true cfg tenv pdesc [set_instr] [(prop, path)]
 
-  (** match any function whose plain name is shared_ptr, and set the ignore attribute *)
-  (* let _ = Builtin.register_plain "shared_ptr" execute___method_set_ignore_attribute *)
-
   let _ = Builtin.register "__method_set_ignore_attribute" execute___method_set_ignore_attribute
   let _ = Builtin.register "__builtin_va_arg" execute___builtin_va_arg (* model va_arg *)
   let _ = Builtin.register "__builtin_va_copy" execute_skip (** NOTE: __builtin_va_copy should have been handled in the translation already (see frontend.ml) *)

infer/src/clang/cFrontend_config.ml

@@ -108,6 +108,10 @@ let cf_autorelease = "CFAutorelease"
 
 let builtin_expect = "__builtin_expect"
 
+let builtin_memset_chk = "__builtin___memset_chk"
+
+let builtin_object_size = "__builtin_object_size"
+
 let assert_fail = "__assert_fail"
 
 let assert_rtn = "__assert_rtn"

infer/src/clang/cFrontend_config.mli

@@ -105,6 +105,10 @@ val cf_autorelease : string
 
 val builtin_expect : string
 
+val builtin_memset_chk : string
+
+val builtin_object_size : string
+
 val fbAssertWithSignalAndLogFunctionHelper : string
 
 val assert_fail : string

infer/src/clang/cTrans.ml

@@ -292,7 +292,7 @@ struct
       { root_nodes = []; leaf_nodes = []; ids = []; instrs = []; exps = [(const_exp, typ)]}
     ) else if is_function then (
       let name =
-        if name = CFrontend_config.builtin_expect then ("infer"^CFrontend_config.builtin_expect)
+        if CTrans_models.is_modeled_builtin name then ("infer" ^ name)
         else name in
       let qt = CTypes.get_raw_qual_type_decl_ref_exp_info decl_ref_expr_info in
       let pname, type_opt =
@@ -525,12 +525,25 @@ struct
     (* afterwards. The 'instructions' function does not do that          *)
     let trans_state_param =
       { trans_state_pri with parent_line_number = line_number; succ_nodes = [] } in
+    let (sil_fe, typ_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
+    let callee_pname_opt =
+        match sil_fe with
+        | Sil.Const (Sil.Cfun pn) ->
+            Some pn
+        | _ -> None (* function pointer *) in
+    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
+    let params_stmt = if should_translate_args then params_stmt else [] in
     let res_trans_par =
       let l = list_map (fun i -> exec_with_self_exception instruction trans_state_param i) params_stmt in
       let rt = collect_res_trans (res_trans_callee :: l) in
       { rt with exps = list_tl rt.exps } in
-    let (sil_fe, typ_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
     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
       if Config.report_assertion_failure then
@@ -538,11 +551,6 @@ struct
       else
         CTrans_utils.trans_assume_false sil_loc context trans_state.succ_nodes
     else
-      let callee_pname_opt =
-        match sil_fe with
-        | Sil.Const (Sil.Cfun pn) ->
-            Some pn
-        | _ -> None (* function pointer *) in
       let act_params = if list_length res_trans_par.exps = list_length params_stmt then
           res_trans_par.exps
         else (Printing.log_err

infer/src/clang/cTrans_models.ml

@@ -66,9 +66,9 @@ let get_builtinname method_name =
     Some SymExec.ModelBuiltins.__objc_release
   else None
 
-(* Not used *)
-let is_builtin_expect funct =
-  funct = CFrontend_config.builtin_expect
+let is_modeled_builtin funct =
+  funct = CFrontend_config.builtin_expect ||
+  funct = CFrontend_config.builtin_memset_chk
 
 let is_assert_log_s funct =
   funct = CFrontend_config.assert_rtn ||

infer/src/clang/cTrans_models.mli

@@ -17,6 +17,8 @@ val is_assert_log : Sil.exp -> bool
 
 val is_handleFailureInMethod : string -> bool
 
+val is_modeled_builtin : string -> bool
+
 val is_toll_free_bridging : Procname.t option -> bool
 
 val get_predefined_model_method_signature : string -> string -> (string -> string -> Procname.t) ->

infer/tests/codetoanalyze/c/errors/memory_leaks/test.c

@@ -4,6 +4,7 @@
  */
 
 #include <stdlib.h>
+#include <string.h>
 
 void simple_leak() {
   int *p;
@@ -31,3 +32,7 @@ void uses_allocator() {
   p = allocate();
   *p = 42;
 }
+
+void * builtin_no_leak(size_t s) {
+  return memset(malloc(s), 0, s);
+}