From 472f155a7a1a5afa95f46d4300137e58cb1fa643 Mon Sep 17 00:00:00 2001 From: Dino Distefano Date: Thu, 13 Jun 2019 09:35:06 -0700 Subject: [PATCH] Improved rule on block capturing CXX Reference Reviewed By: jvillard Differential Revision: D15737387 fbshipit-source-id: efe677b3d --- infer/lib/linter_rules/linters.al | 34 ++++++++++++--- infer/src/clang/cAst_utils.ml | 1 + infer/src/clang/cPredicates.ml | 29 +++++++++++++ infer/src/clang/cPredicates.mli | 2 + infer/src/clang/cTL.ml | 2 + .../linters/cxx_reference_in_block/block2.mm | 41 +++++++++++++++++++ .../codetoanalyze/objcpp/linters/issues.exp | 2 + 7 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 infer/tests/codetoanalyze/objcpp/linters/cxx_reference_in_block/block2.mm diff --git a/infer/lib/linter_rules/linters.al b/infer/lib/linter_rules/linters.al index 24546e098..596b5190e 100644 --- a/infer/lib/linter_rules/linters.al +++ b/infer/lib/linter_rules/linters.al @@ -176,11 +176,35 @@ DEFINE-CHECKER GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL = { DEFINE-CHECKER CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK = { - SET report_when = - WHEN - ((is_node("BlockDecl") AND captures_cxx_references()) - HOLDS-NEXT) - HOLDS-IN-NODE BlockExpr; + + // The current node is block definition capturing a C++ reference + LET capture_reference = + WHEN + ((is_node("BlockDecl") AND captures_cxx_references()) HOLDS-NEXT) + HOLDS-IN-NODE BlockExpr; + + // At some point we encounter a block definition capturing a C++ reference + LET block_definition_capture_reference = + capture_reference HOLDS-EVENTUALLY; + + // A variable definition initialized with a block capturing a C++ reference + LET variable_initialized_with_block = + IN-NODE VarDecl WITH-TRANSITION InitExpr + (block_definition_capture_reference) + HOLDS-EVENTUALLY; + + // Reference to a variable initialized with a capturing block + LET variable_block_definition = + IN-NODE DeclRefExpr WITH-TRANSITION PointerToDecl + (variable_initialized_with_block) + HOLDS-EVENTUALLY; + + // Report when a function that does not have NoEscapeAttribute call a block or a variable definied with + // a block capturing a C++ ref + SET report_when = + WHEN + (NOT has_no_escape_attribute) AND (block_definition_capture_reference OR variable_block_definition) + HOLDS-IN-NODE CallExpr; SET message = "C++ Reference variable(s) %cxx_ref_captured_in_block% captured by Objective-C block"; diff --git a/infer/src/clang/cAst_utils.ml b/infer/src/clang/cAst_utils.ml index 03ce2c401..5f22c14a9 100644 --- a/infer/src/clang/cAst_utils.ml +++ b/infer/src/clang/cAst_utils.ml @@ -130,6 +130,7 @@ let get_stmt_opt stmt_ptr_opt source_range = let get_decl_opt_with_decl_ref decl_ref_opt = match decl_ref_opt with | Some decl_ref -> + L.debug Capture Verbose "#####POINTER LOOK UP: '%i'@\n" decl_ref.Clang_ast_t.dr_decl_pointer ; get_decl decl_ref.Clang_ast_t.dr_decl_pointer | None -> None diff --git a/infer/src/clang/cPredicates.ml b/infer/src/clang/cPredicates.ml index 3b411c0a3..b059239eb 100644 --- a/infer/src/clang/cPredicates.ml +++ b/infer/src/clang/cPredicates.ml @@ -1402,6 +1402,30 @@ let rec get_decl_attributes_for_callexpr an = [] +let rec get_decl_attributes_for_callexpr_param an = + let open Clang_ast_t in + let open Ctl_parser_types in + let get_attr_param p = match p with ParmVarDecl (di, _, _, _) -> di.di_attributes | _ -> [] in + match an with + | Stmt (CallExpr (_, func :: _, _)) -> + get_decl_attributes_for_callexpr_param (Stmt func) + | Stmt (ImplicitCastExpr (_, [stmt], _, _)) -> + get_decl_attributes_for_callexpr_param (Stmt stmt) + | Stmt (DeclRefExpr (si, _, _, drti)) -> ( + L.debug Linters Verbose "#####POINTER LOOP UP: '%i'@\n" si.si_pointer ; + match CAst_utils.get_decl_opt_with_decl_ref drti.drti_decl_ref with + | Some (FunctionDecl (_, _, _, fdi)) -> + List.fold fdi.fdi_parameters + ~f:(fun acc p -> List.append (get_attr_param p) acc) + ~init:[] + | Some (ParmVarDecl _ as d) -> + get_attr_param d + | _ -> + [] ) + | _ -> + [] + + let visibility_matches vis_str (visibility : Clang_ast_t.visibility_attr) = match (visibility, vis_str) with | DefaultVisibility, "Default" -> @@ -1426,6 +1450,11 @@ let has_visibility_attribute an visibility = match visibility with ALVar.Const vis -> has_visibility_attr attributes vis | _ -> false +let has_no_escape_attribute an = + let attributes = get_decl_attributes_for_callexpr_param an in + List.exists ~f:(fun attr -> match attr with `NoEscapeAttr _ -> true | _ -> false) attributes + + let has_used_attribute an = let attributes = get_decl_attributes_for_callexpr an in List.exists ~f:(fun attr -> match attr with `UsedAttr _ -> true | _ -> false) attributes diff --git a/infer/src/clang/cPredicates.mli b/infer/src/clang/cPredicates.mli index 1ff5273b9..69bf13665 100644 --- a/infer/src/clang/cPredicates.mli +++ b/infer/src/clang/cPredicates.mli @@ -488,6 +488,8 @@ val cxx_construct_expr_has_name : Ctl_parser_types.ast_node -> ALVar.alexp -> bo val has_used_attribute : Ctl_parser_types.ast_node -> bool +val has_no_escape_attribute : Ctl_parser_types.ast_node -> bool + val iphoneos_target_sdk_version_by_path : CLintersContext.context -> string option val iphoneos_target_sdk_version_greater_or_equal : CLintersContext.context -> string -> bool diff --git a/infer/src/clang/cTL.ml b/infer/src/clang/cTL.ml index c53927976..9c897fa19 100644 --- a/infer/src/clang/cTL.ml +++ b/infer/src/clang/cTL.ml @@ -1191,6 +1191,8 @@ let rec eval_Atomic pred_name_ args an lcxt = CPredicates.has_visibility_attribute an vis | "has_used_attribute", [], an -> CPredicates.has_used_attribute an + | "has_no_escape_attribute", [], an -> + CPredicates.has_no_escape_attribute an | "has_unavailable_attribute", [], an -> CPredicates.has_unavailable_attribute an | "within_available_class_block", [], an -> diff --git a/infer/tests/codetoanalyze/objcpp/linters/cxx_reference_in_block/block2.mm b/infer/tests/codetoanalyze/objcpp/linters/cxx_reference_in_block/block2.mm new file mode 100644 index 000000000..2f6b3211f --- /dev/null +++ b/infer/tests/codetoanalyze/objcpp/linters/cxx_reference_in_block/block2.mm @@ -0,0 +1,41 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +#import + +void runBlock(__attribute__((noescape)) dispatch_block_t block) { block(); } + +void safe() { + + int x = 5; + int& xref = x; + runBlock(^{ + xref = 6; + }); // safe +} + +void unsafe() { + int x = 5; + int& xref = x; + dispatch_queue_t queue = dispatch_queue_create("queue", NULL); + dispatch_async(queue, ^{ + // Infer flags the following: + xref = 6; // CRASH! local has gone away + }); +} + +void both_safe_and_unsafe() { + + int x = 5; + int& xref = x; + dispatch_block_t my_block = ^{ + xref = 6; + }; + dispatch_queue_t queue = dispatch_queue_create("queue", NULL); + + runBlock(my_block); // safe + dispatch_async(queue, my_block); // unsafe +} diff --git a/infer/tests/codetoanalyze/objcpp/linters/issues.exp b/infer/tests/codetoanalyze/objcpp/linters/issues.exp index 37f2225bc..94cf263b0 100644 --- a/infer/tests/codetoanalyze/objcpp/linters/issues.exp +++ b/infer/tests/codetoanalyze/objcpp/linters/issues.exp @@ -28,6 +28,8 @@ codetoanalyze/objcpp/linters/componentkit/MutableLocalVariablesTest.mm, SomeClas codetoanalyze/objcpp/linters/componentkit/UnconventionalSuperclassTest.h, Linters_dummy_method, 15, COMPONENT_WITH_UNCONVENTIONAL_SUPERCLASS, no_bucket, ADVICE, [] codetoanalyze/objcpp/linters/cxx_reference_in_block/block.mm, A::foo, 18, CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK, no_bucket, ERROR, [] codetoanalyze/objcpp/linters/cxx_reference_in_block/block.mm, A::foo3, 35, CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK, no_bucket, ERROR, [] +codetoanalyze/objcpp/linters/cxx_reference_in_block/block2.mm, both_safe_and_unsafe, 40, CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK, no_bucket, ERROR, [] +codetoanalyze/objcpp/linters/cxx_reference_in_block/block2.mm, unsafe, 24, CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK, no_bucket, ERROR, [] codetoanalyze/objcpp/linters/global-var/B.mm, Linters_dummy_method, 28, GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL, no_bucket, WARNING, [] codetoanalyze/objcpp/linters/global-var/B.mm, Linters_dummy_method, 30, GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL, no_bucket, WARNING, [] codetoanalyze/objcpp/linters/global-var/B.mm, Linters_dummy_method, 32, GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL, no_bucket, WARNING, []