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 
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 
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 
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
360