[frontend] Add a return param of struct type in ObjC methods

Summary: This diff adds an additional parameter of struct return type in ObjC's methods. The additional parameter had been supported only in C/C++ functions/methods for 5 years (D2865091 (ec80d40bdd)).  If there is no specific reason not to do that, let's do it and fix the incorrect frontend translations.

Reviewed By: jvillard

Differential Revision: D26049748

fbshipit-source-id: 414b3011f

infer/src/clang/CMethodProperties.ml

@@ -145,10 +145,6 @@ let get_pointer_to_property method_decl =
       None
 
 
-let is_objc_method method_decl =
-  match method_decl with Clang_ast_t.ObjCMethodDecl _ -> true | _ -> false
-
-
 let is_no_return method_decl =
   match Clang_ast_proj.get_function_decl_tuple method_decl with
   | Some (_, _, _, {Clang_ast_t.fdi_is_no_return= true}) ->

infer/src/clang/CMethodProperties.mli

@@ -27,8 +27,6 @@ val get_init_list_instrs : Clang_ast_t.decl -> CFrontend_config.instr_type list
 
 val get_pointer_to_property : Clang_ast_t.decl -> Clang_ast_t.pointer option
 
-val is_objc_method : Clang_ast_t.decl -> bool
-
 val is_no_return : Clang_ast_t.decl -> bool
 
 val is_variadic : Clang_ast_t.decl -> bool

infer/src/clang/CType_decl.ml

@@ -53,12 +53,11 @@ module BuildMethodSignature = struct
         raise CFrontend_errors.Invalid_declaration
 
 
-  let should_add_return_param return_type ~is_objc_method =
+  let should_add_return_param return_type =
-    match return_type.Typ.desc with Tstruct _ -> not is_objc_method | _ -> false
+    match return_type.Typ.desc with Tstruct _ -> true | _ -> false
 
 
   let get_return_param qual_type_to_sil_type tenv ~block_return_type method_decl =
-    let is_objc_method = CMethodProperties.is_objc_method method_decl in
     let return_qual_type =
       match block_return_type with
       | Some return_type ->
@@ -67,7 +66,7 @@ module BuildMethodSignature = struct
           CMethodProperties.get_return_type method_decl
     in
     let return_typ = qual_type_to_sil_type tenv return_qual_type in
-    if should_add_return_param return_typ ~is_objc_method then
+    if should_add_return_param return_typ then
       let name = Mangled.from_string CFrontend_config.return_param in
       let return_qual_type = Ast_expressions.create_pointer_qual_type return_qual_type in
       param_type_of_qual_type qual_type_to_sil_type tenv name return_qual_type
@@ -149,8 +148,7 @@ module BuildMethodSignature = struct
     in
     let return_typ = qual_type_to_sil_type tenv return_qual_type in
     let return_typ_annot = CAst_utils.sil_annot_of_type return_qual_type in
-    let is_objc_method = CMethodProperties.is_objc_method method_decl in
+    if should_add_return_param return_typ then
-    if should_add_return_param return_typ ~is_objc_method then
       (StdTyp.void, Some (CType.add_pointer_to_typ return_typ), Annot.Item.empty, true)
     else (return_typ, None, return_typ_annot, false)
 

infer/src/clang/CType_decl.mli

@@ -65,7 +65,7 @@ val method_signature_body_of_decl :
   -> Procname.t
   -> CMethodSignature.t * Clang_ast_t.stmt option * CFrontend_config.instr_type list
 
-val should_add_return_param : Typ.t -> is_objc_method:bool -> bool
+val should_add_return_param : Typ.t -> bool
 
 val type_of_captured_var :
   Tenv.t -> is_block_inside_objc_class_method:bool -> Clang_ast_t.decl_ref -> Typ.t option

infer/src/clang/cTrans.ml

@@ -233,11 +233,11 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
 
 
   let create_call_instr trans_state (return_type : Typ.t) function_sil params_sil sil_loc call_flags
-      ~is_objc_method ~is_inherited_ctor =
+      ~is_inherited_ctor =
     let ret_id_typ = (Ident.create_fresh Ident.knormal, return_type) in
     let ret_id', params, initd_exps, ret_exps, call_flags =
       (* Assumption: should_add_return_param will return true only for struct types *)
-      if CType_decl.should_add_return_param return_type ~is_objc_method then
+      if CType_decl.should_add_return_param return_type then
         let param_type = Typ.mk (Typ.Tptr (return_type, Typ.Pk_pointer)) in
         let var_exp =
           match trans_state.var_exp_typ with
@@ -1151,7 +1151,7 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
           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 ~is_inherited_ctor:false
+            ~is_inherited_ctor:false
         in
         let node_name = Procdesc.Node.Call (Exp.to_string sil_fe) in
         let all_res_trans = res_trans_callee :: (result_trans_params @ [res_trans_call]) in
