Introducing checker for observer of notification centers

Summary:public
An observer object that registered to a notification center needs to be
unregistered before it is deallocated.
If not, the notification center may send a notification to a gost object.
This diff introduce a checker for this problem.

Reviewed By: dulmarod

Differential Revision: D2949692

fb-gh-sync-id: 1653cec
shipit-source-id: 1653cec

infer/lib/python/inferlib/issues.py

@@ -48,6 +48,7 @@ ISSUE_TYPES = [
     'PREMATURE_NIL_TERMINATION_ARGUMENT',
     'DIRECT_ATOMIC_PROPERTY_ACCESS',
     'CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK',
+    'REGISTERED_OBSERVER_BEING_DEALLOCATED',
 ]
 
 NULL_STYLE_ISSUE_TYPES = [

infer/src/backend/abs.ml

@@ -1016,6 +1016,22 @@ let cycle_has_weak_or_unretained_or_assign_field cycle =
         else do_cycle c' in
   do_cycle cycle
 
+let check_observer_is_unsubscribed_deallocation prop e =
+  let pvar_opt = match Prop.get_resource_attribute prop e with
+    | Some (Sil.Aresource ({Sil.ra_vpath =  Some (Sil.Dpvar pvar) })) -> Some pvar
+    | _ -> None in
+  let loc = State.get_loc () in
+  match Prop.get_observer_attribute prop e with
+  | Some Sil.Aobserver ->
+      (match pvar_opt with
+       |  Some pvar ->
+           L.d_strln (" ERROR: Object " ^ (Sil.pvar_to_string pvar) ^
+                      " is being deallocated while still registered in a notification center");
+           let desc = Localise.desc_registered_observer_being_deallocated pvar loc in
+           raise (Exceptions.Registered_observer_being_deallocated (desc, __POS__))
+       | _ -> ())
+  | _ -> ()
+
 let check_junk ?original_prop pname tenv prop =
   let fav_sub_sigmafp = Sil.fav_new () in
   Sil.sub_fav_add fav_sub_sigmafp (Prop.get_sub prop);
@@ -1070,6 +1086,7 @@ let check_junk ?original_prop pname tenv prop =
                 (* find the alloc attribute of one of the roots of hpred, if it exists *)
                 let res = ref None in
                 let do_entry e =
+                  check_observer_is_unsubscribed_deallocation prop e;
                   match Prop.get_resource_attribute prop e with
                   | Some (Sil.Aresource ({ Sil.ra_kind = Sil.Racquire }) as a) ->
                       L.d_str "ATTRIBUTE: "; Sil.d_exp (Sil.Const (Sil.Cattribute a)); L.d_ln ();

infer/src/backend/exceptions.ml

@@ -67,6 +67,7 @@ exception Pointer_size_mismatch of Localise.error_desc * ml_loc
 exception Precondition_not_found of Localise.error_desc * ml_loc
 exception Precondition_not_met of Localise.error_desc * ml_loc
 exception Retain_cycle of Prop.normal Prop.t * Sil.hpred * Localise.error_desc * ml_loc
+exception Registered_observer_being_deallocated of Localise.error_desc * ml_loc
 exception Return_expression_required of Localise.error_desc * ml_loc
 exception Return_statement_missing of Localise.error_desc * ml_loc
 exception Return_value_ignored of Localise.error_desc * ml_loc
@@ -234,6 +235,9 @@ let recognize_exception exn =
     | Retain_cycle (_, _, desc, ml_loc) ->
         (Localise.retain_cycle,
          desc, Some ml_loc, Exn_user, High, None, Prover)
+    | Registered_observer_being_deallocated (desc, ml_loc) ->
+        (Localise.registered_observer_being_deallocated,
+         desc, Some ml_loc, Exn_user, High, Some Kerror, Nocat)
     | Return_expression_required (desc, ml_loc) ->
         (Localise.return_expression_required,
          desc, Some ml_loc, Exn_user, Medium, None, Nocat)

infer/src/backend/exceptions.mli

@@ -66,6 +66,7 @@ exception Pointer_size_mismatch of Localise.error_desc * ml_loc
 exception Precondition_not_found of Localise.error_desc * ml_loc
 exception Precondition_not_met of Localise.error_desc * ml_loc
 exception Retain_cycle of Prop.normal Prop.t * Sil.hpred * Localise.error_desc * ml_loc
+exception Registered_observer_being_deallocated of Localise.error_desc * ml_loc
 exception Return_expression_required of Localise.error_desc * ml_loc
 exception Return_statement_missing of Localise.error_desc * ml_loc
 exception Return_value_ignored of Localise.error_desc * ml_loc

infer/src/backend/localise.ml

@@ -53,6 +53,7 @@ let pointer_size_mismatch = "POINTER_SIZE_MISMATCH"
 let precondition_not_found = "PRECONDITION_NOT_FOUND"
 let precondition_not_met = "PRECONDITION_NOT_MET"
 let premature_nil_termination = "PREMATURE_NIL_TERMINATION_ARGUMENT"
+let registered_observer_being_deallocated = "REGISTERED_OBSERVER_BEING_DEALLOCATED"
 let resource_leak = "RESOURCE_LEAK"
 let retain_cycle = "RETAIN_CYCLE"
 let return_value_ignored = "RETURN_VALUE_IGNORED"
@@ -734,6 +735,16 @@ let desc_retain_cycle prop cycle loc cycle_dotty =
       !str_cycle (at_line tags loc) in
   { no_desc with descriptions = [desc]; tags = !tags; dotty = cycle_dotty }
 
+let desc_registered_observer_being_deallocated pvar loc =
+  let tags = Tags.create () in
+  let obj_str = Sil.pvar_to_string pvar in
+  { no_desc with descriptions = ["Object " ^ obj_str ^
+                                 " has not been unregistered from a notification " ^
+                                 "center and is being deallocated " ^ at_line tags loc ^
+                                 ". Being still registered as observer of the notification " ^
+                                 "center, the deallocated object "
+                                 ^ obj_str ^ " may be notified in the future." ]; tags = !tags }
+
 let desc_return_statement_missing loc =
   let tags = Tags.create () in
   { no_desc with descriptions = ["Return statement missing " ^ at_line tags loc]; tags = !tags }

infer/src/backend/localise.mli

@@ -51,6 +51,7 @@ val pointer_size_mismatch : t
 val precondition_not_found : t
 val precondition_not_met : t
 val premature_nil_termination : t
+val registered_observer_being_deallocated : t
 val retain_cycle : t
 val resource_leak : t
 val return_value_ignored : t
@@ -227,6 +228,8 @@ val desc_retain_cycle :
   Prop.normal Prop.t -> ((Sil.strexp * Sil.typ) * Ident.fieldname * Sil.strexp) list ->
   Location.t -> string option -> error_desc
 
+val desc_registered_observer_being_deallocated : Sil.pvar -> Location.t -> error_desc
+
 val desc_return_statement_missing : Location.t -> error_desc
 
 val desc_return_value_ignored : Procname.t -> Location.t -> error_desc

infer/src/backend/prop.ml

@@ -1788,6 +1788,9 @@ let get_objc_null_attribute prop exp =
 let get_div0_attribute prop exp =
   get_attribute prop exp Sil.ACdiv0
 
+let get_observer_attribute prop exp =
+  get_attribute prop exp Sil.ACobserver
+
 let has_dangling_uninit_attribute prop exp =
   let la = get_exp_attributes prop exp in
   IList.exists (fun a -> Sil.attribute_equal a (Sil.Adangling (Sil.DAuninit))) la

infer/src/backend/prop.mli

@@ -289,6 +289,9 @@ val get_autorelease_attribute : 'a t -> exp -> attribute option
 (** Get the div0 attribute associated to the expression, if any *)
 val get_div0_attribute : 'a t -> exp -> attribute option
 
+(** Get the observer attribute associated to the expression, if any *)
+val get_observer_attribute : 'a t -> exp -> attribute option
+
 (** Get the objc null attribute associated to the expression, if any *)
 val get_objc_null_attribute : 'a t -> exp -> attribute option
 

infer/tests/codetoanalyze/objc/errors/registered_observer/ViewController.m

@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2016 - 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 <Cocoa/Cocoa.h>
+@interface ViewController : NSViewController
+@end
+
+@implementation ViewController
+
+- (instancetype)init {
+
+  [[NSNotificationCenter defaultCenter] addObserver:self
+                                           selector:@selector(fired)
+                                               name:@"some_notification"
+                                             object:nil];
+  return self;
+}
+
+- (void)fired {
+  NSLog(@"This codepath fired");
+}
+
+- (void)invalidate1 {
+  [[NSNotificationCenter defaultCenter] removeObserver:self];
+}
+
+- (void)invalidate2 {
+  [[NSNotificationCenter defaultCenter] removeObserver:self
+                                                  name:@"some_notification"
+                                                object:nil];
+}
+@end
+
+int fooError() {
+  ViewController* vc = [[ViewController alloc] init];
+  ViewController* vc2;
+  vc2 = vc;
+  [vc fired];
+  return 0;
+}
+
+int fooOK1() {
+  ViewController* vc = [[ViewController alloc] init];
+  ViewController* vc2;
+  vc2 = vc;
+  [vc fired];
+  [vc invalidate1];
+  return 0;
+}
+
+int fooOK2() {
+  ViewController* vc = [[ViewController alloc] init];
+  ViewController* vc2;
+  vc2 = vc;
+  [vc fired];
+  [vc invalidate2];
+  return 0;
+}
+
+int barError() {
+  ViewController* vc = [[ViewController alloc] init];
+  [vc fired];
+
+  vc = [[ViewController alloc] init];
+  return 0;
+}
+
+int barOK1() {
+  ViewController* vc = [[ViewController alloc] init];
+  [vc invalidate1];
+  vc = [[ViewController alloc] init];
+  [vc invalidate1];
+  return 0;
+}
+
+int barOK2() {
+  ViewController* vc = [[ViewController alloc] init];
+  [vc invalidate2];
+  vc = [[ViewController alloc] init];
+  [vc invalidate1];
+  return 0;
+}

infer/tests/endtoend/objc/RegisteredObserver.java

@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2015 - 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.
+ */
+
+package endtoend.objc;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static utils.matchers.ResultContainsExactly.containsExactly;
+
+import com.google.common.collect.ImmutableList;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+import java.io.IOException;
+
+import utils.DebuggableTemporaryFolder;
+import utils.InferException;
+import utils.InferResults;
+import utils.InferRunner;
+
+public class RegisteredObserver {
+
+  public static final String VCFile =
+      "infer/tests/codetoanalyze/objc/errors/registered_observer/ViewController.m";
+
+  private static ImmutableList<String> inferCmd;
+
+  public static final String REGISTERED_OBSERVER = "REGISTERED_OBSERVER_BEING_DEALLOCATED";
+
+  @ClassRule
+  public static DebuggableTemporaryFolder folder = new DebuggableTemporaryFolder();
+
+  @BeforeClass
+  public static void runInfer() throws InterruptedException, IOException {
+    inferCmd = InferRunner.createObjCInferCommandSimple(
+        folder,
+        VCFile,
+        "cf");
+  }
+
+  @Test
+  public void RegisteredObserverShouldBeFound()
+      throws InterruptedException, IOException, InferException {
+    InferResults inferResults = InferRunner.runInferObjC(inferCmd);
+    String[] methods = {
+      "fooError", "barError"
+    };
+    assertThat(
+        "Results should contain " + REGISTERED_OBSERVER,
+        inferResults,
+        containsExactly(
+            REGISTERED_OBSERVER,
+            VCFile,
+            methods
+        )
+    );
+  }
+
+}

infer/tests/utils/InferResults.java

@@ -61,6 +61,7 @@ public class InferResults {
           errorType.equals("STRONG_DELEGATE_WARNING") ||
           errorType.equals("DIRECT_ATOMIC_PROPERTY_ACCESS") ||
           errorType.equals("CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK") ||
+          errorType.equals("REGISTERED_OBSERVER_BEING_DEALLOCATED") ||
           errorType.equals("IMMUTABLE_CAST") ||
           errorType.equals("PARAMETER_NOT_NULL_CHECKED") ||
           errorType.equals("DANGLING_POINTER_DEREFERENCE") ||

infer/tests/utils/InferRunner.java

@@ -393,6 +393,20 @@ public class InferRunner {
         false);
   }
 
+  public static ImmutableList<String> createObjCInferCommandSimple(
+      TemporaryFolder folder,
+      String sourceFile,
+      String ml_bucket) throws IOException, InterruptedException {
+    return createClangInferCommand(
+        folder,
+        sourceFile,
+        Language.ObjC,
+        true,
+        null,
+        ml_bucket,
+        false);
+  }
+
   public static ImmutableList<String> createObjCInferCommandWithMLBuckets(
       TemporaryFolder folder,
       String sourceFile,