From df543b900b25deae5ea0d8ab5115b9e285704d45 Mon Sep 17 00:00:00 2001 From: Dino Distefano Date: Thu, 16 Mar 2017 02:59:28 -0700 Subject: [PATCH] Making easier to specify subclassing Reviewed By: dulmarod Differential Revision: D4699748 fbshipit-source-id: 39f7c05 --- Makefile | 2 +- infer/lib/linter_rules/linters.al | 22 +++---- infer/src/clang/cFrontend_errors.ml | 7 ++- infer/src/clang/cPredicates.ml | 29 +++++++++ infer/src/clang/cPredicates.mli | 8 +++ infer/src/clang/cTL.ml | 31 ++++++--- infer/src/clang/ctl_lexer.mll | 2 +- .../objc/linters-for-test-only/Makefile | 19 ++++++ .../objc/linters-for-test-only/issues.exp | 4 ++ .../linters-for-test-only/linters_example.al | 10 +++ .../objc/linters-for-test-only/subclassing.m | 63 +++++++++++++++++++ 11 files changed, 175 insertions(+), 22 deletions(-) create mode 100644 infer/tests/codetoanalyze/objc/linters-for-test-only/Makefile create mode 100644 infer/tests/codetoanalyze/objc/linters-for-test-only/issues.exp create mode 100644 infer/tests/codetoanalyze/objc/linters-for-test-only/linters_example.al create mode 100644 infer/tests/codetoanalyze/objc/linters-for-test-only/subclassing.m diff --git a/Makefile b/Makefile index 974b1ee37..a07e00228 100644 --- a/Makefile +++ b/Makefile @@ -80,7 +80,7 @@ DIRECT_TESTS += \ endif ifneq ($(XCODE_SELECT),no) DIRECT_TESTS += \ - objc_frontend objc_errors objc_linters objc_ioslints objcpp_frontend objcpp_linters + objc_frontend objc_errors objc_linters objc_ioslints objcpp_frontend objcpp_linters objc_linters-for-test-only endif .PHONY: all diff --git a/infer/lib/linter_rules/linters.al b/infer/lib/linter_rules/linters.al index 763322361..102a58453 100644 --- a/infer/lib/linter_rules/linters.al +++ b/infer/lib/linter_rules/linters.al @@ -40,13 +40,13 @@ DEFINE-CHECKER ASSIGN_POINTER_WARNING = { // Fires whenever a NSNumber is dangerously coerced to a boolean in a comparison DEFINE-CHECKER BAD_POINTER_COMPARISON = { - LET is_binop = in_node(BinaryOperator); + LET is_binop = is_node(BinaryOperator); LET is_binop_eq = is_binop_with_kind(EQ); LET is_binop_ne = is_binop_with_kind(NE); LET is_binop_neq = is_binop_eq OR is_binop_ne; LET is_unop_lnot = is_unop_with_kind(LNot); - LET is_implicit_cast_expr = in_node(ImplicitCastExpr); - LET is_expr_with_cleanups = in_node(ExprWithCleanups); + LET is_implicit_cast_expr = is_node(ImplicitCastExpr); + LET is_expr_with_cleanups = is_node(ExprWithCleanups); LET is_nsnumber = isa(NSNumber); LET eu =( @@ -119,7 +119,7 @@ DEFINE-CHECKER REGISTERED_OBSERVER_BEING_DEALLOCATED = { LET eventually_removeObserver = IN-NODE ObjCImplementationDecl, ObjCProtocolDecl WITH-TRANSITION _ (remove_observer_in_method OR - remove_observer_in_method INSTANCEOF ObjCImplementationDecl, ObjCProtocolDecl) + remove_observer_in_method HOLDS-IN-SOME-SUPERCLASS-OF ObjCImplementationDecl) HOLDS-EVENTUALLY; SET report_when = @@ -157,11 +157,11 @@ DEFINE-CHECKER GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL = { is_objc_extension() AND is_global_var() AND (NOT is_const_var()); LET makes_an_expensive_call = - (in_node(CallExpr) AND NOT call_function_named(CGPointMake)) - OR in_node(CXXTemporaryObjectExpr) - OR in_node(CXXMemberCallExpr) - OR in_node(CXXOperatorCallExpr) - OR in_node(ObjCMessageExpr); + (is_node(CallExpr) AND NOT call_function_named(CGPointMake)) + OR is_node(CXXTemporaryObjectExpr) + OR is_node(CXXMemberCallExpr) + OR is_node(CXXOperatorCallExpr) + OR is_node(ObjCMessageExpr); LET is_initialized_with_expensive_call = IN-NODE VarDecl WITH-TRANSITION InitExpr @@ -183,7 +183,7 @@ DEFINE-CHECKER GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL = { DEFINE-CHECKER CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK = { SET report_when = WHEN - ((in_node(BlockDecl) AND captures_cxx_references()) + ((is_node(BlockDecl) AND captures_cxx_references()) HOLDS-NEXT) HOLDS-IN-NODE BlockExpr; @@ -194,7 +194,7 @@ DEFINE-CHECKER CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK = { // HOLDS-IN-NODE BlockDecl; // // SET report_when = -// in_node(BlockDecl) AND captures_cxx_references(); +// is_node(BlockDecl) AND captures_cxx_references(); SET message = "C++ Reference variable(s) %cxx_ref_captured_in_block% captured by Objective-C block"; diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index 9b3710415..62250fa4f 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/infer/src/clang/cFrontend_errors.ml @@ -252,5 +252,10 @@ let invoke_set_of_parsed_checkers_an parsed_linters context (an : Ctl_parser_typ (* We decouple the hardcoded checkers from the parsed ones *) let invoke_set_of_checkers_on_node context an = - invoke_set_of_parsed_checkers_an !parsed_linters context an; + (match an with + | Ctl_parser_types.Decl (Clang_ast_t.TranslationUnitDecl _) -> + (* Don't run parsed linters on TranslationUnitDecl node. + Because depending on the formula it may give an error at line -1 *) + () + | _ -> invoke_set_of_parsed_checkers_an !parsed_linters context an); invoke_set_of_hard_coded_checkers_an context an diff --git a/infer/src/clang/cPredicates.ml b/infer/src/clang/cPredicates.ml index cfc88e36d..01bbb0e85 100644 --- a/infer/src/clang/cPredicates.ml +++ b/infer/src/clang/cPredicates.ml @@ -285,6 +285,7 @@ let is_node nodename an = | Ctl_parser_types.Decl d -> Clang_ast_proj.get_decl_kind_string d in String.equal nodename an_str +(* node an is of class classname *) let isa classname an = match an with | Ctl_parser_types.Stmt stmt -> @@ -295,6 +296,34 @@ let isa classname an = | _ -> false) | _ -> false +let _declaration_has_name comp an name = + match an with + | Ctl_parser_types.Decl d -> + (match Clang_ast_proj.get_named_decl_tuple d with + | Some (_, ndi) -> comp ndi.ni_name name + | _ -> false) + | _ -> false + +(* an is a declaration whose name contains a regexp defined by re *) +let declaration_has_name an re = + _declaration_has_name (str_contains) an re + +(* an is a declaration called precisely name *) +let declaration_has_name_strict an name = + _declaration_has_name (String.equal) an name + +let _is_class comp an re = + match an with + | Ctl_parser_types.Decl (Clang_ast_t.ObjCInterfaceDecl _) + | Ctl_parser_types.Decl (Clang_ast_t.ObjCImplementationDecl _) -> + _declaration_has_name comp an re + | _ -> false + +let is_class an re = + _is_class (str_contains) an re + +let is_class_strict an name = + _is_class (String.equal) an name let decl_unavailable_in_supported_ios_sdk (cxt : CLintersContext.context) an = let allowed_os_versions = diff --git a/infer/src/clang/cPredicates.mli b/infer/src/clang/cPredicates.mli index 12dbbd4ee..d4591f30e 100644 --- a/infer/src/clang/cPredicates.mli +++ b/infer/src/clang/cPredicates.mli @@ -61,6 +61,14 @@ val isa : string -> Ctl_parser_types.ast_node -> bool val is_node : string -> Ctl_parser_types.ast_node -> bool +val declaration_has_name : Ctl_parser_types.ast_node -> string -> bool + +val declaration_has_name_strict : Ctl_parser_types.ast_node -> string -> bool + +val is_class : Ctl_parser_types.ast_node -> string -> bool + +val is_class_strict : Ctl_parser_types.ast_node -> string -> bool + val pp_predicate : Format.formatter -> t -> unit val decl_unavailable_in_supported_ios_sdk : CLintersContext.context -> Ctl_parser_types.ast_node diff --git a/infer/src/clang/cTL.ml b/infer/src/clang/cTL.ml index 70a97cbc9..e0772803d 100644 --- a/infer/src/clang/cTL.ml +++ b/infer/src/clang/cTL.ml @@ -366,12 +366,21 @@ let transition_decl_to_stmt d trs = | _ -> None let transition_decl_to_decl_via_super d = - match CAst_utils.get_impl_decl_info d with - | Some idi -> - (match CAst_utils.get_super_ObjCImplementationDecl idi with - | Some d -> Some (Decl d) - | _ -> None) - | None -> None + let decl_opt_to_ast_node_opt d_opt = + match d_opt with + | Some d' -> Some (Decl d') + | None -> None in + let do_ObjCImplementationDecl d = + match CAst_utils.get_impl_decl_info d with + | Some idi -> + decl_opt_to_ast_node_opt (CAst_utils.get_super_ObjCImplementationDecl idi) + | None -> None in + match d with + | Clang_ast_t.ObjCImplementationDecl _ -> + do_ObjCImplementationDecl d + | Clang_ast_t.ObjCInterfaceDecl (_, _, _, _, idi) -> + decl_opt_to_ast_node_opt (CAst_utils.get_decl_opt_with_decl_ref idi.otdi_super) + | _ -> None let transition_stmt_to_stmt_via_condition st = let open Clang_ast_t in @@ -442,8 +451,14 @@ let rec eval_Atomic pred_name args an lcxt = CPredicates.is_binop_with_kind str_kind an | "is_unop_with_kind", [str_kind], an -> CPredicates.is_unop_with_kind str_kind an - | "in_node", [nodename], an -> CPredicates.is_node nodename an + | "is_node", [nodename], an -> CPredicates.is_node nodename an | "isa", [classname], an -> CPredicates.isa classname an + | "declaration_has_name", [decl_name], an -> + CPredicates.declaration_has_name an decl_name + | "declaration_has_name_strict", [decl_name], an -> + CPredicates.declaration_has_name_strict an decl_name + | "is_class", [cname], an -> CPredicates.is_class an cname + | "is_class_strict", [cname], an -> CPredicates.is_class_strict an cname | "decl_unavailable_in_supported_ios_sdk", [], an -> CPredicates.decl_unavailable_in_supported_ios_sdk lcxt an | "within_responds_to_selector_block", [], an -> @@ -527,7 +542,7 @@ and in_node node_type_list phi an lctx = such that d,lcxt |= phi *) and eval_EH classes phi an lcxt = (* Define EH[Classes] phi = ET[Classes](EF[->Super] phi) *) - let f = ET (classes, None, EF (Some Super, phi)) in + let f = ET (classes, None, EX (Some Super, EF (Some Super, phi))) in eval_formula f an lcxt (* an, lcxt |= ET[T][->l]phi <=> diff --git a/infer/src/clang/ctl_lexer.mll b/infer/src/clang/ctl_lexer.mll index b29861e9d..7d66babe4 100644 --- a/infer/src/clang/ctl_lexer.mll +++ b/infer/src/clang/ctl_lexer.mll @@ -38,7 +38,7 @@ rule token = parse | "HOLDS-EVERYWHERE-NEXT" { AX } | "ALWAYS-HOLDS" { EG } | "ALSWAYS-HOLDS-EVERYWHERE" { AG } - | "INSTANCEOF" { EH } + | "HOLDS-IN-SOME-SUPERCLASS-OF" { EH } | "IN-NODE" { ET } | "IN-EXCLUSIVE-NODE" { ETX } | "WHEN" { WHEN } diff --git a/infer/tests/codetoanalyze/objc/linters-for-test-only/Makefile b/infer/tests/codetoanalyze/objc/linters-for-test-only/Makefile new file mode 100644 index 000000000..6f043a511 --- /dev/null +++ b/infer/tests/codetoanalyze/objc/linters-for-test-only/Makefile @@ -0,0 +1,19 @@ +# 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. + +TESTS_DIR = ../../.. + +ANALYZER = linters +CLANG_OPTIONS = -x objective-c -fobjc-arc -c +INFER_OPTIONS = --linters-def-file linters_example.al --no-filtering --debug-exceptions --project-root $(TESTS_DIR) --no-failures-allowed +INFERPRINT_OPTIONS = --issues-tests + +SOURCES = \ + $(wildcard *.m) \ + $(wildcard */*.m) \ + +include $(TESTS_DIR)/clang.make diff --git a/infer/tests/codetoanalyze/objc/linters-for-test-only/issues.exp b/infer/tests/codetoanalyze/objc/linters-for-test-only/issues.exp new file mode 100644 index 000000000..49c82d7ac --- /dev/null +++ b/infer/tests/codetoanalyze/objc/linters-for-test-only/issues.exp @@ -0,0 +1,4 @@ +codetoanalyze/objc/linters-for-test-only/subclassing.m, Linters_dummy_method, 26, SUBCLASSING_TEST_EXAMPLE, [] +codetoanalyze/objc/linters-for-test-only/subclassing.m, Linters_dummy_method, 41, SUBCLASSING_TEST_EXAMPLE, [] +codetoanalyze/objc/linters-for-test-only/subclassing.m, Linters_dummy_method, 47, SUBCLASSING_TEST_EXAMPLE, [] +codetoanalyze/objc/linters-for-test-only/subclassing.m, Linters_dummy_method, 53, SUBCLASSING_TEST_EXAMPLE, [] diff --git a/infer/tests/codetoanalyze/objc/linters-for-test-only/linters_example.al b/infer/tests/codetoanalyze/objc/linters-for-test-only/linters_example.al new file mode 100644 index 000000000..cb8fed34b --- /dev/null +++ b/infer/tests/codetoanalyze/objc/linters-for-test-only/linters_example.al @@ -0,0 +1,10 @@ + +// Check that class A is not subclassed. +DEFINE-CHECKER SUBCLASSING_TEST_EXAMPLE = { + + SET report_when = + is_class(A) HOLDS-IN-SOME-SUPERCLASS-OF ObjCInterfaceDecl; + + SET message = "This is subclassing A. Class A should not be subclassed."; + +}; diff --git a/infer/tests/codetoanalyze/objc/linters-for-test-only/subclassing.m b/infer/tests/codetoanalyze/objc/linters-for-test-only/subclassing.m new file mode 100644 index 000000000..ba8dbf92b --- /dev/null +++ b/infer/tests/codetoanalyze/objc/linters-for-test-only/subclassing.m @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2017 - 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 + +@interface A : NSObject + +- (int)foo:(int)foo_par; + +@end + +@implementation A + +- (int)foo:(int)foo_par { + + return foo_par; +} + +@end + +@interface B : A // Error: A subclass + +- (void)bar; + +@end + +@implementation B + +- (void)bar { + A* a = [[A alloc] init]; + [a foo:5]; +} + +@end + +@interface C : B // Error: C subclass of B subclass of A +@end + +@implementation C +@end + +@interface D : C // Error: D subclass of C ... subclass of A +@end + +@implementation D +@end + +@interface E : D // Error: E subclass of D ... subclass of A +@end + +@implementation E +@end + +@interface F : NSObject +@end + +@implementation F +@end