[clang-tools-extra] r253401 - [clang-tidy] add check cppcoreguidelines-pro-bounds-constant-array-index
Bruno Cardoso Lopes via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 17 16:50:52 PST 2015
Hi Matthias,
This is failing on the green dragon bot:
http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_check/13184/
http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_check/13184/consoleFull#54616856949ba4694-19c4-4d7e-bec5-911270d8a58c
Thanks,
On Tue, Nov 17, 2015 at 3:43 PM, Matthias Gehre via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> Author: mgehre
> Date: Tue Nov 17 17:43:20 2015
> New Revision: 253401
>
> URL: http://llvm.org/viewvc/llvm-project?rev=253401&view=rev
> Log:
> [clang-tidy] add check cppcoreguidelines-pro-bounds-constant-array-index
>
> Summary:
> 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
>
> Reviewers: alexfh, sbenza, bkramer, aaron.ballman
>
> Subscribers: cfe-commits
>
> Differential Revision: http://reviews.llvm.org/D13746
>
> Added:
> clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
> clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h
> clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
> clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
> Modified:
> clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
> clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
> clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
>
> Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt?rev=253401&r1=253400&r2=253401&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt (original)
> +++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt Tue Nov 17 17:43:20 2015
> @@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support)
> add_clang_library(clangTidyCppCoreGuidelinesModule
> CppCoreGuidelinesTidyModule.cpp
> ProBoundsArrayToPointerDecayCheck.cpp
> + ProBoundsConstantArrayIndexCheck.cpp
> ProBoundsPointerArithmeticCheck.cpp
> ProTypeConstCastCheck.cpp
> ProTypeCstyleCastCheck.cpp
>
> Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp?rev=253401&r1=253400&r2=253401&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp (original)
> +++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp Tue Nov 17 17:43:20 2015
> @@ -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 @@ public:
> void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
> CheckFactories.registerCheck<ProBoundsArrayToPointerDecayCheck>(
> "cppcoreguidelines-pro-bounds-array-to-pointer-decay");
> + CheckFactories.registerCheck<ProBoundsConstantArrayIndexCheck>(
> + "cppcoreguidelines-pro-bounds-constant-array-index");
> CheckFactories.registerCheck<ProBoundsPointerArithmeticCheck>(
> "cppcoreguidelines-pro-bounds-pointer-arithmetic");
> CheckFactories.registerCheck<ProTypeConstCastCheck>(
>
> Added: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp?rev=253401&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp (added)
> +++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp Tue Nov 17 17:43:20 2015
> @@ -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>("expr");
> + const auto *IndexExpr = Result.Nodes.getNodeAs<Expr>("index");
> + llvm::APSInt Index;
> + if (!IndexExpr->isIntegerConstantExpr(Index, *Result.Context, nullptr,
> + /*isEvaluated=*/true)) {
> + SourceRange BaseRange;
> + if (const auto *ArraySubscriptE = dyn_cast<ArraySubscriptExpr>(Matched))
> + BaseRange = ArraySubscriptE->getBase()->getSourceRange();
> + else
> + BaseRange =
> + dyn_cast<CXXOperatorCallExpr>(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<ClassTemplateSpecializationDecl>("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
>
> Added: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h?rev=253401&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h (added)
> +++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h Tue Nov 17 17:43:20 2015
> @@ -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<IncludeInserter> 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
>
> Added: clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst?rev=253401&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst (added)
> +++ clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst Tue Nov 17 17:43:20 2015
> @@ -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.
>
> Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=253401&r1=253400&r2=253401&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
> +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Tue Nov 17 17:43:20 2015
> @@ -6,6 +6,7 @@ List of clang-tidy Checks
> 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
>
> Added: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp?rev=253401&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp (added)
> +++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp Tue Nov 17 17:43:20 2015
> @@ -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 <array>
> +// CHECK-FIXES: #include "dir1/gslheader.h"
> +
> +namespace gsl {
> + template<class T, size_t N>
> + T& at( T(&a)[N], size_t index );
> +
> + template<class T, size_t N>
> + T& at( std::array<T, N> &a, size_t index );
> +}
> +
> +constexpr int const_index(int base) {
> + return base + 3;
> +}
> +
> +void f(std::array<int, 10> 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
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
--
Bruno Cardoso Lopes
http://www.brunocardoso.cc
More information about the cfe-commits
mailing list