From ec80d40bddf577ec68e5a4f15ed0d18c368f59a3 Mon Sep 17 00:00:00 2001 From: Andrzej Kotulski Date: Thu, 28 Jan 2016 02:31:53 -0800 Subject: [PATCH] Populate return parameter when returning + turn on new feature for C/C++ Summary: public 1. When function uses return parameter instead of returning directly, populate that parameter. 2. Turn on new feature for C/C++ functions/methods that return structured types Reviewed By: jberdine Differential Revision: D2865091 fb-gh-sync-id: e15e6eb --- infer/src/clang/cMethod_trans.ml | 16 ++++-- infer/src/clang/cMethod_trans.mli | 2 +- infer/src/clang/cTrans.ml | 42 ++++++++++------ .../cpp/frontend/methods/return_struct.cpp | 24 +++++++++ .../frontend/methods/return_struct.cpp.dot | 46 +++++++++++++++++ .../cpp/frontend/types/return_struct.cpp | 24 +++++++++ .../cpp/frontend/types/return_struct.cpp.dot | 50 +++++++++++++++++++ infer/tests/frontend/cpp/FunctionsTest.java | 6 +++ infer/tests/frontend/cpp/MethodsTest.java | 6 +++ 9 files changed, 197 insertions(+), 19 deletions(-) create mode 100644 infer/tests/codetoanalyze/cpp/frontend/methods/return_struct.cpp create mode 100644 infer/tests/codetoanalyze/cpp/frontend/methods/return_struct.cpp.dot create mode 100644 infer/tests/codetoanalyze/cpp/frontend/types/return_struct.cpp create mode 100644 infer/tests/codetoanalyze/cpp/frontend/types/return_struct.cpp.dot diff --git a/infer/src/clang/cMethod_trans.ml b/infer/src/clang/cMethod_trans.ml index 3253c8438..25b34aef2 100644 --- a/infer/src/clang/cMethod_trans.ml +++ b/infer/src/clang/cMethod_trans.ml @@ -60,15 +60,22 @@ let get_class_param function_method_decl_info = | _ -> [] else [] -let should_add_return_param return_type = + +let should_add_return_param return_type ~is_objc_method = match return_type with - | Sil.Tstruct _ -> false (* will return true once everything is in place *) + | Sil.Tstruct _ -> not is_objc_method + | _ -> false + +let is_objc_method function_method_decl_info = + match function_method_decl_info with + | ObjC_Meth_decl_info _ -> true | _ -> false let get_return_param tenv function_method_decl_info = + let is_objc_method = is_objc_method function_method_decl_info in let return_type_ptr = get_original_return_type function_method_decl_info in let return_typ = CTypes_decl.type_ptr_to_sil_type tenv return_type_ptr in - if should_add_return_param return_typ then + if should_add_return_param return_typ is_objc_method then [(CFrontend_config.return_param, Ast_expressions.create_pointer_type return_type_ptr)] else [] @@ -106,7 +113,8 @@ let get_parameters tenv function_method_decl_info = let get_return_type tenv function_method_decl_info = let return_type_ptr = get_original_return_type function_method_decl_info in let return_typ = CTypes_decl.type_ptr_to_sil_type tenv return_type_ptr in - if should_add_return_param return_typ then + let is_objc_method = is_objc_method function_method_decl_info in + if should_add_return_param return_typ is_objc_method then Ast_expressions.create_void_type, true else return_type_ptr, false diff --git a/infer/src/clang/cMethod_trans.mli b/infer/src/clang/cMethod_trans.mli index 91d104d5e..e500ea137 100644 --- a/infer/src/clang/cMethod_trans.mli +++ b/infer/src/clang/cMethod_trans.mli @@ -20,7 +20,7 @@ type method_call_type = | MCNoVirtual | MCStatic -val should_add_return_param : Sil.typ -> bool +val should_add_return_param : Sil.typ -> is_objc_method:bool -> bool val create_local_procdesc : Cfg.cfg -> Sil.tenv -> CMethod_signature.method_signature -> Clang_ast_t.stmt list -> (Sil.pvar * Sil.typ) list -> bool -> bool diff --git a/infer/src/clang/cTrans.ml b/infer/src/clang/cTrans.ml index 81ef184a5..5b9a7bb94 100644 --- a/infer/src/clang/cTrans.ml +++ b/infer/src/clang/cTrans.ml @@ -248,11 +248,12 @@ struct let typ = CTypes_decl.type_ptr_to_sil_type tenv type_ptr in (mk_temp_sil_var tenv procdesc var_name_prefix, typ) - let create_call_instr trans_state return_type function_sil params_sil sil_loc call_flags = + let create_call_instr trans_state return_type function_sil params_sil sil_loc + call_flags ~is_objc_method = let ret_id = if (Sil.typ_equal return_type Sil.Tvoid) then [] else [Ident.create_fresh Ident.knormal] in let ret_id_call, params, instrs_after_call, initd_exps = - if CMethod_trans.should_add_return_param return_type then + if CMethod_trans.should_add_return_param return_type is_objc_method then let param_type = Sil.Tptr (return_type, Sil.Pk_pointer) in let var_exp = match trans_state.var_exp with | Some e -> e @@ -696,7 +697,7 @@ struct (* 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 = [] } in + { trans_state_pri with succ_nodes = []; var_exp = None } 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 = @@ -743,7 +744,8 @@ struct | Some (id, instr, _) -> { empty_res_trans with ids = [id]; instrs = [instr] } | None -> let call_flags = { Sil.cf_default with Sil.cf_is_objc_block = is_call_to_block; } in - create_call_instr trans_state function_type sil_fe act_params sil_loc call_flags 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 = @@ -777,7 +779,7 @@ struct (* 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 = [] } in + { trans_state_pri with succ_nodes = []; var_exp = None } in let result_trans_subexprs = let instruction' = exec_with_self_exception (exec_with_lvalue_as_reference instruction) in let res_trans_p = IList.map (instruction' trans_state_param) params_stmt in @@ -791,7 +793,7 @@ struct Sil.cf_is_objc_block = false; } in let res_trans_call = create_call_instr trans_state_pri function_type sil_method actual_params - sil_loc call_flags in + 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 = @@ -901,7 +903,7 @@ struct let method_type_no_ref = CTypes_decl.get_type_from_expr_info expr_info context.CContext.tenv in let method_type = add_reference_if_lvalue method_type_no_ref expr_info in let trans_state_pri = PriorityNode.try_claim_priority_node trans_state si in - let trans_state_param = { trans_state_pri with succ_nodes = [] } in + let trans_state_param = { trans_state_pri with succ_nodes = []; var_exp = None } in let obj_c_message_expr_info, res_trans_subexpr_list = objCMessageExpr_deal_with_static_self trans_state_param stmt_list obj_c_message_expr_info in let subexpr_exprs = collect_exprs res_trans_subexpr_list in @@ -928,7 +930,7 @@ struct let call_flags = { Sil.cf_default with Sil.cf_virtual = is_virtual; } in let method_sil = Sil.Const (Sil.Cfun callee_name) in let res_trans_call = create_call_instr trans_state method_type method_sil param_exps - sil_loc call_flags in + sil_loc call_flags ~is_objc_method:true in let selector = obj_c_message_expr_info.Clang_ast_t.omei_selector in let nname = "Message Call: "^selector in let all_res_trans = res_trans_subexpr_list @ [res_trans_block; res_trans_call] in @@ -1697,16 +1699,28 @@ struct ret_node in let trans_result = (match stmt_list with | [stmt] -> (* return exp; *) - let trans_state' = { trans_state_pri with succ_nodes = [] } in + let procdesc = context.CContext.procdesc in + let ret_exp, var_instrs, var_ids = if context.CContext.has_return_param then + let name = CFrontend_config.return_param in + let procname = Cfg.Procdesc.get_proc_name procdesc in + let pvar = Sil.mk_pvar (Mangled.from_string name) procname in + let id = Ident.create_fresh Ident.knormal in + let ret_type_ptr = Sil.Tptr (Sil.Tvoid, Sil.Pk_pointer) in + let instr = Sil.Letderef (id, Sil.Lvar pvar, ret_type_ptr, sil_loc) in + Sil.Var id, [instr], [id] + else + Sil.Lvar (Cfg.Procdesc.get_ret_var procdesc), [], [] in + let ret_type = Cfg.Procdesc.get_ret_type procdesc in + let trans_state' = { trans_state_pri with succ_nodes = []; var_exp = Some ret_exp} in let res_trans_stmt = exec_with_self_exception instruction trans_state' stmt in let (sil_expr, sil_typ) = extract_exp_from_list res_trans_stmt.exps "WARNING: There should be only one return expression.\n" in - let ret_var = Cfg.Procdesc.get_ret_var context.CContext.procdesc in - let ret_type = Cfg.Procdesc.get_ret_type context.CContext.procdesc in - let ret_instr = Sil.Set (Sil.Lvar ret_var, ret_type, sil_expr, sil_loc) in + let ret_instrs = if IList.exists (Sil.exp_equal ret_exp) res_trans_stmt.initd_exps + then [] + else [Sil.Set (ret_exp, ret_type, sil_expr, sil_loc)] in let autorelease_ids, autorelease_instrs = add_autorelease_call context sil_expr ret_type sil_loc in - let instrs = res_trans_stmt.instrs @ [ret_instr] @ autorelease_instrs in - let ids = res_trans_stmt.ids@autorelease_ids in + let instrs = var_instrs @ res_trans_stmt.instrs @ ret_instrs @ autorelease_instrs in + let ids = var_ids @ res_trans_stmt.ids @ autorelease_ids in let ret_node = mk_ret_node ids instrs in IList.iter (fun n -> Cfg.Node.set_succs_exn n [ret_node] []) res_trans_stmt.leaf_nodes; let root_nodes_to_parent = diff --git a/infer/tests/codetoanalyze/cpp/frontend/methods/return_struct.cpp b/infer/tests/codetoanalyze/cpp/frontend/methods/return_struct.cpp new file mode 100644 index 000000000..fd57b809a --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/frontend/methods/return_struct.cpp @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2016 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +struct X { + int f; +}; + +struct A { + X get(int p) { + X x; + return x; + } +}; + +int test(A *a) { + X x = a->get(1); + return 1 / x.f; +} diff --git a/infer/tests/codetoanalyze/cpp/frontend/methods/return_struct.cpp.dot b/infer/tests/codetoanalyze/cpp/frontend/methods/return_struct.cpp.dot new file mode 100644 index 000000000..fb45ab368 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/frontend/methods/return_struct.cpp.dot @@ -0,0 +1,46 @@ +digraph iCFG { +12 [label="12: DeclStmt \n n$2=*&a:class A * [line 22]\n _fun_A_get(n$2:class A *,1:int ,&SIL_materialize_temp__n$1:struct X *) [line 22]\n n$3=*&SIL_materialize_temp__n$1:struct X [line 22]\n _fun_X_X(&x:struct X *,&SIL_materialize_temp__n$1:struct X ) [line 22]\n REMOVE_TEMPS(n$2,n$3); [line 22]\n NULLIFY(&a,false); [line 22]\n " shape="box"] + + + 12 -> 11 ; +11 [label="11: Return Stmt \n n$0=*&x.f:int [line 23]\n *&return:int =(1 / n$0) [line 23]\n REMOVE_TEMPS(n$0); [line 23]\n NULLIFY(&SIL_materialize_temp__n$1,false); [line 23]\n NULLIFY(&x,false); [line 23]\n APPLY_ABSTRACTION; [line 23]\n " shape="box"] + + + 11 -> 10 ; +10 [label="10: Exit test \n " color=yellow style=filled] + + +9 [label="9: Start test\nFormals: a:class A *\nLocals: x:struct X SIL_materialize_temp__n$1:struct X \n DECLARE_LOCALS(&return,&x,&SIL_materialize_temp__n$1); [line 21]\n " color=yellow style=filled] + + + 9 -> 12 ; +8 [label="8: DeclStmt \n _fun_X_X(&x:struct X *) [line 16]\n " shape="box"] + + + 8 -> 7 ; +7 [label="7: Return Stmt \n n$0=*&__return_param:void * [line 17]\n _fun_X_X(n$0:struct X *,&x:struct X ) [line 17]\n REMOVE_TEMPS(n$0); [line 17]\n NULLIFY(&__return_param,false); [line 17]\n NULLIFY(&x,false); [line 17]\n APPLY_ABSTRACTION; [line 17]\n " shape="box"] + + + 7 -> 6 ; +6 [label="6: Exit A_get \n " color=yellow style=filled] + + +5 [label="5: Start A_get\nFormals: this:class A * p:int __return_param:struct X *\nLocals: x:struct X \n DECLARE_LOCALS(&return,&x); [line 15]\n NULLIFY(&p,false); [line 15]\n NULLIFY(&this,false); [line 15]\n " color=yellow style=filled] + + + 5 -> 8 ; +4 [label="4: Exit X_X \n " color=yellow style=filled] + + +3 [label="3: Start X_X\nFormals: this:class X * :void \nLocals: \n DECLARE_LOCALS(&return); [line 10]\n NULLIFY(&,false); [line 10]\n NULLIFY(&this,false); [line 10]\n " color=yellow style=filled] + + + 3 -> 4 ; +2 [label="2: Exit X_X \n " color=yellow style=filled] + + +1 [label="1: Start X_X\nFormals: this:class X *\nLocals: \n DECLARE_LOCALS(&return); [line 10]\n NULLIFY(&this,false); [line 10]\n " color=yellow style=filled] + + + 1 -> 2 ; +} diff --git a/infer/tests/codetoanalyze/cpp/frontend/types/return_struct.cpp b/infer/tests/codetoanalyze/cpp/frontend/types/return_struct.cpp new file mode 100644 index 000000000..e5dcae04a --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/frontend/types/return_struct.cpp @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2016 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +struct X { + int f; +}; + + +X get(int a) { + X x; + x.f = a; + return x; +} + +int test() { + X x = get(0); + return 1 / x.f; +} diff --git a/infer/tests/codetoanalyze/cpp/frontend/types/return_struct.cpp.dot b/infer/tests/codetoanalyze/cpp/frontend/types/return_struct.cpp.dot new file mode 100644 index 000000000..0ad911ecf --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/frontend/types/return_struct.cpp.dot @@ -0,0 +1,50 @@ +digraph iCFG { +13 [label="13: DeclStmt \n _fun_get(0:int ,&SIL_materialize_temp__n$1:struct X *) [line 22]\n n$2=*&SIL_materialize_temp__n$1:struct X [line 22]\n _fun_X_X(&x:struct X *,&SIL_materialize_temp__n$1:struct X ) [line 22]\n REMOVE_TEMPS(n$2); [line 22]\n " shape="box"] + + + 13 -> 12 ; +12 [label="12: Return Stmt \n n$0=*&x.f:int [line 23]\n *&return:int =(1 / n$0) [line 23]\n REMOVE_TEMPS(n$0); [line 23]\n NULLIFY(&SIL_materialize_temp__n$1,false); [line 23]\n NULLIFY(&x,false); [line 23]\n APPLY_ABSTRACTION; [line 23]\n " shape="box"] + + + 12 -> 11 ; +11 [label="11: Exit test \n " color=yellow style=filled] + + +10 [label="10: Start test\nFormals: \nLocals: x:struct X SIL_materialize_temp__n$1:struct X \n DECLARE_LOCALS(&return,&x,&SIL_materialize_temp__n$1); [line 21]\n " color=yellow style=filled] + + + 10 -> 13 ; +9 [label="9: DeclStmt \n _fun_X_X(&x:struct X *) [line 16]\n " shape="box"] + + + 9 -> 8 ; +8 [label="8: BinaryOperatorStmt: Assign \n n$1=*&a:int [line 17]\n *&x.f:int =n$1 [line 17]\n REMOVE_TEMPS(n$1); [line 17]\n NULLIFY(&a,false); [line 17]\n " shape="box"] + + + 8 -> 7 ; +7 [label="7: Return Stmt \n n$0=*&__return_param:void * [line 18]\n _fun_X_X(n$0:struct X *,&x:struct X ) [line 18]\n REMOVE_TEMPS(n$0); [line 18]\n NULLIFY(&__return_param,false); [line 18]\n NULLIFY(&x,false); [line 18]\n APPLY_ABSTRACTION; [line 18]\n " shape="box"] + + + 7 -> 6 ; +6 [label="6: Exit get \n " color=yellow style=filled] + + +5 [label="5: Start get\nFormals: a:int __return_param:struct X *\nLocals: x:struct X \n DECLARE_LOCALS(&return,&x); [line 15]\n " color=yellow style=filled] + + + 5 -> 9 ; +4 [label="4: Exit X_X \n " color=yellow style=filled] + + +3 [label="3: Start X_X\nFormals: this:class X * :void \nLocals: \n DECLARE_LOCALS(&return); [line 10]\n NULLIFY(&,false); [line 10]\n NULLIFY(&this,false); [line 10]\n " color=yellow style=filled] + + + 3 -> 4 ; +2 [label="2: Exit X_X \n " color=yellow style=filled] + + +1 [label="1: Start X_X\nFormals: this:class X *\nLocals: \n DECLARE_LOCALS(&return); [line 10]\n NULLIFY(&this,false); [line 10]\n " color=yellow style=filled] + + + 1 -> 2 ; +} diff --git a/infer/tests/frontend/cpp/FunctionsTest.java b/infer/tests/frontend/cpp/FunctionsTest.java index 0a902f32a..a78f57032 100644 --- a/infer/tests/frontend/cpp/FunctionsTest.java +++ b/infer/tests/frontend/cpp/FunctionsTest.java @@ -41,4 +41,10 @@ public class FunctionsTest { throws InterruptedException, IOException, InferException { frontendTest("operator_overload.cpp"); } + + @Test + public void testReturnStructDotFilesMatch() + throws InterruptedException, IOException, InferException { + frontendTest("return_struct.cpp"); + } } diff --git a/infer/tests/frontend/cpp/MethodsTest.java b/infer/tests/frontend/cpp/MethodsTest.java index b5f748fce..7c5930432 100644 --- a/infer/tests/frontend/cpp/MethodsTest.java +++ b/infer/tests/frontend/cpp/MethodsTest.java @@ -58,4 +58,10 @@ public class MethodsTest { throws InterruptedException, IOException, InferException { frontendTest("static.cpp"); } + + @Test + public void testReturnStructDotFilesMatch() + throws InterruptedException, IOException, InferException { + frontendTest("return_struct.cpp"); + } }