From f6586908469c1502f9597bd45f5b9afd5f0b2b8e Mon Sep 17 00:00:00 2001 From: Jia Chen Date: Thu, 27 Jul 2017 14:07:07 -0700 Subject: [PATCH] Whitelist more iterator-related functions and classes Reviewed By: jeremydubreil Differential Revision: D5503746 fbshipit-source-id: 945c552 --- infer/src/IR/QualifiedCppName.ml | 14 ++-- infer/src/base/Config.ml | 66 ++++++++++++++--- infer/tests/codetoanalyze/cpp/errors/Makefile | 2 +- .../tests/codetoanalyze/cpp/errors/issues.exp | 5 +- .../codetoanalyze/cpp/errors/models/pair.cpp | 6 -- .../cpp/errors/vector/iterator_cmp.cpp | 73 +++++++++++++++++++ 6 files changed, 140 insertions(+), 26 deletions(-) create mode 100644 infer/tests/codetoanalyze/cpp/errors/vector/iterator_cmp.cpp diff --git a/infer/src/IR/QualifiedCppName.ml b/infer/src/IR/QualifiedCppName.ml index 1df962fa5..8e70388ee 100644 --- a/infer/src/IR/QualifiedCppName.ml +++ b/infer/src/IR/QualifiedCppName.ml @@ -76,15 +76,11 @@ module Match = struct (* Fail if we detect templates in the fuzzy name. Template instantiations are not taken into account when fuzzy matching, and templates may produce wrong results when parsing qualified names. *) - let filtered_qual_name = - (* Filter out the '<' in operator< and operator<= *) - let operator_less_length = 14 in - if String.is_prefix qual_name ~prefix:"std::operator<" then - String.drop_prefix qual_name operator_less_length - else qual_name - in - if String.contains filtered_qual_name '<' then - failwithf "Unexpected template in fuzzy qualified name %s." qual_name ; + let colon_splits = String.split qual_name ~on:':' in + List.iter colon_splits ~f:(fun s -> + (* Filter out the '<' in operator< and operator<= *) + if not (String.is_prefix s ~prefix:"operator<") && String.contains s '<' then + failwithf "Unexpected template in fuzzy qualified name %s." qual_name ) ; of_qual_string qual_name let of_fuzzy_qual_names fuzzy_qual_names = diff --git a/infer/src/base/Config.ml b/infer/src/base/Config.ml index da3ddbbf0..511abf24c 100644 --- a/infer/src/base/Config.ml +++ b/infer/src/base/Config.ml @@ -241,10 +241,15 @@ let use_jar_cache = true let weak = "<\"Weak\">" -let whitelisted_cpp_methods = - [ "google::CheckNotNull" +(* Whitelists for C++ library functions *) + +let std_whitelisted_cpp_methods = + [ "std::back_inserter" ; "std::forward" + ; "std::front_inserter" ; "std::get" + ; "std::inserter" + ; "std::make_move_iterator" ; "std::make_pair" ; "std::max" ; "std::min" @@ -257,19 +262,62 @@ let whitelisted_cpp_methods = ; "std::operator>=" ; "std::swap" ] -let whitelisted_cpp_classes = - [ "__gnu_cxx::__normal_iterator" (* libstdc++ internal name of vector iterator *) - ; "std::__less" - ; "std::__wrap_iter" (* libc++ internal name of vector iterator *) - ; "std::__get_pair" (* libc++ internal support class for std::get *) - ; "std::__pair_get" (* libstdc++ internal support class for std::get *) +let libstdcxx_whitelisted_cpp_methods = + [ "__gnu_cxx::operator!=" + ; "__gnu_cxx::operator<" + ; "__gnu_cxx::operator<=" + ; "__gnu_cxx::operator==" + ; "__gnu_cxx::operator>" + ; "__gnu_cxx::operator>=" + ; "__gnu_cxx::operator+" + ; "__gnu_cxx::operator-" ] + +let libcxx_whitelisted_cpp_methods = [] + +let other_whitelisted_cpp_methods = ["google::CheckNotNull"] + +let whitelisted_cpp_methods = + List.concat + [ std_whitelisted_cpp_methods + ; libstdcxx_whitelisted_cpp_methods + ; libcxx_whitelisted_cpp_methods + ; other_whitelisted_cpp_methods ] + +(* Whitelists for C++ library classes *) + +let std_whitelisted_cpp_classes = + [ "std::back_insert_iterator" ; "std::equal_to" + ; "std::front_insert_iterator" ; "std::greater" ; "std::greater_equal" + ; "std::insert_iterator" ; "std::less" ; "std::less_equal" + ; "std::move_iterator" ; "std::not_equal_to" - ; "std::pair" ] + ; "std::pair" + ; "std::reverse_iterator" ] + +let libstdcxx_whitelisted_cpp_classes = + (* libstdc++ internal support class for std::get *) + [ "__gnu_cxx::__normal_iterator" (* libstdc++ internal name of vector iterator *) + ; "std::__pair_get" ] + +let libcxx_whitelisted_cpp_classes = + (* libc++ internal support class for std::get *) + [ "std::__less" + ; "std::__wrap_iter" (* libc++ internal name of vector iterator *) + ; "std::__get_pair" ] + +let other_whitelisted_cpp_classes = [] + +let whitelisted_cpp_classes = + List.concat + [ std_whitelisted_cpp_classes + ; libstdcxx_whitelisted_cpp_classes + ; libcxx_whitelisted_cpp_classes + ; other_whitelisted_cpp_classes ] type dynamic_dispatch_policy = [`None | `Interface | `Sound | `Lazy] diff --git a/infer/tests/codetoanalyze/cpp/errors/Makefile b/infer/tests/codetoanalyze/cpp/errors/Makefile index be13a34fd..672e2d6a4 100644 --- a/infer/tests/codetoanalyze/cpp/errors/Makefile +++ b/infer/tests/codetoanalyze/cpp/errors/Makefile @@ -9,7 +9,7 @@ TESTS_DIR = ../../.. ANALYZER = infer -CLANG_OPTIONS = -x c++ -std=c++14 -isystem$(ROOT_DIR) -c +CLANG_OPTIONS = -x c++ -std=c++1y -isystem$(ROOT_DIR) -c INFER_OPTIONS = --ml-buckets cpp --no-filtering --debug-exceptions --project-root $(TESTS_DIR) \ --no-keep-going --pmd-xml --report-custom-error INFERPRINT_OPTIONS = --issues-tests diff --git a/infer/tests/codetoanalyze/cpp/errors/issues.exp b/infer/tests/codetoanalyze/cpp/errors/issues.exp index df65b9af7..9c637d06c 100644 --- a/infer/tests/codetoanalyze/cpp/errors/issues.exp +++ b/infer/tests/codetoanalyze/cpp/errors/issues.exp @@ -32,7 +32,6 @@ codetoanalyze/cpp/errors/models/move.cpp, move::div0_moved_from, 3, DIVIDE_BY_ZE codetoanalyze/cpp/errors/models/move.cpp, move::div0_moved_to, 3, DIVIDE_BY_ZERO, [start of procedure move::div0_moved_to(),start of procedure X,return from a call to move::X_X,start of procedure X,return from a call to move::X_X] codetoanalyze/cpp/errors/models/pair.cpp, pair::deref_pair_null0_bad, 3, NULL_DEREFERENCE, [start of procedure pair::deref_pair_null0_bad(),start of procedure pair::pairOfZeroNull(),return from a call to pair::pairOfZeroNull] codetoanalyze/cpp/errors/models/pair.cpp, pair::deref_pair_null1_bad, 3, NULL_DEREFERENCE, [start of procedure pair::deref_pair_null1_bad(),start of procedure pair::pairOfZeroNull(),return from a call to pair::pairOfZeroNull] -codetoanalyze/cpp/errors/models/pair.cpp, pair::deref_pair_null2_bad, 3, NULL_DEREFERENCE, [start of procedure pair::deref_pair_null2_bad(),start of procedure pair::pairOfZeroNull(),return from a call to pair::pairOfZeroNull] codetoanalyze/cpp/errors/models/pair.cpp, pair::deref_pair_null3_bad, 3, NULL_DEREFERENCE, [start of procedure pair::deref_pair_null3_bad(),start of procedure pair::pairOfZeroNull2(),return from a call to pair::pairOfZeroNull2] codetoanalyze/cpp/errors/models/swap.cpp, swap_null_bad, 4, NULL_DEREFERENCE, [start of procedure swap_null_bad()] codetoanalyze/cpp/errors/models/throw_wrapper.cpp, nothrow_if_null_bad, 4, NULL_DEREFERENCE, [start of procedure nothrow_if_null_bad(),start of procedure get_null(),return from a call to get_null,Condition is true,start of procedure do_nothing(),return from a call to do_nothing] @@ -213,6 +212,10 @@ codetoanalyze/cpp/errors/vector/empty_access.cpp, vector_as_param_by_value_empty codetoanalyze/cpp/errors/vector/empty_access.cpp, vector_as_param_clear, 3, EMPTY_VECTOR_ACCESS, [start of procedure vector_as_param_clear(),start of procedure vector_param_clear(),return from a call to vector_param_clear] codetoanalyze/cpp/errors/vector/empty_access.cpp, vector_as_param_empty, 2, EMPTY_VECTOR_ACCESS, [start of procedure vector_as_param_empty(),start of procedure vector_param_access(),return from a call to vector_param_access] codetoanalyze/cpp/errors/vector/iterator_access.cpp, iterator_access::possible_npe, 4, NULL_DEREFERENCE, [start of procedure iterator_access::possible_npe(),Condition is true,Condition is true,Condition is true] +codetoanalyze/cpp/errors/vector/iterator_cmp.cpp, iterator_compare::empty_deref1_bad, 4, NULL_DEREFERENCE, [start of procedure iterator_compare::empty_deref1_bad(),start of procedure iterator_compare::is_empty(),return from a call to iterator_compare::is_empty,Condition is true] +codetoanalyze/cpp/errors/vector/iterator_cmp.cpp, iterator_compare::empty_deref2_bad, 4, NULL_DEREFERENCE, [start of procedure iterator_compare::empty_deref2_bad(),start of procedure iterator_compare::not_empty(),return from a call to iterator_compare::not_empty,Condition is false,Condition is true] +codetoanalyze/cpp/errors/vector/iterator_cmp.cpp, iterator_compare::not_empty_deref1_bad, 4, NULL_DEREFERENCE, [start of procedure iterator_compare::not_empty_deref1_bad(),Skipped call: function or method not found,start of procedure iterator_compare::is_empty(),return from a call to iterator_compare::is_empty,Condition is false,Condition is true] +codetoanalyze/cpp/errors/vector/iterator_cmp.cpp, iterator_compare::not_empty_deref2_bad, 4, NULL_DEREFERENCE, [start of procedure iterator_compare::not_empty_deref2_bad(),Skipped call: function or method not found,start of procedure iterator_compare::not_empty(),return from a call to iterator_compare::not_empty,Condition is true] codetoanalyze/cpp/shared/attributes/annotate.cpp, derefFirstArg2_null_deref, 2, NULL_DEREFERENCE, [start of procedure derefFirstArg2_null_deref()] codetoanalyze/cpp/shared/attributes/annotate.cpp, derefFirstArg3_null_deref, 2, NULL_DEREFERENCE, [start of procedure derefFirstArg3_null_deref(),start of procedure derefFirstArg3()] codetoanalyze/cpp/shared/attributes/annotate.cpp, derefFirstArg_null_deref, 2, NULL_DEREFERENCE, [start of procedure derefFirstArg_null_deref()] diff --git a/infer/tests/codetoanalyze/cpp/errors/models/pair.cpp b/infer/tests/codetoanalyze/cpp/errors/models/pair.cpp index b47ab4423..c698aa8e4 100644 --- a/infer/tests/codetoanalyze/cpp/errors/models/pair.cpp +++ b/infer/tests/codetoanalyze/cpp/errors/models/pair.cpp @@ -30,12 +30,6 @@ int deref_pair_null1_bad() { return std::get<0>(p) + *std::get<1>(p); } -int deref_pair_null2_bad() { - auto p = pairOfZeroNull(); - // Should report an NPE here as p.second is NULL - return std::get(p) + *std::get(p); -} - int deref_pair_null3_bad() { auto p = pairOfZeroNull2(); // Should report an NPE here as p.second is NULL diff --git a/infer/tests/codetoanalyze/cpp/errors/vector/iterator_cmp.cpp b/infer/tests/codetoanalyze/cpp/errors/vector/iterator_cmp.cpp new file mode 100644 index 000000000..3457b5340 --- /dev/null +++ b/infer/tests/codetoanalyze/cpp/errors/vector/iterator_cmp.cpp @@ -0,0 +1,73 @@ +/* + * 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. + */ +#include + +namespace iterator_compare { + +bool is_empty(const std::vector& vec) { return vec.begin() == vec.end(); } + +bool not_empty(const std::vector& vec) { return vec.begin() != vec.end(); } + +void empty_no_deref1_ok() { + int* p = nullptr; + std::vector vec; + if (!is_empty(vec)) + *p = 42; +} + +void empty_no_deref2_ok() { + int* p = nullptr; + std::vector vec; + if (not_empty(vec)) + *p = 42; +} + +void empty_deref1_bad() { + int* p = nullptr; + std::vector vec; + if (is_empty(vec)) + *p = 42; +} + +void empty_deref2_bad() { + int* p = nullptr; + std::vector vec; + if (!not_empty(vec)) + *p = 42; +} + +void not_empty_no_deref1_ok() { + int* p = nullptr; + std::vector vec = {1, 2, 3, 4}; + if (is_empty(vec)) + *p = 42; +} + +void not_empty_no_deref2_ok() { + int* p = nullptr; + std::vector vec = {1, 2, 3, 4}; + if (!not_empty(vec)) + *p = 42; +} + +void not_empty_deref1_bad() { + int* p = nullptr; + std::vector vec = {1, 2, 3, 4}; + if (!is_empty(vec)) + *p = 42; +} + +void not_empty_deref2_bad() { + int* p = nullptr; + std::vector vec = {1, 2, 3, 4}; + if (not_empty(vec)) + *p = 42; +} + +} // namespace iterator_compare