From 1baaa96fcc111a920ae56503bccfa815308eb048 Mon Sep 17 00:00:00 2001 From: Sam Blackshear Date: Mon, 29 Jun 2015 07:49:18 -0600 Subject: [PATCH] [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 | 5 ++++ infer/src/backend/symExec.ml | 3 --- infer/src/clang/cFrontend_config.ml | 4 ++++ infer/src/clang/cFrontend_config.mli | 4 ++++ infer/src/clang/cTrans.ml | 24 ++++++++++++------- infer/src/clang/cTrans_models.ml | 6 ++--- infer/src/clang/cTrans_models.mli | 2 ++ .../c/errors/memory_leaks/test.c | 5 ++++ 8 files changed, 39 insertions(+), 14 deletions(-) diff --git a/infer/models/c/src/infer_builtins.c b/infer/models/c/src/infer_builtins.c index 433714e57..b46bd64ee 100644 --- a/infer/models/c/src/infer_builtins.c +++ b/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; +} diff --git a/infer/src/backend/symExec.ml b/infer/src/backend/symExec.ml index b6c7b1c2e..057ea3ed2 100644 --- a/infer/src/backend/symExec.ml +++ b/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) *) diff --git a/infer/src/clang/cFrontend_config.ml b/infer/src/clang/cFrontend_config.ml index 7e1476cc4..7c712b32a 100644 --- a/infer/src/clang/cFrontend_config.ml +++ b/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" diff --git a/infer/src/clang/cFrontend_config.mli b/infer/src/clang/cFrontend_config.mli index ab250d933..b8e524023 100644 --- a/infer/src/clang/cFrontend_config.mli +++ b/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 diff --git a/infer/src/clang/cTrans.ml b/infer/src/clang/cTrans.ml index 130e5df73..3ab68795d 100644 --- a/infer/src/clang/cTrans.ml +++ b/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 diff --git a/infer/src/clang/cTrans_models.ml b/infer/src/clang/cTrans_models.ml index 40c1e844f..fc12d0690 100644 --- a/infer/src/clang/cTrans_models.ml +++ b/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 || diff --git a/infer/src/clang/cTrans_models.mli b/infer/src/clang/cTrans_models.mli index 98845865e..ffab9c5e1 100644 --- a/infer/src/clang/cTrans_models.mli +++ b/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) -> diff --git a/infer/tests/codetoanalyze/c/errors/memory_leaks/test.c b/infer/tests/codetoanalyze/c/errors/memory_leaks/test.c index 617664f36..588f43fd5 100644 --- a/infer/tests/codetoanalyze/c/errors/memory_leaks/test.c +++ b/infer/tests/codetoanalyze/c/errors/memory_leaks/test.c @@ -4,6 +4,7 @@ */ #include +#include 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); +}