@@ -1170,7 +1170,7 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
     let result_trans_param = exec_with_glvalue_as_reference instruction trans_state_param stmt in
     let res_trans_call =
       create_call_instr trans_state function_type sil_fe [result_trans_param.return] sil_loc
-        CallFlags.default ~is_objc_method:false ~is_inherited_ctor:false
+        CallFlags.default ~is_inherited_ctor:false
     in
     let node_name = Procdesc.Node.Call (Exp.to_string sil_fe) in
     let all_res_trans = [result_trans_param; res_trans_call] in
@@ -1209,7 +1209,7 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
         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 ~is_inherited_ctor
+            call_flags ~is_inherited_ctor
         in
         let node_name = Procdesc.Node.Call (Exp.to_string sil_method) in
         let all_res_trans =
@@ -1427,7 +1427,7 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
         let method_sil = Exp.Const (Const.Cfun callee_name) in
         let res_trans_call =
           create_call_instr trans_state method_type method_sil subexpr_exprs sil_loc call_flags
-            ~is_objc_method:true ~is_inherited_ctor:false
+            ~is_inherited_ctor:false
         in
         let selector = obj_c_message_expr_info.Clang_ast_t.omei_selector in
         let node_name = Procdesc.Node.MessageCall selector in
@@ -3132,7 +3132,7 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
         }
       in
       create_call_instr trans_state method_type method_sil actuals loc call_flags
-        ~is_objc_method:true ~is_inherited_ctor:false
+        ~is_inherited_ctor:false
     in
     collect_trans_results procdesc ~return:res_trans_call.return
       (res_trans_elems @ [res_trans_array; res_trans_call])
@@ -3255,7 +3255,7 @@ module CTrans_funct (F : CModule_type.CFrontend) : CModule_type.CTranslation = s
         }
       in
       create_call_instr trans_state method_type method_sil actuals loc call_flags
-        ~is_objc_method:true ~is_inherited_ctor:false
+        ~is_inherited_ctor:false
     in
     collect_trans_results procdesc ~return:res_trans_call.return
       (res_trans_elems @ [res_trans_array1; res_trans_array2; res_trans_call])

infer/src/cost/costModels.ml

@@ -231,7 +231,7 @@ module Call = struct
         ; +PatternMatch.ObjectiveC.implements "NSString"
           &:: "substringFromIndex:" <>$ capt_exp $+ capt_exp $!--> NSString.substring_from_index
         ; +PatternMatch.ObjectiveC.implements "NSString"
-          &:: "rangeOfString:" <>$ capt_exp $+ capt_exp
+          &:: "rangeOfString:" <>$ capt_exp $+ capt_exp $+ any_arg
           $!--> NSString.op_on_two_str BasicCost.mult ~of_function:"NSString.rangeOfString:"
         ; +PatternMatch.ObjectiveC.implements "NSMutableString"
           &:: "appendString:" <>$ any_arg $+ capt_exp

infer/tests/codetoanalyze/objc/bufferoverrun/issues.exp

@@ -1 +1,2 @@
 codetoanalyze/objc/bufferoverrun/block.m, Block.block_in_field_Bad, 7, BUFFER_OVERRUN_L1, no_bucket, ERROR, [<Offset trace>,Call,Assignment,<Length trace>,Array declaration,Array access: Offset: 15 Size: 10]
+codetoanalyze/objc/bufferoverrun/return_struct.m, ReturnStruct.call_get_struct_Bad, 3, BUFFER_OVERRUN_L1, no_bucket, ERROR, [<Offset trace>,Call,Parameter `n`,Assignment,Assignment,<Length trace>,Array declaration,Array access: Offset: 5 Size: 3]

infer/tests/codetoanalyze/objc/bufferoverrun/return_struct.m

@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+#import <Foundation/Foundation.h>
+
+struct s {
+  int a;
+};
+
+@interface ReturnStruct : NSObject
+@end
+
+@implementation ReturnStruct
+
+- (struct s)get_struct:(int)n {
+  struct s x;
+  x.a = n;
+  return x;
+}
+
+- (void)call_get_struct_Ok {
+  int arr[5];
+  struct s x = [self get_struct:3];
+  arr[x.a] = 0;
+}
+
+- (void)call_get_struct_Bad {
+  int arr[3];
+  struct s x = [self get_struct:5];
+  arr[x.a] = 0;
+}
+
+@end

infer/tests/codetoanalyze/objc/frontend/types/testloop.m.dot

