[self-in-block] Add new issue type MULTIPLE_WEAKSELF

Summary: Raises an error when weakSelf is used multiple times in a block. The issue is that there could be unexpected behaviour. Even if `weakSelf` is not nil in the first use, it could be nil in the following uses since the object that `weakSelf` points to could be freed anytime.

Reviewed By: skcho

Differential Revision: D18765493

fbshipit-source-id: 37fc652e3

infer/man/man1/infer-full.txt

@@ -449,6 +449,7 @@ OPTIONS
              MEMORY_LEAK (enabled by default),
              MISSING_REQUIRED_PROP (enabled by default),
              MIXED_SELF_WEAKSELF (enabled by default),
+             MULTIPLE_WEAKSELF (enabled by default),
              MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE (enabled by default),
              Missing_fld (enabled by default),
              NULLPTR_DEREFERENCE (disabled by default),

infer/man/man1/infer-report.txt

@@ -191,6 +191,7 @@ OPTIONS
              MEMORY_LEAK (enabled by default),
              MISSING_REQUIRED_PROP (enabled by default),
              MIXED_SELF_WEAKSELF (enabled by default),
+             MULTIPLE_WEAKSELF (enabled by default),
              MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE (enabled by default),
              Missing_fld (enabled by default),
              NULLPTR_DEREFERENCE (disabled by default),

infer/man/man1/infer.txt

@@ -449,6 +449,7 @@ OPTIONS
              MEMORY_LEAK (enabled by default),
              MISSING_REQUIRED_PROP (enabled by default),
              MIXED_SELF_WEAKSELF (enabled by default),
+             MULTIPLE_WEAKSELF (enabled by default),
              MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE (enabled by default),
              Missing_fld (enabled by default),
              NULLPTR_DEREFERENCE (disabled by default),

infer/src/base/IssueType.ml

@@ -362,6 +362,8 @@ let missing_required_prop = register_from_string "MISSING_REQUIRED_PROP"
 
 let mixed_self_weakself = register_from_string "MIXED_SELF_WEAKSELF" ~hum:"Mixed Self WeakSelf"
 
+let multiple_weakself = register_from_string "MULTIPLE_WEAKSELF" ~hum:"Multiple WeakSelf Use"
+
 let mutable_local_variable_in_component_file =
   register_from_string "MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE"
 

infer/src/base/IssueType.mli

@@ -235,6 +235,8 @@ val missing_required_prop : t
 
 val mixed_self_weakself : t
 
+val multiple_weakself : t
+
 val mutable_local_variable_in_component_file : t
 
 val null_dereference : t

infer/src/checkers/SelfInBlock.ml

@@ -210,7 +210,7 @@ module TransferFunctions = struct
         astate
 end
 
-let make_trace_mixed_self_weakself domain =
+let make_trace_use_self_weakself domain =
   let trace_elems =
     Vars.fold
       (fun _ {pvar; loc; kind} trace_elems ->
@@ -240,12 +240,38 @@ let report_mix_self_weakself_issues summary domain =
            unexpected behavior."
           (Pvar.pp Pp.text) weakSelf Location.pp weakLoc (Pvar.pp Pp.text) self Location.pp selfLoc
       in
-      let ltr = make_trace_mixed_self_weakself domain in
+      let ltr = make_trace_use_self_weakself domain in
       Reporting.log_error summary ~ltr ~loc:selfLoc IssueType.mixed_self_weakself message
   | _ ->
       ()
 
 
+let report_weakself_multiple_issues summary domain =
+  let weakSelfs =
+    Vars.filter (fun _ {kind} -> DomainData.is_weak_self kind) domain
+    |> Vars.bindings |> List.map ~f:snd
+  in
+  let sorted_WeakSelfs =
+    List.sort weakSelfs ~compare:(fun {DomainData.loc= loc1} {DomainData.loc= loc2} ->
+        Location.compare loc1 loc2 )
+  in
+  match sorted_WeakSelfs with
+  | {pvar= weakSelf; loc= weakLoc1} :: {loc= weakLoc2} :: _ ->
+      let message =
+        F.asprintf
+          "This block uses the weak pointer `%a` more than once (%a) and (%a). This could lead to \
+           unexpected behavior. Even if `%a` is not nil in the first use, it could be nil in the \
+           following uses since the object that `%a` points to could be freed anytime; assign it \
+           to a strong variable first."
+          (Pvar.pp Pp.text) weakSelf Location.pp weakLoc1 Location.pp weakLoc2 (Pvar.pp Pp.text)
+          weakSelf (Pvar.pp Pp.text) weakSelf
+      in
+      let ltr = make_trace_use_self_weakself domain in
+      Reporting.log_error summary ~ltr ~loc:weakLoc1 IssueType.multiple_weakself message
+  | _ ->
+      ()
+
+
 let make_trace_unchecked_strongself (domain : Domain.t) =
   let trace_elems_strongVars =
     StrongEqualToWeakCapturedVars.fold
@@ -304,7 +330,8 @@ let checker {Callbacks.exe_env; summary} =
     match Analyzer.compute_post proc_data ~initial with
     | Some domain ->
         report_mix_self_weakself_issues summary domain.vars ;
-        report_unchecked_strongself_issues summary domain
+        report_unchecked_strongself_issues summary domain ;
+        report_weakself_multiple_issues summary domain.vars
     | None ->
         () ) ;
   summary

infer/tests/codetoanalyze/objc/self-in-block/StrongSelf.m

@@ -11,6 +11,8 @@
 
 - (void)foo;
 
+- (void)bar;
+
 @end
 
 @implementation SelfInBlockTest {
@@ -20,6 +22,9 @@
 - (void)foo {
 }
 
+- (void)bar {
+}
+
 - (void)mixSelfWeakSelf_bad {
   __weak __typeof(self) weakSelf = self;
   int (^my_block)() = ^() {
@@ -116,4 +121,12 @@
   };
 }
 
+- (void)wekSelfMultiple_bad {
+  __weak __typeof(self) weakSelf = self;
+  int (^my_block)(BOOL) = ^(BOOL isTapped) {
+    [weakSelf foo];
+    [weakSelf bar];
+    return 0;
+  };
+}
 @end

infer/tests/codetoanalyze/objc/self-in-block/issues.exp

@@ -3,3 +3,4 @@ codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strong
 codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfCheckOnce_bad_5, 8, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null,Using &strongSelf not checked for null]
 codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfNoCheck2_bad_4, 2, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null]
 codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::strongSelfNoCheck_bad_3, 2, STRONG_SELF_NOT_CHECKED, no_bucket, ERROR, [&strongSelf assigned here,Using &strongSelf not checked for null]
+codetoanalyze/objc/self-in-block/StrongSelf.m, objc_blockSelfInBlockTest::wekSelfMultiple_bad_9, 1, MULTIPLE_WEAKSELF, no_bucket, ERROR, [Using &weakSelf,Using &weakSelf]