diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index 0ea4d3919..a14c4d776 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/infer/src/clang/cFrontend_errors.ml @@ -462,14 +462,16 @@ let log_frontend_issue method_decl_opt (node: Ctl_parser_types.ast_node) ~node_id:(0, key) ?linters_def_file ?doc_url:issue_desc.doc_url -let fill_issue_desc_info_and_log context an (issue_desc: CIssue.issue_desc) linters_def_file loc = +let fill_issue_desc_info_and_log context ~witness ~current_node (issue_desc: CIssue.issue_desc) + linters_def_file loc = let process_message message = - remove_new_lines_and_whitespace (expand_message_string context message an) + remove_new_lines_and_whitespace (expand_message_string context message current_node) in let description = process_message issue_desc.description in let suggestion = Option.map ~f:process_message issue_desc.suggestion in let issue_desc' = {issue_desc with description; loc; suggestion} in - try log_frontend_issue context.CLintersContext.current_method an issue_desc' linters_def_file + try + log_frontend_issue context.CLintersContext.current_method witness issue_desc' linters_def_file with CFrontend_config.IncorrectAssumption e -> let trans_unit_ctx = context.CLintersContext.translation_unit_context in ClangLogging.log_caught_exception trans_unit_ctx "IncorrectAssumption" e.position @@ -485,7 +487,8 @@ let invoke_set_of_hard_coded_checkers_an context (an: Ctl_parser_types.ast_node) List.iter ~f:(fun issue_desc -> if CIssue.should_run_check issue_desc.CIssue.mode then - fill_issue_desc_info_and_log context an issue_desc None issue_desc.CIssue.loc ) + fill_issue_desc_info_and_log context ~witness:an ~current_node:an issue_desc None + issue_desc.CIssue.loc ) issue_desc_list ) checkers @@ -500,7 +503,8 @@ let invoke_set_of_parsed_checkers_an parsed_linters context (an: Ctl_parser_type () | Some witness -> let loc = CFrontend_checkers.location_from_an context witness in - fill_issue_desc_info_and_log context witness linter.issue_desc linter.def_file loc ) + fill_issue_desc_info_and_log context ~witness ~current_node:an linter.issue_desc + linter.def_file loc ) parsed_linters diff --git a/infer/src/clang/cFrontend_errors.mli b/infer/src/clang/cFrontend_errors.mli index 55d086a24..ad448d3f3 100644 --- a/infer/src/clang/cFrontend_errors.mli +++ b/infer/src/clang/cFrontend_errors.mli @@ -49,5 +49,6 @@ val create_parsed_linters : string -> CTL.ctl_checker list -> linter list val remove_new_lines_and_whitespace : string -> string val fill_issue_desc_info_and_log : - CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.issue_desc -> string option - -> Location.t -> unit + CLintersContext.context -> witness:Ctl_parser_types.ast_node + -> current_node:Ctl_parser_types.ast_node -> CIssue.issue_desc -> string option -> Location.t + -> unit diff --git a/infer/src/clang/tableaux.ml b/infer/src/clang/tableaux.ml index fdb4cd0d9..c3eb311f6 100644 --- a/infer/src/clang/tableaux.ml +++ b/infer/src/clang/tableaux.ml @@ -295,7 +295,9 @@ let report_issue an lcxt linter (*npo_condition*) = CTL.Debug.pp_formula linter.condition ;*) let loc = CFrontend_checkers.location_from_an lcxt an in let should_report = match an with Decl dec -> is_decl_allowed lcxt dec | Stmt _ -> true in - if should_report then fill_issue_desc_info_and_log lcxt an linter.issue_desc linter.def_file loc + if should_report then + fill_issue_desc_info_and_log lcxt ~witness:an ~current_node:an linter.issue_desc + linter.def_file loc let check_linter_map linter_map_contex phi = diff --git a/infer/tests/codetoanalyze/cpp/linters/Makefile b/infer/tests/codetoanalyze/cpp/linters/Makefile new file mode 100644 index 000000000..ba078f419 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/linters/Makefile @@ -0,0 +1,18 @@ +# Copyright (c) 2016-present, Facebook, Inc. +# +# This source code is licensed under the MIT license found in the +# LICENSE file in the root directory of this source tree. + +TESTS_DIR = ../../.. + +ANALYZER = linters +CLANG_OPTIONS = -std=c++11 -c +INFER_OPTIONS = -a linters --linters-def-file extracopy.al --no-filtering --debug-exceptions --project-root $(TESTS_DIR) \ +--enable-issue-type GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL +INFERPRINT_OPTIONS = --issues-tests + +SOURCES = \ + $(wildcard *.cpp) \ + + +include $(TESTS_DIR)/clang.make diff --git a/infer/tests/codetoanalyze/cpp/linters/extracopy.al b/infer/tests/codetoanalyze/cpp/linters/extracopy.al new file mode 100644 index 000000000..335f7253b --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/linters/extracopy.al @@ -0,0 +1,26 @@ +// Copyright (c) 2018-present, Facebook, Inc. +// +// This source code is licensed under the MIT license found in the +// LICENSE file in the root directory of this source tree. +DEFINE-CHECKER EXTRA_COPY = { + LET name_not_contains_copy = NOT declaration_has_name(REGEXP("[cC]opy")); + + LET is_local_var = NOT is_global_var AND NOT is_static_local_var; + + LET is_copy_contructor = + HOLDS-NEXT WITH-TRANSITION InitExpr + (is_node("CXXConstructExpr") AND is_cxx_copy_constructor); + + SET report_when = + WHEN name_not_contains_copy AND is_local_var AND is_copy_contructor + HOLDS-IN-NODE VarDecl; + + SET message = "Potentially unnecessary to copy var %name%"; + SET severity = "WARNING"; + SET mode = "ON"; + //SET whitelist_path = { + // REGEXP("admarket/.*"), + // REGEXP("multifeed/.*") + //}; + +}; diff --git a/infer/tests/codetoanalyze/cpp/linters/extracopy.cpp b/infer/tests/codetoanalyze/cpp/linters/extracopy.cpp new file mode 100644 index 000000000..e5221e887 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/linters/extracopy.cpp @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2018-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include +#include + +struct A { + int a; +}; + +const A& get_a_ref() { + static A a; + return a; +} + +A get_a() { + A a; + return a; +} + +int test_a() { + static auto x = get_a_ref(); + auto y = get_a_ref(); + auto copy_y = get_a_ref(); + auto z = get_a(); + return 0; +} + +auto global_a = get_a_ref(); + +void test_map() { + std::map intMap{{1, 2}, {3, 4}}; + for (auto p : intMap) { + std::cout << p.first << "->" << p.second << std::endl; + } + for (auto copy_p : intMap) { + std::cout << copy_p.first << "->" << copy_p.second << std::endl; + } + for (const auto& p : intMap) { + std::cout << p.first << "->" << p.second << std::endl; + } +} diff --git a/infer/tests/codetoanalyze/cpp/linters/issues.exp b/infer/tests/codetoanalyze/cpp/linters/issues.exp new file mode 100644 index 000000000..ce0ea43f5 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/linters/issues.exp @@ -0,0 +1,2 @@ +codetoanalyze/cpp/linters/extracopy.cpp, test_a, 33, EXTRA_COPY, no_bucket, WARNING, [] +codetoanalyze/cpp/linters/extracopy.cpp, test_map, 43, EXTRA_COPY, no_bucket, WARNING, []