@@ -22,21 +22,21 @@ digraph cfg {
 	
 
 	 "__infer_globals_initializer___iPhoneVideoAdLayout#774934d200ab6ea201ea7444923ebf03.1e6bd750ce4ce65119ad54cee8ee01a8_3" -> "__infer_globals_initializer___iPhoneVideoAdLayout#774934d200ab6ea201ea7444923ebf03.1e6bd750ce4ce65119ad54cee8ee01a8_2" ;
-"layoutToUse#FBScrollViewDelegateProxy#class.0fb14252876875c85e9253ab00bfb755_1" [label="1: Start FBScrollViewDelegateProxy.layoutToUse\nFormals: \nLocals:  \n  " color=yellow style=filled]
+"layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_1" [label="1: Start FBScrollViewDelegateProxy.layoutToUse\nFormals:  __return_param:FBVideoAdLayout*\nLocals:  \n  " color=yellow style=filled]
 	
 
-	 "layoutToUse#FBScrollViewDelegateProxy#class.0fb14252876875c85e9253ab00bfb755_1" -> "layoutToUse#FBScrollViewDelegateProxy#class.0fb14252876875c85e9253ab00bfb755_3" ;
+	 "layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_1" -> "layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_3" ;
-"layoutToUse#FBScrollViewDelegateProxy#class.0fb14252876875c85e9253ab00bfb755_2" [label="2: Exit FBScrollViewDelegateProxy.layoutToUse \n  " color=yellow style=filled]
+"layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_2" [label="2: Exit FBScrollViewDelegateProxy.layoutToUse \n  " color=yellow style=filled]
 	
 
-"layoutToUse#FBScrollViewDelegateProxy#class.0fb14252876875c85e9253ab00bfb755_3" [label="3:  Return Stmt \n   n$0=*&#GB<codetoanalyze/objc/frontend/types/testloop.m>$__iPhoneVideoAdLayout:FBVideoAdLayout [line 43, column 10]\n " shape="box"]
+"layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_3" [label="3:  Return Stmt \n   n$0=*&__return_param:FBVideoAdLayout* [line 43, column 3]\n " shape="box"]
 	
 
-	 "layoutToUse#FBScrollViewDelegateProxy#class.0fb14252876875c85e9253ab00bfb755_3" -> "layoutToUse#FBScrollViewDelegateProxy#class.0fb14252876875c85e9253ab00bfb755_4" ;
+	 "layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_3" -> "layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_4" ;
-"layoutToUse#FBScrollViewDelegateProxy#class.0fb14252876875c85e9253ab00bfb755_4" [label="4:  Return Stmt \n   *&return:FBVideoAdLayout=n$0 [line 43, column 3]\n " shape="box"]
+"layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_4" [label="4:  Return Stmt \n   n$1=*&#GB<codetoanalyze/objc/frontend/types/testloop.m>$__iPhoneVideoAdLayout.placeHolderWidth:float [line 43, column 3]\n  *n$0.placeHolderWidth:float=n$1 [line 43, column 3]\n  n$2=*&#GB<codetoanalyze/objc/frontend/types/testloop.m>$__iPhoneVideoAdLayout.placeHolderHeight:float [line 43, column 3]\n  *n$0.placeHolderHeight:float=n$2 [line 43, column 3]\n  n$3=*&#GB<codetoanalyze/objc/frontend/types/testloop.m>$__iPhoneVideoAdLayout.contentLeftSidePadding:float [line 43, column 3]\n  *n$0.contentLeftSidePadding:float=n$3 [line 43, column 3]\n  n$4=*&#GB<codetoanalyze/objc/frontend/types/testloop.m>$__iPhoneVideoAdLayout.contentRightSidePadding:float [line 43, column 3]\n  *n$0.contentRightSidePadding:float=n$4 [line 43, column 3]\n  n$5=*&#GB<codetoanalyze/objc/frontend/types/testloop.m>$__iPhoneVideoAdLayout.additionalPlaceholderOffset:float [line 43, column 3]\n  *n$0.additionalPlaceholderOffset:float=n$5 [line 43, column 3]\n  n$6=*&#GB<codetoanalyze/objc/frontend/types/testloop.m>$__iPhoneVideoAdLayout.contentGap:float [line 43, column 3]\n  *n$0.contentGap:float=n$6 [line 43, column 3]\n " shape="box"]
 	
 
-	 "layoutToUse#FBScrollViewDelegateProxy#class.0fb14252876875c85e9253ab00bfb755_4" -> "layoutToUse#FBScrollViewDelegateProxy#class.0fb14252876875c85e9253ab00bfb755_2" ;
+	 "layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_4" -> "layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_2" ;
 "dealloc#FBScrollViewDelegateProxy#instance.be70c6b49a0df60d48868e383f3399dc_1" [label="1: Start FBScrollViewDelegateProxy.dealloc\nFormals:  self:FBScrollViewDelegateProxy*\nLocals:  \n  " color=yellow style=filled]