[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

llvm-svn: 253401

1 files changed, 131 insertions, 0 deletions

diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
new file mode 100644
index 00000000000..b976cb24b15
--- /dev/null
+++ b/clang-tools-extra/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>("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