[retain cycles] Find cycles about blocks

Reviewed By: mbouaziz

Differential Revision: D6835738

fbshipit-source-id: 5d46a5c

infer/src/IR/Sil.ml

@@ -431,9 +431,6 @@ let add_with_block_parameters_flag instr =
 (** Check if a pvar is a local pointing to a block in objc *)
 let is_block_pvar pvar = Typ.has_block_prefix (Mangled.to_string (Pvar.get_name pvar))
 
-(** A block pvar used to explain retain cycles *)
-let block_pvar = Pvar.mk (Mangled.from_string "block") (Typ.Procname.from_string_c_fun "")
-
 (** Dump an instruction. *)
 let d_instr (i: instr) = L.add_print_action (L.PTinstr, Obj.repr i)
 

infer/src/IR/Sil.mli

@@ -255,9 +255,6 @@ val zero_value_of_numerical_type : Typ.t -> Exp.t
 val is_static_local_name : string -> Pvar.t -> bool
 (** Check if a pvar is a local static in objc *)
 
-val block_pvar : Pvar.t
-(** A block pvar used to explain retain cycles *)
-
 val is_block_pvar : Pvar.t -> bool
 (** Check if a pvar is a local pointing to a block in objc *)
 

infer/src/backend/RetainCycles.ml

@@ -17,27 +17,34 @@ let desc_retain_cycle tenv (cycle: RetainCyclesType.t) =
   let do_edge index_ edge =
     let index = index_ + 1 in
     let node = State.get_node () in
-    let from_exp_str =
-      match edge.rc_from.rc_node_exp with
-      | Exp.Lvar pvar when Pvar.equal pvar Sil.block_pvar ->
-          "a block"
-      | _ ->
-        match Errdesc.find_outermost_dereference tenv node edge.rc_from.rc_node_exp with
+    let from_exp_str edge_obj =
+      match Errdesc.find_outermost_dereference tenv node edge_obj.rc_from.rc_node_exp with
       | Some de ->
           DecompiledExp.to_string de
       | None ->
-            Format.sprintf "(object of type %s)" (Typ.to_string edge.rc_from.rc_node_typ)
+          Format.sprintf "(object of type %s)" (Typ.to_string edge_obj.rc_from.rc_node_typ)
+    in
+    let location_str =
+      match edge with
+      | Object obj ->
+          let update_str_list =
+            Localise.access_desc (ref []) (Errdesc.access_opt obj.rc_field.rc_field_inst)
           in
-    let inst_str inst =
-      let update_str_list = Localise.access_desc (ref []) (Errdesc.access_opt inst) in
-      if List.is_empty update_str_list then " " else ", " ^ String.concat ~sep:"," update_str_list
+          if List.is_empty update_str_list then " "
+          else ", " ^ String.concat ~sep:"," update_str_list
+      | Block ->
+          ""
     in
     let cycle_item_str =
-      Format.sprintf "%s->%s" from_exp_str (Typ.Fieldname.to_string edge.rc_field.rc_field_name)
+      match edge with
+      | Object obj ->
+          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"
     in
-    Format.sprintf "(%d) %s%s" index
-      (MF.monospaced_to_string cycle_item_str)
-      (inst_str edge.rc_field.rc_field_inst)
+    Format.sprintf "(%d) %s%s" index cycle_item_str location_str
   in
   let cycle_str = List.mapi ~f:do_edge cycle.rc_elements in
   List.fold_left cycle_str ~f:(fun acc s -> Format.sprintf "%s\n %s" acc s) ~init:""
@@ -55,14 +62,38 @@ let get_cycle root prop =
    Returns a pair (path, bool) where path is a list of edges 
    describing the path to e_root and bool is true if e_root is reached. *)
   let rec dfs root_node from_node path fields visited =
