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(Node); } + CharUnits getSizeOfType(const ASTContext &Ctx, const Type *Ty) { if (!Ty || Ty->isIncompleteType() || Ty->isDependentType() || isa(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( + "sizeof-in-ptr-arithmetic-plusminus")) { + const auto *PointeeTy = Result.Nodes.getNodeAs("pointee-type"); + const auto *ScaleExpr = + Result.Nodes.getNodeAs("sizeof-in-ptr-arithmetic-scale-expr"); + const CharUnits PointeeSize = getSizeOfType(Ctx, PointeeTy->getTypePtr()); + const int ScaleKind = [ScaleExpr]() { + if (const auto *UTTE = dyn_cast(ScaleExpr)) + switch (UTTE->getKind()) { + case UETT_SizeOf: + return 0; + case UETT_AlignOf: + return 1; + default: + return -1; + } + + if (isa(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( + "cert-arr39-c"); // CON CheckFactories.registerCheck( "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 ` to + :doc:`bugprone-sizeof-expression + ` was added. + Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -115,14 +119,19 @@ Changes in existing checks ` check by fixing a crash when determining if an ``enable_if[_t]`` was found. -- Improved :doc:`cert-flp30-c` check to +- Improved :doc:`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 ` check to fix false positive that floating point variable is only used in increment expression. - Improved :doc:`cppcoreguidelines-prefer-member-initializer - ` check to avoid - false positive when member initialization depends on a structured binging - variable. + ` check to + avoid false positive when member initialization depends on a structured + binding variable. - Improved :doc:`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 difference +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Subtracting two pointers results in an integer expression (of type +``ptrdiff_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 difference 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 +`_. + +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 `, :doc:`cppcoreguidelines-narrowing-conversions `, + :doc:`cert-arr39-c `, :doc:`bugprone-sizeof-expression `, :doc:`cert-con36-c `, :doc:`bugprone-spuriously-wake-up-functions `, :doc:`cert-con54-cpp `, :doc:`bugprone-spuriously-wake-up-functions `, :doc:`cert-ctr56-cpp `, :doc:`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 +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 // 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;