From 76ad9915a1d9ff39eb51625b2b937ec37b5a3069 Mon Sep 17 00:00:00 2001 From: Sungkeun Cho Date: Thu, 17 Sep 2020 09:28:25 -0700 Subject: [PATCH] [cost] Increase autoreleasepool size when non-ARC call ARC Summary: This diff increases autoreleasepool size when * caller is non-ARC-compiled * callee is ARC-compiled * return type is a pointer to objc object To distinguish non-ARC-/ARC-compiled: * extended `translation_unit_decl_info` to have a boolean field `is_objc_arc_on` * then copied it to `ProcAttributes.t` for each procedures. Reviewed By: ezgicicek Differential Revision: D23565003 fbshipit-source-id: dee22ea82 --- .../libtooling/ASTExporter.h | 5 +- .../tests/BiniouASTExporter/Hello.m.exp | 1 + .../ObjCBridgeTransferTest.m.exp | 1 + .../tests/BiniouASTExporter/ObjCTest.m.exp | 1 + .../available_expression.m.exp | 1 + .../objcpp_template_unboxing.mm.exp | 1 + .../tests/BiniouASTExporter/optional.m.exp | 1 + .../tests/JsonASTExporter/Hello.m.exp | 1 + .../ObjCBridgeTransferTest.m.exp | 1 + .../tests/JsonASTExporter/ObjCTest.m.exp | 1 + .../objcpp_template_unboxing.mm.exp | 1 + .../tests/JsonASTExporter/optional.m.exp | 1 + .../tests/YojsonASTExporter/Hello.m.exp | 1 + .../ObjCBridgeTransferTest.m.exp | 1 + .../tests/YojsonASTExporter/ObjCTest.m.exp | 1 + .../available_expression.m.exp | 1 + .../objcpp_template_unboxing.mm.exp | 1 + .../tests/YojsonASTExporter/optional.m.exp | 1 + infer/src/IR/ProcAttributes.ml | 4 ++ infer/src/IR/ProcAttributes.mli | 1 + infer/src/IR/Procdesc.ml | 2 + infer/src/IR/Procdesc.mli | 3 + infer/src/IR/Typ.ml | 2 + infer/src/IR/Typ.mli | 2 + infer/src/bufferoverrun/bounds.ml | 10 +++- infer/src/bufferoverrun/bounds.mli | 2 + infer/src/clang/Capture.ml | 3 +- infer/src/clang/cFrontend_config.ml | 5 +- infer/src/clang/cFrontend_config.mli | 5 +- infer/src/clang/cMethod_trans.ml | 1 + infer/src/cost/cost.ml | 55 ++++++++++++------ .../objc/autoreleasepool/arc_callee.h | 23 ++++++++ .../objc/autoreleasepool/arc_callee.m | 39 +++++++++++++ .../objc/autoreleasepool/cost-issues.exp | 15 +++++ .../objc/autoreleasepool/no_arc_caller.m | 57 +++++++++++++++++++ 35 files changed, 228 insertions(+), 23 deletions(-) create mode 100644 infer/tests/codetoanalyze/objc/autoreleasepool/arc_callee.h create mode 100644 infer/tests/codetoanalyze/objc/autoreleasepool/arc_callee.m create mode 100644 infer/tests/codetoanalyze/objc/autoreleasepool/no_arc_caller.m diff --git a/facebook-clang-plugins/libtooling/ASTExporter.h b/facebook-clang-plugins/libtooling/ASTExporter.h index d5e83148f..60c50e87f 100644 --- a/facebook-clang-plugins/libtooling/ASTExporter.h +++ b/facebook-clang-plugins/libtooling/ASTExporter.h @@ -1360,6 +1360,7 @@ void ASTExporter::dumpIntegerTypeWidths(const TargetInfo &info) { //@atd input_path : source_file; //@atd input_kind : input_kind; //@atd integer_type_widths : integer_type_widths; +//@atd ~is_objc_arc_on : bool; //@atd types : c_type list; //@atd } template @@ -1367,7 +1368,8 @@ void ASTExporter::VisitTranslationUnitDecl( const TranslationUnitDecl *D) { VisitDecl(D); VisitDeclContext(D); - ObjectScope Scope(OF, 4); + bool IsObjCArcOn = D->getASTContext().getLangOpts().ObjCAutoRefCount; + ObjectScope Scope(OF, 4 + IsObjCArcOn); OF.emitTag("input_path"); OF.emitString( Options.normalizeSourcePath(Options.inputFile.getFile().str().c_str())); @@ -1375,6 +1377,7 @@ void ASTExporter::VisitTranslationUnitDecl( dumpInputKind(Options.inputFile.getKind()); OF.emitTag("integer_type_widths"); dumpIntegerTypeWidths(Context.getTargetInfo()); + OF.emitFlag("is_objc_arc_on", IsObjCArcOn); OF.emitTag("types"); const auto &types = Context.getTypes(); ArrayScope aScope(OF, types.size() + 1); // + 1 for nullptr diff --git a/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/Hello.m.exp b/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/Hello.m.exp index 1a3a0cb95..2f016d64d 100644 --- a/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/Hello.m.exp +++ b/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/Hello.m.exp @@ -1363,6 +1363,7 @@ #d7cd079d: 64, #048ad2a1: 64 }, + #247f0baa: true, #1acb7079: [ <#22d546dd: ({ #d121c0bd: 67 }, <#392cef74>)>, <#22d546dd: ({ #d121c0bd: 122 }, <#2bf4b04a>)>, diff --git a/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/ObjCBridgeTransferTest.m.exp b/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/ObjCBridgeTransferTest.m.exp index c2185232f..a76621dcf 100644 --- a/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/ObjCBridgeTransferTest.m.exp +++ b/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/ObjCBridgeTransferTest.m.exp @@ -1015,6 +1015,7 @@ #d7cd079d: 64, #048ad2a1: 64 }, + #247f0baa: true, #1acb7079: [ <#22d546dd: ({ #d121c0bd: 75 }, <#392cef74>)>, <#22d546dd: ({ #d121c0bd: 92 }, <#2bf4b04a>)>, diff --git a/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/ObjCTest.m.exp b/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/ObjCTest.m.exp index 04c16a8a9..b7d44e938 100644 --- a/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/ObjCTest.m.exp +++ b/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/ObjCTest.m.exp @@ -9420,6 +9420,7 @@ #d7cd079d: 64, #048ad2a1: 64 }, + #247f0baa: true, #1acb7079: [ <#22d546dd: ({ #d121c0bd: 69 }, <#392cef74>)>, <#22d546dd: ({ #d121c0bd: 373 }, <#2bf4b04a>)>, diff --git a/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/available_expression.m.exp b/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/available_expression.m.exp index c8d33acc6..d37c63ef2 100644 --- a/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/available_expression.m.exp +++ b/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/available_expression.m.exp @@ -303,6 +303,7 @@ tests/available_expression.m:8:20: note: add a super class to fix this problem #d7cd079d: 64, #048ad2a1: 64 }, + #247f0baa: true, #1acb7079: [ <#22d546dd: ({ #d121c0bd: 18 }, <#392cef74>)>, <#22d546dd: ({ #d121c0bd: 28 }, <#2bf4b04a>)>, diff --git a/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/objcpp_template_unboxing.mm.exp b/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/objcpp_template_unboxing.mm.exp index 35a3cb4ad..d8efacc4a 100644 --- a/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/objcpp_template_unboxing.mm.exp +++ b/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/objcpp_template_unboxing.mm.exp @@ -91,6 +91,7 @@ #d7cd079d: 64, #048ad2a1: 64 }, + #247f0baa: true, #1acb7079: [ <#22d546dd: ({ #d121c0bd: 15 }, <#392cef74>)>, <#22d546dd: ({ #d121c0bd: 16 }, <#2bf4b04a>)>, diff --git a/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/optional.m.exp b/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/optional.m.exp index 2494946e3..d04eb733f 100644 --- a/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/optional.m.exp +++ b/facebook-clang-plugins/libtooling/tests/BiniouASTExporter/optional.m.exp @@ -777,6 +777,7 @@ #d7cd079d: 64, #048ad2a1: 64 }, + #247f0baa: true, #1acb7079: [ <#22d546dd: ({ #d121c0bd: 17 }, <#392cef74>)>, <#22d546dd: ({ #d121c0bd: 48 }, <#2bf4b04a>)>, diff --git a/facebook-clang-plugins/libtooling/tests/JsonASTExporter/Hello.m.exp b/facebook-clang-plugins/libtooling/tests/JsonASTExporter/Hello.m.exp index e5df30bc4..51abdfecb 100644 --- a/facebook-clang-plugins/libtooling/tests/JsonASTExporter/Hello.m.exp +++ b/facebook-clang-plugins/libtooling/tests/JsonASTExporter/Hello.m.exp @@ -2600,6 +2600,7 @@ "long_type" : 64, "longlong_type" : 64 }, + "is_objc_arc_on" : true, "types" : [ ["BuiltinType" , [ { diff --git a/facebook-clang-plugins/libtooling/tests/JsonASTExporter/ObjCBridgeTransferTest.m.exp b/facebook-clang-plugins/libtooling/tests/JsonASTExporter/ObjCBridgeTransferTest.m.exp index 6b60882d3..4df9b4453 100644 --- a/facebook-clang-plugins/libtooling/tests/JsonASTExporter/ObjCBridgeTransferTest.m.exp +++ b/facebook-clang-plugins/libtooling/tests/JsonASTExporter/ObjCBridgeTransferTest.m.exp @@ -1983,6 +1983,7 @@ "long_type" : 64, "longlong_type" : 64 }, + "is_objc_arc_on" : true, "types" : [ ["BuiltinType" , [ { diff --git a/facebook-clang-plugins/libtooling/tests/JsonASTExporter/ObjCTest.m.exp b/facebook-clang-plugins/libtooling/tests/JsonASTExporter/ObjCTest.m.exp index 803c256ea..5ec6aef38 100644 --- a/facebook-clang-plugins/libtooling/tests/JsonASTExporter/ObjCTest.m.exp +++ b/facebook-clang-plugins/libtooling/tests/JsonASTExporter/ObjCTest.m.exp @@ -12362,6 +12362,7 @@ "long_type" : 64, "longlong_type" : 64 }, + "is_objc_arc_on" : true, "types" : [ ["BuiltinType" , [ { diff --git a/facebook-clang-plugins/libtooling/tests/JsonASTExporter/objcpp_template_unboxing.mm.exp b/facebook-clang-plugins/libtooling/tests/JsonASTExporter/objcpp_template_unboxing.mm.exp index 825947cfe..4b5acb0fd 100644 --- a/facebook-clang-plugins/libtooling/tests/JsonASTExporter/objcpp_template_unboxing.mm.exp +++ b/facebook-clang-plugins/libtooling/tests/JsonASTExporter/objcpp_template_unboxing.mm.exp @@ -260,6 +260,7 @@ "long_type" : 64, "longlong_type" : 64 }, + "is_objc_arc_on" : true, "types" : [ ["BuiltinType" , [ { diff --git a/facebook-clang-plugins/libtooling/tests/JsonASTExporter/optional.m.exp b/facebook-clang-plugins/libtooling/tests/JsonASTExporter/optional.m.exp index cdaab24f9..c92a56129 100644 --- a/facebook-clang-plugins/libtooling/tests/JsonASTExporter/optional.m.exp +++ b/facebook-clang-plugins/libtooling/tests/JsonASTExporter/optional.m.exp @@ -1302,6 +1302,7 @@ "long_type" : 64, "longlong_type" : 64 }, + "is_objc_arc_on" : true, "types" : [ ["BuiltinType" , [ { diff --git a/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/Hello.m.exp b/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/Hello.m.exp index 06a01b631..ffbdeaf5d 100644 --- a/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/Hello.m.exp +++ b/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/Hello.m.exp @@ -2600,6 +2600,7 @@ "long_type" : 64, "longlong_type" : 64 }, + "is_objc_arc_on" : true, "types" : [ <"BuiltinType" : ( { diff --git a/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/ObjCBridgeTransferTest.m.exp b/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/ObjCBridgeTransferTest.m.exp index 5a60b67ce..0cf9be373 100644 --- a/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/ObjCBridgeTransferTest.m.exp +++ b/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/ObjCBridgeTransferTest.m.exp @@ -1983,6 +1983,7 @@ "long_type" : 64, "longlong_type" : 64 }, + "is_objc_arc_on" : true, "types" : [ <"BuiltinType" : ( { diff --git a/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/ObjCTest.m.exp b/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/ObjCTest.m.exp index cc3541e64..a2362c0ed 100644 --- a/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/ObjCTest.m.exp +++ b/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/ObjCTest.m.exp @@ -12362,6 +12362,7 @@ "long_type" : 64, "longlong_type" : 64 }, + "is_objc_arc_on" : true, "types" : [ <"BuiltinType" : ( { diff --git a/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/available_expression.m.exp b/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/available_expression.m.exp index 77b5e6117..8fc5c1a58 100644 --- a/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/available_expression.m.exp +++ b/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/available_expression.m.exp @@ -676,6 +676,7 @@ tests/available_expression.m:8:20: note: add a super class to fix this problem "long_type" : 64, "longlong_type" : 64 }, + "is_objc_arc_on" : true, "types" : [ <"BuiltinType" : ( { diff --git a/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/objcpp_template_unboxing.mm.exp b/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/objcpp_template_unboxing.mm.exp index 47ba89b98..ee1f37b85 100644 --- a/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/objcpp_template_unboxing.mm.exp +++ b/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/objcpp_template_unboxing.mm.exp @@ -260,6 +260,7 @@ "long_type" : 64, "longlong_type" : 64 }, + "is_objc_arc_on" : true, "types" : [ <"BuiltinType" : ( { diff --git a/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/optional.m.exp b/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/optional.m.exp index 6b82b7669..39dd4c07d 100644 --- a/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/optional.m.exp +++ b/facebook-clang-plugins/libtooling/tests/YojsonASTExporter/optional.m.exp @@ -1302,6 +1302,7 @@ "long_type" : 64, "longlong_type" : 64 }, + "is_objc_arc_on" : true, "types" : [ <"BuiltinType" : ( { diff --git a/infer/src/IR/ProcAttributes.ml b/infer/src/IR/ProcAttributes.ml index a2271166a..5f67e6ad6 100644 --- a/infer/src/IR/ProcAttributes.ml +++ b/infer/src/IR/ProcAttributes.ml @@ -59,6 +59,7 @@ type t = (** Present if the procedure is an Objective-C block that has been passed to the given method in a position annotated with the NS_NOESCAPE attribute. *) ; is_no_return: bool (** the procedure is known not to return *) + ; is_objc_arc_on: bool (** the ObjC procedure is compiled with ARC *) ; is_specialized: bool (** the procedure is a clone specialized for dynamic dispatch handling *) ; is_synthetic_method: bool (** the procedure is a synthetic method *) ; is_variadic: bool (** the procedure is variadic, only supported for Clang procedures *) @@ -123,6 +124,7 @@ let default translation_unit proc_name = ; is_java_synchronized_method= false ; passed_as_noescape_block_to= None ; is_no_return= false + ; is_objc_arc_on= false ; is_specialized= false ; specialized_with_blocks_info= None ; is_synthetic_method= false @@ -173,6 +175,7 @@ let pp f ; is_java_synchronized_method ; passed_as_noescape_block_to ; is_no_return + ; is_objc_arc_on ; is_specialized ; specialized_with_blocks_info ; is_synthetic_method @@ -221,6 +224,7 @@ let pp f F.fprintf f "; passed_as_noescape_block_to %a" (Pp.option Procname.pp) passed_as_noescape_block_to ; pp_bool_default ~default:default.is_no_return "is_no_return" is_no_return f () ; + pp_bool_default ~default:default.is_objc_arc_on "is_objc_arc_on" is_objc_arc_on f () ; pp_bool_default ~default:default.is_specialized "is_specialized" is_specialized f () ; if not diff --git a/infer/src/IR/ProcAttributes.mli b/infer/src/IR/ProcAttributes.mli index f7e19d69a..66bcc1291 100644 --- a/infer/src/IR/ProcAttributes.mli +++ b/infer/src/IR/ProcAttributes.mli @@ -41,6 +41,7 @@ type t = (** Present if the procedure is an Objective-C block that has been passed to the given method in a position annotated with the NS_NOESCAPE attribute. *) ; is_no_return: bool (** the procedure is known not to return *) + ; is_objc_arc_on: bool (** the ObjC procedure is compiled with ARC *) ; is_specialized: bool (** the procedure is a clone specialized for dynamic dispatch handling *) ; is_synthetic_method: bool (** the procedure is a synthetic method *) ; is_variadic: bool (** the procedure is variadic, only supported for Clang procedures *) diff --git a/infer/src/IR/Procdesc.ml b/infer/src/IR/Procdesc.ml index 3cc38c74f..5464dbfe9 100644 --- a/infer/src/IR/Procdesc.ml +++ b/infer/src/IR/Procdesc.ml @@ -529,6 +529,8 @@ let is_defined pdesc = pdesc.attributes.is_defined let is_java_synchronized pdesc = pdesc.attributes.is_java_synchronized_method +let is_objc_arc_on pdesc = pdesc.attributes.is_objc_arc_on + let iter_nodes f pdesc = List.iter ~f (get_nodes pdesc) let iter_instrs f pdesc = diff --git a/infer/src/IR/Procdesc.mli b/infer/src/IR/Procdesc.mli index dbcbcf2fe..eae14467d 100644 --- a/infer/src/IR/Procdesc.mli +++ b/infer/src/IR/Procdesc.mli @@ -271,6 +271,9 @@ val is_defined : t -> bool val is_java_synchronized : t -> bool (** Return [true] if the procedure signature has the Java synchronized keyword *) +val is_objc_arc_on : t -> bool +(** Return [true] iff the ObjC procedure is compiled with ARC *) + val iter_instrs : (Node.t -> Sil.instr -> unit) -> t -> unit (** iterate over all nodes and their instructions *) diff --git a/infer/src/IR/Typ.ml b/infer/src/IR/Typ.ml index 508ed59f9..034c19308 100644 --- a/infer/src/IR/Typ.ml +++ b/infer/src/IR/Typ.ml @@ -569,6 +569,8 @@ let is_struct typ = match typ.desc with Tstruct _ -> true | _ -> false let is_pointer_to_cpp_class typ = match typ.desc with Tptr (t, _) -> is_cpp_class t | _ -> false +let is_pointer_to_objc_class typ = match typ.desc with Tptr (t, _) -> is_objc_class t | _ -> false + let is_pointer_to_void typ = match typ.desc with Tptr ({desc= Tvoid}, _) -> true | _ -> false let is_void typ = match typ.desc with Tvoid -> true | _ -> false diff --git a/infer/src/IR/Typ.mli b/infer/src/IR/Typ.mli index e51953929..5c527d22f 100644 --- a/infer/src/IR/Typ.mli +++ b/infer/src/IR/Typ.mli @@ -324,6 +324,8 @@ val is_cpp_class : t -> bool val is_pointer_to_cpp_class : t -> bool +val is_pointer_to_objc_class : t -> bool + val is_pointer_to_void : t -> bool val is_void : t -> bool diff --git a/infer/src/bufferoverrun/bounds.ml b/infer/src/bufferoverrun/bounds.ml index 0e3de67a1..d5d93c63d 100644 --- a/infer/src/bufferoverrun/bounds.ml +++ b/infer/src/bufferoverrun/bounds.ml @@ -1291,10 +1291,11 @@ module BoundTrace = struct | Loop of Location.t | Call of {callee_pname: Procname.t; callee_trace: t; location: Location.t} | ModeledFunction of {pname: string; location: Location.t} + | ArcFromNonArc of {pname: string; location: Location.t} [@@deriving compare] let rec length = function - | Loop _ | ModeledFunction _ -> + | Loop _ | ModeledFunction _ | ArcFromNonArc _ -> 1 | Call {callee_trace} -> 1 + length callee_trace @@ -1309,6 +1310,8 @@ module BoundTrace = struct F.fprintf f "Loop (%a)" Location.pp loc | ModeledFunction {pname; location} -> F.fprintf f "ModeledFunction `%s` (%a)" pname Location.pp location + | ArcFromNonArc {pname; location} -> + F.fprintf f "ArcFromNonArc `%s` (%a)" pname Location.pp location | Call {callee_pname; callee_trace; location} -> F.fprintf f "%a -> Call `%a` (%a)" pp callee_trace Procname.pp callee_pname Location.pp location @@ -1327,11 +1330,16 @@ module BoundTrace = struct | ModeledFunction {pname; location} -> let desc = F.asprintf "Modeled call to %s" pname in [Errlog.make_trace_element depth location desc []] + | ArcFromNonArc {pname; location} -> + let desc = F.asprintf "ARC function call to %s from non-ARC caller" pname in + [Errlog.make_trace_element depth location desc []] let of_loop location = Loop location let of_modeled_function pname location = ModeledFunction {pname; location} + + let of_arc_from_non_arc pname location = ArcFromNonArc {pname; location} end (** A NonNegativeBound is a Bound that is either non-negative or symbolic but will be evaluated to a diff --git a/infer/src/bufferoverrun/bounds.mli b/infer/src/bufferoverrun/bounds.mli index 8aba1f5d6..4c975e0ab 100644 --- a/infer/src/bufferoverrun/bounds.mli +++ b/infer/src/bufferoverrun/bounds.mli @@ -156,6 +156,8 @@ module BoundTrace : sig val of_loop : Location.t -> t val of_modeled_function : string -> Location.t -> t + + val of_arc_from_non_arc : string -> Location.t -> t end type ('c, 's, 't) valclass = Constant of 'c | Symbolic of 's | ValTop of 't diff --git a/infer/src/clang/Capture.ml b/infer/src/clang/Capture.ml index 1f50ca39a..05e46052b 100644 --- a/infer/src/clang/Capture.ml +++ b/infer/src/clang/Capture.ml @@ -69,7 +69,8 @@ let run_clang_frontend ast_source = ; long_width= widths.itw_long_type ; longlong_width= widths.itw_longlong_type } in - {CFrontend_config.source_file; lang; integer_type_widths} + let is_objc_arc_on = info.Clang_ast_t.tudi_is_objc_arc_on in + {CFrontend_config.source_file; lang; integer_type_widths; is_objc_arc_on} | _ -> assert false in diff --git a/infer/src/clang/cFrontend_config.ml b/infer/src/clang/cFrontend_config.ml index bce29b450..4bbb6bc66 100644 --- a/infer/src/clang/cFrontend_config.ml +++ b/infer/src/clang/cFrontend_config.ml @@ -14,7 +14,10 @@ type clang_lang = C | CPP | ObjC | ObjCPP [@@deriving compare] let equal_clang_lang = [%compare.equal: clang_lang] type translation_unit_context = - {lang: clang_lang; source_file: SourceFile.t; integer_type_widths: Typ.IntegerWidths.t} + { lang: clang_lang + ; source_file: SourceFile.t + ; integer_type_widths: Typ.IntegerWidths.t + ; is_objc_arc_on: bool } type decl_trans_context = [`DeclTraversal | `Translation | `CppLambdaExprTranslation] diff --git a/infer/src/clang/cFrontend_config.mli b/infer/src/clang/cFrontend_config.mli index 6cd0eb9fa..9ae92868a 100644 --- a/infer/src/clang/cFrontend_config.mli +++ b/infer/src/clang/cFrontend_config.mli @@ -14,7 +14,10 @@ type clang_lang = C | CPP | ObjC | ObjCPP [@@deriving compare] val equal_clang_lang : clang_lang -> clang_lang -> bool type translation_unit_context = - {lang: clang_lang; source_file: SourceFile.t; integer_type_widths: Typ.IntegerWidths.t} + { lang: clang_lang + ; source_file: SourceFile.t + ; integer_type_widths: Typ.IntegerWidths.t + ; is_objc_arc_on: bool } type decl_trans_context = [`DeclTraversal | `Translation | `CppLambdaExprTranslation] diff --git a/infer/src/clang/cMethod_trans.ml b/infer/src/clang/cMethod_trans.ml index 19822c5de..2dd3a56c1 100644 --- a/infer/src/clang/cMethod_trans.ml +++ b/infer/src/clang/cMethod_trans.ml @@ -267,6 +267,7 @@ let create_local_procdesc ?(set_objc_accessor_attr = false) ?(record_lambda_capt ; is_biabduction_model= Config.biabduction_models_mode ; passed_as_noescape_block_to= ms.CMethodSignature.passed_as_noescape_block_to ; is_no_return= ms.CMethodSignature.is_no_return + ; is_objc_arc_on= trans_unit_ctx.CFrontend_config.is_objc_arc_on ; is_variadic= ms.CMethodSignature.is_variadic ; sentinel_attr= find_sentinel_attribute ms.CMethodSignature.attributes ; loc= loc_start diff --git a/infer/src/cost/cost.ml b/infer/src/cost/cost.ml index 21fa88a0b..7ca209885 100644 --- a/infer/src/cost/cost.ml +++ b/infer/src/cost/cost.ml @@ -22,7 +22,8 @@ type extras_WorstCaseCost = ; inferbo_get_summary: BufferOverrunAnalysisSummary.get_summary ; get_node_nb_exec: Node.t -> BasicCost.t ; get_summary: Procname.t -> CostDomain.summary option - ; get_formals: Procname.t -> (Pvar.t * Typ.t) list option } + ; get_formals: Procname.t -> (Pvar.t * Typ.t) list option + ; proc_resolve_attributes: Procname.t -> ProcAttributes.t option } let instantiate_cost integer_type_widths ~inferbo_caller_mem ~callee_pname ~callee_formals ~params ~callee_cost ~loc = @@ -55,9 +56,15 @@ module InstrBasicCostWithReason = struct String.equal (Procname.get_method callee_pname) "autorelease" - let get_instr_cost_record tenv extras instr_node instr = + let is_objc_call_from_no_arc_to_arc {proc_resolve_attributes} caller_pdesc callee_pname = + (not (Procdesc.is_objc_arc_on caller_pdesc)) + && Option.exists (proc_resolve_attributes callee_pname) + ~f:(fun {ProcAttributes.is_objc_arc_on} -> is_objc_arc_on) + + + let get_instr_cost_record tenv extras cfg instr_node instr = match instr with - | Sil.Call (ret, Exp.Const (Const.Cfun callee_pname), params, location, _) + | Sil.Call (((_, ret_typ) as ret), Exp.Const (Const.Cfun callee_pname), params, location, _) when Config.inclusive_cost -> let { inferbo_invariant_map ; integer_type_widths @@ -66,7 +73,7 @@ module InstrBasicCostWithReason = struct ; get_formals } = extras in - let operation_cost = + let cost = match BufferOverrunAnalysis.extract_pre (InstrCFG.Node.id instr_node) inferbo_invariant_map with @@ -107,14 +114,25 @@ module InstrBasicCostWithReason = struct callee_pname) ; CostDomain.unit_cost_atomic_operation ) ) in - if is_allocation_function callee_pname then - CostDomain.plus CostDomain.unit_cost_allocation operation_cost - else if is_autorelease_function callee_pname then - CostDomain.plus - (CostDomain.unit_cost_autoreleasepool_size - ~autoreleasepool_trace:(Bounds.BoundTrace.of_modeled_function "autorelease" location)) - operation_cost - else operation_cost + let cost = + if is_allocation_function callee_pname then + CostDomain.plus CostDomain.unit_cost_allocation cost + else cost + in + if is_autorelease_function callee_pname then + let autoreleasepool_trace = + Bounds.BoundTrace.of_modeled_function "autorelease" location + in + CostDomain.plus cost (CostDomain.unit_cost_autoreleasepool_size ~autoreleasepool_trace) + else if + is_objc_call_from_no_arc_to_arc extras cfg callee_pname + && Typ.is_pointer_to_objc_class ret_typ + then + let autoreleasepool_trace = + Bounds.BoundTrace.of_arc_from_non_arc (Procname.to_string callee_pname) location + in + CostDomain.plus cost (CostDomain.unit_cost_autoreleasepool_size ~autoreleasepool_trace) + else cost | Sil.Call (_, Exp.Const (Const.Cfun _), _, _, _) -> CostDomain.zero_record | Sil.Load {id= lhs_id} when Ident.is_none lhs_id -> @@ -134,7 +152,7 @@ module InstrBasicCostWithReason = struct CostDomain.zero_record - let get_instr_node_cost_record tenv extras instr_node = + let get_instr_node_cost_record tenv extras cfg instr_node = let instrs = InstrCFG.instrs instr_node in let instr = match IContainer.singleton_or_more instrs ~fold:Instrs.fold with @@ -145,7 +163,7 @@ module InstrBasicCostWithReason = struct | More -> assert false in - let cost = get_instr_cost_record tenv extras instr_node instr in + let cost = get_instr_cost_record tenv extras cfg instr_node instr in let operation_cost = CostDomain.get_operation_cost cost in let log_msg top_or_bottom = Logging.d_printfln_escaped "Statement cost became %s at %a (%a)." top_or_bottom @@ -169,10 +187,10 @@ let compute_errlog_extras cost = module WorstCaseCost = struct (** We don't report when the cost is Top as it corresponds to subsequent 'don't know's. Instead, we report Top cost only at the top level per function. *) - let exec_node tenv extras instr_node = + let exec_node tenv extras cfg instr_node = let {get_node_nb_exec} = extras in let instr_cost_record = - InstrBasicCostWithReason.get_instr_node_cost_record tenv extras instr_node + InstrBasicCostWithReason.get_instr_node_cost_record tenv extras cfg instr_node in let node = InstrCFG.Node.underlying_node instr_node in let nb_exec = get_node_nb_exec node in @@ -187,7 +205,7 @@ module WorstCaseCost = struct let cost = let nodes_in_autoreleasepool = CostUtils.get_nodes_in_autoreleasepool cfg in InstrCFG.fold_nodes cfg ~init ~f:(fun acc ((node, _) as pair) -> - let cost = exec_node tenv extras pair in + let cost = exec_node tenv extras cfg pair in let cost = if Procdesc.NodeSet.mem node nodes_in_autoreleasepool then CostDomain.set_autoreleasepool_size_zero cost @@ -326,7 +344,8 @@ let checker ({InterproceduralAnalysis.proc_desc; exe_env; analyze_dependency} as ; integer_type_widths ; get_node_nb_exec ; get_summary - ; get_formals } + ; get_formals + ; proc_resolve_attributes= AnalysisCallbacks.proc_resolve_attributes } in AnalysisCallbacks.html_debug_new_node_session (NodeCFG.start_node node_cfg) ~pp_name:(fun f -> F.pp_print_string f "cost(worst-case)") diff --git a/infer/tests/codetoanalyze/objc/autoreleasepool/arc_callee.h b/infer/tests/codetoanalyze/objc/autoreleasepool/arc_callee.h new file mode 100644 index 000000000..c12542868 --- /dev/null +++ b/infer/tests/codetoanalyze/objc/autoreleasepool/arc_callee.h @@ -0,0 +1,23 @@ +/* + * 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 + +@interface ArcCallee : NSObject + ++ (ArcCallee*)allocObject; + ++ (ArcCallee*)newObject; + ++ (ArcCallee*)copyObject:(ArcCallee*)obj; + ++ (ArcCallee*)mutableCopyObject:(ArcCallee*)obj; + ++ (ArcCallee*)giveMeObject; + ++ (int)giveMeInt; + +@end diff --git a/infer/tests/codetoanalyze/objc/autoreleasepool/arc_callee.m b/infer/tests/codetoanalyze/objc/autoreleasepool/arc_callee.m new file mode 100644 index 000000000..35de0efa1 --- /dev/null +++ b/infer/tests/codetoanalyze/objc/autoreleasepool/arc_callee.m @@ -0,0 +1,39 @@ +/* + * 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 +#import "arc_callee.h" + +@implementation ArcCallee + ++ (ArcCallee*)allocObject { + return [ArcCallee alloc]; +} + ++ (ArcCallee*)newObject { + return [ArcCallee new]; +} + +/* Since the checker cares about calling `autorelease` only, we define the + following functions as returning a new object simply. */ + ++ (ArcCallee*)copyObject:(ArcCallee*)obj { + return [ArcCallee new]; +} + ++ (ArcCallee*)mutableCopyObject:(ArcCallee*)obj { + return [ArcCallee new]; +} + ++ (ArcCallee*)giveMeObject { + return [ArcCallee new]; +} + ++ (int)giveMeInt { + return 100; +} + +@end diff --git a/infer/tests/codetoanalyze/objc/autoreleasepool/cost-issues.exp b/infer/tests/codetoanalyze/objc/autoreleasepool/cost-issues.exp index 4f32fd95d..f7620d2c8 100644 --- a/infer/tests/codetoanalyze/objc/autoreleasepool/cost-issues.exp +++ b/infer/tests/codetoanalyze/objc/autoreleasepool/cost-issues.exp @@ -2,6 +2,13 @@ ${XCODE_ISYSROOT}/System/Library/Frameworks/Foundation.framework/Headers/NSArray codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.callIndexOfObjectPassingTest_linear_FN:, 0, OnUIThread:false, [] codetoanalyze/objc/autoreleasepool/arc_block.m, ArcBlock.dealloc, 0, OnUIThread:false, [] codetoanalyze/objc/autoreleasepool/arc_block.m, objc_blockArcBlock.callIndexOfObjectPassingTest_linear_FN:_1, 1, OnUIThread:false, [autorelease,Call to NoArcCallee.giveMeObject,Modeled call to autorelease] +codetoanalyze/objc/autoreleasepool/arc_callee.m, ArcCallee.allocObject, 0, OnUIThread:false, [] +codetoanalyze/objc/autoreleasepool/arc_callee.m, ArcCallee.copyObject:, 0, OnUIThread:false, [] +codetoanalyze/objc/autoreleasepool/arc_callee.m, ArcCallee.dealloc, 0, OnUIThread:false, [] +codetoanalyze/objc/autoreleasepool/arc_callee.m, ArcCallee.giveMeInt, 0, OnUIThread:false, [] +codetoanalyze/objc/autoreleasepool/arc_callee.m, ArcCallee.giveMeObject, 0, OnUIThread:false, [] +codetoanalyze/objc/autoreleasepool/arc_callee.m, ArcCallee.mutableCopyObject:, 0, OnUIThread:false, [] +codetoanalyze/objc/autoreleasepool/arc_callee.m, ArcCallee.newObject, 0, OnUIThread:false, [] codetoanalyze/objc/autoreleasepool/arc_caller.m, ArcCaller.callAllocObject_zero:, 0, OnUIThread:false, [] codetoanalyze/objc/autoreleasepool/arc_caller.m, ArcCaller.callCopyObject_zero:x:, 0, OnUIThread:false, [] codetoanalyze/objc/autoreleasepool/arc_caller.m, ArcCaller.callGiveMeObject_autoreleasepool_zero:, 0, OnUIThread:false, [] @@ -27,3 +34,11 @@ codetoanalyze/objc/autoreleasepool/no_arc_callee.m, NoArcCallee.dealloc, 0, OnU codetoanalyze/objc/autoreleasepool/no_arc_callee.m, NoArcCallee.giveMeObject, 1, OnUIThread:false, [autorelease,Modeled call to autorelease] codetoanalyze/objc/autoreleasepool/no_arc_callee.m, NoArcCallee.mutableCopyObject:, 0, OnUIThread:false, [] codetoanalyze/objc/autoreleasepool/no_arc_callee.m, NoArcCallee.newObject, 0, OnUIThread:false, [] +codetoanalyze/objc/autoreleasepool/no_arc_caller.m, NoArcCaller.callAllocObject_zero_FP:, n, OnUIThread:false, [{n},Loop,autorelease,ARC function call to ArcCallee.allocObject from non-ARC caller] +codetoanalyze/objc/autoreleasepool/no_arc_caller.m, NoArcCaller.callCopyObject_zero_FP:x:, n, OnUIThread:false, [{n},Loop,autorelease,ARC function call to ArcCallee.copyObject: from non-ARC caller] +codetoanalyze/objc/autoreleasepool/no_arc_caller.m, NoArcCaller.callGiveMeInt_zero, 0, OnUIThread:false, [] +codetoanalyze/objc/autoreleasepool/no_arc_caller.m, NoArcCaller.callGiveMeObject_autoreleasepool_zero:, 0, OnUIThread:false, [] +codetoanalyze/objc/autoreleasepool/no_arc_caller.m, NoArcCaller.callGiveMeObject_linear:, n, OnUIThread:false, [{n},Loop,autorelease,ARC function call to ArcCallee.giveMeObject from non-ARC caller] +codetoanalyze/objc/autoreleasepool/no_arc_caller.m, NoArcCaller.callMutableCopyObject_zero_FP:x:, n, OnUIThread:false, [{n},Loop,autorelease,ARC function call to ArcCallee.mutableCopyObject: from non-ARC caller] +codetoanalyze/objc/autoreleasepool/no_arc_caller.m, NoArcCaller.callNewObject_zero_FP:, n, OnUIThread:false, [{n},Loop,autorelease,ARC function call to ArcCallee.newObject from non-ARC caller] +codetoanalyze/objc/autoreleasepool/no_arc_caller.m, NoArcCaller.dealloc, 0, OnUIThread:false, [] diff --git a/infer/tests/codetoanalyze/objc/autoreleasepool/no_arc_caller.m b/infer/tests/codetoanalyze/objc/autoreleasepool/no_arc_caller.m new file mode 100644 index 000000000..5a04669cd --- /dev/null +++ b/infer/tests/codetoanalyze/objc/autoreleasepool/no_arc_caller.m @@ -0,0 +1,57 @@ +/* + * 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 +#import "arc_callee.h" + +@interface NoArcCaller : NSObject +@end + +@implementation NoArcCaller + +- (void)callAllocObject_zero_FP:(int)n { + for (int i = 0; i < n; i++) { + ArcCallee* obj = [ArcCallee allocObject]; + } +} + +- (void)callNewObject_zero_FP:(int)n { + for (int i = 0; i < n; i++) { + ArcCallee* obj = [ArcCallee newObject]; + } +} + +- (void)callCopyObject_zero_FP:(int)n x:(ArcCallee*)x { + for (int i = 0; i < n; i++) { + ArcCallee* obj = [ArcCallee copyObject:x]; + } +} + +- (void)callMutableCopyObject_zero_FP:(int)n x:(ArcCallee*)x { + for (int i = 0; i < n; i++) { + ArcCallee* obj = [ArcCallee mutableCopyObject:x]; + } +} + +- (void)callGiveMeObject_linear:(int)n { + for (int i = 0; i < n; i++) { + ArcCallee* obj = [ArcCallee giveMeObject]; + } +} + +- (void)callGiveMeObject_autoreleasepool_zero:(int)n { + for (int i = 0; i < n; i++) { + @autoreleasepool { + ArcCallee* obj = [ArcCallee giveMeObject]; + } + } +} + +- (void)callGiveMeInt_zero { + int i = [ArcCallee giveMeInt]; +} + +@end