Add NPE check for weak variables captured in blocks

Reviewed By: sblackshear

Differential Revision: D3593746

fbshipit-source-id: bc5dea0

infer/src/backend/localise.ml

@@ -136,6 +136,7 @@ module Tags = struct
   let parameter_not_null_checked = "parameter_not_null_checked" (* describes a NPE that comes from parameter not nullable *)
   let field_not_null_checked = "field_not_null_checked" (* describes a NPE that comes from field not nullable *)
   let nullable_src = "nullable_src" (* @Nullable-annoted field/param/retval that causes a warning *)
+  let weak_captured_var_src = "weak_captured_var_src" (* Weak variable captured in a block that causes a warning *)
   let empty_vector_access = "empty_vector_access"
   let create () = ref []
   let add tags tag value = tags := (tag, value) :: !tags
@@ -307,6 +308,13 @@ let deref_str_nullable proc_name_opt nullable_obj_str =
   let problem_str = "" in
   _deref_str_null proc_name_opt problem_str tags
 
+(** dereference strings for null dereference due to weak captured variable in block *)
+let deref_str_weak_variable_in_block proc_name_opt nullable_obj_str =
+  let tags = Tags.create () in
+  Tags.add tags Tags.weak_captured_var_src nullable_obj_str;
+  let problem_str = "" in
+  _deref_str_null proc_name_opt problem_str tags
+
 (** dereference strings for nonterminal nil arguments in c/objc variadic methods *)
 let deref_str_nil_argument_in_variadic_method pn total_args arg_number =
   let tags = Tags.create () in
@@ -517,11 +525,16 @@ let dereference_string deref_str value_str access_opt loc =
   let problem_desc =
     let nullable_text = if !Config.curr_language = Config.Java then "@Nullable" else "__nullable" in
     let problem_str =
-      match Tags.get !tags Tags.nullable_src with
+      match Tags.get !tags Tags.nullable_src, Tags.get !tags Tags.weak_captured_var_src with
-      | Some nullable_src ->
+      | Some nullable_src, _ ->
           if nullable_src = value_str then "is annotated with " ^ nullable_text ^ " and is dereferenced without a null check"
           else "is indirectly marked " ^ nullable_text ^ " (source: " ^ nullable_src ^ ") and is dereferenced without a null check"
-      | None -> deref_str.problem_str in
+      | None, Some weak_var_str ->
+          if weak_var_str = value_str then
+            "is a weak pointer captured in the block and is dereferenced without a null check"
+          else "is equal to the variable " ^ weak_var_str ^
+               ", a weak pointer captured in the block, and is dereferenced without a null check"
+      | None, None -> deref_str.problem_str in
     [(problem_str ^ " " ^ at_line tags loc)] in
   { no_desc with descriptions = value_desc:: access_desc @ problem_desc; tags = !tags }
 

infer/src/backend/localise.mli

@@ -146,6 +146,9 @@ val deref_str_null : Procname.t option -> deref_str
 (** dereference strings for null dereference due to Nullable annotation *)
 val deref_str_nullable : Procname.t option -> string -> deref_str
 
+(** dereference strings for null dereference due to weak captured variable in block *)
+val deref_str_weak_variable_in_block : Procname.t option -> string -> deref_str
+
 (** dereference strings for an undefined value coming from the given procedure *)
 val deref_str_undef : Procname.t * Location.t -> deref_str
 

infer/src/backend/rearrange.ml

@@ -1156,9 +1156,23 @@ let rec iter_rearrange
   end;
   res
 
+let is_weak_captured_var pdesc pvar =
+  let pname = Cfg.Procdesc.get_proc_name pdesc in
+  match pname with
+  | Block _ ->
+      let is_weak_captured (var, typ) =
+        match typ with
+        | Typ.Tptr (_, Pk_objc_weak) ->
+            Mangled.equal (Pvar.get_name pvar) var
+        | _ -> false in
+      IList.exists is_weak_captured (Cfg.Procdesc.get_captured pdesc)
+  | _ -> false
+
+
 (** Check for dereference errors: dereferencing 0, a freed value, or an undefined value *)
 let check_dereference_error pdesc (prop : Prop.normal Prop.t) lexp loc =
   let nullable_obj_str = ref None in
