diff --git a/infer/src/IR/Procname.ml b/infer/src/IR/Procname.ml index edaf95724..4bc193168 100644 --- a/infer/src/IR/Procname.ml +++ b/infer/src/IR/Procname.ml @@ -234,6 +234,12 @@ module ObjC_Cpp = struct let make_dealloc name = make name "dealloc" ObjCInstanceMethod Typ.NoTemplate [] + let make_copyWithZone ~is_mutable name = + let zone = Typ.CStruct (QualifiedCppName.of_qual_string "_NSZone") in + let method_name = if is_mutable then "mutableCopyWithZone:" else "copyWithZone:" in + make name method_name ObjCInstanceMethod Typ.NoTemplate [Parameter.clang_param_of_name zone] + + let get_class_name objc_cpp = Typ.Name.name objc_cpp.class_name let get_class_type_name objc_cpp = objc_cpp.class_name @@ -725,6 +731,8 @@ let make_java ~class_name ~return_type ~method_name ~parameters ~kind () = let make_objc_dealloc name = ObjC_Cpp (ObjC_Cpp.make_dealloc name) +let make_objc_copyWithZone ~is_mutable name = ObjC_Cpp (ObjC_Cpp.make_copyWithZone ~is_mutable name) + module Hashable = struct type nonrec t = t [@@deriving compare, equal] diff --git a/infer/src/IR/Procname.mli b/infer/src/IR/Procname.mli index 9b848ca18..e5d074f4a 100644 --- a/infer/src/IR/Procname.mli +++ b/infer/src/IR/Procname.mli @@ -265,6 +265,9 @@ val make_objc_dealloc : Typ.Name.t -> t (** Create a Objective-C dealloc name. This is a destructor for an Objective-C class. This procname is given by the class name, since it is always an instance method with the name "dealloc" *) +val make_objc_copyWithZone : is_mutable:bool -> Typ.Name.t -> t +(** Create an Objective-C method for copyWithZone: or mutableCopyWithZone: according to is_mutable. *) + val empty_block : t (** Empty block name. *) diff --git a/infer/src/IR/Typ.ml b/infer/src/IR/Typ.ml index 515432963..14d20cf91 100644 --- a/infer/src/IR/Typ.ml +++ b/infer/src/IR/Typ.ml @@ -689,3 +689,5 @@ let rec is_java_type t = let pointer_to_java_lang_object = mk_ptr (mk_struct Name.Java.java_lang_object) let pointer_to_java_lang_string = mk_ptr (mk_struct Name.Java.java_lang_string) + +let pointer_to_objc_nszone = mk_ptr (mk_struct (CStruct (QualifiedCppName.of_qual_string "NSZone"))) diff --git a/infer/src/IR/Typ.mli b/infer/src/IR/Typ.mli index be69ab72a..7de4f04cc 100644 --- a/infer/src/IR/Typ.mli +++ b/infer/src/IR/Typ.mli @@ -171,6 +171,8 @@ val pointer_to_java_lang_object : t val pointer_to_java_lang_string : t +val pointer_to_objc_nszone : t + val get_ikind_opt : t -> ikind option (** Get ikind if the type is integer. *) diff --git a/infer/src/absint/PatternMatch.ml b/infer/src/absint/PatternMatch.ml index a266027b9..655a2a8a6 100644 --- a/infer/src/absint/PatternMatch.ml +++ b/infer/src/absint/PatternMatch.ml @@ -18,6 +18,15 @@ let rec supertype_exists tenv pred name = false +(** Holds iff the predicate holds on a protocol of the named type *) +let protocol_exists tenv pred name = + match Tenv.lookup tenv name with + | Some {objc_protocols} -> + List.exists ~f:(fun name -> pred name) objc_protocols + | None -> + false + + let rec supertype_find_map_opt tenv f name = match f name with | None -> ( @@ -292,6 +301,11 @@ module ObjectiveC = struct supertype_exists tenv is_interface (Typ.Name.Objc.from_string typename) + let conforms_to ~protocol tenv typename = + let is_protocol s = String.equal protocol (Typ.Name.name s) in + protocol_exists tenv is_protocol (Typ.Name.Objc.from_string typename) + + let is_core_graphics_create_or_copy _ procname = String.is_prefix ~prefix:"CG" procname && ( String.is_substring ~substring:"Create" procname diff --git a/infer/src/absint/PatternMatch.mli b/infer/src/absint/PatternMatch.mli index 78f9915a8..4872cbf88 100644 --- a/infer/src/absint/PatternMatch.mli +++ b/infer/src/absint/PatternMatch.mli @@ -191,6 +191,9 @@ module ObjectiveC : sig val implements : string -> Tenv.t -> string -> bool (** Check whether class implements a given ObjC class *) + val conforms_to : protocol:string -> Tenv.t -> string -> bool + (** Check whether class conforms to a given ObjC protocol *) + val is_core_graphics_create_or_copy : Tenv.t -> string -> bool val is_core_foundation_create_or_copy : Tenv.t -> string -> bool diff --git a/infer/src/backend/preanal.ml b/infer/src/backend/preanal.ml index 8b3d3a573..1b7de0115 100644 --- a/infer/src/backend/preanal.ml +++ b/infer/src/backend/preanal.ml @@ -35,6 +35,95 @@ module AddAbstractionInstructions = struct Procdesc.iter_nodes do_node pdesc end +(** In ObjC, [NSObject.copy] returns the object returned by [copyWithZone:] on the given class. This + method must be implemented if the class complies with [NSCopying] protocol. Since we don't have + access to NSObject's code, to follow calls into [copyWithZone:], we replace such [copy] calls + with calls to [copyWithZone] when i) such a method exists in the class and 2) class conforms to + NSCopying protocol. + + TODO: handle calls into superclasses. + + Even though [NSObject] doesn't itself conform to [NSCopying], it supports the above pattern. + Hence, we consider all subclasses that extend it to conform to the protocol. Similarly for: + [mutableCopy] -> [mutableCopyWithZone:] for classes implementing [NSMutableCopying] protocol. *) +module ReplaceObjCCopy = struct + type copy_kind = + {protocol: string; method_name: string; method_with_zone: string; is_mutable: bool} + + let get_first_arg_typ = function + | [(_, {Typ.desc= Tptr ({desc= Tstruct objc_class}, _)})] -> + Some objc_class + | _ -> + None + + + let get_copy_kind_opt pname = + let matches_nsobject_proc method_name = + String.equal (Procname.get_method pname) method_name + && Procname.get_class_type_name pname + |> Option.exists ~f:(fun type_name -> String.equal (Typ.Name.name type_name) "NSObject") + in + if matches_nsobject_proc "copy" then + Some + { protocol= "NSCopying" + ; method_name= "copy" + ; method_with_zone= "copyWithZone:" + ; is_mutable= false } + else if matches_nsobject_proc "mutableCopy" then + Some + { protocol= "NSMutableCopying" + ; method_name= "mutableCopy" + ; method_with_zone= "mutableCopyWithZone:" + ; is_mutable= true } + else None + + + let method_exists_in_sources pdesc ~method_name ~class_name = + let pname = Procdesc.get_proc_name pdesc in + let procs = SourceFiles.get_procs_in_file pname in + List.exists procs ~f:(fun pn -> + let class_name_opt = Procname.get_class_name pn in + String.equal method_name (Procname.get_method pn) + && Option.exists class_name_opt ~f:(String.equal class_name) ) + + + let get_replaced_instr {protocol; method_name; method_with_zone; is_mutable} pdesc tenv params + instr ret_id_typ loc flags = + match get_first_arg_typ params with + | Some cl -> + let class_name = Typ.Name.name cl in + if + ( PatternMatch.ObjectiveC.conforms_to ~protocol tenv class_name + || PatternMatch.ObjectiveC.implements "NSObject" tenv class_name ) + && method_exists_in_sources pdesc ~method_name:method_with_zone ~class_name + then ( + let pname = Procname.make_objc_copyWithZone cl ~is_mutable in + let function_exp = Exp.Const (Const.Cfun pname) in + (* Zone parameter is ignored: Memory zones are no longer + used by Objective-C. We still need to satisfy the + signature though. *) + L.(debug Capture Verbose) "REPLACING %s with '%s'@\n" method_name method_with_zone ; + Sil.Call + (ret_id_typ, function_exp, params @ [(Exp.null, Typ.pointer_to_objc_nszone)], loc, flags) + ) + else instr + | _ -> + instr + + + let process tenv pdesc = + let instr_replace_copy_method _node (instr : Sil.instr) = + match instr with + | Call (ret_id_typ, Const (Cfun pn), params, loc, flags) -> + get_copy_kind_opt pn + |> Option.value_map ~default:instr ~f:(fun copy_kind -> + get_replaced_instr copy_kind pdesc tenv params instr ret_id_typ loc flags ) + | _ -> + instr + in + Procdesc.replace_instrs pdesc ~f:instr_replace_copy_method |> ignore +end + (** Find synthetic (including access and bridge) Java methods in the procedure and inline them in the cfg. @@ -376,7 +465,8 @@ let do_preanalysis exe_env pdesc = (* NOTE: It is important that this preanalysis stays before Liveness *) if not (Procname.is_java proc_name) then ( ClosuresSubstitution.process_closure_call summary ; - ClosureSubstSpecializedMethod.process summary ) ; + ClosureSubstSpecializedMethod.process summary ; + ReplaceObjCCopy.process tenv pdesc ) ; Liveness.process summary tenv ; AddAbstractionInstructions.process pdesc ; if Procname.is_java proc_name then Devirtualizer.process summary tenv ; diff --git a/infer/tests/codetoanalyze/objc/performance/copy_test.m b/infer/tests/codetoanalyze/objc/performance/copy_test.m new file mode 100644 index 000000000..dbad636d0 --- /dev/null +++ b/infer/tests/codetoanalyze/objc/performance/copy_test.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 + +@interface MyCopyObj : NSObject +@property(nonatomic, strong, nonnull) NSArray* mObjects; +@end + +@implementation MyCopyObj + +- (NSArray*)objects { + return self.mObjects; +} + +- (id)copyWithZone:(NSZone*)zone { + MyCopyObj* copy = [[MyCopyObj alloc] init]; + if (copy != nil) { + NSArray* arr = [NSArray new]; + [arr initWithArray:self.mObjects copyItems:YES]; + copy->_mObjects = arr; + } + return copy; +} + +@end + +void loop_over_copied_objects_linear(MyCopyObj* b) { + MyCopyObj* c = [b copy]; // calls into copyWithZone: + NSArray* objects = c.objects; + for (int i = 0; i < objects.count; i++) { + } +} diff --git a/infer/tests/codetoanalyze/objc/performance/copy_test_object.m b/infer/tests/codetoanalyze/objc/performance/copy_test_object.m new file mode 100644 index 000000000..2ff905f78 --- /dev/null +++ b/infer/tests/codetoanalyze/objc/performance/copy_test_object.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 + +@interface MyObj : NSObject +@property(nonatomic, strong, nonnull) NSArray* mObjects; +@end + +@implementation MyObj + +- (NSArray*)objects { + return self.mObjects; +} + +- (id)copyWithZone:(NSZone*)zone { + MyObj* copy = [[MyObj alloc] init]; + if (copy != nil) { + NSArray* arr = [NSArray new]; + [arr initWithArray:self.mObjects copyItems:YES]; + copy->_mObjects = arr; + } + return copy; +} + +@end + +void loop_over_copied_myobj_linear(MyObj* b) { + MyObj* c = [b copy]; // calls into copyWithZone: + NSArray* objects = c.objects; + for (int i = 0; i < objects.count; i++) { + } +} diff --git a/infer/tests/codetoanalyze/objc/performance/cost-issues.exp b/infer/tests/codetoanalyze/objc/performance/cost-issues.exp index f6c1353b7..051fcaf96 100644 --- a/infer/tests/codetoanalyze/objc/performance/cost-issues.exp +++ b/infer/tests/codetoanalyze/objc/performance/cost-issues.exp @@ -120,6 +120,18 @@ codetoanalyze/objc/performance/compound_loop_guard.m, simulated_while_with_and_l codetoanalyze/objc/performance/compound_loop_guard.m, while_and_or, ⊤, OnUIThread:false, [Unbounded loop,Loop] codetoanalyze/objc/performance/control.m, __infer_globals_initializer_gvar, 2, OnUIThread:false, [] codetoanalyze/objc/performance/control.m, wrong_cvar_FP, ⊤, OnUIThread:false, [Unbounded loop,Loop] +codetoanalyze/objc/performance/copy_test.m, MyCopyObj.copyWithZone:, 22, OnUIThread:false, [] +codetoanalyze/objc/performance/copy_test.m, MyCopyObj.dealloc, 4, OnUIThread:false, [] +codetoanalyze/objc/performance/copy_test.m, MyCopyObj.mObjects, 4, OnUIThread:false, [] +codetoanalyze/objc/performance/copy_test.m, MyCopyObj.objects, 7, OnUIThread:false, [] +codetoanalyze/objc/performance/copy_test.m, MyCopyObj.setMObjects:, 4, OnUIThread:false, [] +codetoanalyze/objc/performance/copy_test.m, loop_over_copied_objects_linear, 36 + 3 ⋅ b->_mObjects->elements.length.ub + 3 ⋅ (b->_mObjects->elements.length.ub + 1), OnUIThread:false, [{b->_mObjects->elements.length.ub + 1},Loop,{b->_mObjects->elements.length.ub},Loop] +codetoanalyze/objc/performance/copy_test_object.m, MyObj.copyWithZone:, 22, OnUIThread:false, [] +codetoanalyze/objc/performance/copy_test_object.m, MyObj.dealloc, 4, OnUIThread:false, [] +codetoanalyze/objc/performance/copy_test_object.m, MyObj.mObjects, 4, OnUIThread:false, [] +codetoanalyze/objc/performance/copy_test_object.m, MyObj.objects, 7, OnUIThread:false, [] +codetoanalyze/objc/performance/copy_test_object.m, MyObj.setMObjects:, 4, OnUIThread:false, [] +codetoanalyze/objc/performance/copy_test_object.m, loop_over_copied_myobj_linear, 36 + 3 ⋅ b->_mObjects->elements.length.ub + 3 ⋅ (b->_mObjects->elements.length.ub + 1), OnUIThread:false, [{b->_mObjects->elements.length.ub + 1},Loop,{b->_mObjects->elements.length.ub},Loop] codetoanalyze/objc/performance/cost_test.m, always, 9, OnUIThread:false, [] codetoanalyze/objc/performance/cost_test.m, call_infinite, ⊤, OnUIThread:false, [Call to infinite,Unbounded loop,Loop] codetoanalyze/objc/performance/cost_test.m, call_while_upto20_10_constant, 56, OnUIThread:false, [] @@ -184,6 +196,12 @@ codetoanalyze/objc/performance/loops.m, if_out_loop, 515, OnUIThread:false, [] codetoanalyze/objc/performance/loops.m, larger_state_FN, 1005, OnUIThread:false, [] codetoanalyze/objc/performance/loops.m, loop_use_global_vars, 4 + 4 ⋅ x + 2 ⋅ (1+max(0, x)), OnUIThread:false, [{1+max(0, x)},Loop,{x},Loop] codetoanalyze/objc/performance/loops.m, ptr_cmp, 5 + 5 ⋅ size + 2 ⋅ (2+max(-1, size)), OnUIThread:false, [{2+max(-1, size)},Loop,{size},Loop] +codetoanalyze/objc/performance/mutable_copy_test.m, MyMutableObj.dealloc, 4, OnUIThread:false, [] +codetoanalyze/objc/performance/mutable_copy_test.m, MyMutableObj.mObjects, 4, OnUIThread:false, [] +codetoanalyze/objc/performance/mutable_copy_test.m, MyMutableObj.mutableCopyWithZone:, 22, OnUIThread:false, [] +codetoanalyze/objc/performance/mutable_copy_test.m, MyMutableObj.objects, 7, OnUIThread:false, [] +codetoanalyze/objc/performance/mutable_copy_test.m, MyMutableObj.setMObjects:, 4, OnUIThread:false, [] +codetoanalyze/objc/performance/mutable_copy_test.m, loop_over_mutable_copy_linear, 36 + 3 ⋅ b->_mObjects->elements.length.ub + 3 ⋅ (b->_mObjects->elements.length.ub + 1), OnUIThread:false, [{b->_mObjects->elements.length.ub + 1},Loop,{b->_mObjects->elements.length.ub},Loop] codetoanalyze/objc/performance/purity.m, loop, 7007, OnUIThread:false, [] codetoanalyze/objc/performance/switch_continue.m, test_switch_FN, 601, OnUIThread:false, [] codetoanalyze/objc/performance/switch_continue.m, unroll_loop, 16 + (n - 1) + 11 ⋅ (max(1, n)), OnUIThread:false, [{max(1, n)},Loop,{n - 1},Loop] diff --git a/infer/tests/codetoanalyze/objc/performance/mutable_copy_test.m b/infer/tests/codetoanalyze/objc/performance/mutable_copy_test.m new file mode 100644 index 000000000..87e3bc041 --- /dev/null +++ b/infer/tests/codetoanalyze/objc/performance/mutable_copy_test.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 + +@interface MyMutableObj : NSObject +@property(nonatomic, strong, nonnull) NSArray* mObjects; +@end + +@implementation MyMutableObj + +- (NSArray*)objects { + return self.mObjects; +} + +- (id)mutableCopyWithZone:(NSZone*)zone { + MyMutableObj* copy = [[MyMutableObj alloc] init]; + if (copy != nil) { + NSArray* arr = [NSArray new]; + [arr initWithArray:self.mObjects copyItems:YES]; + copy->_mObjects = arr; + } + return copy; +} + +@end + +void loop_over_mutable_copy_linear(MyMutableObj* b) { + MyMutableObj* c = [b mutableCopy]; // calls into mutableCopyWithZone: + NSArray* objects = c.objects; + for (int i = 0; i < objects.count; i++) { + } +}