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