+    let get_cycle_blocks exp =
+      match exp with
+      | Exp.Closure {captured_vars}
+      (* will turn on in prod when false positives have been addressed *)
+        when Config.debug_exceptions ->
+          List.find ~f:(fun (e, _, _) -> Exp.equal e root_node.rc_node_exp) captured_vars
+          |> Option.map ~f:snd3
+      | _ ->
+          None
+    in
     match fields with
     | [] ->
         (path, false)
     | (field, Sil.Eexp (f_exp, f_inst)) :: el' ->
         if Exp.equal f_exp root_node.rc_node_exp then
           let rc_field = {rc_field_name= field; rc_field_inst= f_inst} in
-          let edge = {rc_from= from_node; rc_field} in
+          let edge = Object {rc_from= from_node; rc_field} in
           (edge :: path, true)
+        else
+          let cycle_block_opt = get_cycle_blocks f_exp in
+          if Option.is_some cycle_block_opt then
+            match cycle_block_opt with
+            | Some var ->
+                let rc_field = {rc_field_name= field; rc_field_inst= f_inst} in
+                (* From the captured variables we get the actual name of the variable 
+                that is more useful for the error message *)
+                let updated_from_node = {from_node with rc_node_exp= Exp.Lvar var} in
+                let edge1 = Object {rc_from= updated_from_node; rc_field} in
+                let edge2 = Block in
+                (edge1 :: edge2 :: path, true)
+            | None ->
+                assert false
           else if List.mem ~equal:Exp.equal visited f_exp then (path, false)
           else
             let visited' = from_node.rc_node_exp :: visited in
@@ -72,7 +103,7 @@ let get_cycle root prop =
                   (path, false)
               | Some Sil.Hpointsto (_, Sil.Estruct (new_fields, _), Exp.Sizeof {typ= te}) ->
                   let rc_field = {rc_field_name= field; rc_field_inst= f_inst} in
-                let edge = {rc_from= from_node; rc_field} in
+                  let edge = Object {rc_from= from_node; rc_field} in
                   let rc_to = {rc_node_exp= f_exp; rc_node_typ= te} in
                   dfs root_node rc_to (edge :: path) new_fields visited'
               | _ ->
@@ -149,8 +180,12 @@ let cycle_has_weak_or_unretained_or_assign_field tenv cycle =
     | [] ->
         false
     | edge :: c' ->
-        let ia = get_item_annotation edge.rc_from.rc_node_typ edge.rc_field.rc_field_name in
+      match edge with
+      | Object obj ->
+          let ia = get_item_annotation obj.rc_from.rc_node_typ obj.rc_field.rc_field_name in
           if List.exists ~f:do_annotation ia then true else do_cycle c'
+      | Block ->
+          false
   in
   do_cycle cycle.rc_elements
 

infer/src/backend/RetainCyclesType.ml

@@ -14,22 +14,29 @@ type retain_cycle_field_objc =
   {rc_field_name: Typ.Fieldname.t; rc_field_inst: Sil.inst}
   [@@deriving compare]
 
-type retain_cycle_edge =
+type retain_cycle_edge_objc =
   {rc_from: retain_cycle_node; rc_field: retain_cycle_field_objc}
   [@@deriving compare]
 
+type retain_cycle_edge = Object of retain_cycle_edge_objc | Block [@@deriving compare]
+
 type t = {rc_elements: retain_cycle_edge list; rc_head: retain_cycle_edge} [@@deriving compare]
 
+let is_inst_rearrange node =
+  match node with
+  | Object obj -> (
+    match obj.rc_field.rc_field_inst with Sil.Irearrange _ -> true | _ -> false )
+  | Block ->
+      false
+
+
 let create_cycle cycle =
   let sorted_cycle = List.sort ~cmp:compare_retain_cycle_edge cycle in
   match sorted_cycle with
-  | [hd] -> (
-    match (* cycles of length 1 created at rearrange are not real *)
-          hd.rc_field.rc_field_inst with
-    | Sil.Irearrange _ ->
+  | [hd] ->
+      if is_inst_rearrange hd then (* cycles of length 1 created at rearrange are not real *)
         None
-    | _ ->
-        Some {rc_elements= sorted_cycle; rc_head= hd} )
+      else Some {rc_elements= sorted_cycle; rc_head= hd}
   | hd :: _ ->
       Some {rc_elements= sorted_cycle; rc_head= hd}
   | [] ->
