1 //===--- SizeofExpressionCheck.cpp - clang-tidy----------------------------===//
2 //
3 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4 // See https://llvm.org/LICENSE.txt for license information.
5 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6 //
7 //===----------------------------------------------------------------------===//
8
9 #include "SizeofExpressionCheck.h"
10 #include "../utils/Matchers.h"
11 #include "clang/AST/ASTContext.h"
12 #include "clang/ASTMatchers/ASTMatchFinder.h"
13
14 using namespace clang::ast_matchers;
15
16 namespace clang {
17 namespace tidy {
18 namespace bugprone {
19
20 namespace {
21
AST_MATCHER_P(IntegerLiteral,isBiggerThan,unsigned,N)22 AST_MATCHER_P(IntegerLiteral, isBiggerThan, unsigned, N) {
23 return Node.getValue().getZExtValue() > N;
24 }
25
AST_MATCHER_P2(Expr,hasSizeOfDescendant,int,Depth,ast_matchers::internal::Matcher<Expr>,InnerMatcher)26 AST_MATCHER_P2(Expr, hasSizeOfDescendant, int, Depth,
27 ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
28 if (Depth < 0)
29 return false;
30
31 const Expr *E = Node.IgnoreParenImpCasts();
32 if (InnerMatcher.matches(*E, Finder, Builder))
33 return true;
34
35 if (const auto *CE = dyn_cast<CastExpr>(E)) {
36 const auto M = hasSizeOfDescendant(Depth - 1, InnerMatcher);
37 return M.matches(*CE->getSubExpr(), Finder, Builder);
38 }
39 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 }
43 if (const auto *BE = dyn_cast<BinaryOperator>(E)) {
44 const auto LHS = hasSizeOfDescendant(Depth - 1, InnerMatcher);
45 const auto RHS = hasSizeOfDescendant(Depth - 1, InnerMatcher);
46 return LHS.matches(*BE->getLHS(), Finder, Builder) ||
47 RHS.matches(*BE->getRHS(), Finder, Builder);
48 }
49
50 return false;
51 }
52
getSizeOfType(const ASTContext & Ctx,const Type * Ty)53 CharUnits getSizeOfType(const ASTContext &Ctx, const Type *Ty) {
54 if (!Ty || Ty->isIncompleteType() || Ty->isDependentType() ||
55 isa<DependentSizedArrayType>(Ty) || !Ty->isConstantSizeType())
56 return CharUnits::Zero();
57 return Ctx.getTypeSizeInChars(Ty);
58 }
59
60 } // namespace
61
SizeofExpressionCheck(StringRef Name,ClangTidyContext * Context)62 SizeofExpressionCheck::SizeofExpressionCheck(StringRef Name,
63 ClangTidyContext *Context)
64 : ClangTidyCheck(Name, Context),
65 WarnOnSizeOfConstant(Options.get("WarnOnSizeOfConstant", true)),
66 WarnOnSizeOfIntegerExpression(
67 Options.get("WarnOnSizeOfIntegerExpression", false)),
68 WarnOnSizeOfThis(Options.get("WarnOnSizeOfThis", true)),
69 WarnOnSizeOfCompareToConstant(
70 Options.get("WarnOnSizeOfCompareToConstant", true)) {}
71
storeOptions(ClangTidyOptions::OptionMap & Opts)72 void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
73 Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant);
74 Options.store(Opts, "WarnOnSizeOfIntegerExpression",
75 WarnOnSizeOfIntegerExpression);
76 Options.store(Opts, "WarnOnSizeOfThis", WarnOnSizeOfThis);
77 Options.store(Opts, "WarnOnSizeOfCompareToConstant",
78 WarnOnSizeOfCompareToConstant);
79 }
80
registerMatchers(MatchFinder * Finder)81 void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
82 // FIXME:
83 // Some of the checks should not match in template code to avoid false
84 // positives if sizeof is applied on template argument.
85
86 const auto IntegerExpr = ignoringParenImpCasts(integerLiteral());
87 const auto ConstantExpr = ignoringParenImpCasts(
88 anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)),
89 binaryOperator(hasLHS(IntegerExpr), hasRHS(IntegerExpr))));
90 const auto IntegerCallExpr = ignoringParenImpCasts(
91 callExpr(anyOf(hasType(isInteger()), hasType(enumType())),
92 unless(isInTemplateInstantiation())));
93 const auto SizeOfExpr = sizeOfExpr(hasArgumentOfType(
94 hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type"))));
95 const auto SizeOfZero =
96 sizeOfExpr(has(ignoringParenImpCasts(integerLiteral(equals(0)))));
97
98 // Detect expression like: sizeof(ARRAYLEN);
99 // Note: The expression 'sizeof(sizeof(0))' is a portable trick used to know
100 // the sizeof size_t.
101 if (WarnOnSizeOfConstant) {
102 Finder->addMatcher(
103 expr(sizeOfExpr(has(ignoringParenImpCasts(ConstantExpr))),
104 unless(SizeOfZero))
105 .bind("sizeof-constant"),
106 this);
107 }
108
109 // Detect sizeof(f())
110 if (WarnOnSizeOfIntegerExpression) {
111 Finder->addMatcher(sizeOfExpr(ignoringParenImpCasts(has(IntegerCallExpr)))
112 .bind("sizeof-integer-call"),
113 this);
114 }
115
116 // Detect expression like: sizeof(this);
117 if (WarnOnSizeOfThis) {
118 Finder->addMatcher(sizeOfExpr(has(ignoringParenImpCasts(cxxThisExpr())))
119 .bind("sizeof-this"),
120 this);
121 }
122
123 // Detect sizeof(kPtr) where kPtr is 'const char* kPtr = "abc"';
124 const auto CharPtrType = pointerType(pointee(isAnyCharacter()));
125 const auto ConstStrLiteralDecl =
126 varDecl(isDefinition(), hasType(hasCanonicalType(CharPtrType)),
127 hasInitializer(ignoringParenImpCasts(stringLiteral())));
128 Finder->addMatcher(
129 sizeOfExpr(has(ignoringParenImpCasts(
130 expr(hasType(hasCanonicalType(CharPtrType)),
131 ignoringParenImpCasts(declRefExpr(
132 hasDeclaration(ConstStrLiteralDecl)))))))
133 .bind("sizeof-charp"),
134 this);
135
136 // Detect sizeof(ptr) where ptr points to an aggregate (i.e. sizeof(&S)).
137 // Do not find it if RHS of a 'sizeof(arr) / sizeof(arr[0])' expression.
138 const auto ArrayExpr =
139 ignoringParenImpCasts(hasType(hasCanonicalType(arrayType())));
140 const auto ArrayCastExpr = expr(anyOf(
141 unaryOperator(hasUnaryOperand(ArrayExpr), unless(hasOperatorName("*"))),
142 binaryOperator(hasEitherOperand(ArrayExpr)),
143 castExpr(hasSourceExpression(ArrayExpr))));
144 const auto PointerToArrayExpr = ignoringParenImpCasts(
145 hasType(hasCanonicalType(pointerType(pointee(arrayType())))));
146
147 const auto StructAddrOfExpr = unaryOperator(
148 hasOperatorName("&"), hasUnaryOperand(ignoringParenImpCasts(
149 hasType(hasCanonicalType(recordType())))));
150 const auto PointerToStructType =
151 hasUnqualifiedDesugaredType(pointerType(pointee(recordType())));
152 const auto PointerToStructExpr = ignoringParenImpCasts(expr(
153 hasType(hasCanonicalType(PointerToStructType)), unless(cxxThisExpr())));
154
155 const auto ArrayOfPointersExpr = ignoringParenImpCasts(
156 hasType(hasCanonicalType(arrayType(hasElementType(pointerType()))
157 .bind("type-of-array-of-pointers"))));
158 const auto ArrayOfSamePointersExpr =
159 ignoringParenImpCasts(hasType(hasCanonicalType(
160 arrayType(equalsBoundNode("type-of-array-of-pointers")))));
161 const auto ZeroLiteral = ignoringParenImpCasts(integerLiteral(equals(0)));
162 const auto ArrayOfSamePointersZeroSubscriptExpr =
163 ignoringParenImpCasts(arraySubscriptExpr(hasBase(ArrayOfSamePointersExpr),
164 hasIndex(ZeroLiteral)));
165 const auto ArrayLengthExprDenom =
166 expr(hasParent(expr(ignoringParenImpCasts(binaryOperator(
167 hasOperatorName("/"), hasLHS(ignoringParenImpCasts(sizeOfExpr(
168 has(ArrayOfPointersExpr)))))))),
169 sizeOfExpr(has(ArrayOfSamePointersZeroSubscriptExpr)));
170
171 Finder->addMatcher(expr(anyOf(sizeOfExpr(has(ignoringParenImpCasts(anyOf(
172 ArrayCastExpr, PointerToArrayExpr,
173 StructAddrOfExpr, PointerToStructExpr)))),
174 sizeOfExpr(has(PointerToStructType))),
175 unless(ArrayLengthExprDenom))
176 .bind("sizeof-pointer-to-aggregate"),
177 this);
178
179 // Detect expression like: sizeof(epxr) <= k for a suspicious constant 'k'.
180 if (WarnOnSizeOfCompareToConstant) {
181 Finder->addMatcher(
182 binaryOperator(matchers::isRelationalOperator(),
183 hasOperands(ignoringParenImpCasts(SizeOfExpr),
184 ignoringParenImpCasts(integerLiteral(anyOf(
185 equals(0), isBiggerThan(0x80000))))))
186 .bind("sizeof-compare-constant"),
187 this);
188 }
189
190 // Detect expression like: sizeof(expr, expr); most likely an error.
191 Finder->addMatcher(sizeOfExpr(has(ignoringParenImpCasts(
192 binaryOperator(hasOperatorName(",")))))
193 .bind("sizeof-comma-expr"),
194 this);
195
196 // Detect sizeof(...) /sizeof(...));
197 // FIXME:
198 // Re-evaluate what cases to handle by the checker.
199 // Probably any sizeof(A)/sizeof(B) should be error if
200 // 'A' is not an array (type) and 'B' the (type of the) first element of it.
201 // Except if 'A' and 'B' are non-pointers, then use the existing size division
202 // rule.
203 const auto ElemType =
204 arrayType(hasElementType(recordType().bind("elem-type")));
205 const auto ElemPtrType = pointerType(pointee(type().bind("elem-ptr-type")));
206
207 Finder->addMatcher(
208 binaryOperator(
209 hasOperatorName("/"),
210 hasLHS(ignoringParenImpCasts(sizeOfExpr(hasArgumentOfType(
211 hasCanonicalType(type(anyOf(ElemType, ElemPtrType, type()))
212 .bind("num-type")))))),
213 hasRHS(ignoringParenImpCasts(sizeOfExpr(
214 hasArgumentOfType(hasCanonicalType(type().bind("denom-type")))))))
215 .bind("sizeof-divide-expr"),
216 this);
217
218 // Detect expression like: sizeof(...) * sizeof(...)); most likely an error.
219 Finder->addMatcher(binaryOperator(hasOperatorName("*"),
220 hasLHS(ignoringParenImpCasts(SizeOfExpr)),
221 hasRHS(ignoringParenImpCasts(SizeOfExpr)))
222 .bind("sizeof-multiply-sizeof"),
223 this);
224
225 Finder->addMatcher(
226 binaryOperator(hasOperatorName("*"),
227 hasOperands(ignoringParenImpCasts(SizeOfExpr),
228 ignoringParenImpCasts(binaryOperator(
229 hasOperatorName("*"),
230 hasEitherOperand(
231 ignoringParenImpCasts(SizeOfExpr))))))
232 .bind("sizeof-multiply-sizeof"),
233 this);
234
235 // Detect strange double-sizeof expression like: sizeof(sizeof(...));
236 // Note: The expression 'sizeof(sizeof(0))' is accepted.
237 Finder->addMatcher(sizeOfExpr(has(ignoringParenImpCasts(hasSizeOfDescendant(
238 8, allOf(SizeOfExpr, unless(SizeOfZero))))))
239 .bind("sizeof-sizeof-expr"),
240 this);
241
242 // Detect sizeof in pointer arithmetic like: N * sizeof(S) == P1 - P2 or
243 // (P1 - P2) / sizeof(S) where P1 and P2 are pointers to type S.
244 const auto PtrDiffExpr = binaryOperator(
245 hasOperatorName("-"),
246 hasLHS(hasType(hasUnqualifiedDesugaredType(pointerType(pointee(
247 hasUnqualifiedDesugaredType(type().bind("left-ptr-type"))))))),
248 hasRHS(hasType(hasUnqualifiedDesugaredType(pointerType(pointee(
249 hasUnqualifiedDesugaredType(type().bind("right-ptr-type"))))))));
250
251 Finder->addMatcher(
252 binaryOperator(
253 hasAnyOperatorName("==", "!=", "<", "<=", ">", ">=", "+", "-"),
254 hasOperands(
255 anyOf(ignoringParenImpCasts(SizeOfExpr),
256 ignoringParenImpCasts(binaryOperator(
257 hasOperatorName("*"),
258 hasEitherOperand(ignoringParenImpCasts(SizeOfExpr))))),
259 ignoringParenImpCasts(PtrDiffExpr)))
260 .bind("sizeof-in-ptr-arithmetic-mul"),
261 this);
262
263 Finder->addMatcher(binaryOperator(hasOperatorName("/"),
264 hasLHS(ignoringParenImpCasts(PtrDiffExpr)),
265 hasRHS(ignoringParenImpCasts(SizeOfExpr)))
266 .bind("sizeof-in-ptr-arithmetic-div"),
267 this);
268 }
269
check(const MatchFinder::MatchResult & Result)270 void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
271 const ASTContext &Ctx = *Result.Context;
272
273 if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-constant")) {
274 diag(E->getBeginLoc(),
275 "suspicious usage of 'sizeof(K)'; did you mean 'K'?");
276 } else if (const auto *E =
277 Result.Nodes.getNodeAs<Expr>("sizeof-integer-call")) {
278 diag(E->getBeginLoc(), "suspicious usage of 'sizeof()' on an expression "
279 "that results in an integer");
280 } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-this")) {
281 diag(E->getBeginLoc(),
282 "suspicious usage of 'sizeof(this)'; did you mean 'sizeof(*this)'");
283 } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-charp")) {
284 diag(E->getBeginLoc(),
285 "suspicious usage of 'sizeof(char*)'; do you mean 'strlen'?");
286 } else if (const auto *E =
287 Result.Nodes.getNodeAs<Expr>("sizeof-pointer-to-aggregate")) {
288 diag(E->getBeginLoc(),
289 "suspicious usage of 'sizeof(A*)'; pointer to aggregate");
290 } else if (const auto *E =
291 Result.Nodes.getNodeAs<Expr>("sizeof-compare-constant")) {
292 diag(E->getBeginLoc(),
293 "suspicious comparison of 'sizeof(expr)' to a constant");
294 } else if (const auto *E =
295 Result.Nodes.getNodeAs<Expr>("sizeof-comma-expr")) {
296 diag(E->getBeginLoc(), "suspicious usage of 'sizeof(..., ...)'");
297 } else if (const auto *E =
298 Result.Nodes.getNodeAs<Expr>("sizeof-divide-expr")) {
299 const auto *NumTy = Result.Nodes.getNodeAs<Type>("num-type");
300 const auto *DenomTy = Result.Nodes.getNodeAs<Type>("denom-type");
301 const auto *ElementTy = Result.Nodes.getNodeAs<Type>("elem-type");
302 const auto *PointedTy = Result.Nodes.getNodeAs<Type>("elem-ptr-type");
303
304 CharUnits NumeratorSize = getSizeOfType(Ctx, NumTy);
305 CharUnits DenominatorSize = getSizeOfType(Ctx, DenomTy);
306 CharUnits ElementSize = getSizeOfType(Ctx, ElementTy);
307
308 if (DenominatorSize > CharUnits::Zero() &&
309 !NumeratorSize.isMultipleOf(DenominatorSize)) {
310 diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
311 " numerator is not a multiple of denominator");
312 } else if (ElementSize > CharUnits::Zero() &&
313 DenominatorSize > CharUnits::Zero() &&
314 ElementSize != DenominatorSize) {
315 diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
316 " numerator is not a multiple of denominator");
317 } else if (NumTy && DenomTy && NumTy == DenomTy) {
318 diag(E->getBeginLoc(),
319 "suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'");
320 } else if (PointedTy && DenomTy && PointedTy == DenomTy) {
321 diag(E->getBeginLoc(),
322 "suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'");
323 } else if (NumTy && DenomTy && NumTy->isPointerType() &&
324 DenomTy->isPointerType()) {
325 diag(E->getBeginLoc(),
326 "suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'");
327 }
328 } else if (const auto *E =
329 Result.Nodes.getNodeAs<Expr>("sizeof-sizeof-expr")) {
330 diag(E->getBeginLoc(), "suspicious usage of 'sizeof(sizeof(...))'");
331 } else if (const auto *E =
332 Result.Nodes.getNodeAs<Expr>("sizeof-multiply-sizeof")) {
333 diag(E->getBeginLoc(), "suspicious 'sizeof' by 'sizeof' multiplication");
334 } else if (const auto *E =
335 Result.Nodes.getNodeAs<Expr>("sizeof-in-ptr-arithmetic-mul")) {
336 const auto *LPtrTy = Result.Nodes.getNodeAs<Type>("left-ptr-type");
337 const auto *RPtrTy = Result.Nodes.getNodeAs<Type>("right-ptr-type");
338 const auto *SizeofArgTy = Result.Nodes.getNodeAs<Type>("sizeof-arg-type");
339
340 if ((LPtrTy == RPtrTy) && (LPtrTy == SizeofArgTy)) {
341 diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
342 "pointer arithmetic");
343 }
344 } else if (const auto *E =
345 Result.Nodes.getNodeAs<Expr>("sizeof-in-ptr-arithmetic-div")) {
346 const auto *LPtrTy = Result.Nodes.getNodeAs<Type>("left-ptr-type");
347 const auto *RPtrTy = Result.Nodes.getNodeAs<Type>("right-ptr-type");
348 const auto *SizeofArgTy = Result.Nodes.getNodeAs<Type>("sizeof-arg-type");
349
350 if ((LPtrTy == RPtrTy) && (LPtrTy == SizeofArgTy)) {
351 diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
352 "pointer arithmetic");
353 }
354 }
355 }
356
357 } // namespace bugprone
358 } // namespace tidy
359 } // namespace clang
360