clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp

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