[objc] fixing false positive for weak pointers inside c++ structs

Reviewed By: jvillard

Differential Revision: D10401459

fbshipit-source-id: a0495426e

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 \
+  objc_frontend objc_errors objc_linters objc_ioslints objcpp_errors objcpp_nullable objcpp_retain-cycles \
-	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_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

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 has_weak_type (t : Typ.t) =
-  let get_item_annotation (t : Typ.t) fn =
+    match t.Typ.desc with
-    match t.desc with
+    | Typ.Tptr (_, Typ.Pk_objc_weak) | Typ.Tptr (_, Typ.Pk_objc_unsafe_unretained) ->
-    | Tstruct name -> (
+        true
-      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 )
     | _ ->
-        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 =
+  let rc_field =
-    match obj_edge.rc_from.rc_node_typ.Typ.desc with
+    match obj_edge.rc_from.rc_node_typ.desc with
-    | Typ.Tptr (_, Typ.Pk_objc_weak) | Typ.Tptr (_, Typ.Pk_objc_unsafe_unretained) ->
+    | Tstruct name -> (
-        true
+      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
+        None
-  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 *)
   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

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)

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 <Foundation/NSObject.h>
+
+@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

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]