[clang-tools-extra] 267ad43 - [clang-tidy] Extend `bugprone-sizeof-expression` with matching `P +- sizeof(T)` and `P +- N */ sizeof(T)` cases, add `cert-arr39-c` alias (#106061)

via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 17 07:43:03 PDT 2024


Author: Zoltán Porkoláb
Date: 2024-09-17T16:42:58+02:00
New Revision: 267ad430fc54d6d548cd7d25c7e59c3b6b650097

URL: https://github.com/llvm/llvm-project/commit/267ad430fc54d6d548cd7d25c7e59c3b6b650097
DIFF: https://github.com/llvm/llvm-project/commit/267ad430fc54d6d548cd7d25c7e59c3b6b650097.diff

LOG: [clang-tidy] Extend `bugprone-sizeof-expression` with matching `P +- sizeof(T)` and `P +- N */ sizeof(T)` cases, add `cert-arr39-c` alias (#106061)

Improved `bugprone-sizeof-expression` check to find suspicious pointer
arithmetic calculations where the pointer is offset by an `alignof()`,
`offsetof()`, or `sizeof()` expression.

Pointer arithmetic expressions implicitly scale the offset added to or
subtracted from the address by the size of the pointee type. Using an
offset expression that is already scaled by the size of the underlying
type effectively results in a squared offset, which is likely an invalid
pointer that points beyond the end of the intended array.

```c
void printEveryEvenIndexElement(int *Array, size_t N) {
  int *P = Array;
  while (P <= Array + N * sizeof(int)) { // Suspicious pointer arithmetics using sizeof()!
    printf("%d ", *P);

    P += 2 * sizeof(int); // Suspicious pointer arithmetics using sizeof()!
  }
}
```

---------

Co-authored-by: Whisperity <whisperity at gmail.com>

Added: 
    clang-tools-extra/docs/clang-tidy/checks/cert/arr39-c.rst
    clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-pointer-arithmetics-c11.c
    clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-pointer-arithmetics.c
    clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.c

Modified: 
    clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
    clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
    clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst
    clang-tools-extra/docs/clang-tidy/checks/list.rst
    clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
    clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp

Removed: 
    clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-2.c


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
index d517e8473d94ae..a30e63f9b0fd6a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -48,6 +48,8 @@ AST_MATCHER_P2(Expr, hasSizeOfDescendant, int, Depth,
   return false;
 }
 
+AST_MATCHER(Expr, offsetOfExpr) { return isa<OffsetOfExpr>(Node); }
+
 CharUnits getSizeOfType(const ASTContext &Ctx, const Type *Ty) {
   if (!Ty || Ty->isIncompleteType() || Ty->isDependentType() ||
       isa<DependentSizedArrayType>(Ty) || !Ty->isConstantSizeType())
@@ -221,17 +223,15 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
   const auto ElemType =
       arrayType(hasElementType(recordType().bind("elem-type")));
   const auto ElemPtrType = pointerType(pointee(type().bind("elem-ptr-type")));
+  const auto SizeofDivideExpr = binaryOperator(
+      hasOperatorName("/"),
+      hasLHS(
+          ignoringParenImpCasts(sizeOfExpr(hasArgumentOfType(hasCanonicalType(
+              type(anyOf(ElemType, ElemPtrType, type())).bind("num-type")))))),
+      hasRHS(ignoringParenImpCasts(sizeOfExpr(
+          hasArgumentOfType(hasCanonicalType(type().bind("denom-type")))))));
 
-  Finder->addMatcher(
-      binaryOperator(
-          hasOperatorName("/"),
-          hasLHS(ignoringParenImpCasts(sizeOfExpr(hasArgumentOfType(
-              hasCanonicalType(type(anyOf(ElemType, ElemPtrType, type()))
-                                   .bind("num-type")))))),
-          hasRHS(ignoringParenImpCasts(sizeOfExpr(
-              hasArgumentOfType(hasCanonicalType(type().bind("denom-type")))))))
-          .bind("sizeof-divide-expr"),
-      this);
+  Finder->addMatcher(SizeofDivideExpr.bind("sizeof-divide-expr"), this);
 
   // Detect expression like: sizeof(...) * sizeof(...)); most likely an error.
   Finder->addMatcher(binaryOperator(hasOperatorName("*"),
@@ -257,8 +257,9 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
                          .bind("sizeof-sizeof-expr"),
                      this);
 
-  // Detect sizeof in pointer arithmetic like: N * sizeof(S) == P1 - P2 or
-  // (P1 - P2) / sizeof(S) where P1 and P2 are pointers to type S.
+  // Detect sizeof usage in comparisons involving pointer arithmetics, such as
+  // N * sizeof(T) == P1 - P2 or (P1 - P2) / sizeof(T), where P1 and P2 are
+  // pointers to a type T.
   const auto PtrDiffExpr = binaryOperator(
       hasOperatorName("-"),
       hasLHS(hasType(hasUnqualifiedDesugaredType(pointerType(pointee(
@@ -285,6 +286,47 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
           hasRHS(ignoringParenImpCasts(SizeOfExpr.bind("sizeof-ptr-div-expr"))))
           .bind("sizeof-in-ptr-arithmetic-div"),
       this);
+
+  // SEI CERT ARR39-C. Do not add or subtract a scaled integer to a pointer.
+  // Detect sizeof, alignof and offsetof usage in pointer arithmetics where
+  // they are used to scale the numeric distance, which is scaled again by
+  // the pointer arithmetic operator. This can result in forming invalid
+  // offsets.
+  //
+  // Examples, where P is a pointer, N is some integer (both compile-time and
+  // run-time): P + sizeof(T), P + sizeof(*P), P + N * sizeof(*P).
+  //
+  // This check does not warn on cases where the pointee type is "1 byte",
+  // as those cases can often come from generics and also do not constitute a
+  // problem because the size does not affect the scale used.
+  const auto InterestingPtrTyForPtrArithmetic =
+      pointerType(pointee(qualType().bind("pointee-type")));
+  const auto SizeofLikeScaleExpr =
+      expr(anyOf(unaryExprOrTypeTraitExpr(ofKind(UETT_SizeOf)),
+                 unaryExprOrTypeTraitExpr(ofKind(UETT_AlignOf)),
+                 offsetOfExpr()))
+          .bind("sizeof-in-ptr-arithmetic-scale-expr");
+  const auto PtrArithmeticIntegerScaleExpr = binaryOperator(
+      hasAnyOperatorName("*", "/"),
+      // sizeof(...) * sizeof(...) and sizeof(...) / sizeof(...) is handled
+      // by this check on another path.
+      hasOperands(expr(hasType(isInteger()), unless(SizeofLikeScaleExpr)),
+                  SizeofLikeScaleExpr));
+  const auto PtrArithmeticScaledIntegerExpr =
+      expr(anyOf(SizeofLikeScaleExpr, PtrArithmeticIntegerScaleExpr),
+           unless(SizeofDivideExpr));
+
+  Finder->addMatcher(
+      expr(anyOf(
+          binaryOperator(hasAnyOperatorName("+", "-"),
+                         hasOperands(hasType(InterestingPtrTyForPtrArithmetic),
+                                     PtrArithmeticScaledIntegerExpr))
+              .bind("sizeof-in-ptr-arithmetic-plusminus"),
+          binaryOperator(hasAnyOperatorName("+=", "-="),
+                         hasLHS(hasType(InterestingPtrTyForPtrArithmetic)),
+                         hasRHS(PtrArithmeticScaledIntegerExpr))
+              .bind("sizeof-in-ptr-arithmetic-plusminus"))),
+      this);
 }
 
 void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
@@ -409,6 +451,43 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
           << SizeOfExpr->getSourceRange() << E->getOperatorLoc()
           << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
     }
+  } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
+                 "sizeof-in-ptr-arithmetic-plusminus")) {
+    const auto *PointeeTy = Result.Nodes.getNodeAs<QualType>("pointee-type");
+    const auto *ScaleExpr =
+        Result.Nodes.getNodeAs<Expr>("sizeof-in-ptr-arithmetic-scale-expr");
+    const CharUnits PointeeSize = getSizeOfType(Ctx, PointeeTy->getTypePtr());
+    const int ScaleKind = [ScaleExpr]() {
+      if (const auto *UTTE = dyn_cast<UnaryExprOrTypeTraitExpr>(ScaleExpr))
+        switch (UTTE->getKind()) {
+        case UETT_SizeOf:
+          return 0;
+        case UETT_AlignOf:
+          return 1;
+        default:
+          return -1;
+        }
+
+      if (isa<OffsetOfExpr>(ScaleExpr))
+        return 2;
+
+      return -1;
+    }();
+
+    if (ScaleKind != -1 && PointeeSize > CharUnits::One()) {
+      diag(E->getExprLoc(),
+           "suspicious usage of '%select{sizeof|alignof|offsetof}0(...)' in "
+           "pointer arithmetic; this scaled value will be scaled again by the "
+           "'%1' operator")
+          << ScaleKind << E->getOpcodeStr() << ScaleExpr->getSourceRange();
+      diag(E->getExprLoc(),
+           "'%0' in pointer arithmetic internally scales with 'sizeof(%1)' == "
+           "%2",
+           DiagnosticIDs::Note)
+          << E->getOpcodeStr()
+          << PointeeTy->getAsString(Ctx.getPrintingPolicy())
+          << PointeeSize.getQuantity();
+    }
   }
 }
 

diff  --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
index 9ca17bc9e6f124..66d7c34cc9e940 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h
@@ -13,7 +13,7 @@
 
 namespace clang::tidy::bugprone {
 
-/// Find suspicious usages of sizeof expression.
+/// Find suspicious usages of sizeof expressions.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/sizeof-expression.html

diff  --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index 8b5be9cd95f767..26befe0de59ae4 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -14,6 +14,7 @@
 #include "../bugprone/ReservedIdentifierCheck.h"
 #include "../bugprone/SignalHandlerCheck.h"
 #include "../bugprone/SignedCharMisuseCheck.h"
+#include "../bugprone/SizeofExpressionCheck.h"
 #include "../bugprone/SpuriouslyWakeUpFunctionsCheck.h"
 #include "../bugprone/SuspiciousMemoryComparisonCheck.h"
 #include "../bugprone/UnhandledSelfAssignmentCheck.h"
@@ -281,6 +282,9 @@ class CERTModule : public ClangTidyModule {
         "cert-oop58-cpp");
 
     // C checkers
+    // ARR
+    CheckFactories.registerCheck<bugprone::SizeofExpressionCheck>(
+        "cert-arr39-c");
     // CON
     CheckFactories.registerCheck<bugprone::SpuriouslyWakeUpFunctionsCheck>(
         "cert-con36-c");
@@ -332,6 +336,12 @@ class CERTModule : public ClangTidyModule {
   ClangTidyOptions getModuleOptions() override {
     ClangTidyOptions Options;
     ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
+    Opts["cert-arr39-c.WarnOnSizeOfConstant"] = "false";
+    Opts["cert-arr39-c.WarnOnSizeOfIntegerExpression"] = "false";
+    Opts["cert-arr39-c.WarnOnSizeOfThis"] = "false";
+    Opts["cert-arr39-c.WarnOnSizeOfCompareToConstant"] = "false";
+    Opts["cert-arr39-c.WarnOnSizeOfPointer"] = "false";
+    Opts["cert-arr39-c.WarnOnSizeOfPointerToAggregate"] = "false";
     Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU";
     Opts["cert-err33-c.CheckedFunctions"] = CertErr33CCheckedFunctions;
     Opts["cert-err33-c.AllowCastToVoid"] = "true";

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index b5b164833e1575..79501b563b4e22 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -104,6 +104,10 @@ New checks
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
+- New alias :doc:`cert-arr39-c <clang-tidy/checks/cert/arr39-c>` to
+  :doc:`bugprone-sizeof-expression
+  <clang-tidy/checks/bugprone/sizeof-expression>` was added.
+
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
@@ -115,14 +119,19 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/forwarding-reference-overload>` check by fixing
   a crash when determining if an ``enable_if[_t]`` was found.
 
-- Improved :doc:`cert-flp30-c<clang-tidy/checks/cert/flp30-c>` check to 
+- Improved :doc:`bugprone-sizeof-expression
+  <clang-tidy/checks/bugprone/sizeof-expression>` check to find suspicious
+  usages of ``sizeof()``, ``alignof()``, and ``offsetof()`` when adding or
+  subtracting from a pointer.
+
+- Improved :doc:`cert-flp30-c <clang-tidy/checks/cert/flp30-c>` check to
   fix false positive that floating point variable is only used in increment
   expression.
 
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
-  <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>` check to avoid
-  false positive when member initialization depends on a structured binging
-  variable.
+  <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>` check to
+  avoid false positive when member initialization depends on a structured
+  binding variable.
 
 - Improved :doc:`misc-definitions-in-headers
   <clang-tidy/checks/misc/definitions-in-headers>` check by rewording the

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst
index ed5bb4fbb89baf..89c88d2740dba2 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst
@@ -164,6 +164,103 @@ hidden through macros.
     memcpy(dst, buf, sizeof(INT_SZ));  // sizeof(sizeof(int)) is suspicious.
   }
 
+Suspicious usages of 'sizeof(...)' in pointer arithmetic
+--------------------------------------------------------
+
+Arithmetic operators on pointers automatically scale the result with the size
+of the pointed typed.
+Further use of ``sizeof`` around pointer arithmetic will typically result in an
+unintended result.
+
+Scaling the result of pointer 
diff erence
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Subtracting two pointers results in an integer expression (of type
+``ptr
diff _t``) which expresses the distance between the two pointed objects in
+"number of objects between".
+A common mistake is to think that the result is "number of bytes between", and
+scale the 
diff erence with ``sizeof``, such as ``P1 - P2 == N * sizeof(T)``
+(instead of ``P1 - P2 == N``) or ``(P1 - P2) / sizeof(T)`` instead of
+``P1 - P2``.
+
+.. code-block:: c++
+
+  void splitFour(const Obj* Objs, size_t N, Obj Delimiter) {
+    const Obj *P = Objs;
+    while (P < Objs + N) {
+      if (*P == Delimiter) {
+        break;
+      }
+    }
+
+    if (P - Objs != 4 * sizeof(Obj)) { // Expecting a distance multiplied by sizeof is suspicious.
+      error();
+    }
+  }
+
+.. code-block:: c++
+
+  void iterateIfEvenLength(int *Begin, int *End) {
+    auto N = (Begin - End) / sizeof(int); // Dividing by sizeof() is suspicious.
+    if (N % 2)
+      return;
+
+    // ...
+  }
+
+Stepping a pointer with a scaled integer
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Conversely, when performing pointer arithmetics to add or subtract from a
+pointer, the arithmetic operator implicitly scales the value actually added to
+the pointer with the size of the pointee, as ``Ptr + N`` expects ``N`` to be
+"number of objects to step", and not "number of bytes to step".
+
+Seeing the calculation of a pointer where ``sizeof`` appears is suspicious,
+and the result is typically unintended, often out of bounds.
+``Ptr + sizeof(T)`` will offset the pointer by ``sizeof(T)`` elements,
+effectively exponentiating the scaling factor to the power of 2.
+
+This case also checks suspicious ``alignof`` and ``offsetof`` usages in
+pointer arithmetic, as both return the "size" in bytes and not elements,
+potentially resulting in doubly-scaled offsets.
+
+.. code-block:: c++
+
+  void printEveryEvenIndexElement(int *Array, size_t N) {
+    int *P = Array;
+    while (P <= Array + N * sizeof(int)) { // Suspicious pointer arithmetic using sizeof()!
+      printf("%d ", *P);
+
+      P += 2 * sizeof(int); // Suspicious pointer arithmetic using sizeof()!
+    }
+  }
+
+.. code-block:: c++
+
+  struct Message { /* ... */; char Flags[8]; };
+  void clearFlags(Message *Array, size_t N) {
+    const Message *End = Array + N;
+    while (Array < End) {
+      memset(Array + offsetof(Message, Flags), // Suspicious pointer arithmetic using offsetof()!
+             0, sizeof(Message::Flags));
+      ++Array;
+    }
+  }
+
+For this checked bogus pattern, `cert-arr39-c` redirects here as an alias of
+this check.
+
+This check corresponds to the CERT C Coding Standard rule
+`ARR39-C. Do not add or subtract a scaled integer to a pointer
+<http://wiki.sei.cmu.edu/confluence/display/c/ARR39-C.+Do+not+add+or+subtract+a+scaled+integer+to+a+pointer>`_.
+
+Limitations
+"""""""""""
+
+Cases where the pointee type has a size of `1` byte (such as, and most
+importantly, ``char``) are excluded.
+
 Options
 -------
 

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/cert/arr39-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert/arr39-c.rst
new file mode 100644
index 00000000000000..21353c61eede86
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/cert/arr39-c.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cert-arr39-c
+.. meta::
+   :http-equiv=refresh: 5;URL=../bugprone/sizeof-expression.html
+
+cert-arr39-c
+============
+
+The `cert-arr39-c` check is an alias, please see
+:doc:`bugprone-sizeof-expression <../bugprone/sizeof-expression>`
+for more information.

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index a931ebf025a10e..1909d7b8d8e246 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -407,6 +407,7 @@ Check aliases
    :header: "Name", "Redirect", "Offers fixes"
 
    :doc:`bugprone-narrowing-conversions <bugprone/narrowing-conversions>`, :doc:`cppcoreguidelines-narrowing-conversions <cppcoreguidelines/narrowing-conversions>`,
+   :doc:`cert-arr39-c <cert/arr39-c>`, :doc:`bugprone-sizeof-expression <bugprone/sizeof-expression>`,
    :doc:`cert-con36-c <cert/con36-c>`, :doc:`bugprone-spuriously-wake-up-functions <bugprone/spuriously-wake-up-functions>`,
    :doc:`cert-con54-cpp <cert/con54-cpp>`, :doc:`bugprone-spuriously-wake-up-functions <bugprone/spuriously-wake-up-functions>`,
    :doc:`cert-ctr56-cpp <cert/ctr56-cpp>`, :doc:`bugprone-pointer-arithmetic-on-polymorphic-object <bugprone/pointer-arithmetic-on-polymorphic-object>`,

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-pointer-arithmetics-c11.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-pointer-arithmetics-c11.c
new file mode 100644
index 00000000000000..944328e61c3404
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-pointer-arithmetics-c11.c
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy -std=c11-or-later %s bugprone-sizeof-expression %t
+
+#define alignof(type_name) _Alignof(type_name)
+extern void sink(const void *P);
+
+enum { BufferSize = 1024 };
+
+struct S {
+  long A, B, C;
+};
+
+void bad4d(void) {
+  struct S Buffer[BufferSize];
+
+  struct S *P = &Buffer[0];
+  struct S *Q = P;
+  while (Q < P + alignof(Buffer)) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: suspicious usage of 'alignof(...)' in pointer arithmetic; this scaled value will be scaled again by the '+' operator [bugprone-sizeof-expression]
+    // CHECK-MESSAGES: :[[@LINE-2]]:16: note: '+' in pointer arithmetic internally scales with 'sizeof(struct S)' == {{[0-9]+}}
+    sink(Q++);
+  }
+}

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-pointer-arithmetics.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-pointer-arithmetics.c
new file mode 100644
index 00000000000000..360ce00a6889d7
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-pointer-arithmetics.c
@@ -0,0 +1,372 @@
+// RUN: %check_clang_tidy %s bugprone-sizeof-expression %t
+
+#define offsetof(type, member) __builtin_offsetof(type, member)
+
+typedef __SIZE_TYPE__ size_t;
+typedef __WCHAR_TYPE__ wchar_t;
+
+extern void *memset(void *Dest, int Ch, size_t Count);
+extern size_t strlen(const char *Str);
+extern size_t wcslen(const wchar_t *Str);
+extern char *strcpy(char *Dest, const char *Src);
+extern wchar_t *wcscpy(wchar_t *Dest, const wchar_t *Src);
+extern int scanf(const char *Format, ...);
+extern int wscanf(const wchar_t *Format, ...);
+
+extern void sink(const void *P);
+
+enum { BufferSize = 1024 };
+
+void bad1a(void) {
+  int Buffer[BufferSize];
+
+  int *P = &Buffer[0];
+  int *Q = P;
+  while (Q < P + sizeof(Buffer)) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic; this scaled value will be scaled again by the '+' operator [bugprone-sizeof-expression]
+    // CHECK-MESSAGES: :[[@LINE-2]]:16: note: '+' in pointer arithmetic internally scales with 'sizeof(int)' == {{[0-9]+}}
+    *Q++ = 0;
+  }
+}
+
+void bad1b(void) {
+  typedef int Integer;
+  Integer Buffer[BufferSize];
+
+  Integer *P = &Buffer[0];
+  Integer *Q = P;
+  while (Q < P + sizeof(Buffer)) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic; this scaled value will be scaled again by the '+' operator
+    // CHECK-MESSAGES: :[[@LINE-2]]:16: note: '+' in pointer arithmetic internally scales with 'sizeof(Integer)' == {{[0-9]+}}
+    *Q++ = 0;
+  }
+}
+
+void good1(void) {
+  int Buffer[BufferSize];
+
+  int *P = &Buffer[0];
+  int *Q = P;
+  while (Q < P + BufferSize) {
+    *Q++ = 0;
+  }
+}
+
+void bad2(void) {
+  int Buffer[BufferSize];
+  int *P = Buffer;
+
+  while (P < Buffer + sizeof(Buffer)) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic; this scaled value will be scaled again by the '+' operator
+    // CHECK-MESSAGES: :[[@LINE-2]]:21: note: '+' in pointer arithmetic internally scales with 'sizeof(int)' == {{[0-9]+}}
+    *P++ = 0;
+  }
+}
+
+void good2(void) {
+  int Buffer[BufferSize];
+  int *P = Buffer;
+
+  while (P < Buffer + BufferSize) {
+    *P++ = 0;
+  }
+}
+
+struct S {
+  long A, B, C;
+};
+
+void bad3a(struct S *S) {
+  const size_t Offset = offsetof(struct S, B);
+  struct S *P = S;
+
+  // This is not captureable by Tidy because the size/offset expression is
+  // not a direct child of the pointer arithmetics.
+  memset(P + Offset, 0, sizeof(struct S) - Offset);
+}
+
+void good3a(struct S *S) {
+  const size_t Offset = offsetof(struct S, B);
+  char *P = (char*)S;
+
+  // This is not captureable by Tidy because the size/offset expression is
+  // not a direct child of the pointer arithmetics.
+  memset(P + Offset, 0, sizeof(struct S) - Offset);
+}
+
+void bad3b(struct S *S) {
+  memset(S + offsetof(struct S, B), 0,
+         sizeof(struct S) - offsetof(struct S, B));
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: suspicious usage of 'offsetof(...)' in pointer arithmetic; this scaled value will be scaled again by the '+' operator
+  // CHECK-MESSAGES: :[[@LINE-3]]:12: note: '+' in pointer arithmetic internally scales with 'sizeof(struct S)' == {{[0-9]+}}
+}
+
+void good3b(struct S *S) {
+  char *P = (char*)S;
+  memset(P + offsetof(struct S, B), 0,
+         sizeof(struct S) - offsetof(struct S, B));
+}
+
+void bad3c(void) {
+  struct S Buffer[BufferSize];
+
+  struct S *P = &Buffer[0];
+  struct S *Q = P;
+  while (Q < P + sizeof(Buffer)) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic; this scaled value will be scaled again by the '+' operator
+    // CHECK-MESSAGES: :[[@LINE-2]]:16: note: '+' in pointer arithmetic internally scales with 'sizeof(struct S)' == {{[0-9]+}}
+    sink(Q++);
+  }
+}
+
+void bad4(void) {
+  int Buffer[BufferSize];
+
+  int *P = &Buffer[0];
+  int *Q = P;
+  while (Q < P + BufferSize) {
+    *Q = 0;
+    Q += sizeof(*Q);
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic; this scaled value will be scaled again by the '+=' operator
+    // CHECK-MESSAGES: :[[@LINE-2]]:7: note: '+=' in pointer arithmetic internally scales with 'sizeof(int)' == {{[0-9]+}}
+  }
+}
+
+void silenced4(void) {
+  char Buffer[BufferSize];
+
+  char *P = &Buffer[0];
+  char *Q = P;
+  while (Q < P + BufferSize) {
+    *Q = 0;
+    Q += sizeof(*Q);
+  }
+}
+
+void good4(void) {
+  char Buffer[BufferSize];
+
+  char *P = &Buffer[0];
+  char *Q = P;
+  while (Q < P + BufferSize) {
+    *Q = 0;
+    Q += 1;
+  }
+}
+
+void good5aa(void) {
+  int Buffer[BufferSize];
+
+  int *P = &Buffer[0];
+  int *Q = P;
+  while (Q < P + BufferSize) {
+    *Q = 0;
+    Q += ( sizeof(Buffer) / sizeof(Buffer[0]) );
+  }
+}
+
+void good5ab(void) {
+  int Buffer[BufferSize];
+
+  int *P = &Buffer[0];
+  int *Q = P;
+  while (Q < P + BufferSize) {
+    *Q = 0;
+    Q = Q + ( sizeof(Buffer) / sizeof(Buffer[0]) );
+  }
+}
+
+void good5ba(void) {
+  int Buffer[BufferSize];
+
+  int *P = &Buffer[0];
+  int *Q = P;
+  while (Q < P + BufferSize) {
+    *Q = 0;
+    Q -= ( sizeof(Buffer) / sizeof(Buffer[0]) );
+  }
+}
+
+void good5bb(void) {
+  int Buffer[BufferSize];
+
+  int *P = &Buffer[0];
+  int *Q = P;
+  while (Q < P + BufferSize) {
+    *Q = 0;
+    Q = Q - ( sizeof(Buffer) / sizeof(Buffer[0]) );
+  }
+}
+
+void bad6(void) {
+  int Buffer[BufferSize];
+
+  int *P = &Buffer[0];
+  int *Q = P;
+  while (Q < P + BufferSize) {
+    *Q = 0;
+    Q = Q + sizeof(*Q);
+    // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic; this scaled value will be scaled again by the '+' operator
+    // CHECK-MESSAGES: :[[@LINE-2]]:11: note: '+' in pointer arithmetic internally scales with 'sizeof(int)' == {{[0-9]+}}
+  }
+}
+
+void silenced6(void) {
+  char Buffer[BufferSize];
+
+  char *P = &Buffer[0];
+  char *Q = P;
+  while (Q < P + BufferSize) {
+    *Q = 0;
+    Q = Q + sizeof(*Q);
+  }
+}
+
+void good6(void) {
+  char Buffer[BufferSize];
+
+  char *P = &Buffer[0];
+  char *Q = P;
+  while (Q < P + BufferSize) {
+    *Q = 0;
+    Q = Q + 1;
+  }
+}
+
+void silenced7(void) {
+  char Buffer[BufferSize];
+
+  char *P = &Buffer[0];
+  const char *Q = P;
+  while (Q < P + BufferSize) {
+    sink(Q);
+    Q = Q + sizeof(*Q);
+  }
+}
+
+void good7(void) {
+  char Buffer[BufferSize];
+
+  char *P = &Buffer[0];
+  const char *Q = P;
+  while (Q < P + BufferSize) {
+    sink(Q);
+    Q = Q + 1;
+  }
+}
+
+void bad8(void) {
+  int Buffer[BufferSize];
+
+  int *P = &Buffer[0];
+  int *Q = P;
+  while (Q >= P) {
+    *Q = 0;
+    Q = Q - sizeof(*Q);
+    // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic; this scaled value will be scaled again by the '-' operator
+    // CHECK-MESSAGES: :[[@LINE-2]]:11: note: '-' in pointer arithmetic internally scales with 'sizeof(int)' == {{[0-9]+}}
+  }
+}
+
+void silenced8(void) {
+  char Buffer[BufferSize];
+
+  char *P = &Buffer[0];
+  char *Q = P;
+  while (Q >= P) {
+    *Q = 0;
+    Q = Q - sizeof(*Q);
+  }
+}
+
+void good8(void) {
+  char Buffer[BufferSize];
+
+  char *P = &Buffer[0];
+  char *Q = P;
+  while (Q >= P) {
+    *Q = 0;
+    Q = Q - 1;
+  }
+}
+
+void good9(void) {
+  int Buffer[BufferSize];
+
+  int *P = &Buffer[0];
+  int *Q = P + BufferSize;
+  int N = Q - P;
+  while (N >= 0) {
+    Q[N] = 0;
+    N = N - 1;
+  }
+}
+
+void good10(void) {
+  int Buffer[BufferSize];
+
+  int *P = &Buffer[0];
+  int *Q = Buffer + BufferSize;
+  int I = sizeof(*P) - sizeof(*Q);
+
+  sink(&I);
+}
+
+void good11(void) {
+  int Buffer[BufferSize];
+
+  int *P = &Buffer[0];
+  int *Q = Buffer + BufferSize;
+  int I = sizeof(Q) - sizeof(*P);
+
+  sink(&I);
+}
+
+void bad12(void) {
+  wchar_t Message[BufferSize];
+  wcscpy(Message, L"Message: ");
+  wscanf(L"%s", Message + wcslen(Message) * sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic; this scaled value will be scaled again by the '+' operator
+  // CHECK-MESSAGES: :[[@LINE-2]]:25: note: '+' in pointer arithmetic internally scales with 'sizeof(wchar_t)' == {{[0-9]+}}
+}
+
+void silenced12(void) {
+  char Message[BufferSize];
+  strcpy(Message, "Message: ");
+  scanf("%s", Message + strlen(Message) * sizeof(char));
+}
+
+void nomatch12(void) {
+  char Message[BufferSize];
+  strcpy(Message, "Message: ");
+  scanf("%s", Message + strlen(Message));
+}
+
+void good12(void) {
+  wchar_t Message[BufferSize];
+  wcscpy(Message, L"Message: ");
+  wscanf(L"%s", Message + wcslen(Message));
+}
+
+void good13(void) {
+  int Buffer[BufferSize];
+
+  int *P = &Buffer[0];
+  while (P < (Buffer + sizeof(Buffer) / sizeof(int))) {
+    // NO-WARNING: Calculating the element count of the buffer here, which is
+    // safe with this idiom (as long as the types don't change).
+    ++P;
+  }
+
+  while (P < (Buffer + sizeof(Buffer) / sizeof(Buffer[0]))) {
+    // NO-WARNING: Calculating the element count of the buffer here, which is
+    // safe with this idiom.
+    ++P;
+  }
+
+  while (P < (Buffer + sizeof(Buffer) / sizeof(*P))) {
+    // NO-WARNING: Calculating the element count of the buffer here, which is
+    // safe with this idiom.
+    ++P;
+  }
+}

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-2.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.c
similarity index 100%
rename from clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-2.c
rename to clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.c

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
index 671fd8370894ff..81efd87345c97e 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
@@ -289,6 +289,35 @@ int Test6() {
   return sum;
 }
 
+static constexpr inline int BufferSize = 1024;
+
+template <typename T>
+T next(const T *&Read) {
+  T value = *Read;
+  Read += sizeof(T);
+  return value;
+}
+
+void Test7() {
+  int Buffer[BufferSize];
+  int *P = &Buffer[0];
+
+  const int *P2 = P;
+  int V1 = next(P2);
+  // CHECK-MESSAGES: :[[@LINE-10]]:8: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic; this scaled value will be scaled again by the '+=' operator
+  // CHECK-MESSAGES: :[[@LINE-11]]:8: note: '+=' in pointer arithmetic internally scales with 'sizeof(const int)' == {{[0-9]+}}
+  int V2 = next(P2);
+  (void)V1;
+  (void)V2;
+
+  int *Q = P;
+  while (Q < P + sizeof(Buffer)) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: suspicious usage of 'sizeof(...)' in pointer arithmetic; this scaled value will be scaled again by the '+' operator
+    // CHECK-MESSAGES: :[[@LINE-2]]:16: note: '+' in pointer arithmetic internally scales with 'sizeof(int)' == {{[0-9]+}}
+    *Q++ = 0;
+  }
+}
+
 #ifdef __SIZEOF_INT128__
 template <__int128_t N>
 #else
@@ -296,7 +325,7 @@ template <long N> // Fallback for platforms which do not define `__int128_t`
 #endif
 bool Baz() { return sizeof(A) < N; }
 // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: suspicious comparison of 'sizeof(expr)' to a constant
-bool Test7() { return Baz<-1>(); }
+bool Test8() { return Baz<-1>(); }
 
 void some_generic_function(const void *arg, int argsize);
 int *IntP, **IntPP;

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp
index fa4307dec02308..3432b1c84a9a50 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/prefer-member-initializer.cpp
@@ -641,9 +641,9 @@ struct S3 {
 }
 
 namespace GH82970 {
-struct InitFromBingingDecl {
+struct InitFromBindingDecl {
   int m;
-  InitFromBingingDecl() {
+  InitFromBindingDecl() {
     struct { int i; } a;
     auto [n] = a;
     m = n;


        


More information about the cfe-commits mailing list