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