SizeofExpressionCheck.cpp 16 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354355356357358359360361
  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. #include "SizeofExpressionCheck.h"
  9. #include "../utils/Matchers.h"
  10. #include "clang/AST/ASTContext.h"
  11. #include "clang/ASTMatchers/ASTMatchFinder.h"
  12. using namespace clang::ast_matchers;
  13. namespace clang::tidy::bugprone {
  14. namespace {
  15. AST_MATCHER_P(IntegerLiteral, isBiggerThan, unsigned, N) {
  16. return Node.getValue().ugt(N);
  17. }
  18. AST_MATCHER_P2(Expr, hasSizeOfDescendant, int, Depth,
  19. ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
  20. if (Depth < 0)
  21. return false;
  22. const Expr *E = Node.IgnoreParenImpCasts();
  23. if (InnerMatcher.matches(*E, Finder, Builder))
  24. return true;
  25. if (const auto *CE = dyn_cast<CastExpr>(E)) {
  26. const auto M = hasSizeOfDescendant(Depth - 1, InnerMatcher);
  27. return M.matches(*CE->getSubExpr(), Finder, Builder);
  28. }
  29. if (const auto *UE = dyn_cast<UnaryOperator>(E)) {
  30. const auto M = hasSizeOfDescendant(Depth - 1, InnerMatcher);
  31. return M.matches(*UE->getSubExpr(), Finder, Builder);
  32. }
  33. if (const auto *BE = dyn_cast<BinaryOperator>(E)) {
  34. const auto LHS = hasSizeOfDescendant(Depth - 1, InnerMatcher);
  35. const auto RHS = hasSizeOfDescendant(Depth - 1, InnerMatcher);
  36. return LHS.matches(*BE->getLHS(), Finder, Builder) ||
  37. RHS.matches(*BE->getRHS(), Finder, Builder);
  38. }
  39. return false;
  40. }
  41. CharUnits getSizeOfType(const ASTContext &Ctx, const Type *Ty) {
  42. if (!Ty || Ty->isIncompleteType() || Ty->isDependentType() ||
  43. isa<DependentSizedArrayType>(Ty) || !Ty->isConstantSizeType())
  44. return CharUnits::Zero();
  45. return Ctx.getTypeSizeInChars(Ty);
  46. }
  47. } // namespace
  48. SizeofExpressionCheck::SizeofExpressionCheck(StringRef Name,
  49. ClangTidyContext *Context)
  50. : ClangTidyCheck(Name, Context),
  51. WarnOnSizeOfConstant(Options.get("WarnOnSizeOfConstant", true)),
  52. WarnOnSizeOfIntegerExpression(
  53. Options.get("WarnOnSizeOfIntegerExpression", false)),
  54. WarnOnSizeOfThis(Options.get("WarnOnSizeOfThis", true)),
  55. WarnOnSizeOfCompareToConstant(
  56. Options.get("WarnOnSizeOfCompareToConstant", true)),
  57. WarnOnSizeOfPointerToAggregate(
  58. Options.get("WarnOnSizeOfPointerToAggregate", true)) {}
  59. void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
  60. Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant);
  61. Options.store(Opts, "WarnOnSizeOfIntegerExpression",
  62. WarnOnSizeOfIntegerExpression);
  63. Options.store(Opts, "WarnOnSizeOfThis", WarnOnSizeOfThis);
  64. Options.store(Opts, "WarnOnSizeOfCompareToConstant",
  65. WarnOnSizeOfCompareToConstant);
  66. Options.store(Opts, "WarnOnSizeOfPointerToAggregate",
  67. WarnOnSizeOfPointerToAggregate);
  68. }
  69. void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
  70. // FIXME:
  71. // Some of the checks should not match in template code to avoid false
  72. // positives if sizeof is applied on template argument.
  73. const auto IntegerExpr = ignoringParenImpCasts(integerLiteral());
  74. const auto ConstantExpr = ignoringParenImpCasts(
  75. anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)),
  76. binaryOperator(hasLHS(IntegerExpr), hasRHS(IntegerExpr))));
  77. const auto IntegerCallExpr = ignoringParenImpCasts(callExpr(
  78. anyOf(hasType(isInteger()), hasType(hasCanonicalType(enumType()))),
  79. unless(isInTemplateInstantiation())));
  80. const auto SizeOfExpr = sizeOfExpr(hasArgumentOfType(
  81. hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type"))));
  82. const auto SizeOfZero =
  83. sizeOfExpr(has(ignoringParenImpCasts(integerLiteral(equals(0)))));
  84. // Detect expression like: sizeof(ARRAYLEN);
  85. // Note: The expression 'sizeof(sizeof(0))' is a portable trick used to know
  86. // the sizeof size_t.
  87. if (WarnOnSizeOfConstant) {
  88. Finder->addMatcher(
  89. expr(sizeOfExpr(has(ignoringParenImpCasts(ConstantExpr))),
  90. unless(SizeOfZero))
  91. .bind("sizeof-constant"),
  92. this);
  93. }
  94. // Detect sizeof(f())
  95. if (WarnOnSizeOfIntegerExpression) {
  96. Finder->addMatcher(sizeOfExpr(ignoringParenImpCasts(has(IntegerCallExpr)))
  97. .bind("sizeof-integer-call"),
  98. this);
  99. }
  100. // Detect expression like: sizeof(this);
  101. if (WarnOnSizeOfThis) {
  102. Finder->addMatcher(sizeOfExpr(has(ignoringParenImpCasts(cxxThisExpr())))
  103. .bind("sizeof-this"),
  104. this);
  105. }
  106. // Detect sizeof(kPtr) where kPtr is 'const char* kPtr = "abc"';
  107. const auto CharPtrType = pointerType(pointee(isAnyCharacter()));
  108. const auto ConstStrLiteralDecl =
  109. varDecl(isDefinition(), hasType(hasCanonicalType(CharPtrType)),
  110. hasInitializer(ignoringParenImpCasts(stringLiteral())));
  111. Finder->addMatcher(
  112. sizeOfExpr(has(ignoringParenImpCasts(
  113. expr(hasType(hasCanonicalType(CharPtrType)),
  114. ignoringParenImpCasts(declRefExpr(
  115. hasDeclaration(ConstStrLiteralDecl)))))))
  116. .bind("sizeof-charp"),
  117. this);
  118. // Detect sizeof(ptr) where ptr points to an aggregate (i.e. sizeof(&S)).
  119. // Do not find it if RHS of a 'sizeof(arr) / sizeof(arr[0])' expression.
  120. if (WarnOnSizeOfPointerToAggregate) {
  121. const auto ArrayExpr =
  122. ignoringParenImpCasts(hasType(hasCanonicalType(arrayType())));
  123. const auto ArrayCastExpr = expr(anyOf(
  124. unaryOperator(hasUnaryOperand(ArrayExpr), unless(hasOperatorName("*"))),
  125. binaryOperator(hasEitherOperand(ArrayExpr)),
  126. castExpr(hasSourceExpression(ArrayExpr))));
  127. const auto PointerToArrayExpr = ignoringParenImpCasts(
  128. hasType(hasCanonicalType(pointerType(pointee(arrayType())))));
  129. const auto StructAddrOfExpr = unaryOperator(
  130. hasOperatorName("&"), hasUnaryOperand(ignoringParenImpCasts(
  131. hasType(hasCanonicalType(recordType())))));
  132. const auto PointerToStructType =
  133. hasUnqualifiedDesugaredType(pointerType(pointee(recordType())));
  134. const auto PointerToStructExpr = ignoringParenImpCasts(expr(
  135. hasType(hasCanonicalType(PointerToStructType)), unless(cxxThisExpr())));
  136. const auto ArrayOfPointersExpr = ignoringParenImpCasts(
  137. hasType(hasCanonicalType(arrayType(hasElementType(pointerType()))
  138. .bind("type-of-array-of-pointers"))));
  139. const auto ArrayOfSamePointersExpr =
  140. ignoringParenImpCasts(hasType(hasCanonicalType(
  141. arrayType(equalsBoundNode("type-of-array-of-pointers")))));
  142. const auto ZeroLiteral = ignoringParenImpCasts(integerLiteral(equals(0)));
  143. const auto ArrayOfSamePointersZeroSubscriptExpr =
  144. ignoringParenImpCasts(arraySubscriptExpr(
  145. hasBase(ArrayOfSamePointersExpr), hasIndex(ZeroLiteral)));
  146. const auto ArrayLengthExprDenom =
  147. expr(hasParent(expr(ignoringParenImpCasts(binaryOperator(
  148. hasOperatorName("/"), hasLHS(ignoringParenImpCasts(sizeOfExpr(
  149. has(ArrayOfPointersExpr)))))))),
  150. sizeOfExpr(has(ArrayOfSamePointersZeroSubscriptExpr)));
  151. Finder->addMatcher(expr(anyOf(sizeOfExpr(has(ignoringParenImpCasts(anyOf(
  152. ArrayCastExpr, PointerToArrayExpr,
  153. StructAddrOfExpr, PointerToStructExpr)))),
  154. sizeOfExpr(has(PointerToStructType))),
  155. unless(ArrayLengthExprDenom))
  156. .bind("sizeof-pointer-to-aggregate"),
  157. this);
  158. }
  159. // Detect expression like: sizeof(expr) <= k for a suspicious constant 'k'.
  160. if (WarnOnSizeOfCompareToConstant) {
  161. Finder->addMatcher(
  162. binaryOperator(matchers::isRelationalOperator(),
  163. hasOperands(ignoringParenImpCasts(SizeOfExpr),
  164. ignoringParenImpCasts(integerLiteral(anyOf(
  165. equals(0), isBiggerThan(0x80000))))))
  166. .bind("sizeof-compare-constant"),
  167. this);
  168. }
  169. // Detect expression like: sizeof(expr, expr); most likely an error.
  170. Finder->addMatcher(sizeOfExpr(has(ignoringParenImpCasts(
  171. binaryOperator(hasOperatorName(",")))))
  172. .bind("sizeof-comma-expr"),
  173. this);
  174. // Detect sizeof(...) /sizeof(...));
  175. // FIXME:
  176. // Re-evaluate what cases to handle by the checker.
  177. // Probably any sizeof(A)/sizeof(B) should be error if
  178. // 'A' is not an array (type) and 'B' the (type of the) first element of it.
  179. // Except if 'A' and 'B' are non-pointers, then use the existing size division
  180. // rule.
  181. const auto ElemType =
  182. arrayType(hasElementType(recordType().bind("elem-type")));
  183. const auto ElemPtrType = pointerType(pointee(type().bind("elem-ptr-type")));
  184. Finder->addMatcher(
  185. binaryOperator(
  186. hasOperatorName("/"),
  187. hasLHS(ignoringParenImpCasts(sizeOfExpr(hasArgumentOfType(
  188. hasCanonicalType(type(anyOf(ElemType, ElemPtrType, type()))
  189. .bind("num-type")))))),
  190. hasRHS(ignoringParenImpCasts(sizeOfExpr(
  191. hasArgumentOfType(hasCanonicalType(type().bind("denom-type")))))))
  192. .bind("sizeof-divide-expr"),
  193. this);
  194. // Detect expression like: sizeof(...) * sizeof(...)); most likely an error.
  195. Finder->addMatcher(binaryOperator(hasOperatorName("*"),
  196. hasLHS(ignoringParenImpCasts(SizeOfExpr)),
  197. hasRHS(ignoringParenImpCasts(SizeOfExpr)))
  198. .bind("sizeof-multiply-sizeof"),
  199. this);
  200. Finder->addMatcher(
  201. binaryOperator(hasOperatorName("*"),
  202. hasOperands(ignoringParenImpCasts(SizeOfExpr),
  203. ignoringParenImpCasts(binaryOperator(
  204. hasOperatorName("*"),
  205. hasEitherOperand(
  206. ignoringParenImpCasts(SizeOfExpr))))))
  207. .bind("sizeof-multiply-sizeof"),
  208. this);
  209. // Detect strange double-sizeof expression like: sizeof(sizeof(...));
  210. // Note: The expression 'sizeof(sizeof(0))' is accepted.
  211. Finder->addMatcher(sizeOfExpr(has(ignoringParenImpCasts(hasSizeOfDescendant(
  212. 8, allOf(SizeOfExpr, unless(SizeOfZero))))))
  213. .bind("sizeof-sizeof-expr"),
  214. this);
  215. // Detect sizeof in pointer arithmetic like: N * sizeof(S) == P1 - P2 or
  216. // (P1 - P2) / sizeof(S) where P1 and P2 are pointers to type S.
  217. const auto PtrDiffExpr = binaryOperator(
  218. hasOperatorName("-"),
  219. hasLHS(hasType(hasUnqualifiedDesugaredType(pointerType(pointee(
  220. hasUnqualifiedDesugaredType(type().bind("left-ptr-type"))))))),
  221. hasRHS(hasType(hasUnqualifiedDesugaredType(pointerType(pointee(
  222. hasUnqualifiedDesugaredType(type().bind("right-ptr-type"))))))));
  223. Finder->addMatcher(
  224. binaryOperator(
  225. hasAnyOperatorName("==", "!=", "<", "<=", ">", ">=", "+", "-"),
  226. hasOperands(
  227. anyOf(ignoringParenImpCasts(SizeOfExpr),
  228. ignoringParenImpCasts(binaryOperator(
  229. hasOperatorName("*"),
  230. hasEitherOperand(ignoringParenImpCasts(SizeOfExpr))))),
  231. ignoringParenImpCasts(PtrDiffExpr)))
  232. .bind("sizeof-in-ptr-arithmetic-mul"),
  233. this);
  234. Finder->addMatcher(binaryOperator(hasOperatorName("/"),
  235. hasLHS(ignoringParenImpCasts(PtrDiffExpr)),
  236. hasRHS(ignoringParenImpCasts(SizeOfExpr)))
  237. .bind("sizeof-in-ptr-arithmetic-div"),
  238. this);
  239. }
  240. void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
  241. const ASTContext &Ctx = *Result.Context;
  242. if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-constant")) {
  243. diag(E->getBeginLoc(),
  244. "suspicious usage of 'sizeof(K)'; did you mean 'K'?");
  245. } else if (const auto *E =
  246. Result.Nodes.getNodeAs<Expr>("sizeof-integer-call")) {
  247. diag(E->getBeginLoc(), "suspicious usage of 'sizeof()' on an expression "
  248. "that results in an integer");
  249. } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-this")) {
  250. diag(E->getBeginLoc(),
  251. "suspicious usage of 'sizeof(this)'; did you mean 'sizeof(*this)'");
  252. } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-charp")) {
  253. diag(E->getBeginLoc(),
  254. "suspicious usage of 'sizeof(char*)'; do you mean 'strlen'?");
  255. } else if (const auto *E =
  256. Result.Nodes.getNodeAs<Expr>("sizeof-pointer-to-aggregate")) {
  257. diag(E->getBeginLoc(),
  258. "suspicious usage of 'sizeof(A*)'; pointer to aggregate");
  259. } else if (const auto *E =
  260. Result.Nodes.getNodeAs<Expr>("sizeof-compare-constant")) {
  261. diag(E->getBeginLoc(),
  262. "suspicious comparison of 'sizeof(expr)' to a constant");
  263. } else if (const auto *E =
  264. Result.Nodes.getNodeAs<Expr>("sizeof-comma-expr")) {
  265. diag(E->getBeginLoc(), "suspicious usage of 'sizeof(..., ...)'");
  266. } else if (const auto *E =
  267. Result.Nodes.getNodeAs<Expr>("sizeof-divide-expr")) {
  268. const auto *NumTy = Result.Nodes.getNodeAs<Type>("num-type");
  269. const auto *DenomTy = Result.Nodes.getNodeAs<Type>("denom-type");
  270. const auto *ElementTy = Result.Nodes.getNodeAs<Type>("elem-type");
  271. const auto *PointedTy = Result.Nodes.getNodeAs<Type>("elem-ptr-type");
  272. CharUnits NumeratorSize = getSizeOfType(Ctx, NumTy);
  273. CharUnits DenominatorSize = getSizeOfType(Ctx, DenomTy);
  274. CharUnits ElementSize = getSizeOfType(Ctx, ElementTy);
  275. if (DenominatorSize > CharUnits::Zero() &&
  276. !NumeratorSize.isMultipleOf(DenominatorSize)) {
  277. diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
  278. " numerator is not a multiple of denominator");
  279. } else if (ElementSize > CharUnits::Zero() &&
  280. DenominatorSize > CharUnits::Zero() &&
  281. ElementSize != DenominatorSize) {
  282. diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
  283. " numerator is not a multiple of denominator");
  284. } else if (NumTy && DenomTy && NumTy == DenomTy) {
  285. diag(E->getBeginLoc(),
  286. "suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'");
  287. } else if (PointedTy && DenomTy && PointedTy == DenomTy) {
  288. diag(E->getBeginLoc(),
  289. "suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'");
  290. } else if (NumTy && DenomTy && NumTy->isPointerType() &&
  291. DenomTy->isPointerType()) {
  292. diag(E->getBeginLoc(),
  293. "suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'");
  294. }
  295. } else if (const auto *E =
  296. Result.Nodes.getNodeAs<Expr>("sizeof-sizeof-expr")) {
  297. diag(E->getBeginLoc(), "suspicious usage of 'sizeof(sizeof(...))'");
  298. } else if (const auto *E =
  299. Result.Nodes.getNodeAs<Expr>("sizeof-multiply-sizeof")) {
  300. diag(E->getBeginLoc(), "suspicious 'sizeof' by 'sizeof' multiplication");
  301. } else if (const auto *E =
  302. Result.Nodes.getNodeAs<Expr>("sizeof-in-ptr-arithmetic-mul")) {
  303. const auto *LPtrTy = Result.Nodes.getNodeAs<Type>("left-ptr-type");
  304. const auto *RPtrTy = Result.Nodes.getNodeAs<Type>("right-ptr-type");
  305. const auto *SizeofArgTy = Result.Nodes.getNodeAs<Type>("sizeof-arg-type");
  306. if ((LPtrTy == RPtrTy) && (LPtrTy == SizeofArgTy)) {
  307. diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
  308. "pointer arithmetic");
  309. }
  310. } else if (const auto *E =
  311. Result.Nodes.getNodeAs<Expr>("sizeof-in-ptr-arithmetic-div")) {
  312. const auto *LPtrTy = Result.Nodes.getNodeAs<Type>("left-ptr-type");
  313. const auto *RPtrTy = Result.Nodes.getNodeAs<Type>("right-ptr-type");
  314. const auto *SizeofArgTy = Result.Nodes.getNodeAs<Type>("sizeof-arg-type");
  315. if ((LPtrTy == RPtrTy) && (LPtrTy == SizeofArgTy)) {
  316. diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
  317. "pointer arithmetic");
  318. }
  319. }
  320. }
  321. } // namespace clang::tidy::bugprone