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

@@ -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
 

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

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_instrs = if IList.exists (Sil.exp_equal ret_exp) res_trans_stmt.initd_exps
-            let ret_type = Cfg.Procdesc.get_ret_type context.CContext.procdesc in
+              then []
-            let ret_instr = Sil.Set (Sil.Lvar ret_var, ret_type, sil_expr, sil_loc) in
+              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 instrs = var_instrs @ res_trans_stmt.instrs @ ret_instrs @ autorelease_instrs in
-            let ids = res_trans_stmt.ids@autorelease_ids 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 =

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;
+}

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 ;
+}

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;
+}

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 ;
+}

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");
+  }
 }

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");
+  }
 }