Implemented new check CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK

Reviewed By: dulmarod

Differential Revision: D2823705

fb-gh-sync-id: 594bfa2

infer/lib/python/inferlib/issues.py

@@ -47,6 +47,7 @@ ISSUE_TYPES = [
     'PARAMETER_NOT_NULL_CHECKED',
     'PREMATURE_NIL_TERMINATION_ARGUMENT',
     'DIRECT_ATOMIC_PROPERTY_ACCESS',
+    'CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK',
 ]
 
 NULL_STYLE_ISSUE_TYPES = [

infer/src/clang/cFrontend_checkers.ml

@@ -62,3 +62,27 @@ let direct_atomic_property_access context stmt_info ivar_name =
       } in
       (condition, Some warning_desc)
   | _ -> (false, None) (* No warning *)
+
+(* CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK: C++ references
+   should not be captured in blocks.  *)
+let captured_cxx_ref_in_objc_block stmt_info captured_vars =
+  let is_cxx_ref (_, typ) =
+    match typ with
+    | Sil.Tptr(_, Sil.Pk_reference) -> true
+    | _ -> false in
+  let capt_refs = IList.filter is_cxx_ref captured_vars in
+  match capt_refs with
+  | [] -> (false, None) (* No warning *)
+  | _ ->
+      let pvar_descs =
+        IList.fold_left (fun s (v, _)  -> s ^ " '" ^ (Sil.pvar_to_string v) ^ "' ") "" capt_refs in
+      let loc = CLocation.get_sil_location_from_range stmt_info.Clang_ast_t.si_source_range true in
+      let warning_desc = {
+        name = "CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK";
+        description = "C++ Reference variable(s) " ^ pvar_descs ^
+                      " captured by Objective-C block";
+        suggestion = "C++ References are unmanaged and may be invalid " ^
+                     "by the time the block executes.";
+        loc = loc;
+      } in
+      (true, Some warning_desc)

infer/src/clang/cFrontend_checkers.mli

@@ -19,3 +19,8 @@ val checker_strong_delegate_warning : Clang_ast_t.decl_info -> Clang_ast_t.named
 val direct_atomic_property_access :
   CContext.t -> Clang_ast_t.stmt_info -> Clang_ast_t.named_decl_info option ->
   (bool * CFrontend_utils.General_utils.warning_desc option)
+
+(* CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK: C++ references
+   should not be captured in blocks.  *)
+val captured_cxx_ref_in_objc_block : Clang_ast_t.stmt_info -> (Sil.pvar * Sil.typ) list ->
+  (bool * CFrontend_utils.General_utils.warning_desc option)

infer/src/clang/cFrontend_errors.ml

@@ -24,6 +24,9 @@ let property_checkers_list = [CFrontend_checkers.checker_strong_delegate_warning
 (* List of checkers on ivar access *)
 let ivar_access_checker_list =  [CFrontend_checkers.direct_atomic_property_access]
 
+(* List of checkers for captured vars in objc blocks *)
+let captured_vars_checker_list = [CFrontend_checkers.captured_cxx_ref_in_objc_block]
+
 (* Add a frontend warning with a description desc at location loc to the errlog of a proc desc *)
 let log_frontend_warning pdesc warn_desc =
   let loc = warn_desc.loc in
@@ -61,3 +64,10 @@ let check_for_ivar_errors context stmt_info obj_c_ivar_ref_expr_info =
       match checker context stmt_info dr_name with
       | true, Some warning_desc -> log_frontend_warning pdesc warning_desc
       | _, _ -> ()) ivar_access_checker_list
+
+let check_for_captured_vars context stmt_info captured_vars =
+  let pdesc = CContext.get_procdesc context in
+  IList.iter (fun checker ->
+      match checker stmt_info captured_vars with
+      | true, Some warning_desc -> log_frontend_warning pdesc warning_desc
+      | _, _ -> ()) captured_vars_checker_list

infer/src/clang/cFrontend_errors.mli

@@ -18,3 +18,6 @@ val check_for_property_errors : Cfg.cfg -> Cg.t -> Sil.tenv -> string -> Clang_a
 (* Call checkers on a specific access of an ivar *)
 val check_for_ivar_errors :
   CContext.t -> Clang_ast_t.stmt_info -> Clang_ast_t.obj_c_ivar_ref_expr_info -> unit
+
+val check_for_captured_vars : CContext.t -> Clang_ast_t.stmt_info ->
+  (Sil.pvar * Sil.typ) list -> unit

infer/src/clang/cTrans.ml

@@ -1753,6 +1753,7 @@ struct
         Cg.add_edge context.cg procname block_pname;
         let captured_block_vars = block_decl_info.Clang_ast_t.bdi_captured_variables in
         let captured_vars = CVar_decl.captured_vars_from_block_info context captured_block_vars in
+        CFrontend_errors.check_for_captured_vars context stmt_info captured_vars;
         let ids_instrs = IList.map assign_captured_var captured_vars in
         let ids, instrs = IList.split ids_instrs in
         let block_data = (context, type_ptr, block_pname, captured_vars) in

infer/tests/codetoanalyze/objcpp/errors/blocks/block.mm

@@ -0,0 +1,49 @@
+/*
+ * 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<Foundation/NSObject.h>
+#include <dispatch/dispatch.h>
+
+@interface A : NSObject
+- (int) foo: (int &) y;
+@end
+
+@implementation A
+
+- (int) foo: (int &) y
+{
+    dispatch_async(dispatch_get_main_queue(), ^{
+        int a = y; // Error
+        return ;
+    });
+    return 1;
+}
+
+- (int) foo2: (int &) y
+{
+    const int copied_y = y;
+    dispatch_async(dispatch_get_main_queue(), ^{
+        int a = copied_y; // OK
+        return ;
+    });
+    return 1;
+}
+
+- (int) foo3: (int &) y param2: (int &) z
+{
+    dispatch_async(dispatch_get_main_queue(), ^{
+        int a = y; // Error
+        int b = z; // Error
+        return ;
+    });
+    return 1;
+}
+
+
+@end

infer/tests/endtoend/objc/BlockCaptureCXXRefTest.java

@@ -0,0 +1,62 @@
+/*
+ * 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 BlockCaptureCXXRefTest {
+
+  public static final String FILE = "infer/tests/codetoanalyze/objcpp/errors/blocks/block.mm";
+
+  private static ImmutableList<String> inferCmd;
+
+  public static final String REFERENCE_CAPTURED = "CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK";
+
+  @ClassRule
+  public static DebuggableTemporaryFolder folder =
+      new DebuggableTemporaryFolder();
+
+  @BeforeClass
+  public static void runInfer() throws InterruptedException, IOException {
+    inferCmd = InferRunner.createObjCPPInferCommand(folder, FILE);
+  }
+
+  @Test
+  public void whenInferRunsOnNavigateToURLInBackgroundThenNPEIsFound()
+      throws InterruptedException, IOException, InferException {
+    InferResults inferResults = InferRunner.runInferObjC(inferCmd);
+    String[] procedures = {
+        "foo:", "foo3:param2:"
+    };
+    assertThat(
+        "Results should contain the expected C++ reference captured in block",
+        inferResults,
+        containsExactly(
+            REFERENCE_CAPTURED,
+            FILE,
+            procedures
+        )
+    );
+  }
+}

infer/tests/utils/InferResults.java

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