@@ -47,9 +54,13 @@ let retain_cycle_field_to_string (field: retain_cycle_field_objc) =
 
 
 let retain_cycle_edge_to_string (edge: retain_cycle_edge) =
+  match edge with
+  | Object obj ->
       Format.sprintf "%s ->{%s}"
-    (retain_cycle_node_to_string edge.rc_from)
-    (retain_cycle_field_to_string edge.rc_field)
+        (retain_cycle_node_to_string obj.rc_from)
+        (retain_cycle_field_to_string obj.rc_field)
+  | Block ->
+      Format.sprintf "a block"
 
 
 let retain_cycle_to_string cycle =

infer/src/backend/RetainCyclesType.mli

@@ -13,7 +13,9 @@ type retain_cycle_node = {rc_node_exp: Exp.t; rc_node_typ: Typ.t}
 
 type retain_cycle_field_objc = {rc_field_name: Typ.Fieldname.t; rc_field_inst: Sil.inst}
 
-type retain_cycle_edge = {rc_from: retain_cycle_node; rc_field: retain_cycle_field_objc}
+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
 
 (** 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/src/backend/dotty.ml

@@ -1080,18 +1080,33 @@ let pp_dotty_prop fmt (prop, cycle) =
 
 
 let dotty_retain_cycle_to_str prop (cycle: RetainCyclesType.t) =
+  let open RetainCyclesType in
+  let get_exp_from_edge edge =
+    match edge with
+    | Object obj ->
+        obj.rc_from.rc_node_exp
+    | Block ->
+        Exp.Lvar (Pvar.mk (Mangled.from_string "block") (Typ.Procname.from_string_c_fun ""))
+  in
+  let get_field_from_edge edge =
+    match edge with
+    | Object obj ->
+        obj.rc_field.rc_field_name
+    | Block ->
+        Typ.Fieldname.Java.from_string ""
+  in
   let open RetainCyclesType in
   let rec cycle_to_list elements =
     match elements with
     | edge1 :: edge2 :: rest ->
-        ( edge1.rc_from.rc_node_exp
-        , edge1.rc_field.rc_field_name
-        , Sil.Eexp (edge2.rc_from.rc_node_exp, Sil.Inone) )
+        ( get_exp_from_edge edge1
+        , get_field_from_edge edge1
+        , Sil.Eexp (get_exp_from_edge edge2, Sil.Inone) )
         :: cycle_to_list (edge2 :: rest)
     | [edge] ->
-        [ ( edge.rc_from.rc_node_exp
-          , edge.rc_field.rc_field_name
-          , Sil.Eexp (cycle.rc_head.rc_from.rc_node_exp, Sil.Inone) ) ]
+        [ ( get_exp_from_edge edge
+          , get_field_from_edge edge
+          , Sil.Eexp (get_exp_from_edge edge, Sil.Inone) ) ]
     | [] ->
         []
   in

infer/tests/codetoanalyze/objc/errors/Makefile

@@ -92,6 +92,7 @@ SOURCES_ARC = \
   memory_leaks_benchmark/RetainReleaseExampleBucketingArc.m \
   memory_leaks_benchmark/retain_cycle.m \
   memory_leaks_benchmark/retain_cycle2.m \
+	memory_leaks_benchmark/RetainCycleBlocks.m \
   npe/BoxedNumberExample.m \
   npe/ObjCMethodCallInCondition.m \
   npe/UpdateDict.m \

infer/tests/codetoanalyze/objc/errors/blocks_in_heap/BlockInHeap.m

@@ -12,7 +12,7 @@
 
 typedef void (^BlockInHeapHandler)(BlockInHeap* name);
 
-@property(nonatomic, strong) BlockInHeapHandler handler;
+@property(nonatomic, weak) BlockInHeapHandler handler;
 
 @property(nonatomic, strong) BlockInHeap* child;
 
@@ -27,8 +27,8 @@ typedef void (^BlockInHeapHandler)(BlockInHeap* name);
 }
 
 @end
