[retain cycles] Take weak pointers into account to avoid false positives

Reviewed By: mbouaziz

Differential Revision: D7323649

fbshipit-source-id: e05a066

infer/src/backend/RetainCycles.ml

@@ -84,7 +84,14 @@ let edge_is_strong tenv obj_edge =
         || String.equal Config.assign att )
       params
   in
-  let weak_edge =
+  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
+    | _ ->
+        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
@@ -97,7 +104,7 @@ let edge_is_strong tenv obj_edge =
         true
     (* Assume the edge is weak if the type cannot be found in the tenv, to avoid FPs *)
   in
-  not weak_edge
+  not (weak_edge_by_type || weak_edge_by_field)
 
 
 exception Max_retain_cycles of RetainCyclesType.Set.t
@@ -128,6 +135,23 @@ let get_cycle_blocks root_node exp =
       None
 
 
+let get_weak_alias_type prop e =
+  let sigma = prop.Prop.sigma in
+  let check_weak_alias hpred =
+    match hpred with
+    | Sil.Hpointsto (_, Sil.Eexp (e', _), Sizeof {typ}) -> (
+      match typ.Typ.desc with
+      | (Typ.Tptr (_, Typ.Pk_objc_weak) | Typ.Tptr (_, Typ.Pk_objc_unsafe_unretained))
+        when Exp.equal e' e ->
+          Some typ
+      | _ ->
+          None )
+    | _ ->
+        None
+  in
+  List.find_map ~f:check_weak_alias sigma
+
+
 let get_cycles found_cycles root tenv prop =
   let open RetainCyclesType in
   let sigma = prop.Prop.sigma in
@@ -138,6 +162,13 @@ let get_cycles found_cycles root tenv prop =
   in
   (* Perform a dfs of a graph stopping when e_root is reached. Returns the set of cycles reached. *)
   let rec dfs ~found_cycles ~root_node ~from_node ~rev_path ~fields ~visited =
+    let from_node =
+      match get_weak_alias_type prop from_node.rc_node_exp with
+      | Some typ ->
+          {from_node with rc_node_typ= typ}
+      | None ->
+          from_node
+    in
     match fields with
     | [] ->
         found_cycles

infer/tests/codetoanalyze/objc/errors/Makefile

@@ -65,7 +65,6 @@ SOURCES_BUCKET_ALL = \
   field_superclass/field.c \
   global_const/global_const.m \
   initialization/compound_literal.c \
-  memory_leaks_benchmark/CADisplayLinkRetainCycle.m \
   memory_leaks_benchmark/RetainCycleStaticVar.m \
   npe/blockenum.m \
   npe/nil_param.m \
@@ -89,6 +88,7 @@ SOURCES_ARC = \
   field_superclass/SubtypingExample.m \
 	blocks_in_heap/BlockInHeap.m \
   initialization/struct_initlistexpr.c \
+	memory_leaks_benchmark/CADisplayLinkRetainCycle.m \
 	memory_leaks_benchmark/RetainCycleExampleWeak.m \
   memory_leaks_benchmark/RetainReleaseExampleBucketingArc.m \
   memory_leaks_benchmark/retain_cycle.m \

infer/tests/codetoanalyze/objc/errors/issues.exp

@@ -2,7 +2,6 @@ codetoanalyze/objc/errors/field_superclass/field.c, field_superclass_main, 3, RE
 codetoanalyze/objc/errors/global_const/global_const.m, SimpleRoot_doSomethingBadWithDict:andString:, 3, NULL_DEREFERENCE, ERROR, [start of procedure doSomethingBadWithDict:andString:,Message stringByAppendingString: with receiver nil returns nil.]
 codetoanalyze/objc/errors/global_const/global_const.m, SimpleRoot_doSomethingOkWithDict:andString:, 3, NULL_DEREFERENCE, ERROR, [start of procedure doSomethingOkWithDict:andString:,Message stringByAppendingString: with receiver nil returns nil.]
 codetoanalyze/objc/errors/initialization/compound_literal.c, init_with_compound_literal, 2, DIVIDE_BY_ZERO, ERROR, [start of procedure init_with_compound_literal()]
-codetoanalyze/objc/errors/memory_leaks_benchmark/CADisplayLinkRetainCycle.m, CADisplay_init, 1, RETAIN_CYCLE, ERROR, [start of procedure init]
 codetoanalyze/objc/errors/memory_leaks_benchmark/RetainCycleStaticVar.m, RetainCSV_foo, 13, RETAIN_CYCLE, ERROR, [start of procedure foo]
 codetoanalyze/objc/errors/npe/null_returned_by_method.m, NullReturnedByMethodA_test1, 1, NULL_DEREFERENCE, ERROR, [start of procedure test1,start of procedure test,return from a call to NullReturnedByMethodA_test]
 codetoanalyze/objc/errors/procdescs/main.c, ProcdescMain, 3, MEMORY_LEAK, ERROR, [start of procedure ProcdescMain(),Skipping plusX:andY:: function or method not found]
@@ -29,6 +28,7 @@ codetoanalyze/objc/errors/field_superclass/SubtypingExample.m, subtyping_test, 0
 codetoanalyze/objc/errors/initialization/struct_initlistexpr.c, field_set_correctly, 2, DIVIDE_BY_ZERO, ERROR, [start of procedure field_set_correctly()]
 codetoanalyze/objc/errors/initialization/struct_initlistexpr.c, implicit_expr_set_correctly, 3, DIVIDE_BY_ZERO, ERROR, [start of procedure implicit_expr_set_correctly()]
 codetoanalyze/objc/errors/initialization/struct_initlistexpr.c, point_coords_set_correctly, 2, DIVIDE_BY_ZERO, ERROR, [start of procedure point_coords_set_correctly()]
+codetoanalyze/objc/errors/memory_leaks_benchmark/CADisplayLinkRetainCycle.m, CADisplay_init, 1, RETAIN_CYCLE, ERROR, [start of procedure init]
 codetoanalyze/objc/errors/memory_leaks_benchmark/RetainCycleBlockCapturedVar.m, LinkResolver_test_bad, 3, RETAIN_CYCLE, ERROR, [start of procedure test_bad]
 codetoanalyze/objc/errors/memory_leaks_benchmark/RetainCycleBlocks.m, RCBlock_retain_self_in_block, 1, RETAIN_CYCLE, ERROR, [start of procedure retain_self_in_block]
 codetoanalyze/objc/errors/memory_leaks_benchmark/RetainCycleBlocks.m, objc_blockretain_a_in_block_cycle_3, 1, RETAIN_CYCLE, ERROR, [start of procedure block]

infer/tests/codetoanalyze/objc/errors/memory_leaks_benchmark/CADisplayLinkRetainCycle.m

@@ -19,13 +19,20 @@
 
 @implementation CADisplay
 
-- init {
+- (instancetype)init {
   _displayLink =
       [CADisplayLink displayLinkWithTarget:self selector:@selector(bla)];
 
   return self;
 }
 
+- (void)cycle_weak_good {
+  __weak __typeof__(self) weakSelf = self;
+  _displayLink =
+      [CADisplayLink displayLinkWithTarget:weakSelf
+                                  selector:@selector(handleDisplayLink:)];
+}
+
 - (void)bla{};
 
 - (void)invalidate {
@@ -33,10 +40,6 @@
   [_displayLink invalidate];
 };
 
-- (void)dealloc {
-  [super dealloc];
-}
-
 @end
 
 void testCycle() {
@@ -49,5 +52,4 @@ void testNoCycle() {
 
   CADisplay* a = [[CADisplay alloc] init];
   [a invalidate]; // break the cycle
-  [a dealloc];
 }