diff --git a/infer/lib/python/inferlib/issues.py b/infer/lib/python/inferlib/issues.py index 04371b852..94a4d610f 100644 --- a/infer/lib/python/inferlib/issues.py +++ b/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 = [ diff --git a/infer/src/clang/cFrontend_checkers.ml b/infer/src/clang/cFrontend_checkers.ml index ee9d8b087..92fec3f67 100644 --- a/infer/src/clang/cFrontend_checkers.ml +++ b/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) diff --git a/infer/src/clang/cFrontend_checkers.mli b/infer/src/clang/cFrontend_checkers.mli index f5caebf6a..26fbce09b 100644 --- a/infer/src/clang/cFrontend_checkers.mli +++ b/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) diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index 9a000b2f3..aae326236 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/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 diff --git a/infer/src/clang/cFrontend_errors.mli b/infer/src/clang/cFrontend_errors.mli index 3c54dbd91..513ef7f2f 100644 --- a/infer/src/clang/cFrontend_errors.mli +++ b/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 diff --git a/infer/src/clang/cTrans.ml b/infer/src/clang/cTrans.ml index 6ea56109d..d84ef9c95 100644 --- a/infer/src/clang/cTrans.ml +++ b/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 diff --git a/infer/tests/codetoanalyze/objcpp/errors/blocks/block.mm b/infer/tests/codetoanalyze/objcpp/errors/blocks/block.mm new file mode 100644 index 000000000..1a0eb48d7 --- /dev/null +++ b/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 +#include + +@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 diff --git a/infer/tests/endtoend/objc/BlockCaptureCXXRefTest.java b/infer/tests/endtoend/objc/BlockCaptureCXXRefTest.java new file mode 100644 index 000000000..9172419aa --- /dev/null +++ b/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 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 + ) + ); + } +} diff --git a/infer/tests/utils/InferResults.java b/infer/tests/utils/InferResults.java index 5b3e3b358..065f13039 100644 --- a/infer/tests/utils/InferResults.java +++ b/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") ||