From 3da5908728d8ede0cc1de8e504d4eae488994a30 Mon Sep 17 00:00:00 2001 From: Dulma Churchill Date: Fri, 18 Aug 2017 02:53:40 -0700 Subject: [PATCH] [linters] Remove unavailable api class false positives and turn it on Summary: This is a check for when an unavailable class is being allocated. This diff also adds a check for the context to remove false positives: If the class is not available but the method calls are wrapped in a check whether the class is available, then don't report. Reviewed By: jvillard Differential Revision: D5631191 fbshipit-source-id: 2082dfe --- infer/lib/linter_rules/linters.al | 7 ++- infer/src/clang/CLintersContext.ml | 5 +- infer/src/clang/cFrontend_checkers_main.ml | 22 ++++++-- infer/src/clang/cPredicates.ml | 53 +++++++++++++++++-- infer/src/clang/cPredicates.mli | 10 ++++ infer/src/clang/cTL.ml | 2 + .../codetoanalyze/objc/ioslints/issues.exp | 5 +- .../unavailable_api_in_supported_ios_sdk.m | 19 ++++++- 8 files changed, 107 insertions(+), 16 deletions(-) diff --git a/infer/lib/linter_rules/linters.al b/infer/lib/linter_rules/linters.al index 16398d367..89a70310f 100644 --- a/infer/lib/linter_rules/linters.al +++ b/infer/lib/linter_rules/linters.al @@ -225,7 +225,10 @@ DEFINE-CHECKER CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK = { DEFINE-CHECKER UNAVAILABLE_CLASS_IN_SUPPORTED_IOS_SDK = { SET report_when = - WHEN (class_unavailable_in_supported_ios_sdk()) + WHEN ((class_unavailable_in_supported_ios_sdk()) AND + NOT within_available_class_block() AND + (call_class_method(REGEXP(".*"), "alloc") OR + call_class_method(REGEXP(".*"), "new"))) HOLDS-IN-NODE ObjCMessageExpr; SET message = @@ -233,7 +236,7 @@ DEFINE-CHECKER CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK = { %iphoneos_target_sdk_version% (only available from version %class_available_ios_sdk%)"; SET name = "Unavailable API In Supported iOS SDK"; SET severity = "ERROR"; - SET mode = "OFF"; + SET mode = "ON"; }; diff --git a/infer/src/clang/CLintersContext.ml b/infer/src/clang/CLintersContext.ml index d83da90db..95cfa116e 100644 --- a/infer/src/clang/CLintersContext.ml +++ b/infer/src/clang/CLintersContext.ml @@ -9,7 +9,10 @@ open! IStd -type if_context = {within_responds_to_selector_block: string list; ios_version_guard: string list} +type if_context = + { within_responds_to_selector_block: string list + ; within_available_class_block: string list + ; ios_version_guard: string list } type context = { translation_unit_context: CFrontend_config.translation_unit_context diff --git a/infer/src/clang/cFrontend_checkers_main.ml b/infer/src/clang/cFrontend_checkers_main.ml index 93678522f..1c00f6a3e 100644 --- a/infer/src/clang/cFrontend_checkers_main.ml +++ b/infer/src/clang/cFrontend_checkers_main.ml @@ -162,19 +162,33 @@ let rec get_current_os_version stmt = let compute_if_context (context: CLintersContext.context) stmt = let selector = get_responds_to_selector stmt in + let receiver_class_method_call = + match + (CPredicates.get_selector (Stmt stmt), CPredicates.receiver_class_method_call (Stmt stmt)) + with + | Some selector, Some receiver when String.equal selector "class" + -> Option.to_list (CPredicates.declaration_name receiver) + | _ + -> [] + in let os_version = get_current_os_version stmt in - let within_responds_to_selector_block, ios_version_guard = + let within_responds_to_selector_block, within_available_class_block, ios_version_guard = match context.if_context with | Some if_context -> let within_responds_to_selector_block = List.append selector if_context.within_responds_to_selector_block in + let within_available_class_block = + List.append receiver_class_method_call if_context.within_available_class_block + in let ios_version_guard = List.append os_version if_context.ios_version_guard in - (within_responds_to_selector_block, ios_version_guard) + (within_responds_to_selector_block, within_available_class_block, ios_version_guard) | None - -> (selector, os_version) + -> (selector, receiver_class_method_call, os_version) in - Some ({within_responds_to_selector_block; ios_version_guard} : CLintersContext.if_context) + Some + ( {within_responds_to_selector_block; within_available_class_block; ios_version_guard} + : CLintersContext.if_context ) let is_factory_method (context: CLintersContext.context) decl = let interface_decl_opt = diff --git a/infer/src/clang/cPredicates.ml b/infer/src/clang/cPredicates.ml index 782eddac2..9b38800a8 100644 --- a/infer/src/clang/cPredicates.ml +++ b/infer/src/clang/cPredicates.ml @@ -29,12 +29,21 @@ let rec objc_class_of_pointer_type type_ptr = | _ -> None -let receiver_method_call an = +let receiver_class_method_call an = match an with - | Ctl_parser_types.Stmt ObjCMessageExpr (_, args, _, obj_c_message_expr_info) -> ( + | Ctl_parser_types.Stmt ObjCMessageExpr (_, _, _, obj_c_message_expr_info) -> ( match obj_c_message_expr_info.omei_receiver_kind with | `Class qt -> CAst_utils.get_decl_from_typ_ptr qt.qt_type_ptr + | _ + -> None ) + | _ + -> None + +let receiver_instance_method_call an = + match an with + | Ctl_parser_types.Stmt ObjCMessageExpr (_, args, _, obj_c_message_expr_info) -> ( + match obj_c_message_expr_info.omei_receiver_kind with | `Instance -> ( match args with | receiver :: _ -> ( @@ -50,6 +59,20 @@ let receiver_method_call an = | _ -> None +let receiver_method_call an = + match receiver_class_method_call an with + | Some decl + -> Some decl + | None + -> receiver_instance_method_call an + +let declaration_name decl = + match Clang_ast_proj.get_named_decl_tuple decl with + | Some (_, ndi) + -> Some ndi.ni_name + | None + -> None + let get_available_attr_ios_sdk an = let open Clang_ast_t in let rec get_available_attr attrs = @@ -137,11 +160,18 @@ let is_object_of_class_named receiver cname = | _ -> false -(* an |= call_method(m) where the name must be exactly m *) -let call_method an m = +let get_selector an = match an with | Ctl_parser_types.Stmt Clang_ast_t.ObjCMessageExpr (_, _, _, omei) - -> ALVar.compare_str_with_alexp omei.omei_selector m + -> Some omei.omei_selector + | _ + -> None + +(* an |= call_method(m) where the name must be exactly m *) +let call_method an m = + match get_selector an with + | Some selector + -> ALVar.compare_str_with_alexp selector m | _ -> false @@ -595,6 +625,19 @@ let within_responds_to_selector_block (cxt: CLintersContext.context) an = | _ -> false +let within_available_class_block (cxt: CLintersContext.context) an = + match (receiver_method_call an, cxt.if_context) with + | Some receiver, Some if_context + -> ( + let in_available_class_block = if_context.within_available_class_block in + match declaration_name receiver with + | Some receiver_name + -> List.mem ~equal:String.equal in_available_class_block receiver_name + | None + -> false ) + | _ + -> false + let objc_method_has_nth_parameter_of_type an _num _typ = let open Clang_ast_t in let num = diff --git a/infer/src/clang/cPredicates.mli b/infer/src/clang/cPredicates.mli index 2b5920239..bf66a378d 100644 --- a/infer/src/clang/cPredicates.mli +++ b/infer/src/clang/cPredicates.mli @@ -21,6 +21,8 @@ val call_class_method : Ctl_parser_types.ast_node -> ALVar.alexp -> ALVar.alexp val call_instance_method : Ctl_parser_types.ast_node -> ALVar.alexp -> ALVar.alexp -> bool +val declaration_name : Clang_ast_t.decl -> string option + val is_enum_constant : Ctl_parser_types.ast_node -> ALVar.alexp -> bool val is_objc_interface_named : Ctl_parser_types.ast_node -> ALVar.alexp -> bool @@ -85,6 +87,8 @@ val has_type_subprotocol_of : Ctl_parser_types.ast_node -> ALVar.alexp -> bool val get_available_attr_ios_sdk : Ctl_parser_types.ast_node -> string option +val get_selector : Ctl_parser_types.ast_node -> string option + val within_responds_to_selector_block : CLintersContext.context -> Ctl_parser_types.ast_node -> bool @@ -93,6 +97,10 @@ val objc_method_has_nth_parameter_of_type : val using_namespace : Ctl_parser_types.ast_node -> ALVar.alexp -> bool +val receiver_class_method_call : Ctl_parser_types.ast_node -> Clang_ast_t.decl option + +val receiver_instance_method_call : Ctl_parser_types.ast_node -> Clang_ast_t.decl option + val receiver_method_call : Ctl_parser_types.ast_node -> Clang_ast_t.decl option val is_at_selector_with_name : Ctl_parser_types.ast_node -> ALVar.alexp -> bool @@ -101,3 +109,5 @@ val is_at_selector_with_name : Ctl_parser_types.ast_node -> ALVar.alexp -> bool val has_visibility_attribute : Ctl_parser_types.ast_node -> ALVar.alexp -> bool val has_used_attribute : Ctl_parser_types.ast_node -> bool + +val within_available_class_block : CLintersContext.context -> Ctl_parser_types.ast_node -> bool diff --git a/infer/src/clang/cTL.ml b/infer/src/clang/cTL.ml index 6e61f9689..48b263806 100644 --- a/infer/src/clang/cTL.ml +++ b/infer/src/clang/cTL.ml @@ -800,6 +800,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 + | "within_available_class_block", [], an + -> CPredicates.within_available_class_block lcxt an | _ -> failwith ("ERROR: Undefined Predicate or wrong set of arguments: '" ^ pred_name ^ "'") diff --git a/infer/tests/codetoanalyze/objc/ioslints/issues.exp b/infer/tests/codetoanalyze/objc/ioslints/issues.exp index 4b461cc77..e8f0bfd45 100644 --- a/infer/tests/codetoanalyze/objc/ioslints/issues.exp +++ b/infer/tests/codetoanalyze/objc/ioslints/issues.exp @@ -4,9 +4,8 @@ codetoanalyze/objc/ioslints/unavailable_api_allowed_cases.m, Unavailable_api_all codetoanalyze/objc/ioslints/unavailable_api_allowed_cases.m, Unavailable_api_allowed_cases_with_responds_to_selector_in_else:, 71, UNAVAILABLE_API_IN_SUPPORTED_IOS_SDK, [] codetoanalyze/objc/ioslints/unavailable_api_allowed_cases.m, Unavailable_api_allowed_cases_without_instances_responds_to_selector, 95, UNAVAILABLE_API_IN_SUPPORTED_IOS_SDK, [] codetoanalyze/objc/ioslints/unavailable_api_allowed_cases.m, Unavailable_api_allowed_cases_without_responds_to_selector:, 64, UNAVAILABLE_API_IN_SUPPORTED_IOS_SDK, [] -codetoanalyze/objc/ioslints/unavailable_api_in_supported_ios_sdk.m, OpenURLOptionsFromSourceApplication, 51, UNAVAILABLE_API_IN_SUPPORTED_IOS_SDK, [] +codetoanalyze/objc/ioslints/unavailable_api_in_supported_ios_sdk.m, OpenURLOptionsFromSourceApplication, 68, UNAVAILABLE_API_IN_SUPPORTED_IOS_SDK, [] codetoanalyze/objc/ioslints/unavailable_api_in_supported_ios_sdk.m, Unavailable_api_in_supported_ios_sdk_test:and:, 28, UNAVAILABLE_API_IN_SUPPORTED_IOS_SDK, [] codetoanalyze/objc/ioslints/unavailable_api_in_supported_ios_sdk.m, Unavailable_api_in_supported_ios_sdk_unsupported_class, 34, UNAVAILABLE_CLASS_IN_SUPPORTED_IOS_SDK, [] -codetoanalyze/objc/ioslints/unavailable_api_in_supported_ios_sdk.m, Unavailable_api_in_supported_ios_sdk_unsupported_class, 34, UNAVAILABLE_CLASS_IN_SUPPORTED_IOS_SDK, [] -codetoanalyze/objc/ioslints/unavailable_api_in_supported_ios_sdk.m, Unavailable_api_in_supported_ios_sdk_unsupported_class_with_attributes:, 44, UNAVAILABLE_CLASS_IN_SUPPORTED_IOS_SDK, [] +codetoanalyze/objc/ioslints/unavailable_api_in_supported_ios_sdk.m, Unavailable_api_in_supported_ios_sdk_unsupported_class_with_attributes:, 53, UNAVAILABLE_CLASS_IN_SUPPORTED_IOS_SDK, [] codetoanalyze/objc/ioslints/unavailable_property_ios.m, FNFPlayerLayer_initWithConfigs:, 22, UNAVAILABLE_API_IN_SUPPORTED_IOS_SDK, [] diff --git a/infer/tests/codetoanalyze/objc/ioslints/unavailable_api_in_supported_ios_sdk.m b/infer/tests/codetoanalyze/objc/ioslints/unavailable_api_in_supported_ios_sdk.m index b484db3f0..794b85b41 100644 --- a/infer/tests/codetoanalyze/objc/ioslints/unavailable_api_in_supported_ios_sdk.m +++ b/infer/tests/codetoanalyze/objc/ioslints/unavailable_api_in_supported_ios_sdk.m @@ -38,11 +38,28 @@ NS_CLASS_AVAILABLE(10_12, 10_0) NSLog(@""); } } +// no bug +- (void)unsupported_class_with_check { + if ([AVPlayerLooper class]) { + AVPlayerLooper* looper = + [[AVPlayerLooper alloc] initWithPlayer:nil + templateItem:nil + timeRange:kCMTimeRangeInvalid]; + } +} // bug - (void)unsupported_class_with_attributes:(nonnull Unav_class*)c { - [c m]; + [Unav_class new]; +} + +// no bug +- (void)unsupported_class_with_attributes_with_check:(nonnull Unav_class*)c { + if ([Unav_class class]) { + [[Unav_class alloc] init]; + } } + @end static NSDictionary* OpenURLOptionsFromSourceApplication(