1 //===--- SizeofExpressionCheck.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 "SizeofExpressionCheck.h"
11 #include "../utils/Matchers.h"
12 #include "clang/AST/ASTContext.h"
13 #include "clang/ASTMatchers/ASTMatchFinder.h"
14
15 using namespace clang::ast_matchers;
16
17 namespace clang {
18 namespace tidy {
19 namespace bugprone {
20
21 namespace {
22
AST_MATCHER_P(IntegerLiteral,isBiggerThan,unsigned,N)23 AST_MATCHER_P(IntegerLiteral, isBiggerThan, unsigned, N) {
24 return Node.getValue().getZExtValue() > N;
25 }
26
AST_MATCHER_P2(Expr,hasSizeOfDescendant,int,Depth,ast_matchers::internal::Matcher<Expr>,InnerMatcher)27 AST_MATCHER_P2(Expr, hasSizeOfDescendant, int, Depth,
28 ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
29 if (Depth < 0)
30 return false;
31
32 const Expr *E = Node.IgnoreParenImpCasts();
33 if (InnerMatcher.matches(*E, Finder, Builder))
34 return true;
35
36 if (const auto *CE = dyn_cast<CastExpr>(E)) {
37 const auto M = hasSizeOfDescendant(Depth - 1, InnerMatcher);
38 return M.matches(*CE->getSubExpr(), Finder, Builder);
39 } else if (const auto *UE = dyn_cast<UnaryOperator>(E)) {
40 const auto M = hasSizeOfDescendant(Depth - 1, InnerMatcher);
41 return M.matches(*UE->getSubExpr(), Finder, Builder);
42 } else if (const auto *BE = dyn_cast<BinaryOperator>(E)) {
43 const auto LHS = hasSizeOfDescendant(Depth - 1, InnerMatcher);
44 const auto RHS = hasSizeOfDescendant(Depth - 1, InnerMatcher);
45 return LHS.matches(*BE->getLHS(), Finder, Builder) ||
46 RHS.matches(*BE->getRHS(), Finder, Builder);
47 }
48
49 return false;
50 }
51
getSizeOfType(const ASTContext & Ctx,const Type * Ty)52 CharUnits getSizeOfType(const ASTContext &Ctx, const Type *Ty) {
53 if (!Ty || Ty->isIncompleteType() || Ty->isDependentType() ||
54 isa<DependentSizedArrayType>(Ty) || !Ty->isConstantSizeType())
55 return CharUnits::Zero();
56 return Ctx.getTypeSizeInChars(Ty);
57 }
58
59 } // namespace
60
SizeofExpressionCheck(StringRef Name,ClangTidyContext * Context)61 SizeofExpressionCheck::SizeofExpressionCheck(StringRef Name,
62 ClangTidyContext *Context)
63 : ClangTidyCheck(Name, Context),
64 WarnOnSizeOfConstant(Options.get("WarnOnSizeOfConstant", 1) != 0),
65 WarnOnSizeOfIntegerExpression(
66 Options.get("WarnOnSizeOfIntegerExpression", 0) != 0),
67 WarnOnSizeOfThis(Options.get("WarnOnSizeOfThis", 1) != 0),
68 WarnOnSizeOfCompareToConstant(
69 Options.get("WarnOnSizeOfCompareToConstant", 1) != 0) {}
70
storeOptions(ClangTidyOptions::OptionMap & Opts)71 void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
72 Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant);
73 Options.store(Opts, "WarnOnSizeOfIntegerExpression",
74 WarnOnSizeOfIntegerExpression);
75 Options.store(Opts, "WarnOnSizeOfThis", WarnOnSizeOfThis);
76 Options.store(Opts, "WarnOnSizeOfCompareToConstant",
77 WarnOnSizeOfCompareToConstant);
78 }
79
registerMatchers(MatchFinder * Finder)80 void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
81 const auto IntegerExpr = ignoringParenImpCasts(integerLiteral());
82 const auto ConstantExpr = expr(ignoringParenImpCasts(
83 anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)),
84 binaryOperator(hasLHS(IntegerExpr), hasRHS(IntegerExpr)))));
85 const auto IntegerCallExpr = expr(ignoringParenImpCasts(
86 callExpr(anyOf(hasType(isInteger()), hasType(enumType())),
87 unless(isInTemplateInstantiation()))));
88 const auto SizeOfExpr =
89 expr(anyOf(sizeOfExpr(has(type())), sizeOfExpr(has(expr()))));
90 const auto SizeOfZero = expr(
91 sizeOfExpr(has(ignoringParenImpCasts(expr(integerLiteral(equals(0)))))));
92
93 // Detect expression like: sizeof(ARRAYLEN);
94 // Note: The expression 'sizeof(sizeof(0))' is a portable trick used to know
95 // the sizeof size_t.
96 if (WarnOnSizeOfConstant) {
97 Finder->addMatcher(
98 expr(sizeOfExpr(has(ignoringParenImpCasts(ConstantExpr))),
99 unless(SizeOfZero))
100 .bind("sizeof-constant"),
101 this);
102 }
103
104 // Detect sizeof(f())
105 if (WarnOnSizeOfIntegerExpression) {
106 Finder->addMatcher(
107 expr(sizeOfExpr(ignoringParenImpCasts(has(IntegerCallExpr))))
108 .bind("sizeof-integer-call"),
109 this);
110 }
111
112 // Detect expression like: sizeof(this);
113 if (WarnOnSizeOfThis) {
114 Finder->addMatcher(
115 expr(sizeOfExpr(has(ignoringParenImpCasts(expr(cxxThisExpr())))))
116 .bind("sizeof-this"),
117 this);
118 }
119
120 // Detect sizeof(kPtr) where kPtr is 'const char* kPtr = "abc"';
121 const auto CharPtrType = pointerType(pointee(isAnyCharacter()));
122 const auto ConstStrLiteralDecl =
123 varDecl(isDefinition(), hasType(qualType(hasCanonicalType(CharPtrType))),
124 hasInitializer(ignoringParenImpCasts(stringLiteral())));
125 Finder->addMatcher(expr(sizeOfExpr(has(ignoringParenImpCasts(expr(
126 hasType(qualType(hasCanonicalType(CharPtrType))),
127 ignoringParenImpCasts(declRefExpr(
128 hasDeclaration(ConstStrLiteralDecl))))))))
129 .bind("sizeof-charp"),
130 this);
131
132 // Detect sizeof(ptr) where ptr points to an aggregate (i.e. sizeof(&S)).
133 const auto ArrayExpr = expr(ignoringParenImpCasts(
134 expr(hasType(qualType(hasCanonicalType(arrayType()))))));
135 const auto ArrayCastExpr = expr(anyOf(
136 unaryOperator(hasUnaryOperand(ArrayExpr), unless(hasOperatorName("*"))),
137 binaryOperator(hasEitherOperand(ArrayExpr)),
138 castExpr(hasSourceExpression(ArrayExpr))));
139 const auto PointerToArrayExpr = expr(ignoringParenImpCasts(expr(
140 hasType(qualType(hasCanonicalType(pointerType(pointee(arrayType()))))))));
141
142 const auto StructAddrOfExpr =
143 unaryOperator(hasOperatorName("&"),
144 hasUnaryOperand(ignoringParenImpCasts(expr(
145 hasType(qualType(hasCanonicalType(recordType())))))));
146
147 Finder->addMatcher(
148 expr(sizeOfExpr(has(expr(ignoringParenImpCasts(
149 anyOf(ArrayCastExpr, PointerToArrayExpr, StructAddrOfExpr))))))
150 .bind("sizeof-pointer-to-aggregate"),
151 this);
152
153 // Detect expression like: sizeof(epxr) <= k for a suspicious constant 'k'.
154 if (WarnOnSizeOfCompareToConstant) {
155 Finder->addMatcher(
156 binaryOperator(matchers::isRelationalOperator(),
157 hasEitherOperand(ignoringParenImpCasts(SizeOfExpr)),
158 hasEitherOperand(ignoringParenImpCasts(
159 anyOf(integerLiteral(equals(0)),
160 integerLiteral(isBiggerThan(0x80000))))))
161 .bind("sizeof-compare-constant"),
162 this);
163 }
164
165 // Detect expression like: sizeof(expr, expr); most likely an error.
166 Finder->addMatcher(expr(sizeOfExpr(has(expr(ignoringParenImpCasts(
167 binaryOperator(hasOperatorName(",")))))))
168 .bind("sizeof-comma-expr"),
169 this);
170
171 // Detect sizeof(...) /sizeof(...));
172 const auto ElemType =
173 arrayType(hasElementType(recordType().bind("elem-type")));
174 const auto ElemPtrType = pointerType(pointee(type().bind("elem-ptr-type")));
175 const auto NumType = qualType(hasCanonicalType(
176 type(anyOf(ElemType, ElemPtrType, type())).bind("num-type")));
177 const auto DenomType = qualType(hasCanonicalType(type().bind("denom-type")));
178
179 Finder->addMatcher(
180 binaryOperator(hasOperatorName("/"),
181 hasLHS(expr(ignoringParenImpCasts(
182 anyOf(sizeOfExpr(has(NumType)),
183 sizeOfExpr(has(expr(hasType(NumType)))))))),
184 hasRHS(expr(ignoringParenImpCasts(
185 anyOf(sizeOfExpr(has(DenomType)),
186 sizeOfExpr(has(expr(hasType(DenomType)))))))))
187 .bind("sizeof-divide-expr"),
188 this);
189
190 // Detect expression like: sizeof(...) * sizeof(...)); most likely an error.
191 Finder->addMatcher(binaryOperator(hasOperatorName("*"),
192 hasLHS(ignoringParenImpCasts(SizeOfExpr)),
193 hasRHS(ignoringParenImpCasts(SizeOfExpr)))
194 .bind("sizeof-multiply-sizeof"),
195 this);
196
197 Finder->addMatcher(
198 binaryOperator(hasOperatorName("*"),
199 hasEitherOperand(ignoringParenImpCasts(SizeOfExpr)),
200 hasEitherOperand(ignoringParenImpCasts(binaryOperator(
201 hasOperatorName("*"),
202 hasEitherOperand(ignoringParenImpCasts(SizeOfExpr))))))
203 .bind("sizeof-multiply-sizeof"),
204 this);
205
206 // Detect strange double-sizeof expression like: sizeof(sizeof(...));
207 // Note: The expression 'sizeof(sizeof(0))' is accepted.
208 Finder->addMatcher(
209 expr(sizeOfExpr(has(ignoringParenImpCasts(expr(
210 hasSizeOfDescendant(8, expr(SizeOfExpr, unless(SizeOfZero))))))))
211 .bind("sizeof-sizeof-expr"),
212 this);
213 }
214
check(const MatchFinder::MatchResult & Result)215 void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
216 const ASTContext &Ctx = *Result.Context;
217
218 if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-constant")) {
219 diag(E->getBeginLoc(),
220 "suspicious usage of 'sizeof(K)'; did you mean 'K'?");
221 } else if (const auto *E =
222 Result.Nodes.getNodeAs<Expr>("sizeof-integer-call")) {
223 diag(E->getBeginLoc(), "suspicious usage of 'sizeof()' on an expression "
224 "that results in an integer");
225 } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-this")) {
226 diag(E->getBeginLoc(),
227 "suspicious usage of 'sizeof(this)'; did you mean 'sizeof(*this)'");
228 } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-charp")) {
229 diag(E->getBeginLoc(),
230 "suspicious usage of 'sizeof(char*)'; do you mean 'strlen'?");
231 } else if (const auto *E =
232 Result.Nodes.getNodeAs<Expr>("sizeof-pointer-to-aggregate")) {
233 diag(E->getBeginLoc(),
234 "suspicious usage of 'sizeof(A*)'; pointer to aggregate");
235 } else if (const auto *E =
236 Result.Nodes.getNodeAs<Expr>("sizeof-compare-constant")) {
237 diag(E->getBeginLoc(),
238 "suspicious comparison of 'sizeof(expr)' to a constant");
239 } else if (const auto *E =
240 Result.Nodes.getNodeAs<Expr>("sizeof-comma-expr")) {
241 diag(E->getBeginLoc(), "suspicious usage of 'sizeof(..., ...)'");
242 } else if (const auto *E =
243 Result.Nodes.getNodeAs<Expr>("sizeof-divide-expr")) {
244 const auto *NumTy = Result.Nodes.getNodeAs<Type>("num-type");
245 const auto *DenomTy = Result.Nodes.getNodeAs<Type>("denom-type");
246 const auto *ElementTy = Result.Nodes.getNodeAs<Type>("elem-type");
247 const auto *PointedTy = Result.Nodes.getNodeAs<Type>("elem-ptr-type");
248
249 CharUnits NumeratorSize = getSizeOfType(Ctx, NumTy);
250 CharUnits DenominatorSize = getSizeOfType(Ctx, DenomTy);
251 CharUnits ElementSize = getSizeOfType(Ctx, ElementTy);
252
253 if (DenominatorSize > CharUnits::Zero() &&
254 !NumeratorSize.isMultipleOf(DenominatorSize)) {
255 diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
256 " numerator is not a multiple of denominator");
257 } else if (ElementSize > CharUnits::Zero() &&
258 DenominatorSize > CharUnits::Zero() &&
259 ElementSize != DenominatorSize) {
260 diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
261 " numerator is not a multiple of denominator");
262 } else if (NumTy && DenomTy && NumTy == DenomTy) {
263 diag(E->getBeginLoc(),
264 "suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'");
265 } else if (PointedTy && DenomTy && PointedTy == DenomTy) {
266 diag(E->getBeginLoc(),
267 "suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'");
268 } else if (NumTy && DenomTy && NumTy->isPointerType() &&
269 DenomTy->isPointerType()) {
270 diag(E->getBeginLoc(),
271 "suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'");
272 }
273 } else if (const auto *E =
274 Result.Nodes.getNodeAs<Expr>("sizeof-sizeof-expr")) {
275 diag(E->getBeginLoc(), "suspicious usage of 'sizeof(sizeof(...))'");
276 } else if (const auto *E =
277 Result.Nodes.getNodeAs<Expr>("sizeof-multiply-sizeof")) {
278 diag(E->getBeginLoc(), "suspicious 'sizeof' by 'sizeof' multiplication");
279 }
280 }
281
282 } // namespace bugprone
283 } // namespace tidy
284 } // namespace clang
285