Skip to content

Commit 5502056

Browse files
mgehremgehre
committedNov 17, 2015
[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 parent 11c938d commit 5502056

7 files changed

+258
-0
lines changed
 

‎clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support)
33
add_clang_library(clangTidyCppCoreGuidelinesModule
44
CppCoreGuidelinesTidyModule.cpp
55
ProBoundsArrayToPointerDecayCheck.cpp
6+
ProBoundsConstantArrayIndexCheck.cpp
67
ProBoundsPointerArithmeticCheck.cpp
78
ProTypeConstCastCheck.cpp
89
ProTypeCstyleCastCheck.cpp

‎clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "../ClangTidyModuleRegistry.h"
1313
#include "../misc/AssignOperatorSignatureCheck.h"
1414
#include "ProBoundsArrayToPointerDecayCheck.h"
15+
#include "ProBoundsConstantArrayIndexCheck.h"
1516
#include "ProBoundsPointerArithmeticCheck.h"
1617
#include "ProTypeConstCastCheck.h"
1718
#include "ProTypeCstyleCastCheck.h"
@@ -30,6 +31,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule {
3031
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
3132
CheckFactories.registerCheck<ProBoundsArrayToPointerDecayCheck>(
3233
"cppcoreguidelines-pro-bounds-array-to-pointer-decay");
34+
CheckFactories.registerCheck<ProBoundsConstantArrayIndexCheck>(
35+
"cppcoreguidelines-pro-bounds-constant-array-index");
3336
CheckFactories.registerCheck<ProBoundsPointerArithmeticCheck>(
3437
"cppcoreguidelines-pro-bounds-pointer-arithmetic");
3538
CheckFactories.registerCheck<ProTypeConstCastCheck>(

‎clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp

+131
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
//===--- ProBoundsConstantArrayIndexCheck.cpp - clang-tidy-----------------===//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#include "ProBoundsConstantArrayIndexCheck.h"
11+
#include "clang/AST/ASTContext.h"
12+
#include "clang/ASTMatchers/ASTMatchFinder.h"
13+
#include "clang/Frontend/CompilerInstance.h"
14+
#include "clang/Lex/Preprocessor.h"
15+
16+
using namespace clang::ast_matchers;
17+
18+
namespace clang {
19+
namespace tidy {
20+
21+
ProBoundsConstantArrayIndexCheck::ProBoundsConstantArrayIndexCheck(
22+
StringRef Name, ClangTidyContext *Context)
23+
: ClangTidyCheck(Name, Context), GslHeader(Options.get("GslHeader", "")),
24+
IncludeStyle(IncludeSorter::parseIncludeStyle(
25+
Options.get("IncludeStyle", "llvm"))) {}
26+
27+
void ProBoundsConstantArrayIndexCheck::storeOptions(
28+
ClangTidyOptions::OptionMap &Opts) {
29+
Options.store(Opts, "GslHeader", GslHeader);
30+
}
31+
32+
void ProBoundsConstantArrayIndexCheck::registerPPCallbacks(
33+
CompilerInstance &Compiler) {
34+
if (!getLangOpts().CPlusPlus)
35+
return;
36+
37+
Inserter.reset(new IncludeInserter(Compiler.getSourceManager(),
38+
Compiler.getLangOpts(), IncludeStyle));
39+
Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks());
40+
}
41+
42+
void ProBoundsConstantArrayIndexCheck::registerMatchers(MatchFinder *Finder) {
43+
if (!getLangOpts().CPlusPlus)
44+
return;
45+
46+
Finder->addMatcher(arraySubscriptExpr(hasBase(ignoringImpCasts(hasType(
47+
constantArrayType().bind("type")))),
48+
hasIndex(expr().bind("index")))
49+
.bind("expr"),
50+
this);
51+
52+
Finder->addMatcher(
53+
cxxOperatorCallExpr(
54+
hasOverloadedOperatorName("[]"),
55+
hasArgument(
56+
0, hasType(cxxRecordDecl(hasName("::std::array")).bind("type"))),
57+
hasArgument(1, expr().bind("index")))
58+
.bind("expr"),
59+
this);
60+
}
61+
62+
void ProBoundsConstantArrayIndexCheck::check(
63+
const MatchFinder::MatchResult &Result) {
64+
const auto *Matched = Result.Nodes.getNodeAs<Expr>("expr");
65+
const auto *IndexExpr = Result.Nodes.getNodeAs<Expr>("index");
66+
llvm::APSInt Index;
67+
if (!IndexExpr->isIntegerConstantExpr(Index, *Result.Context, nullptr,
68+
/*isEvaluated=*/true)) {
69+
SourceRange BaseRange;
70+
if (const auto *ArraySubscriptE = dyn_cast<ArraySubscriptExpr>(Matched))
71+
BaseRange = ArraySubscriptE->getBase()->getSourceRange();
72+
else
73+
BaseRange =
74+
dyn_cast<CXXOperatorCallExpr>(Matched)->getArg(0)->getSourceRange();
75+
SourceRange IndexRange = IndexExpr->getSourceRange();
76+
77+
auto Diag = diag(Matched->getExprLoc(),
78+
"do not use array subscript when the index is "
79+
"not a compile-time constant; use gsl::at() "
80+
"instead");
81+
if (!GslHeader.empty()) {
82+
Diag << FixItHint::CreateInsertion(BaseRange.getBegin(), "gsl::at(")
83+
<< FixItHint::CreateReplacement(
84+
SourceRange(BaseRange.getEnd().getLocWithOffset(1),
85+
IndexRange.getBegin().getLocWithOffset(-1)),
86+
", ")
87+
<< FixItHint::CreateReplacement(Matched->getLocEnd(), ")");
88+
89+
auto Insertion = Inserter->CreateIncludeInsertion(
90+
Result.SourceManager->getMainFileID(), GslHeader,
91+
/*IsAngled=*/false);
92+
if (Insertion.hasValue())
93+
Diag << Insertion.getValue();
94+
}
95+
return;
96+
}
97+
98+
const auto *StdArrayDecl =
99+
Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("type");
100+
101+
// For static arrays, this is handled in clang-diagnostic-array-bounds.
102+
if (!StdArrayDecl)
103+
return;
104+
105+
if (Index.isSigned() && Index.isNegative()) {
106+
diag(Matched->getExprLoc(),
107+
"std::array<> index %0 is before the beginning of the array")
108+
<< Index.toString(10);
109+
return;
110+
}
111+
112+
const auto &TemplateArgs = StdArrayDecl->getTemplateArgs();
113+
if (TemplateArgs.size() < 2)
114+
return;
115+
// First template arg of std::array is the type, second arg is the size.
116+
const auto &SizeArg = TemplateArgs[1];
117+
if (SizeArg.getKind() != TemplateArgument::Integral)
118+
return;
119+
llvm::APInt ArraySize = SizeArg.getAsIntegral();
120+
121+
// Get uint64_t values, because different bitwidths would lead to an assertion
122+
// in APInt::uge.
123+
if (Index.getZExtValue() >= ArraySize.getZExtValue()) {
124+
diag(Matched->getExprLoc(), "std::array<> index %0 is past the end of the array "
125+
"(which contains %1 elements)")
126+
<< Index.toString(10) << ArraySize.toString(10, false);
127+
}
128+
}
129+
130+
} // namespace tidy
131+
} // namespace clang

‎clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
//===--- ProBoundsConstantArrayIndexCheck.h - clang-tidy---------*- C++ -*-===//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_CONSTANT_ARRAY_INDEX_H
11+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_CONSTANT_ARRAY_INDEX_H
12+
13+
#include "../ClangTidy.h"
14+
#include "../utils/IncludeInserter.h"
15+
16+
namespace clang {
17+
namespace tidy {
18+
19+
/// This checks that all array subscriptions on static arrays and std::arrays
20+
/// have a constant index and are within bounds
21+
///
22+
/// For the user-facing documentation see:
23+
/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.html
24+
class ProBoundsConstantArrayIndexCheck : public ClangTidyCheck {
25+
std::string GslHeader;
26+
const IncludeSorter::IncludeStyle IncludeStyle;
27+
std::unique_ptr<IncludeInserter> Inserter;
28+
29+
public:
30+
ProBoundsConstantArrayIndexCheck(StringRef Name, ClangTidyContext *Context);
31+
void registerPPCallbacks(CompilerInstance &Compiler) override;
32+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
33+
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
34+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
35+
};
36+
37+
} // namespace tidy
38+
} // namespace clang
39+
40+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_CONSTANT_ARRAY_INDEX_H

‎clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
cppcoreguidelines-pro-bounds-constant-array-index
2+
=================================================
3+
4+
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).
5+
For out-of-bounds checking of static arrays, see the clang-diagnostic-array-bounds check.
6+
7+
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.
8+
9+
The check can generated fixes after the option cppcoreguidelines-pro-bounds-constant-array-index.GslHeader has been set to the name of the
10+
include file that contains gsl::at(), e.g. "gsl/gsl.h".
11+
12+
This rule is part of the "Bounds safety" profile of the C++ Core Guidelines, see
13+
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-bounds2-only-index-into-arrays-using-constant-expressions.

‎clang-tools-extra/docs/clang-tidy/checks/list.rst

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ List of clang-tidy Checks
66
cert-thrown-exception-type
77
cert-variadic-function-def
88
cppcoreguidelines-pro-bounds-array-to-pointer-decay
9+
cppcoreguidelines-pro-bounds-constant-array-index
910
cppcoreguidelines-pro-bounds-pointer-arithmetic
1011
cppcoreguidelines-pro-type-const-cast
1112
cppcoreguidelines-pro-type-cstyle-cast

‎clang-tools-extra/test/clang-tidy/cppcoreguidelines-pro-bounds-constant-array-index.cpp

+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// 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
2+
#include <array>
3+
// CHECK-FIXES: #include "dir1/gslheader.h"
4+
5+
namespace gsl {
6+
template<class T, size_t N>
7+
T& at( T(&a)[N], size_t index );
8+
9+
template<class T, size_t N>
10+
T& at( std::array<T, N> &a, size_t index );
11+
}
12+
13+
constexpr int const_index(int base) {
14+
return base + 3;
15+
}
16+
17+
void f(std::array<int, 10> a, int pos) {
18+
a [ pos / 2 /*comment*/] = 1;
19+
// 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]
20+
// CHECK-FIXES: gsl::at(a, pos / 2 /*comment*/) = 1;
21+
int j = a[pos - 1];
22+
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead
23+
// CHECK-FIXES: int j = gsl::at(a, pos - 1);
24+
25+
a.at(pos-1) = 2; // OK, at() instead of []
26+
gsl::at(a, pos-1) = 2; // OK, gsl::at() instead of []
27+
28+
a[-1] = 3;
29+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index -1 is before the beginning of the array [cppcoreguidelines-pro-bounds-constant-array-index]
30+
a[10] = 4;
31+
// 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]
32+
33+
a[const_index(7)] = 3;
34+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: std::array<> index 10 is past the end of the array (which contains 10 elements)
35+
36+
a[0] = 3; // OK, constant index and inside bounds
37+
a[1] = 3; // OK, constant index and inside bounds
38+
a[9] = 3; // OK, constant index and inside bounds
39+
a[const_index(6)] = 3; // OK, constant index and inside bounds
40+
}
41+
42+
void g() {
43+
int a[10];
44+
for (int i = 0; i < 10; ++i) {
45+
a[i] = i;
46+
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use array subscript when the index is not a compile-time constant; use gsl::at() instead
47+
// CHECK-FIXES: gsl::at(a, i) = i;
48+
gsl::at(a, i) = i; // OK, gsl::at() instead of []
49+
}
50+
51+
a[-1] = 3; // flagged by clang-diagnostic-array-bounds
52+
a[10] = 4; // flagged by clang-diagnostic-array-bounds
53+
a[const_index(7)] = 3; // flagged by clang-diagnostic-array-bounds
54+
55+
a[0] = 3; // OK, constant index and inside bounds
56+
a[1] = 3; // OK, constant index and inside bounds
57+
a[9] = 3; // OK, constant index and inside bounds
58+
a[const_index(6)] = 3; // OK, constant index and inside bounds
59+
}
60+
61+
struct S {
62+
int& operator[](int i);
63+
};
64+
65+
void customOperator() {
66+
S s;
67+
int i = 0;
68+
s[i] = 3; // OK, custom operator
69+
}

0 commit comments

Comments
 (0)
Please sign in to comment.