From 5569ee751a9958719d2289efed69009bb13d674c Mon Sep 17 00:00:00 2001 From: Dulma Churchill Date: Mon, 18 Sep 2017 06:43:06 -0700 Subject: [PATCH] [linters] Add transitions Fields and FieldName Reviewed By: jvillard Differential Revision: D5834422 fbshipit-source-id: 11519b3 --- infer/src/clang/cTL.ml | 62 ++++++++++++++++++- infer/src/clang/cTL.mli | 24 +++---- infer/src/clang/ctl_lexer.mll | 2 + infer/src/clang/ctl_parser.mly | 4 ++ .../TestStructFieldChecks.mm | 42 +++++++++++++ .../objcpp/linters-for-test-only/issues.exp | 4 ++ .../linters-for-test-only/linters_example.al | 45 ++++++++++++++ 7 files changed, 165 insertions(+), 18 deletions(-) create mode 100644 infer/tests/codetoanalyze/objcpp/linters-for-test-only/TestStructFieldChecks.mm diff --git a/infer/src/clang/cTL.ml b/infer/src/clang/cTL.ml index 70948dce9..87a0a352d 100644 --- a/infer/src/clang/cTL.ml +++ b/infer/src/clang/cTL.ml @@ -15,20 +15,23 @@ module L = Logging are intepreted over the AST of the program. A checker is defined by a CTL formula which express a condition saying when the checker should report a problem *) -(* Transition labels used for example to switch from decl to stmt *) + +(** Transition labels used for example to switch from decl to stmt *) type transitions = | Body (** decl to stmt *) + | FieldName of ALVar.alexp (** stmt to stmt, decl to decl *) + | Fields (** stmt to stmt, decl to decl *) | InitExpr (** decl to stmt *) | Super (** decl to decl *) | ParameterName of ALVar.alexp (** stmt to stmt, decl to decl *) - | Parameters (** decl to decl *) + | Parameters (** stmt to stmt, decl to decl *) | Cond | PointerToDecl (** stmt to decl *) | Protocol (** decl to decl *) let is_transition_to_successor trans = match trans with - | Body | InitExpr | ParameterName _ | Parameters | Cond + | Body | InitExpr | FieldName _ | Fields | ParameterName _ | Parameters | Cond -> true | Super | PointerToDecl | Protocol -> false @@ -126,6 +129,10 @@ module Debug = struct match trans with | Body -> Format.pp_print_string fmt "Body" + | FieldName name + -> Format.pp_print_string fmt ("FieldName " ^ ALVar.alexp_to_string name) + | Fields + -> Format.pp_print_string fmt "Fields" | InitExpr -> Format.pp_print_string fmt "InitExpr" | Super @@ -714,11 +721,60 @@ let transition_via_parameter_name an name = | _ -> [] +let transition_via_fields an = + let open Clang_ast_t in + match an with + | Decl RecordDecl (_, _, _, _, decls, _, _) | Decl CXXRecordDecl (_, _, _, _, decls, _, _, _) + -> List.filter_map ~f:(fun d -> match d with FieldDecl _ -> Some (Decl d) | _ -> None) decls + | Stmt InitListExpr (_, stmts, _) + -> List.map ~f:(fun stmt -> Stmt stmt) stmts + | _ + -> [] + +let field_has_name name node = + match node with + | Decl FieldDecl (_, name_info, _, _) + -> ALVar.compare_str_with_alexp name_info.Clang_ast_t.ni_name name + | _ + -> false + +let field_of_name name nodes = List.filter ~f:(field_has_name name) nodes + +let field_of_corresp_name_from_init_list_expr name init_nodes (expr_info: Clang_ast_t.expr_info) = + match CAst_utils.get_decl_from_typ_ptr expr_info.ei_qual_type.qt_type_ptr with + | Some decl + -> ( + let fields = transition_via_fields (Decl decl) in + match List.zip init_nodes fields with + | Some init_nodes_fields + -> List.filter ~f:(fun (_, field) -> field_has_name name field) init_nodes_fields + |> List.map ~f:(fun (node, _) -> node) + | None + -> [] ) + | None + -> [] + +let transition_via_field_name node name = + let open Clang_ast_t in + match node with + | Decl RecordDecl _ | Decl CXXRecordDecl _ + -> let fields = transition_via_fields node in + field_of_name name fields + | Stmt InitListExpr (_, stmts, expr_info) + -> let nodes = List.map ~f:(fun stmt -> Stmt stmt) stmts in + field_of_corresp_name_from_init_list_expr name nodes expr_info + | _ + -> [] + (* given a node an returns a list of nodes an' such that an transition to an' via label trans *) let next_state_via_transition an trans = match (an, trans) with | Decl d, Super -> transition_decl_to_decl_via_super d + | _, FieldName name + -> transition_via_field_name an name + | _, Fields + -> transition_via_fields an | _, Parameters -> transition_via_parameters an | Decl d, InitExpr | Decl d, Body diff --git a/infer/src/clang/cTL.mli b/infer/src/clang/cTL.mli index ae1d8dfbd..1502f0bf8 100644 --- a/infer/src/clang/cTL.mli +++ b/infer/src/clang/cTL.mli @@ -9,27 +9,22 @@ open! IStd open Ctl_parser_types - (* This module defines a language to define checkers. These checkers are intepreted over the AST of the program. A checker is defined by a CTL formula which express a condition saying when the checker should report a problem *) -(* Transition labels used for example to switch from decl to stmt *) +(** Transition labels used for example to switch from decl to stmt *) type transitions = - | Body - (* decl to stmt *) - | InitExpr - (* decl to stmt *) - | Super - (* decl to decl *) - | ParameterName of ALVar.alexp - (* stmt to stmt, decl to decl *) - | Parameters - (* decl to decl *) + | Body (** decl to stmt *) + | FieldName of ALVar.alexp (** stmt to stmt, decl to decl *) + | Fields (** stmt to stmt, decl to decl *) + | InitExpr (** decl to stmt *) + | Super (** decl to decl *) + | ParameterName of ALVar.alexp (** stmt to stmt, decl to decl *) + | Parameters (** stmt to stmt, decl to decl *) | Cond - | PointerToDecl - (* stmt to decl *) + | PointerToDecl (** stmt to decl *) | Protocol (** decl to decl *) (* In formulas below prefix @@ -72,7 +67,6 @@ type t = there exists a descentant an of the current node such that an is of type in set T making a transition to a node an' via label l, such that in an phi holds. *) - (* "set" clauses are used for defining mandatory variables that will be used by when reporting issues: eg for defining the condition. diff --git a/infer/src/clang/ctl_lexer.mll b/infer/src/clang/ctl_lexer.mll index 1c0842476..b01dceefa 100644 --- a/infer/src/clang/ctl_lexer.mll +++ b/infer/src/clang/ctl_lexer.mll @@ -71,6 +71,8 @@ rule token = parse | "IMPLIES" { IMPLIES } | "REGEXP" { REGEXP } | "Any" {ANY} + | "Fields" { FIELDS } + | "FieldName" { FIELD_NAME } | "Parameters" { PARAMETERS } | "ParameterName" { PARAMETER_NAME } | "Body" {BODY} diff --git a/infer/src/clang/ctl_parser.mly b/infer/src/clang/ctl_parser.mly index 61673c887..0d211e878 100644 --- a/infer/src/clang/ctl_parser.mly +++ b/infer/src/clang/ctl_parser.mly @@ -79,6 +79,8 @@ %token BODY %token COND %token INIT_EXPR +%token FIELDS +%token FIELD_NAME %token PARAMETERS %token PARAMETER_NAME %token POINTER_TO_DECL @@ -237,6 +239,8 @@ transition_label: | ANY { None } | BODY { Some CTL.Body } | COND { Some CTL.Cond } + | FIELDS { Some CTL.Fields } + | FIELD_NAME alexp { Some (CTL.FieldName $2) } | INIT_EXPR { Some CTL.InitExpr } | PARAMETERS { Some CTL.Parameters } | PARAMETER_NAME alexp { Some (CTL.ParameterName $2) } diff --git a/infer/tests/codetoanalyze/objcpp/linters-for-test-only/TestStructFieldChecks.mm b/infer/tests/codetoanalyze/objcpp/linters-for-test-only/TestStructFieldChecks.mm new file mode 100644 index 000000000..49afbcb46 --- /dev/null +++ b/infer/tests/codetoanalyze/objcpp/linters-for-test-only/TestStructFieldChecks.mm @@ -0,0 +1,42 @@ +/* + * 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 + +struct Origin { + int x; + int y; +}; + +struct Rectangle { + int width; + int height; + NSString* title; // bug + Origin origin; +}; + +@interface SomeButton : NSObject + ++ (instancetype)newWithFrame:(Rectangle)aRect; + +@end + +SomeButton* buttonComponent(void); +SomeButton* buttonComponent(void) { + SomeButton* c; + // bug + c = [SomeButton newWithFrame:{}]; + // bug + c = [SomeButton newWithFrame:{.width = 100, .height = 200}]; + // no bug + c = [SomeButton newWithFrame:{.title = @"Button"}]; + // bug + c = [SomeButton newWithFrame:{.origin = {.x = 20, .y = 20}}]; + + return c; +}; diff --git a/infer/tests/codetoanalyze/objcpp/linters-for-test-only/issues.exp b/infer/tests/codetoanalyze/objcpp/linters-for-test-only/issues.exp index 4e4603c1f..bd0b3bbfe 100644 --- a/infer/tests/codetoanalyze/objcpp/linters-for-test-only/issues.exp +++ b/infer/tests/codetoanalyze/objcpp/linters-for-test-only/issues.exp @@ -22,4 +22,8 @@ codetoanalyze/objcpp/linters-for-test-only/TestParamterLabelsChecks.mm, foo, 54, codetoanalyze/objcpp/linters-for-test-only/TestParamterLabelsChecks.mm, foo, 54, TEST_PARAMETER_SELECTOR_BY_TYPE, [] codetoanalyze/objcpp/linters-for-test-only/TestParamterLabelsChecks.mm, foo, 56, TEST_PARAMETER_SELECTOR, [] codetoanalyze/objcpp/linters-for-test-only/TestParamterLabelsChecks.mm, foo, 56, TEST_PARAMETER_SELECTOR_BY_TYPE, [] +codetoanalyze/objcpp/linters-for-test-only/TestStructFieldChecks.mm, Linters_dummy_method, 19, FIELD_STRUCT_STRING, [] +codetoanalyze/objcpp/linters-for-test-only/TestStructFieldChecks.mm, buttonComponent, 33, TITLE_NOT_INITIALIZED, [] +codetoanalyze/objcpp/linters-for-test-only/TestStructFieldChecks.mm, buttonComponent, 35, TITLE_NOT_INITIALIZED, [] +codetoanalyze/objcpp/linters-for-test-only/TestStructFieldChecks.mm, buttonComponent, 39, TITLE_NOT_INITIALIZED, [] codetoanalyze/objcpp/linters-for-test-only/hash_test.mm, std::hash_NSObject_*__operator(), 14, DISCOURAGED_HASH_METHOD_INVOCATION, [] diff --git a/infer/tests/codetoanalyze/objcpp/linters-for-test-only/linters_example.al b/infer/tests/codetoanalyze/objcpp/linters-for-test-only/linters_example.al index 89d85abf5..801ec121f 100644 --- a/infer/tests/codetoanalyze/objcpp/linters-for-test-only/linters_example.al +++ b/infer/tests/codetoanalyze/objcpp/linters-for-test-only/linters_example.al @@ -149,3 +149,48 @@ DEFINE-CHECKER NEW_COMPONENT_USING_MEM_MODEL = { SET message = "Consider using fragment models instead."; }; + + +DEFINE-CHECKER TITLE_NOT_INITIALIZED = { + LET rectangle_is_not_initialized_with_title = + WHEN + HOLDS-NEXT WITH-TRANSITION FieldName "title" + (is_node("ImplicitValueInitExpr")) + HOLDS-IN-NODE InitListExpr; + + LET exists_rectangle_is_not_initialized_with_title = + rectangle_is_not_initialized_with_title HOLDS-EVENTUALLY; + + LET parameter_rectangle_is_not_initialized_with_title = + WHEN + HOLDS-NEXT WITH-TRANSITION Parameters + (exists_rectangle_is_not_initialized_with_title) + HOLDS-IN-NODE ObjCMessageExpr; + + SET report_when = + WHEN + call_method(REGEXP("^new.*:$")) AND + (parameter_rectangle_is_not_initialized_with_title) + HOLDS-IN-NODE ObjCMessageExpr; + + SET message = "The title is not initialized"; + SET severity = "WARNING"; + SET mode = "ON"; +}; + +DEFINE-CHECKER FIELD_STRUCT_STRING = { + LET has_field_string = + WHEN + HOLDS-NEXT WITH-TRANSITION Fields + (has_type("NSString*")) + HOLDS-IN-NODE CXXRecordDecl; + + SET report_when = + WHEN + declaration_has_name("Rectangle") AND + (has_field_string) + HOLDS-IN-NODE CXXRecordDecl; + SET message = "Found a field of type NSString"; + SET severity = "WARNING"; + SET mode = "ON"; +};