diff --git a/Makefile b/Makefile index 0961ccc60..2d8894284 100644 --- a/Makefile +++ b/Makefile @@ -80,9 +80,9 @@ endif ifeq ($(HAS_OBJC),yes) BUILD_SYSTEMS_TESTS += objc_getters_setters objc_missing_fld objc_retain_cycles objc_retain_cycles_weak DIRECT_TESTS += \ - objc_frontend objc_errors objc_linters objc_ioslints objcpp_errors objcpp_nullable \ - objcpp_frontend objcpp_linters cpp_linters objc_linters-for-test-only objcpp_linters-for-test-only \ - objc_linters-def-folder objc_nullable objc_liveness objcpp_liveness objc_uninit + objc_frontend objc_errors objc_linters objc_ioslints objcpp_errors objcpp_nullable objcpp_retain-cycles \ + objc_linters-def-folder objc_nullable objc_liveness objcpp_liveness objc_uninit \ + objcpp_frontend objcpp_linters cpp_linters objc_linters-for-test-only objcpp_linters-for-test-only ifneq ($(XCODE_SELECT),no) BUILD_SYSTEMS_TESTS += xcodebuild_no_xcpretty endif diff --git a/infer/src/biabduction/RetainCycles.ml b/infer/src/biabduction/RetainCycles.ml index 1dd841e88..dad3ba23e 100644 --- a/infer/src/biabduction/RetainCycles.ml +++ b/infer/src/biabduction/RetainCycles.ml @@ -57,21 +57,12 @@ let desc_retain_cycle tenv (cycle : RetainCyclesType.t) = let edge_is_strong tenv obj_edge = let open RetainCyclesType in - (* returns items annotation for field fn in struct t *) - let get_item_annotation (t : Typ.t) fn = - match t.desc with - | Tstruct name -> ( - match Tenv.lookup tenv name with - | Some {fields} -> ( - match List.find ~f:(fun (fn', _, _) -> Typ.Fieldname.equal fn fn') fields with - | None -> - None - | Some (_, _, ann) -> - Some ann (* Only returns annotations when the type and field are found in the tenv *) ) - | None -> - None ) + let has_weak_type (t : Typ.t) = + match t.Typ.desc with + | Typ.Tptr (_, Typ.Pk_objc_weak) | Typ.Tptr (_, Typ.Pk_objc_unsafe_unretained) -> + true | _ -> - None + false in let has_weak_or_unretained_or_assign params = List.exists @@ -80,27 +71,37 @@ let edge_is_strong tenv obj_edge = || String.equal Config.weak att || String.equal Config.assign att ) params in - let weak_edge_by_type = - match obj_edge.rc_from.rc_node_typ.Typ.desc with - | Typ.Tptr (_, Typ.Pk_objc_weak) | Typ.Tptr (_, Typ.Pk_objc_unsafe_unretained) -> - true + let rc_field = + match obj_edge.rc_from.rc_node_typ.desc with + | Tstruct name -> ( + match Tenv.lookup tenv name with + | Some {fields} -> + List.find + ~f:(fun (fn, _, _) -> Typ.Fieldname.equal obj_edge.rc_field.rc_field_name fn) + fields + | None -> + None ) | _ -> - false - in - let weak_edge_by_field = - match get_item_annotation obj_edge.rc_from.rc_node_typ obj_edge.rc_field.rc_field_name with - | Some ia -> - List.exists - ~f:(fun ((ann : Annot.t), _) -> - ( String.equal ann.class_name Config.property_attributes - || String.equal ann.class_name Config.ivar_attributes ) - && has_weak_or_unretained_or_assign ann.parameters ) - ia - | None -> - true - (* Assume the edge is weak if the type or field cannot be found in the tenv, to avoid FPs *) + None in - not (weak_edge_by_type || weak_edge_by_field) + not + ( (* Weak edge - by type of from-node *) + has_weak_type obj_edge.rc_from.rc_node_typ + (* Weak edge - by annotation of from-node/field *) + || ( match rc_field with + | Some (_, _, ia) -> + List.exists + ~f:(fun ((ann : Annot.t), _) -> + ( String.equal ann.class_name Config.property_attributes + || String.equal ann.class_name Config.ivar_attributes ) + && has_weak_or_unretained_or_assign ann.parameters ) + ia + | _ -> + (* Assume the edge is weak if the type or field cannot be found in the tenv, to avoid FPs *) + true ) + || + (* Weak edge - by type of from-node/field *) + match rc_field with Some (_, typ, _) -> has_weak_type typ | None -> false ) exception Max_retain_cycles of RetainCyclesType.Set.t diff --git a/infer/tests/codetoanalyze/objcpp/retain-cycles/Makefile b/infer/tests/codetoanalyze/objcpp/retain-cycles/Makefile new file mode 100644 index 000000000..b6c9a29be --- /dev/null +++ b/infer/tests/codetoanalyze/objcpp/retain-cycles/Makefile @@ -0,0 +1,17 @@ +# Copyright (c) 2017-present, Facebook, Inc. +# +# This source code is licensed under the MIT license found in the +# LICENSE file in the root directory of this source tree. + +TESTS_DIR = ../../.. + +CLANG_OPTIONS = -c $(OBJCPP_CLANG_OPTIONS) -fobjc-arc +INFER_OPTIONS = --debug-exceptions --project-root $(TESTS_DIR) +INFERPRINT_OPTIONS = --issues-tests + +SOURCES = $(wildcard *.mm) + +include $(TESTS_DIR)/clang.make +include $(TESTS_DIR)/objc.make + +infer-out/report.json: $(MAKEFILE_LIST) diff --git a/infer/tests/codetoanalyze/objcpp/retain-cycles/ObjCPPStruct.mm b/infer/tests/codetoanalyze/objcpp/retain-cycles/ObjCPPStruct.mm new file mode 100644 index 000000000..4b276a15c --- /dev/null +++ b/infer/tests/codetoanalyze/objcpp/retain-cycles/ObjCPPStruct.mm @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2018-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +#import + +@interface A : NSObject +@end + +struct WeakHolder { + NSObject* ref; + __weak NSObject* weakRef; +}; + +@implementation A { + WeakHolder* _weakHolder; +} + +- (instancetype)aFnGood { + _weakHolder = new WeakHolder(); + _weakHolder->weakRef = self; + return self; +} + +- (instancetype)aFnBad { + _weakHolder = new WeakHolder(); + _weakHolder->ref = self; + return self; +} + +@end diff --git a/infer/tests/codetoanalyze/objcpp/retain-cycles/issues.exp b/infer/tests/codetoanalyze/objcpp/retain-cycles/issues.exp new file mode 100644 index 000000000..865dd4da3 --- /dev/null +++ b/infer/tests/codetoanalyze/objcpp/retain-cycles/issues.exp @@ -0,0 +1 @@ +codetoanalyze/objcpp/retain-cycles/ObjCPPStruct.mm, A_aFnBad, 2, RETAIN_CYCLE, no_bucket, ERROR, [start of procedure aFnBad,start of procedure WeakHolder,return from a call to WeakHolder_WeakHolder]