From f3a29d1c9c3713e31accbf695033486996e9c94b Mon Sep 17 00:00:00 2001 From: Sungkeun Cho Date: Wed, 27 Jan 2021 10:20:10 -0800 Subject: [PATCH] [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 (https://github.com/facebook/infer/commit/ec80d40bddf577ec68e5a4f15ed0d18c368f59a3)). 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 | 4 --- infer/src/clang/CMethodProperties.mli | 2 -- infer/src/clang/CType_decl.ml | 10 +++--- infer/src/clang/CType_decl.mli | 2 +- infer/src/clang/cTrans.ml | 16 ++++----- infer/src/cost/costModels.ml | 2 +- .../objc/bufferoverrun/issues.exp | 1 + .../objc/bufferoverrun/return_struct.m | 36 +++++++++++++++++++ .../objc/frontend/types/testloop.m.dot | 14 ++++---- 9 files changed, 58 insertions(+), 29 deletions(-) create mode 100644 infer/tests/codetoanalyze/objc/bufferoverrun/return_struct.m diff --git a/infer/src/clang/CMethodProperties.ml b/infer/src/clang/CMethodProperties.ml index 6a793c63a..986110aee 100644 --- a/infer/src/clang/CMethodProperties.ml +++ b/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}) -> diff --git a/infer/src/clang/CMethodProperties.mli b/infer/src/clang/CMethodProperties.mli index a0b8b047e..fdbb43fac 100644 --- a/infer/src/clang/CMethodProperties.mli +++ b/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 diff --git a/infer/src/clang/CType_decl.ml b/infer/src/clang/CType_decl.ml index 8661d17ec..a588187d2 100644 --- a/infer/src/clang/CType_decl.ml +++ b/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 = - match return_type.Typ.desc with Tstruct _ -> not is_objc_method | _ -> false + let should_add_return_param return_type = + 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 ~is_objc_method then + if should_add_return_param return_typ then (StdTyp.void, Some (CType.add_pointer_to_typ return_typ), Annot.Item.empty, true) else (return_typ, None, return_typ_annot, false) diff --git a/infer/src/clang/CType_decl.mli b/infer/src/clang/CType_decl.mli index 380c16081..1df139534 100644 --- a/infer/src/clang/CType_decl.mli +++ b/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 diff --git a/infer/src/clang/cTrans.ml b/infer/src/clang/cTrans.ml index 9f56731a9..3215986fe 100644 --- a/infer/src/clang/cTrans.ml +++ b/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]) diff --git a/infer/src/cost/costModels.ml b/infer/src/cost/costModels.ml index 9f62938c8..8134445ce 100644 --- a/infer/src/cost/costModels.ml +++ b/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 diff --git a/infer/tests/codetoanalyze/objc/bufferoverrun/issues.exp b/infer/tests/codetoanalyze/objc/bufferoverrun/issues.exp index 1989e23d5..ee8d451ad 100644 --- a/infer/tests/codetoanalyze/objc/bufferoverrun/issues.exp +++ b/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, [,Call,Assignment,,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, [,Call,Parameter `n`,Assignment,Assignment,,Array declaration,Array access: Offset: 5 Size: 3] diff --git a/infer/tests/codetoanalyze/objc/bufferoverrun/return_struct.m b/infer/tests/codetoanalyze/objc/bufferoverrun/return_struct.m new file mode 100644 index 000000000..3e9099963 --- /dev/null +++ b/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 + +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 diff --git a/infer/tests/codetoanalyze/objc/frontend/types/testloop.m.dot b/infer/tests/codetoanalyze/objc/frontend/types/testloop.m.dot index 5da4e7d40..65cb9adc0 100644 --- a/infer/tests/codetoanalyze/objc/frontend/types/testloop.m.dot +++ b/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#class.0fb14252876875c85e9253ab00bfb755_2" [label="2: Exit FBScrollViewDelegateProxy.layoutToUse \n " color=yellow style=filled] + "layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_1" -> "layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_3" ; +"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$__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#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_3" -> "layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_4" ; +"layoutToUse#FBScrollViewDelegateProxy(struct FBVideoAdLayout)#class.c0c71b15760b8f43ca05dc598adc586c_4" [label="4: Return Stmt \n n$1=*&#GB$__iPhoneVideoAdLayout.placeHolderWidth:float [line 43, column 3]\n *n$0.placeHolderWidth:float=n$1 [line 43, column 3]\n n$2=*&#GB$__iPhoneVideoAdLayout.placeHolderHeight:float [line 43, column 3]\n *n$0.placeHolderHeight:float=n$2 [line 43, column 3]\n n$3=*&#GB$__iPhoneVideoAdLayout.contentLeftSidePadding:float [line 43, column 3]\n *n$0.contentLeftSidePadding:float=n$3 [line 43, column 3]\n n$4=*&#GB$__iPhoneVideoAdLayout.contentRightSidePadding:float [line 43, column 3]\n *n$0.contentRightSidePadding:float=n$4 [line 43, column 3]\n n$5=*&#GB$__iPhoneVideoAdLayout.additionalPlaceholderOffset:float [line 43, column 3]\n *n$0.additionalPlaceholderOffset:float=n$5 [line 43, column 3]\n n$6=*&#GB$__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]