[retain cycles] Improve the message of cycles with blocks by adding more info about captured variables

Reviewed By: mbouaziz

Differential Revision: D7235627

fbshipit-source-id: b38cb3b

infer/src/backend/RetainCycles.ml

@@ -41,8 +41,8 @@ let desc_retain_cycle tenv (cycle: RetainCyclesType.t) =
           MF.monospaced_to_string
             (Format.sprintf "%s->%s" (from_exp_str obj)
                (Typ.Fieldname.to_string obj.rc_field.rc_field_name))
-      | Block _ ->
-          Format.sprintf "a block"
+      | Block (_, var) ->
+          Format.sprintf "a block that captures %s" (MF.monospaced_to_string (Pvar.to_string var))
     in
     Format.sprintf "(%d) %s%s" index cycle_item_str location_str
   in
@@ -107,16 +107,15 @@ let add_cycle found_cycles rev_path =
 let get_cycle_blocks root_node exp =
   match exp with
   | Exp.Closure {name; captured_vars} ->
-      if List.exists
-           ~f:(fun (e, _, typ) ->
-             match typ.Typ.desc with
-             | Typ.Tptr (_, Typ.Pk_objc_weak) | Typ.Tptr (_, Typ.Pk_objc_unsafe_unretained) ->
-                 false
-             | _ ->
-                 Exp.equal e root_node.RetainCyclesType.rc_node_exp )
-           captured_vars
-      then Some name
-      else None
+      List.find_map
+        ~f:(fun (e, var, typ) ->
+          match typ.Typ.desc with
+          | Typ.Tptr (_, Typ.Pk_objc_weak) | Typ.Tptr (_, Typ.Pk_objc_unsafe_unretained) ->
+              None
+          | _ ->
+              if Exp.equal e root_node.RetainCyclesType.rc_node_exp then Some (name, var) else None
+          )
+        captured_vars
   | _ ->
       None
 
@@ -148,8 +147,8 @@ let get_cycles found_cycles root tenv prop =
             (* cycle with a block *)
             let cycle_opt = get_cycle_blocks root_node f_exp in
             if edge_is_strong tenv obj_edge && Option.is_some cycle_opt then
-              let procname = Option.value_exn cycle_opt in
-              let edge2 = Block procname in
+              let procname, captured_var = Option.value_exn cycle_opt in
+              let edge2 = Block (procname, captured_var) in
               let rev_path' = edge2 :: edge :: rev_path in
               add_cycle found_cycles rev_path'
             else

infer/src/backend/RetainCyclesType.ml

@@ -20,7 +20,7 @@ type retain_cycle_edge_objc =
 
 type retain_cycle_edge =
   | Object of retain_cycle_edge_objc
-  | Block of Typ.Procname.t
+  | Block of Typ.Procname.t * Pvar.t
   [@@deriving compare]
 
 let retain_cycle_edge_equal = [%compare.equal : retain_cycle_edge]
@@ -116,7 +116,7 @@ let pp_dotty fmt cycle =
         Format.fprintf fmt "%s_%a"
           (Typ.to_string obj.rc_from.rc_node_typ)
           Typ.Fieldname.pp obj.rc_field.rc_field_name
-    | Block name ->
+    | Block (name, _) ->
         Format.fprintf fmt "%s" (Typ.Procname.to_unique_id name)
   in
   let pp_dotty_field fmt element =

infer/src/backend/RetainCyclesType.mli

@@ -15,7 +15,7 @@ type retain_cycle_field_objc = {rc_field_name: Typ.Fieldname.t; rc_field_inst: S
 
 type retain_cycle_edge_objc = {rc_from: retain_cycle_node; rc_field: retain_cycle_field_objc}
 
-type retain_cycle_edge = Object of retain_cycle_edge_objc | Block of Typ.Procname.t
+type retain_cycle_edge = Object of retain_cycle_edge_objc | Block of Typ.Procname.t * Pvar.t
 
 (** A retain cycle is a non-empty list of paths. It also contains a pointer to the head of the list
 to model the cycle structure. The next element from the end of the list is the head. *)

infer/tests/codetoanalyze/objc/errors/Makefile

@@ -95,6 +95,7 @@ SOURCES_ARC = \
   memory_leaks_benchmark/retain_cycle2.m \
 	memory_leaks_benchmark/RetainCycleBlocks.m \
 	memory_leaks_benchmark/RetainCyclePropertyInProtocol.m \
+	memory_leaks_benchmark/RetainCycleBlockCapturedVar.m \
   npe/BoxedNumberExample.m \
   npe/ObjCMethodCallInCondition.m \
   npe/UpdateDict.m \

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

@@ -29,6 +29,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/RetainCycleBlockCapturedVar.m, LinkResolver_test_bad, 3, RETAIN_CYCLE, ERROR, [start of procedure test_bad]
 codetoanalyze/objc/errors/memory_leaks_benchmark/RetainCycleBlocks.m, call_retain_self_in_block_cycle, 2, RETAIN_CYCLE, ERROR, [start of procedure call_retain_self_in_block_cycle(),start of procedure retain_self_in_block,return from a call to RCBlock_retain_self_in_block]
 codetoanalyze/objc/errors/memory_leaks_benchmark/RetainCycleBlocks.m, retain_a_in_block_cycle, 5, RETAIN_CYCLE, ERROR, [start of procedure retain_a_in_block_cycle()]
 codetoanalyze/objc/errors/memory_leaks_benchmark/RetainCycleBlocks.m, retain_a_in_block_cycle, 5, RETAIN_CYCLE, ERROR, [start of procedure retain_a_in_block_cycle()]

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

@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2018 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+#import <Foundation/NSObject.h>
+
+@interface Listener : NSObject
+
+@property(nonatomic, copy) void (^didFinishLoad)();
+
+@end
+
+@implementation Listener
+
+- (void)dealloc {
+  NSLog(@"dealloc Listener");
+}
+
+@end
+
+@interface LinkResolver : NSObject
+
+@end
+
+@implementation LinkResolver
+
+- (void)test_bad {
+  Listener* listener = [[Listener alloc] init];
+  __block Listener* retainedListener = listener;
+  listener.didFinishLoad = ^() {
+    if (retainedListener) {
+      retainedListener = nil;
+    }
+  };
+}
+
+- (void)dealloc {
+  NSLog(@"dealloc LinkResolver");
+}
+
+@end
+
+int main() {
+  LinkResolver* a = [LinkResolver new];
+  [a test_bad];
+  return 0;
+}