-
-int block_in_heap_executed_after_bi_abduction_ok() {
+// no retain cycle because handler is a weak pointer.
+int block_in_heap_executed_after_bi_abduction_ok_no_retain_cycle() {
   BlockInHeap* c = [[BlockInHeap alloc] init];
   [c assign_block_to_ivar];
   BlockInHeap* b = [[BlockInHeap alloc] init];
@@ -37,7 +37,7 @@ int block_in_heap_executed_after_bi_abduction_ok() {
 }
 
 int block_in_heap_executed_after_bi_abduction_ok_test() {
-  if (block_in_heap_executed_after_bi_abduction_ok() == 5) {
+  if (block_in_heap_executed_after_bi_abduction_ok_no_retain_cycle() == 5) {
     int* p = 0;
     return *p;
   } else {

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

@@ -3,6 +3,7 @@ codetoanalyze/objc/errors/global_const/global_const.m, SimpleRoot_doSomethingBad
 codetoanalyze/objc/errors/global_const/global_const.m, SimpleRoot_doSomethingOkWithDict:andString:, 3, NULL_DEREFERENCE, [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, [start of procedure init_with_compound_literal()]
 codetoanalyze/objc/errors/memory_leaks_benchmark/CADisplayLinkRetainCycle.m, testCycle, 3, RETAIN_CYCLE, [start of procedure testCycle(),start of procedure init,return from a call to CADisplay_init]
+codetoanalyze/objc/errors/memory_leaks_benchmark/RetainCycleStaticVar.m, RetainCSVycleStaticVar, 2, RETAIN_CYCLE, [start of procedure RetainCSVycleStaticVar(),start of procedure init,return from a call to RetainCSV_init,start of procedure foo,return from a call to RetainCSV_foo]
 codetoanalyze/objc/errors/npe/null_returned_by_method.m, NullReturnedByMethodA_test1, 1, NULL_DEREFERENCE, [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, [start of procedure ProcdescMain(),Skipping plusX:andY:: function or method not found]
 codetoanalyze/objc/errors/procdescs/main.c, call_nslog, 3, MEMORY_LEAK, [start of procedure call_nslog(),Skipping NSLog(): function or method not found]
@@ -22,13 +23,14 @@ codetoanalyze/objc/shared/block/block_no_args.m, My_manager_m, 10, NULL_DEREFERE
 codetoanalyze/objc/shared/block/block_release.m, My_manager_blockReleaseTODO, 9, MEMORY_LEAK, [start of procedure blockReleaseTODO]
 codetoanalyze/objc/shared/category_procdesc/main.c, CategoryProcdescMain, 3, MEMORY_LEAK, [start of procedure CategoryProcdescMain(),Skipping performDaysWork: function or method not found]
 codetoanalyze/objc/shared/field_superclass/SuperExample.m, ASuper_init, 2, NULL_DEREFERENCE, [start of procedure init,start of procedure init,return from a call to BSuper_init]
-codetoanalyze/objc/errors/blocks_in_heap/BlockInHeap.m, block_in_heap_executed_after_bi_abduction_ok_test, 3, NULL_DEREFERENCE, [start of procedure block_in_heap_executed_after_bi_abduction_ok_test(),start of procedure block_in_heap_executed_after_bi_abduction_ok(),start of procedure assign_block_to_ivar,return from a call to BlockInHeap_assign_block_to_ivar,start of procedure block,return from a call to objc_blockBlockInHeap_assign_block_to_ivar_1,return from a call to block_in_heap_executed_after_bi_abduction_ok,Condition is true]
+codetoanalyze/objc/errors/blocks_in_heap/BlockInHeap.m, block_in_heap_executed_after_bi_abduction_ok_test, 3, NULL_DEREFERENCE, [start of procedure block_in_heap_executed_after_bi_abduction_ok_test(),start of procedure block_in_heap_executed_after_bi_abduction_ok_no_retain_cycle(),start of procedure assign_block_to_ivar,return from a call to BlockInHeap_assign_block_to_ivar,start of procedure block,return from a call to objc_blockBlockInHeap_assign_block_to_ivar_1,return from a call to block_in_heap_executed_after_bi_abduction_ok_no_retain_cycle,Condition is true]
 codetoanalyze/objc/errors/field_superclass/SubtypingExample.m, Employee_initWithName:andAge:andEducation:, 3, NULL_TEST_AFTER_DEREFERENCE, [start of procedure initWithName:andAge:andEducation:,start of procedure initWithName:andAge:,return from a call to Person_initWithName:andAge:,Condition is false]
 codetoanalyze/objc/errors/field_superclass/SubtypingExample.m, Employee_initWithName:andAge:andEducation:, 6, DIVIDE_BY_ZERO, [start of procedure initWithName:andAge:andEducation:,start of procedure initWithName:andAge:,return from a call to Person_initWithName:andAge:,Condition is true]
 codetoanalyze/objc/errors/field_superclass/SubtypingExample.m, subtyping_test, 0, DIVIDE_BY_ZERO, [start of procedure subtyping_test(),start of procedure testFields(),start of procedure setEmployeeEducation:,return from a call to Employee_setEmployeeEducation:,start of procedure setAge:,return from a call to Person_setAge:,start of procedure setEmployeeEducation:,return from a call to Employee_setEmployeeEducation:,start of procedure getAge,return from a call to Person_getAge,return from a call to testFields]
 codetoanalyze/objc/errors/initialization/struct_initlistexpr.c, field_set_correctly, 2, DIVIDE_BY_ZERO, [start of procedure field_set_correctly()]
 codetoanalyze/objc/errors/initialization/struct_initlistexpr.c, implicit_expr_set_correctly, 3, DIVIDE_BY_ZERO, [start of procedure implicit_expr_set_correctly()]
 codetoanalyze/objc/errors/initialization/struct_initlistexpr.c, point_coords_set_correctly, 2, DIVIDE_BY_ZERO, [start of procedure point_coords_set_correctly()]
+codetoanalyze/objc/errors/memory_leaks_benchmark/RetainCycleBlocks.m, main, 2, RETAIN_CYCLE, [start of procedure main(),start of procedure retain_self_in_block,return from a call to RCBlock_retain_self_in_block]
 codetoanalyze/objc/errors/memory_leaks_benchmark/retain_cycle.m, strongcycle, 6, RETAIN_CYCLE, [start of procedure strongcycle()]
 codetoanalyze/objc/errors/memory_leaks_benchmark/retain_cycle2.m, strongcycle2, 4, RETAIN_CYCLE, [start of procedure strongcycle2(),start of procedure init,return from a call to Parent_init,start of procedure init,return from a call to Child_init,start of procedure setChild:,return from a call to Parent_setChild:,start of procedure setParent:,return from a call to Child_setParent:]
 codetoanalyze/objc/errors/npe/UpdateDict.m, add_nil_in_dict, 10, NULL_DEREFERENCE, [start of procedure add_nil_in_dict(),Skipping dictionaryWithObjectsAndKeys:: function or method not found]

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

@@ -0,0 +1,45 @@
+/*
+ * 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>
+
+@class RCBlock;
+
+typedef void (^MyHandler)(RCBlock* name);
+
+@interface RCBlock : NSObject {
+  RCBlock* child;
+}
+
+@property(nonatomic, strong) MyHandler handler;
+
+@property(nonatomic, strong) RCBlock* child;
+
+@end
+
+@implementation RCBlock
+
+// This code can be executed and one can check that there's a cycle because
+// "Dealloc" is not being printed.
+- (void)dealloc {
+  NSLog(@"Dealloc");
+}
+
+- (void)retain_self_in_block {
+  self.handler = ^(RCBlock* b) {
+    self->_child = b;
+  };
+}
+
+@end
+
+int main() {
+  RCBlock* c = [[RCBlock alloc] init];
+  [c retain_self_in_block];
+  return 0;
+}