clang-tools 19.0.0git
SizeofExpressionCheck.cpp
Go to the documentation of this file.
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
10#include "../utils/Matchers.h"
11#include "clang/AST/ASTContext.h"
12#include "clang/ASTMatchers/ASTMatchFinder.h"
13
14using namespace clang::ast_matchers;
15
16namespace clang::tidy::bugprone {
17
18namespace {
19
20AST_MATCHER_P(IntegerLiteral, isBiggerThan, unsigned, N) {
21 return Node.getValue().ugt(N);
22}
23
24AST_MATCHER_P2(Expr, hasSizeOfDescendant, int, Depth,
25 ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
26 if (Depth < 0)
27 return false;
28
29 const Expr *E = Node.IgnoreParenImpCasts();
30 if (InnerMatcher.matches(*E, Finder, Builder))
31 return true;
32
33 if (const auto *CE = dyn_cast<CastExpr>(E)) {
34 const auto M = hasSizeOfDescendant(Depth - 1, InnerMatcher);
35 return M.matches(*CE->getSubExpr(), Finder, Builder);
36 }
37 if (const auto *UE = dyn_cast<UnaryOperator>(E)) {
38 const auto M = hasSizeOfDescendant(Depth - 1, InnerMatcher);
39 return M.matches(*UE->getSubExpr(), Finder, Builder);
40 }
41 if (const auto *BE = dyn_cast<BinaryOperator>(E)) {
42 const auto LHS = hasSizeOfDescendant(Depth - 1, InnerMatcher);
43 const auto RHS = hasSizeOfDescendant(Depth - 1, InnerMatcher);
44 return LHS.matches(*BE->getLHS(), Finder, Builder) ||
45 RHS.matches(*BE->getRHS(), Finder, Builder);
46 }
47
48 return false;
49}
50
51CharUnits getSizeOfType(const ASTContext &Ctx, const Type *Ty) {
52 if (!Ty || Ty->isIncompleteType() || Ty->isDependentType() ||
53 isa<DependentSizedArrayType>(Ty) || !Ty->isConstantSizeType())
54 return CharUnits::Zero();
55 return Ctx.getTypeSizeInChars(Ty);
56}
57
58} // namespace
59
61 ClangTidyContext *Context)
62 : ClangTidyCheck(Name, Context),
63 WarnOnSizeOfConstant(Options.get("WarnOnSizeOfConstant", true)),
64 WarnOnSizeOfIntegerExpression(
65 Options.get("WarnOnSizeOfIntegerExpression", false)),
66 WarnOnSizeOfThis(Options.get("WarnOnSizeOfThis", true)),
67 WarnOnSizeOfCompareToConstant(
68 Options.get("WarnOnSizeOfCompareToConstant", true)),
69 WarnOnSizeOfPointerToAggregate(
70 Options.get("WarnOnSizeOfPointerToAggregate", true)),
71 WarnOnSizeOfPointer(Options.get("WarnOnSizeOfPointer", false)) {}
72
74 Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant);
75 Options.store(Opts, "WarnOnSizeOfIntegerExpression",
76 WarnOnSizeOfIntegerExpression);
77 Options.store(Opts, "WarnOnSizeOfThis", WarnOnSizeOfThis);
78 Options.store(Opts, "WarnOnSizeOfCompareToConstant",
79 WarnOnSizeOfCompareToConstant);
80 Options.store(Opts, "WarnOnSizeOfPointerToAggregate",
81 WarnOnSizeOfPointerToAggregate);
82 Options.store(Opts, "WarnOnSizeOfPointer", WarnOnSizeOfPointer);
83}
84
85void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
86 // FIXME:
87 // Some of the checks should not match in template code to avoid false
88 // positives if sizeof is applied on template argument.
89
90 const auto IntegerExpr = ignoringParenImpCasts(integerLiteral());
91 const auto ConstantExpr = ignoringParenImpCasts(
92 anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)),
93 binaryOperator(hasLHS(IntegerExpr), hasRHS(IntegerExpr))));
94 const auto IntegerCallExpr = ignoringParenImpCasts(callExpr(
95 anyOf(hasType(isInteger()), hasType(hasCanonicalType(enumType()))),
96 unless(isInTemplateInstantiation())));
97 const auto SizeOfExpr = sizeOfExpr(hasArgumentOfType(
98 hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type"))));
99 const auto SizeOfZero =
100 sizeOfExpr(has(ignoringParenImpCasts(integerLiteral(equals(0)))));
101
102 // Detect expression like: sizeof(ARRAYLEN);
103 // Note: The expression 'sizeof(sizeof(0))' is a portable trick used to know
104 // the sizeof size_t.
105 if (WarnOnSizeOfConstant) {
106 Finder->addMatcher(
107 expr(sizeOfExpr(has(ignoringParenImpCasts(ConstantExpr))),
108 unless(SizeOfZero))
109 .bind("sizeof-constant"),
110 this);
111 }
112
113 // Detect sizeof(f())
114 if (WarnOnSizeOfIntegerExpression) {
115 Finder->addMatcher(sizeOfExpr(ignoringParenImpCasts(has(IntegerCallExpr)))
116 .bind("sizeof-integer-call"),
117 this);
118 }
119
120 // Detect expression like: sizeof(this);
121 if (WarnOnSizeOfThis) {
122 Finder->addMatcher(sizeOfExpr(has(ignoringParenImpCasts(cxxThisExpr())))
123 .bind("sizeof-this"),
124 this);
125 }
126
127 // Detect sizeof(kPtr) where kPtr is 'const char* kPtr = "abc"';
128 const auto CharPtrType = pointerType(pointee(isAnyCharacter()));
129 const auto ConstStrLiteralDecl =
130 varDecl(isDefinition(), hasType(hasCanonicalType(CharPtrType)),
131 hasInitializer(ignoringParenImpCasts(stringLiteral())));
132 const auto VarWithConstStrLiteralDecl = expr(
133 hasType(hasCanonicalType(CharPtrType)),
134 ignoringParenImpCasts(declRefExpr(hasDeclaration(ConstStrLiteralDecl))));
135 Finder->addMatcher(
136 sizeOfExpr(has(ignoringParenImpCasts(VarWithConstStrLiteralDecl)))
137 .bind("sizeof-charp"),
138 this);
139
140 // Detect sizeof(ptr) where ptr is a pointer (CWE-467).
141 //
142 // In WarnOnSizeOfPointerToAggregate mode only report cases when ptr points
143 // to an aggregate type or ptr is an expression that (implicitly or
144 // explicitly) casts an array to a pointer type. (These are more suspicious
145 // than other sizeof(ptr) expressions because they can appear as distorted
146 // forms of the common sizeof(aggregate) expressions.)
147 //
148 // To avoid false positives, the check doesn't report expressions like
149 // 'sizeof(pp[0])' and 'sizeof(*pp)' where `pp` is a pointer-to-pointer or
150 // array of pointers. (This filters out both `sizeof(arr) / sizeof(arr[0])`
151 // expressions and other cases like `p = realloc(p, newsize * sizeof(*p));`.)
152 //
153 // Moreover this generic message is suppressed in cases that are also matched
154 // by the more concrete matchers 'sizeof-this' and 'sizeof-charp'.
155 if (WarnOnSizeOfPointerToAggregate || WarnOnSizeOfPointer) {
156 const auto ArrayExpr =
157 ignoringParenImpCasts(hasType(hasCanonicalType(arrayType())));
158 const auto ArrayCastExpr = expr(anyOf(
159 unaryOperator(hasUnaryOperand(ArrayExpr), unless(hasOperatorName("*"))),
160 binaryOperator(hasEitherOperand(ArrayExpr)),
161 castExpr(hasSourceExpression(ArrayExpr))));
162 const auto PointerToArrayExpr =
163 hasType(hasCanonicalType(pointerType(pointee(arrayType()))));
164
165 const auto PointerToStructType =
166 hasUnqualifiedDesugaredType(pointerType(pointee(recordType())));
167 const auto PointerToStructTypeWithBinding =
168 type(PointerToStructType).bind("struct-type");
169 const auto PointerToStructExpr =
170 expr(hasType(hasCanonicalType(PointerToStructType)));
171
172 const auto PointerToDetectedExpr =
173 WarnOnSizeOfPointer
174 ? expr(hasType(hasUnqualifiedDesugaredType(pointerType())))
175 : expr(anyOf(ArrayCastExpr, PointerToArrayExpr,
176 PointerToStructExpr));
177
178 const auto ZeroLiteral = ignoringParenImpCasts(integerLiteral(equals(0)));
179 const auto SubscriptExprWithZeroIndex =
180 arraySubscriptExpr(hasIndex(ZeroLiteral));
181 const auto DerefExpr =
182 ignoringParenImpCasts(unaryOperator(hasOperatorName("*")));
183
184 Finder->addMatcher(
185 expr(sizeOfExpr(anyOf(has(ignoringParenImpCasts(
186 expr(PointerToDetectedExpr, unless(DerefExpr),
187 unless(SubscriptExprWithZeroIndex),
188 unless(VarWithConstStrLiteralDecl),
189 unless(cxxThisExpr())))),
190 has(PointerToStructTypeWithBinding))))
191 .bind("sizeof-pointer"),
192 this);
193 }
194
195 // Detect expression like: sizeof(expr) <= k for a suspicious constant 'k'.
196 if (WarnOnSizeOfCompareToConstant) {
197 Finder->addMatcher(
198 binaryOperator(matchers::isRelationalOperator(),
199 hasOperands(ignoringParenImpCasts(SizeOfExpr),
200 ignoringParenImpCasts(integerLiteral(anyOf(
201 equals(0), isBiggerThan(0x80000))))))
202 .bind("sizeof-compare-constant"),
203 this);
204 }
205
206 // Detect expression like: sizeof(expr, expr); most likely an error.
207 Finder->addMatcher(
208 sizeOfExpr(
209 has(ignoringParenImpCasts(
210 binaryOperator(hasOperatorName(",")).bind("sizeof-comma-binop"))))
211 .bind("sizeof-comma-expr"),
212 this);
213
214 // Detect sizeof(...) /sizeof(...));
215 // FIXME:
216 // Re-evaluate what cases to handle by the checker.
217 // Probably any sizeof(A)/sizeof(B) should be error if
218 // 'A' is not an array (type) and 'B' the (type of the) first element of it.
219 // Except if 'A' and 'B' are non-pointers, then use the existing size division
220 // rule.
221 const auto ElemType =
222 arrayType(hasElementType(recordType().bind("elem-type")));
223 const auto ElemPtrType = pointerType(pointee(type().bind("elem-ptr-type")));
224
225 Finder->addMatcher(
226 binaryOperator(
227 hasOperatorName("/"),
228 hasLHS(ignoringParenImpCasts(sizeOfExpr(hasArgumentOfType(
229 hasCanonicalType(type(anyOf(ElemType, ElemPtrType, type()))
230 .bind("num-type")))))),
231 hasRHS(ignoringParenImpCasts(sizeOfExpr(
232 hasArgumentOfType(hasCanonicalType(type().bind("denom-type")))))))
233 .bind("sizeof-divide-expr"),
234 this);
235
236 // Detect expression like: sizeof(...) * sizeof(...)); most likely an error.
237 Finder->addMatcher(binaryOperator(hasOperatorName("*"),
238 hasLHS(ignoringParenImpCasts(SizeOfExpr)),
239 hasRHS(ignoringParenImpCasts(SizeOfExpr)))
240 .bind("sizeof-multiply-sizeof"),
241 this);
242
243 Finder->addMatcher(
244 binaryOperator(hasOperatorName("*"),
245 hasOperands(ignoringParenImpCasts(SizeOfExpr),
246 ignoringParenImpCasts(binaryOperator(
247 hasOperatorName("*"),
248 hasEitherOperand(
249 ignoringParenImpCasts(SizeOfExpr))))))
250 .bind("sizeof-multiply-sizeof"),
251 this);
252
253 // Detect strange double-sizeof expression like: sizeof(sizeof(...));
254 // Note: The expression 'sizeof(sizeof(0))' is accepted.
255 Finder->addMatcher(sizeOfExpr(has(ignoringParenImpCasts(hasSizeOfDescendant(
256 8, allOf(SizeOfExpr, unless(SizeOfZero))))))
257 .bind("sizeof-sizeof-expr"),
258 this);
259
260 // Detect sizeof in pointer arithmetic like: N * sizeof(S) == P1 - P2 or
261 // (P1 - P2) / sizeof(S) where P1 and P2 are pointers to type S.
262 const auto PtrDiffExpr = binaryOperator(
263 hasOperatorName("-"),
264 hasLHS(hasType(hasUnqualifiedDesugaredType(pointerType(pointee(
265 hasUnqualifiedDesugaredType(type().bind("left-ptr-type"))))))),
266 hasRHS(hasType(hasUnqualifiedDesugaredType(pointerType(pointee(
267 hasUnqualifiedDesugaredType(type().bind("right-ptr-type"))))))));
268
269 Finder->addMatcher(
270 binaryOperator(
271 hasAnyOperatorName("==", "!=", "<", "<=", ">", ">=", "+", "-"),
272 hasOperands(anyOf(ignoringParenImpCasts(
273 SizeOfExpr.bind("sizeof-ptr-mul-expr")),
274 ignoringParenImpCasts(binaryOperator(
275 hasOperatorName("*"),
276 hasEitherOperand(ignoringParenImpCasts(
277 SizeOfExpr.bind("sizeof-ptr-mul-expr")))))),
278 ignoringParenImpCasts(PtrDiffExpr)))
279 .bind("sizeof-in-ptr-arithmetic-mul"),
280 this);
281
282 Finder->addMatcher(
283 binaryOperator(
284 hasOperatorName("/"), hasLHS(ignoringParenImpCasts(PtrDiffExpr)),
285 hasRHS(ignoringParenImpCasts(SizeOfExpr.bind("sizeof-ptr-div-expr"))))
286 .bind("sizeof-in-ptr-arithmetic-div"),
287 this);
288}
289
290void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
291 const ASTContext &Ctx = *Result.Context;
292
293 if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-constant")) {
294 diag(E->getBeginLoc(), "suspicious usage of 'sizeof(K)'; did you mean 'K'?")
295 << E->getSourceRange();
296 } else if (const auto *E =
297 Result.Nodes.getNodeAs<Expr>("sizeof-integer-call")) {
298 diag(E->getBeginLoc(), "suspicious usage of 'sizeof()' on an expression "
299 "of integer type")
300 << E->getSourceRange();
301 } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-this")) {
302 diag(E->getBeginLoc(),
303 "suspicious usage of 'sizeof(this)'; did you mean 'sizeof(*this)'")
304 << E->getSourceRange();
305 } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-charp")) {
306 diag(E->getBeginLoc(),
307 "suspicious usage of 'sizeof(char*)'; do you mean 'strlen'?")
308 << E->getSourceRange();
309 } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-pointer")) {
310 if (Result.Nodes.getNodeAs<Type>("struct-type")) {
311 diag(E->getBeginLoc(),
312 "suspicious usage of 'sizeof(A*)' on pointer-to-aggregate type; did "
313 "you mean 'sizeof(A)'?")
314 << E->getSourceRange();
315 } else {
316 diag(E->getBeginLoc(), "suspicious usage of 'sizeof()' on an expression "
317 "of pointer type")
318 << E->getSourceRange();
319 }
320 } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
321 "sizeof-compare-constant")) {
322 diag(E->getOperatorLoc(),
323 "suspicious comparison of 'sizeof(expr)' to a constant")
324 << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
325 } else if (const auto *E =
326 Result.Nodes.getNodeAs<Expr>("sizeof-comma-expr")) {
327 const auto *BO =
328 Result.Nodes.getNodeAs<BinaryOperator>("sizeof-comma-binop");
329 assert(BO);
330 diag(BO->getOperatorLoc(), "suspicious usage of 'sizeof(..., ...)'")
331 << E->getSourceRange();
332 } else if (const auto *E =
333 Result.Nodes.getNodeAs<BinaryOperator>("sizeof-divide-expr")) {
334 const auto *NumTy = Result.Nodes.getNodeAs<Type>("num-type");
335 const auto *DenomTy = Result.Nodes.getNodeAs<Type>("denom-type");
336 const auto *ElementTy = Result.Nodes.getNodeAs<Type>("elem-type");
337 const auto *PointedTy = Result.Nodes.getNodeAs<Type>("elem-ptr-type");
338
339 CharUnits NumeratorSize = getSizeOfType(Ctx, NumTy);
340 CharUnits DenominatorSize = getSizeOfType(Ctx, DenomTy);
341 CharUnits ElementSize = getSizeOfType(Ctx, ElementTy);
342
343 if (DenominatorSize > CharUnits::Zero() &&
344 !NumeratorSize.isMultipleOf(DenominatorSize)) {
345 diag(E->getOperatorLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
346 " numerator is not a multiple of denominator")
347 << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
348 } else if (ElementSize > CharUnits::Zero() &&
349 DenominatorSize > CharUnits::Zero() &&
350 ElementSize != DenominatorSize) {
351 // FIXME: Apparently there are no testcases that cover this branch!
352 diag(E->getOperatorLoc(),
353 "suspicious usage of 'sizeof(array)/sizeof(...)';"
354 " denominator differs from the size of array elements")
355 << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
356 } else if (NumTy && DenomTy && NumTy == DenomTy) {
357 diag(E->getOperatorLoc(),
358 "suspicious usage of 'sizeof(...)/sizeof(...)'; both expressions "
359 "have the same type")
360 << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
361 } else if (!WarnOnSizeOfPointer) {
362 // When 'WarnOnSizeOfPointer' is enabled, these messages become redundant:
363 if (PointedTy && DenomTy && PointedTy == DenomTy) {
364 diag(E->getOperatorLoc(),
365 "suspicious usage of 'sizeof(...)/sizeof(...)'; size of pointer "
366 "is divided by size of pointed type")
367 << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
368 } else if (NumTy && DenomTy && NumTy->isPointerType() &&
369 DenomTy->isPointerType()) {
370 diag(E->getOperatorLoc(),
371 "suspicious usage of 'sizeof(...)/sizeof(...)'; both expressions "
372 "have pointer types")
373 << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
374 }
375 }
376 } else if (const auto *E =
377 Result.Nodes.getNodeAs<Expr>("sizeof-sizeof-expr")) {
378 diag(E->getBeginLoc(), "suspicious usage of 'sizeof(sizeof(...))'")
379 << E->getSourceRange();
380 } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
381 "sizeof-multiply-sizeof")) {
382 diag(E->getOperatorLoc(), "suspicious 'sizeof' by 'sizeof' multiplication")
383 << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
384 } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
385 "sizeof-in-ptr-arithmetic-mul")) {
386 const auto *LPtrTy = Result.Nodes.getNodeAs<Type>("left-ptr-type");
387 const auto *RPtrTy = Result.Nodes.getNodeAs<Type>("right-ptr-type");
388 const auto *SizeofArgTy = Result.Nodes.getNodeAs<Type>("sizeof-arg-type");
389 const auto *SizeOfExpr =
390 Result.Nodes.getNodeAs<UnaryExprOrTypeTraitExpr>("sizeof-ptr-mul-expr");
391
392 if ((LPtrTy == RPtrTy) && (LPtrTy == SizeofArgTy)) {
393 diag(SizeOfExpr->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
394 "pointer arithmetic")
395 << SizeOfExpr->getSourceRange() << E->getOperatorLoc()
396 << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
397 }
398 } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
399 "sizeof-in-ptr-arithmetic-div")) {
400 const auto *LPtrTy = Result.Nodes.getNodeAs<Type>("left-ptr-type");
401 const auto *RPtrTy = Result.Nodes.getNodeAs<Type>("right-ptr-type");
402 const auto *SizeofArgTy = Result.Nodes.getNodeAs<Type>("sizeof-arg-type");
403 const auto *SizeOfExpr =
404 Result.Nodes.getNodeAs<UnaryExprOrTypeTraitExpr>("sizeof-ptr-div-expr");
405
406 if ((LPtrTy == RPtrTy) && (LPtrTy == SizeofArgTy)) {
407 diag(SizeOfExpr->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
408 "pointer arithmetic")
409 << SizeOfExpr->getSourceRange() << E->getOperatorLoc()
410 << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
411 }
412 }
413}
414
415} // namespace clang::tidy::bugprone
const Expr * E
CaptureExpr CE
llvm::SmallString< 256U > Name
CodeCompletionBuilder Builder
NodeType Type
::clang::DynTypedNode Node
const google::protobuf::Message & M
Definition: Server.cpp:309
void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, StringRef Value) const
Stores an option with the check-local name LocalName with string value Value to Options.
Base class for all clang-tidy checks.
DiagnosticBuilder diag(SourceLocation Loc, StringRef Description, DiagnosticIDs::Level Level=DiagnosticIDs::Warning)
Add a diagnostic with the check's name.
Every ClangTidyCheck reports errors through a DiagnosticsEngine provided by this context.
void registerMatchers(ast_matchers::MatchFinder *Finder) override
Override this to register AST matchers with Finder.
SizeofExpressionCheck(StringRef Name, ClangTidyContext *Context)
void storeOptions(ClangTidyOptions::OptionMap &Opts) override
Should store all options supported by this check with their current values or default values for opti...
void check(const ast_matchers::MatchFinder::MatchResult &Result) override
ClangTidyChecks that register ASTMatchers should do the actual work in here.
AST_MATCHER_P(FunctionDecl, parameterCountGE, unsigned, N)
Matches functions that have at least the specified amount of parameters.
llvm::StringMap< ClangTidyValue > OptionMap