+  let nullable_str_is_weak_captured_var = ref false in
   (* return true if deref_exp is only pointed to by fields/params with @Nullable annotations *)
   let is_only_pt_by_nullable_fld_or_param deref_exp =
     let ann_sig = Models.get_modelled_annotated_signature (Specs.pdesc_resolve_attributes pdesc) in
@@ -1167,11 +1181,13 @@ let check_dereference_error pdesc (prop : Prop.normal Prop.t) lexp loc =
          match hpred with
          | Sil.Hpointsto (Sil.Lvar pvar, Sil.Eexp (Sil.Var _ as exp, _), _)
            when Sil.exp_equal exp deref_exp ->
+             let is_weak_captured_var = is_weak_captured_var pdesc pvar in
              let is_nullable =
-               if Annotations.param_is_nullable pvar ann_sig
+               if Annotations.param_is_nullable pvar ann_sig || is_weak_captured_var
                then
                  begin
                    nullable_obj_str := Some (Pvar.to_string pvar);
+                   nullable_str_is_weak_captured_var := is_weak_captured_var;
                    true
                  end
                else
@@ -1230,7 +1246,10 @@ let check_dereference_error pdesc (prop : Prop.normal Prop.t) lexp loc =
       let deref_str =
         if is_deref_of_nullable then
           match !nullable_obj_str with
-          | Some str -> Localise.deref_str_nullable None str
+          | Some str ->
+              if !nullable_str_is_weak_captured_var then
+                Localise.deref_str_weak_variable_in_block None str
+              else Localise.deref_str_nullable None str
           | None -> Localise.deref_str_nullable None ""
         else Localise.deref_str_null None in
       let err_desc =

infer/tests/codetoanalyze/objc/errors/npe/WeakCapturedVarsNPE.m

@@ -0,0 +1,47 @@
+/*
+ * 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.
+ */
+
+#include <Foundation/NSObject.h>
+
+@interface A : NSObject
+
+@end
+
+@implementation A {
+  int x;
+}
+
+- (void)strongSelfNoCheckNotWeakSelf {
+  __typeof(self) weakSelf = self;
+  int (^my_block)(BOOL) = ^(BOOL isTapped) {
+    __strong __typeof(weakSelf) strongSelf = weakSelf;
+    return strongSelf->x;
+  };
+}
+
+- (void)strongSelfNoCheck {
+  __weak __typeof(self) weakSelf = self;
+  int (^my_block)(BOOL) = ^(BOOL isTapped) {
+    __strong __typeof(weakSelf) strongSelf = weakSelf;
+    return strongSelf->x;
+  };
+}
+
+- (void)strongSelfCheck {
+  __weak __typeof(self) weakSelf = self;
+  int (^my_block)(BOOL) = ^(BOOL isTapped) {
+    __strong __typeof(weakSelf) strongSelf = weakSelf;
+    if (strongSelf)
+      return strongSelf->x;
+    else
+      return 0;
+  };
+}
+
+@end

infer/tests/endtoend/objc/infer/WeakVariableInBlockNPETest.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2013 - 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.infer;
+
+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 WeakVariableInBlockNPETest {
+
+  public static final String FILE =
+      "infer/tests/codetoanalyze/objc/errors/npe/WeakCapturedVarsNPE.m";
+
+  private static ImmutableList<String> inferCmdNPD;
+
+  public static final String NULL_DEREFERENCE = "NULL_DEREFERENCE";
+
+  @ClassRule
+  public static DebuggableTemporaryFolder folderNPD =
+  new DebuggableTemporaryFolder();
+
+  @BeforeClass
+  public static void runInfer() throws InterruptedException, IOException {
+    inferCmdNPD = InferRunner.createiOSInferCommandWithMLBuckets(
+        folderNPD,
+        FILE,
+        "cf",
+        true);
+  }
+
+  @Test
+  public void whenInferRunsOnWeakCapturedVariablesNPEThenNPEsFound()
+      throws InterruptedException, IOException, InferException {
+    InferResults inferResults = InferRunner.runInferC(inferCmdNPD);
+    String[] procedures = {"__objc_anonymous_block_A_strongSelfNoCheck______2"};
+    assertThat(
+        "Results should not contain null dereference",
+        inferResults,
+        containsExactly(
+            NULL_DEREFERENCE,
+            FILE,
+            procedures
+        )
+    );
+  }
+}