Index: clang-tidy/cppcoreguidelines/CMakeLists.txt =================================================================== --- clang-tidy/cppcoreguidelines/CMakeLists.txt +++ clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -2,6 +2,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule CppCoreGuidelinesTidyModule.cpp + ProBoundsConstantArrayIndexCheck.cpp ProBoundsPointerArithmeticCheck.cpp ProTypeConstCastCheck.cpp ProTypeReinterpretCastCheck.cpp Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp =================================================================== --- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "ProBoundsConstantArrayIndexCheck.h" #include "ProBoundsPointerArithmeticCheck.h" #include "ProTypeConstCastCheck.h" #include "ProTypeReinterpretCastCheck.h" @@ -23,6 +24,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "cppcoreguidelines-pro-bounds-constant-array-index"); CheckFactories.registerCheck( "cppcoreguidelines-pro-bounds-pointer-arithmetic"); CheckFactories.registerCheck( Index: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h =================================================================== --- /dev/null +++ clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h @@ -0,0 +1,34 @@ +//===--- ProBoundsConstantArrayIndexCheck.h - clang-tidy---------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_CONSTANT_ARRAY_INDEX_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_CONSTANT_ARRAY_INDEX_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { + +/// This checks that all array subscriptions on static arrays and std::arrays have a constant index and are within bounds +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.html +class ProBoundsConstantArrayIndexCheck : public ClangTidyCheck { +public: + ProBoundsConstantArrayIndexCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_CONSTANT_ARRAY_INDEX_H + Index: clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp @@ -0,0 +1,74 @@ +//===--- ProBoundsConstantArrayIndexCheck.cpp - clang-tidy-----------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "ProBoundsConstantArrayIndexCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { + +void ProBoundsConstantArrayIndexCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) + return; + + Finder->addMatcher(arraySubscriptExpr(hasBase(ignoringImpCasts(hasType(constantArrayType().bind("type")))), + hasIndex(expr().bind("index")) + ).bind("expr"), this); + + Finder->addMatcher(cxxOperatorCallExpr(hasOverloadedOperatorName("[]"), + hasArgument(0,hasType(qualType(hasDeclaration(classTemplateSpecializationDecl(hasName("::std::array")).bind("type"))))), + hasArgument(1, expr().bind("index")) + ).bind("expr"), + this); +} + +void ProBoundsConstantArrayIndexCheck::check(const MatchFinder::MatchResult &Result) { + llvm::APInt ArraySize; + const auto *Matched = Result.Nodes.getNodeAs("expr"); + if (const auto *ConstArrayType = Result.Nodes.getNodeAs("type")) { + ArraySize = ConstArrayType->getSize(); + } + + if (const auto *StdArrayDecl = Result.Nodes.getNodeAs("type")) { + const auto &TemplateArgs = StdArrayDecl->getTemplateArgs(); + if(TemplateArgs.size() < 2) + return; + // First template arg of std::array is the type, second arg is the size + const auto &SizeArg = TemplateArgs[1]; + if(SizeArg.getKind() != TemplateArgument::Integral) + return; + ArraySize = SizeArg.getAsIntegral(); + } + + const auto *IndexExpr = Result.Nodes.getNodeAs("index"); + llvm::APSInt Index; + if (!IndexExpr->isIntegerConstantExpr(Index, *Result.Context, nullptr, true)) { + //Fixit would need gsl::at() + diag(Matched->getExprLoc(), "do not use array subscript when the index is not a compile-time constant; use gsl::at() instead"); + return; + } + + if (Index.isSigned() && Index.isNegative()) { + diag(Matched->getExprLoc(), "array index %0 is before the beginning of the array") + << Index.toString(10); + return; + } + + if (Index.uge(ArraySize)) { + diag(Matched->getExprLoc(), "array index %0 is past the end of the array (which contains %1 elements)") + << Index.toString(10) << ArraySize.toString(10, false); + } +} + +} // namespace tidy +} // namespace clang + Index: docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst @@ -0,0 +1,9 @@ +cppcoreguidelines-pro-bounds-constant-array-index +================================================= + +This check flags all array subscriptions on static arrays and std::arrays that either have a non-compile-time constant index or are out of bounds. + +Dynamic accesses into arrays are difficult for both tools and humans to validate as safe. array_view is a bounds-checked, safe type for accessing arrays of data. at() is another alternative that ensures single accesses are bounds-checked. If iterators are needed to access an array, use the iterators from an array_view constructed over the array. + +This rule is part of the "Bounds safety" profile of the C++ Core Guidelines, see +https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-bounds2-only-index-into-arrays-using-constant-expressions Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -4,6 +4,7 @@ .. toctree:: cert-setlongjmp cert-variadic-function-def + cppcoreguidelines-pro-bounds-constant-array-index cppcoreguidelines-pro-bounds-pointer-arithmetic cppcoreguidelines-pro-type-const-cast cppcoreguidelines-pro-type-reinterpret-cast Index: test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp =================================================================== --- /dev/null +++ test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp @@ -0,0 +1,68 @@ +// RUN: %python %S/check_clang_tidy.py %s cppcoreguidelines-pro-bounds-constant-array-index %t +#include + +namespace gsl { + template + T& at( T(&a)[N], size_t index ); + + template + T& at( std::array &a, size_t index ); +} + +constexpr int const_index(int base) { + return base + 3; +} + +void f(std::array a, int pos) { + a[pos / 2] = 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index] + a[pos - 1] = 2; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead + + a.at(pos-1) = 2; // OK, at() instead of [] + gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of [] + + a[-1] = 3; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index -1 is before the beginning of the array [cppcoreguidelines-pro-bounds-constant-array-index] + a[10] = 4; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index 10 is past the end of the array (which contains 10 elements) [cppcoreguidelines-pro-bounds-constant-array-index] + + a[const_index(7)] = 3; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index 10 is past the end of the array (which contains 10 elements) + + a[0] = 3; // OK, constant index and inside bounds + a[1] = 3; // OK, constant index and inside bounds + a[9] = 3; // OK, constant index and inside bounds + a[const_index(6)] = 3; // OK, constant index and inside bounds +} + +void g() { + int a[10]; + for (int i = 0; i < 10; ++i) { + a[i] = i; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead + gsl::at(a, i) = i; // OK, gsl::at() instead of [] + } + + a[-1] = 3; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index -1 is before the beginning of the array + a[10] = 4; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index 10 is past the end of the array (which contains 10 elements) + a[const_index(7)] = 3; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: array index 10 is past the end of the array (which contains 10 elements) + + a[0] = 3; // OK, constant index and inside bounds + a[1] = 3; // OK, constant index and inside bounds + a[9] = 3; // OK, constant index and inside bounds + a[const_index(6)] = 3; // OK, constant index and inside bounds +} + +struct S { + int& operator[](int i); +}; + +void customOperator() { + S s; + int i = 0; + s[i] = 3; // OK, custom operator +}