clang-tools  14.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 
14 using namespace clang::ast_matchers;
15 
16 namespace clang {
17 namespace tidy {
18 namespace bugprone {
19 
20 namespace {
21 
22 AST_MATCHER_P(IntegerLiteral, isBiggerThan, unsigned, N) {
23  return Node.getValue().getZExtValue() > N;
24 }
25 
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 
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 
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 
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 
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(expr) <= 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 
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
clang::tidy::ClangTidyOptions::OptionMap
llvm::StringMap< ClangTidyValue > OptionMap
Definition: ClangTidyOptions.h:115
clang::tidy::bugprone::SizeofExpressionCheck::storeOptions
void storeOptions(ClangTidyOptions::OptionMap &Opts) override
Should store all options supported by this check with their current values or default values for opti...
Definition: SizeofExpressionCheck.cpp:72
E
const Expr * E
Definition: AvoidBindCheck.cpp:88
Type
NodeType Type
Definition: HTMLGenerator.cpp:73
clang::tidy::bugprone::SizeofExpressionCheck::check
void check(const ast_matchers::MatchFinder::MatchResult &Result) override
ClangTidyChecks that register ASTMatchers should do the actual work in here.
Definition: SizeofExpressionCheck.cpp:270
Ctx
Context Ctx
Definition: TUScheduler.cpp:460
clang::tidy::ClangTidyCheck
Base class for all clang-tidy checks.
Definition: ClangTidyCheck.h:54
SizeofExpressionCheck.h
clang::ast_matchers
Definition: AbseilMatcher.h:14
M
const google::protobuf::Message & M
Definition: Server.cpp:309
clang::tidy::ClangTidyCheck::Options
OptionsView Options
Definition: ClangTidyCheck.h:416
Builder
CodeCompletionBuilder Builder
Definition: CodeCompletionStringsTests.cpp:36
clang::tidy::ClangTidyContext
Every ClangTidyCheck reports errors through a DiagnosticsEngine provided by this context.
Definition: ClangTidyDiagnosticConsumer.h:71
Name
static constexpr llvm::StringLiteral Name
Definition: UppercaseLiteralSuffixCheck.cpp:28
clang::tidy::ClangTidyCheck::diag
DiagnosticBuilder diag(SourceLocation Loc, StringRef Description, DiagnosticIDs::Level Level=DiagnosticIDs::Warning)
Add a diagnostic with the check's name.
Definition: ClangTidyCheck.cpp:25
CE
CaptureExpr CE
Definition: AvoidBindCheck.cpp:67
clang
===– Representation.cpp - ClangDoc Representation --------—*- C++ -*-===//
Definition: ApplyReplacements.h:27
clang::tidy::ClangTidyCheck::OptionsView::store
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.
Definition: ClangTidyCheck.cpp:120
clang::tidy::bugprone::SizeofExpressionCheck::registerMatchers
void registerMatchers(ast_matchers::MatchFinder *Finder) override
Override this to register AST matchers with Finder.
Definition: SizeofExpressionCheck.cpp:81
clang::tidy::bugprone::AST_MATCHER_P
AST_MATCHER_P(FunctionDecl, parameterCountGE, unsigned, N)
Matches functions that have at least the specified amount of parameters.
Definition: EasilySwappableParametersCheck.cpp:1877