Index: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -3,6 +3,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule CppCoreGuidelinesTidyModule.cpp ProBoundsArrayToPointerDecayCheck.cpp + ProBoundsConstantArrayIndexCheck.cpp ProBoundsPointerArithmeticCheck.cpp ProTypeConstCastCheck.cpp ProTypeCstyleCastCheck.cpp Index: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -12,6 +12,7 @@ #include "../ClangTidyModuleRegistry.h" #include "../misc/AssignOperatorSignatureCheck.h" #include "ProBoundsArrayToPointerDecayCheck.h" +#include "ProBoundsConstantArrayIndexCheck.h" #include "ProBoundsPointerArithmeticCheck.h" #include "ProTypeConstCastCheck.h" #include "ProTypeCstyleCastCheck.h" @@ -30,6 +31,8 @@ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck( "cppcoreguidelines-pro-bounds-array-to-pointer-decay"); + CheckFactories.registerCheck( + "cppcoreguidelines-pro-bounds-constant-array-index"); CheckFactories.registerCheck( "cppcoreguidelines-pro-bounds-pointer-arithmetic"); CheckFactories.registerCheck( Index: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h +++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h @@ -0,0 +1,40 @@ +//===--- 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" +#include "../utils/IncludeInserter.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 { + std::string GslHeader; + const IncludeSorter::IncludeStyle IncludeStyle; + std::unique_ptr Inserter; + +public: + ProBoundsConstantArrayIndexCheck(StringRef Name, ClangTidyContext *Context); + void registerPPCallbacks(CompilerInstance &Compiler) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) 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-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp @@ -0,0 +1,131 @@ +//===--- 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" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/Preprocessor.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { + +ProBoundsConstantArrayIndexCheck::ProBoundsConstantArrayIndexCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), GslHeader(Options.get("GslHeader", "")), + IncludeStyle(IncludeSorter::parseIncludeStyle( + Options.get("IncludeStyle", "llvm"))) {} + +void ProBoundsConstantArrayIndexCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "GslHeader", GslHeader); +} + +void ProBoundsConstantArrayIndexCheck::registerPPCallbacks( + CompilerInstance &Compiler) { + if (!getLangOpts().CPlusPlus) + return; + + Inserter.reset(new IncludeInserter(Compiler.getSourceManager(), + Compiler.getLangOpts(), IncludeStyle)); + Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks()); +} + +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(cxxRecordDecl(hasName("::std::array")).bind("type"))), + hasArgument(1, expr().bind("index"))) + .bind("expr"), + this); +} + +void ProBoundsConstantArrayIndexCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *Matched = Result.Nodes.getNodeAs("expr"); + const auto *IndexExpr = Result.Nodes.getNodeAs("index"); + llvm::APSInt Index; + if (!IndexExpr->isIntegerConstantExpr(Index, *Result.Context, nullptr, + /*isEvaluated=*/true)) { + SourceRange BaseRange; + if (const auto *ArraySubscriptE = dyn_cast(Matched)) + BaseRange = ArraySubscriptE->getBase()->getSourceRange(); + else + BaseRange = + dyn_cast(Matched)->getArg(0)->getSourceRange(); + SourceRange IndexRange = IndexExpr->getSourceRange(); + + auto Diag = diag(Matched->getExprLoc(), + "do not use array subscript when the index is " + "not a compile-time constant; use gsl::at() " + "instead"); + if (!GslHeader.empty()) { + Diag << FixItHint::CreateInsertion(BaseRange.getBegin(), "gsl::at(") + << FixItHint::CreateReplacement( + SourceRange(BaseRange.getEnd().getLocWithOffset(1), + IndexRange.getBegin().getLocWithOffset(-1)), + ", ") + << FixItHint::CreateReplacement(Matched->getLocEnd(), ")"); + + auto Insertion = Inserter->CreateIncludeInsertion( + Result.SourceManager->getMainFileID(), GslHeader, + /*IsAngled=*/false); + if (Insertion.hasValue()) + Diag << Insertion.getValue(); + } + return; + } + + const auto *StdArrayDecl = + Result.Nodes.getNodeAs("type"); + + // For static arrays, this is handled in clang-diagnostic-array-bounds. + if (!StdArrayDecl) + return; + + if (Index.isSigned() && Index.isNegative()) { + diag(Matched->getExprLoc(), + "std::array<> index %0 is before the beginning of the array") + << Index.toString(10); + return; + } + + 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; + llvm::APInt ArraySize = SizeArg.getAsIntegral(); + + // Get uint64_t values, because different bitwidths would lead to an assertion + // in APInt::uge. + if (Index.getZExtValue() >= ArraySize.getZExtValue()) { + diag(Matched->getExprLoc(), "std::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: clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst @@ -0,0 +1,13 @@ +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 (for std::array). +For out-of-bounds checking of static arrays, see the clang-diagnostic-array-bounds check. + +Dynamic accesses into arrays are difficult for both tools and humans to validate as safe. gsl::span is a bounds-checked, safe type for accessing arrays of data. gsl::at() is another alternative that ensures single accesses are bounds-checked. If iterators are needed to access an array, use the iterators from an gsl::span constructed over the array. + +The check can generated fixes after the option cppcoreguidelines-pro-bounds-constant-array-index.GslHeader has been set to the name of the +include file that contains gsl::at(), e.g. "gsl/gsl.h". + +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: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst @@ -6,6 +6,7 @@ cert-thrown-exception-type cert-variadic-function-def cppcoreguidelines-pro-bounds-array-to-pointer-decay + cppcoreguidelines-pro-bounds-constant-array-index cppcoreguidelines-pro-bounds-pointer-arithmetic cppcoreguidelines-pro-type-const-cast cppcoreguidelines-pro-type-cstyle-cast Index: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp +++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp @@ -0,0 +1,69 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-constant-array-index %t -- -config='{CheckOptions: [{key: cppcoreguidelines-pro-bounds-constant-array-index.GslHeader, value: "dir1/gslheader.h"}]}' -- -std=c++11 +#include +// CHECK-FIXES: #include "dir1/gslheader.h" + +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 /*comment*/] = 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] + // CHECK-FIXES: gsl::at(a, pos / 2 /*comment*/) = 1; + int j = a[pos - 1]; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead + // CHECK-FIXES: int j = gsl::at(a, pos - 1); + + 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: std::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: std::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: std::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 + // CHECK-FIXES: gsl::at(a, i) = i; + gsl::at(a, i) = i; // OK, gsl::at() instead of [] + } + + a[-1] = 3; // flagged by clang-diagnostic-array-bounds + a[10] = 4; // flagged by clang-diagnostic-array-bounds + a[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds + + 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 +}