summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--clang-tools-extra/clang-tidy/misc/CMakeLists.txt1
-rw-r--r--clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp3
-rw-r--r--clang-tools-extra/clang-tidy/misc/SizeofExpressionCheck.cpp265
-rw-r--r--clang-tools-extra/clang-tidy/misc/SizeofExpressionCheck.h40
-rw-r--r--clang-tools-extra/docs/ReleaseNotes.rst7
-rw-r--r--clang-tools-extra/docs/clang-tidy/checks/list.rst1
-rw-r--r--clang-tools-extra/docs/clang-tidy/checks/misc-sizeof-expression.rst137
-rw-r--r--clang-tools-extra/test/clang-tidy/misc-sizeof-expression.cpp186
8 files changed, 639 insertions, 1 deletions
diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index ae0b0dfdb78..e3ab929e677 100644
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -23,6 +23,7 @@ add_clang_library(clangTidyMiscModule
NonCopyableObjects.cpp
PointerAndIntegralOperationCheck.cpp
SizeofContainerCheck.cpp
+ SizeofExpressionCheck.cpp
StaticAssertCheck.cpp
StringIntegerAssignmentCheck.cpp
StringLiteralWithEmbeddedNulCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
index bbc0eb714e8..f4393c27351 100644
--- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -31,6 +31,7 @@
#include "NonCopyableObjects.h"
#include "PointerAndIntegralOperationCheck.h"
#include "SizeofContainerCheck.h"
+#include "SizeofExpressionCheck.h"
#include "StaticAssertCheck.h"
#include "StringIntegerAssignmentCheck.h"
#include "StringLiteralWithEmbeddedNulCheck.h"
@@ -92,6 +93,8 @@ public:
CheckFactories.registerCheck<PointerAndIntegralOperationCheck>(
"misc-pointer-and-integral-operation");
CheckFactories.registerCheck<SizeofContainerCheck>("misc-sizeof-container");
+ CheckFactories.registerCheck<SizeofExpressionCheck>(
+ "misc-sizeof-expression");
CheckFactories.registerCheck<StaticAssertCheck>(
"misc-static-assert");
CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
diff --git a/clang-tools-extra/clang-tidy/misc/SizeofExpressionCheck.cpp b/clang-tools-extra/clang-tidy/misc/SizeofExpressionCheck.cpp
new file mode 100644
index 00000000000..c0a223d17a7
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/SizeofExpressionCheck.cpp
@@ -0,0 +1,265 @@
+//===--- SizeofExpressionCheck.cpp - clang-tidy----------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "SizeofExpressionCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+namespace {
+
+AST_MATCHER(BinaryOperator, isRelationalOperator) {
+ return Node.isRelationalOp();
+}
+
+AST_MATCHER_P(IntegerLiteral, isBiggerThan, unsigned, N) {
+ return Node.getValue().getZExtValue() > N;
+}
+
+AST_MATCHER_P2(Expr, hasSizeOfDescendant, int, Depth,
+ ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
+ if (Depth < 0)
+ return false;
+
+ const Expr *E = Node.IgnoreParenImpCasts();
+ if (InnerMatcher.matches(*E, Finder, Builder))
+ return true;
+
+ if (const auto *CE = dyn_cast<CastExpr>(E)) {
+ const auto M = hasSizeOfDescendant(Depth - 1, InnerMatcher);
+ return M.matches(*CE->getSubExpr(), Finder, Builder);
+ } else if (const auto *UE = dyn_cast<UnaryOperator>(E)) {
+ const auto M = hasSizeOfDescendant(Depth - 1, InnerMatcher);
+ return M.matches(*UE->getSubExpr(), Finder, Builder);
+ } else if (const auto *BE = dyn_cast<BinaryOperator>(E)) {
+ const auto LHS = hasSizeOfDescendant(Depth - 1, InnerMatcher);
+ const auto RHS = hasSizeOfDescendant(Depth - 1, InnerMatcher);
+ return LHS.matches(*BE->getLHS(), Finder, Builder) ||
+ RHS.matches(*BE->getRHS(), Finder, Builder);
+ }
+
+ return false;
+}
+
+CharUnits getSizeOfType(const ASTContext &Ctx, const Type *Ty) {
+ if (!Ty || Ty->isIncompleteType() || Ty->isDependentType() ||
+ isa<DependentSizedArrayType>(Ty) || !Ty->isConstantSizeType())
+ return CharUnits::Zero();
+ return Ctx.getTypeSizeInChars(Ty);
+}
+
+} // namespace
+
+SizeofExpressionCheck::SizeofExpressionCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ WarnOnSizeOfConstant(Options.get("WarnOnSizeOfConstant", 1) != 0),
+ WarnOnSizeOfThis(Options.get("WarnOnSizeOfThis", 1) != 0),
+ WarnOnSizeOfCompareToConstant(
+ Options.get("WarnOnSizeOfCompareToConstant", 1) != 0) {}
+
+void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant);
+ Options.store(Opts, "WarnOnSizeOfThis", WarnOnSizeOfThis);
+ Options.store(Opts, "WarnOnSizeOfCompareToConstant",
+ WarnOnSizeOfCompareToConstant);
+}
+
+void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
+ const auto IntegerExpr = ignoringParenImpCasts(integerLiteral());
+ const auto ConstantExpr = expr(ignoringParenImpCasts(
+ anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)),
+ binaryOperator(hasLHS(IntegerExpr), hasRHS(IntegerExpr)))));
+ const auto SizeOfExpr =
+ expr(anyOf(sizeOfExpr(has(type())), sizeOfExpr(has(expr()))));
+ const auto SizeOfZero = expr(
+ sizeOfExpr(has(expr(ignoringParenImpCasts(integerLiteral(equals(0)))))));
+
+ // Detect expression like: sizeof(ARRAYLEN);
+ // Note: The expression 'sizeof(sizeof(0))' is a portable trick used to know
+ // the sizeof size_t.
+ if (WarnOnSizeOfConstant) {
+ Finder->addMatcher(expr(sizeOfExpr(has(ConstantExpr)), unless(SizeOfZero))
+ .bind("sizeof-constant"),
+ this);
+ }
+
+ // Detect expression like: sizeof(this);
+ if (WarnOnSizeOfThis) {
+ Finder->addMatcher(
+ expr(sizeOfExpr(has(expr(ignoringParenImpCasts(cxxThisExpr())))))
+ .bind("sizeof-this"),
+ this);
+ }
+
+ // Detect sizeof(kPtr) where kPtr is 'const char* kPtr = "abc"';
+ const auto CharPtrType = pointerType(pointee(isAnyCharacter()));
+ const auto ConstStrLiteralDecl =
+ varDecl(isDefinition(), hasType(qualType(hasCanonicalType(CharPtrType))),
+ hasInitializer(ignoringParenImpCasts(stringLiteral())));
+ Finder->addMatcher(
+ expr(sizeOfExpr(has(expr(hasType(qualType(hasCanonicalType(CharPtrType))),
+ ignoringParenImpCasts(declRefExpr(
+ hasDeclaration(ConstStrLiteralDecl)))))))
+ .bind("sizeof-charp"),
+ this);
+
+ // Detect sizeof(ptr) where ptr points to an aggregate (i.e. sizeof(&S)).
+ const auto ArrayExpr = expr(ignoringParenImpCasts(
+ expr(hasType(qualType(hasCanonicalType(arrayType()))))));
+ const auto ArrayCastExpr = expr(anyOf(
+ unaryOperator(hasUnaryOperand(ArrayExpr), unless(hasOperatorName("*"))),
+ binaryOperator(hasEitherOperand(ArrayExpr)),
+ castExpr(hasSourceExpression(ArrayExpr))));
+ const auto PointerToArrayExpr = expr(ignoringParenImpCasts(expr(
+ hasType(qualType(hasCanonicalType(pointerType(pointee(arrayType()))))))));
+
+ const auto StructAddrOfExpr =
+ unaryOperator(hasOperatorName("&"),
+ hasUnaryOperand(ignoringParenImpCasts(expr(
+ hasType(qualType(hasCanonicalType(recordType())))))));
+
+ Finder->addMatcher(
+ expr(sizeOfExpr(has(expr(ignoringParenImpCasts(
+ anyOf(ArrayCastExpr, PointerToArrayExpr, StructAddrOfExpr))))))
+ .bind("sizeof-pointer-to-aggregate"),
+ this);
+
+ // Detect expression like: sizeof(epxr) <= k for a suspicious constant 'k'.
+ if (WarnOnSizeOfCompareToConstant) {
+ Finder->addMatcher(
+ binaryOperator(isRelationalOperator(),
+ hasEitherOperand(ignoringParenImpCasts(SizeOfExpr)),
+ hasEitherOperand(ignoringParenImpCasts(
+ anyOf(integerLiteral(equals(0)),
+ integerLiteral(isBiggerThan(0x80000))))))
+ .bind("sizeof-compare-constant"),
+ this);
+ }
+
+ // Detect expression like: sizeof(expr, expr); most likely an error.
+ Finder->addMatcher(expr(sizeOfExpr(has(expr(ignoringParenImpCasts(
+ binaryOperator(hasOperatorName(",")))))))
+ .bind("sizeof-comma-expr"),
+ this);
+
+ // Detect sizeof(...) /sizeof(...));
+ const auto ElemType =
+ arrayType(hasElementType(recordType().bind("elem-type")));
+ const auto ElemPtrType = pointerType(pointee(type().bind("elem-ptr-type")));
+ const auto NumType = qualType(hasCanonicalType(
+ type(anyOf(ElemType, ElemPtrType, type())).bind("num-type")));
+ const auto DenomType = qualType(hasCanonicalType(type().bind("denom-type")));
+
+ Finder->addMatcher(
+ binaryOperator(hasOperatorName("/"),
+ hasLHS(expr(ignoringParenImpCasts(
+ anyOf(sizeOfExpr(has(NumType)),
+ sizeOfExpr(has(expr(hasType(NumType)))))))),
+ hasRHS(expr(ignoringParenImpCasts(
+ anyOf(sizeOfExpr(has(DenomType)),
+ sizeOfExpr(has(expr(hasType(DenomType)))))))))
+ .bind("sizeof-divide-expr"),
+ this);
+
+ // Detect expression like: sizeof(...) * sizeof(...)); most likely an error.
+ Finder->addMatcher(binaryOperator(hasOperatorName("*"),
+ hasLHS(ignoringParenImpCasts(SizeOfExpr)),
+ hasRHS(ignoringParenImpCasts(SizeOfExpr)))
+ .bind("sizeof-multiply-sizeof"),
+ this);
+
+ Finder->addMatcher(
+ binaryOperator(hasOperatorName("*"),
+ hasEitherOperand(ignoringParenImpCasts(SizeOfExpr)),
+ hasEitherOperand(ignoringParenImpCasts(binaryOperator(
+ hasOperatorName("*"),
+ hasEitherOperand(ignoringParenImpCasts(SizeOfExpr))))))
+ .bind("sizeof-multiply-sizeof"),
+ this);
+
+ // Detect strange double-sizeof expression like: sizeof(sizeof(...));
+ // Note: The expression 'sizeof(sizeof(0))' is accepted.
+ Finder->addMatcher(expr(sizeOfExpr(has(expr(hasSizeOfDescendant(
+ 8, expr(SizeOfExpr, unless(SizeOfZero)))))))
+ .bind("sizeof-sizeof-expr"),
+ this);
+}
+
+void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
+ const ASTContext &Ctx = *Result.Context;
+
+ if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-constant")) {
+ diag(E->getLocStart(),
+ "suspicious usage of 'sizeof(K)'; did you mean 'K'?");
+ } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-this")) {
+ diag(E->getLocStart(),
+ "suspicious usage of 'sizeof(this)'; did you mean 'sizeof(*this)'");
+ } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-charp")) {
+ diag(E->getLocStart(),
+ "suspicious usage of 'sizeof(char*)'; do you mean 'strlen'?");
+ } else if (const auto *E =
+ Result.Nodes.getNodeAs<Expr>("sizeof-pointer-to-aggregate")) {
+ diag(E->getLocStart(),
+ "suspicious usage of 'sizeof(A*)'; pointer to aggregate");
+ } else if (const auto *E =
+ Result.Nodes.getNodeAs<Expr>("sizeof-compare-constant")) {
+ diag(E->getLocStart(),
+ "suspicious comparison of 'sizeof(expr)' to a constant");
+ } else if (const auto *E =
+ Result.Nodes.getNodeAs<Expr>("sizeof-comma-expr")) {
+ diag(E->getLocStart(), "suspicious usage of 'sizeof(..., ...)'");
+ } else if (const auto *E =
+ Result.Nodes.getNodeAs<Expr>("sizeof-divide-expr")) {
+ const auto *NumTy = Result.Nodes.getNodeAs<Type>("num-type");
+ const auto *DenomTy = Result.Nodes.getNodeAs<Type>("denom-type");
+ const auto *ElementTy = Result.Nodes.getNodeAs<Type>("elem-type");
+ const auto *PointedTy = Result.Nodes.getNodeAs<Type>("elem-ptr-type");
+
+ CharUnits NumeratorSize = getSizeOfType(Ctx, NumTy);
+ CharUnits DenominatorSize = getSizeOfType(Ctx, DenomTy);
+ CharUnits ElementSize = getSizeOfType(Ctx, ElementTy);
+
+ if (DenominatorSize > CharUnits::Zero() &&
+ !NumeratorSize.isMultipleOf(DenominatorSize)) {
+ diag(E->getLocStart(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
+ " numerator is not a multiple of denominator");
+ } else if (ElementSize > CharUnits::Zero() &&
+ DenominatorSize > CharUnits::Zero() &&
+ ElementSize != DenominatorSize) {
+ diag(E->getLocStart(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
+ " numerator is not a multiple of denominator");
+ } else if (NumTy && DenomTy && NumTy == DenomTy) {
+ diag(E->getLocStart(),
+ "suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'");
+ } else if (PointedTy && DenomTy && PointedTy == DenomTy) {
+ diag(E->getLocStart(),
+ "suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'");
+ } else if (NumTy && DenomTy && NumTy->isPointerType() &&
+ DenomTy->isPointerType()) {
+ diag(E->getLocStart(),
+ "suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'");
+ }
+ } else if (const auto *E =
+ Result.Nodes.getNodeAs<Expr>("sizeof-sizeof-expr")) {
+ diag(E->getLocStart(), "suspicious usage of 'sizeof(sizeof(...))'");
+ } else if (const auto *E =
+ Result.Nodes.getNodeAs<Expr>("sizeof-multiply-sizeof")) {
+ diag(E->getLocStart(), "suspicious 'sizeof' by 'sizeof' multiplication");
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/misc/SizeofExpressionCheck.h b/clang-tools-extra/clang-tidy/misc/SizeofExpressionCheck.h
new file mode 100644
index 00000000000..82388d142f5
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/SizeofExpressionCheck.h
@@ -0,0 +1,40 @@
+//===--- SizeofExpressionCheck.h - clang-tidy--------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_EXPRESSION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_EXPRESSION_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Find suspicious usages of sizeof expression.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-sizeof-expression.html
+class SizeofExpressionCheck : public ClangTidyCheck {
+public:
+ SizeofExpressionCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ const bool WarnOnSizeOfConstant;
+ const bool WarnOnSizeOfThis;
+ const bool WarnOnSizeOfCompareToConstant;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_EXPRESSION_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 578551e4da4..9d7afd38965 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@ identified. The improvements since the 3.8 release include:
Warns about suspicious operations involving pointers and integral types.
+- New `misc-sizeof-expression
+ <http://clang.llvm.org/extra/clang-tidy/checks/misc-sizeof-expression.html>`_ check
+
+ Warns about incorrect uses of ``sizeof`` operator.
+
- New `misc-string-literal-with-embedded-nul
<http://clang.llvm.org/extra/clang-tidy/checks/misc-string-literal-with-embedded-nul.html>`_ check
@@ -123,7 +128,7 @@ identified. The improvements since the 3.8 release include:
<http://clang.llvm.org/extra/clang-tidy/checks/misc-suspicious-semicolon.html>`_ check
Finds most instances of stray semicolons that unexpectedly alter the meaning
- of the code.
+ of the code.
- New `modernize-deprecated-headers
<http://clang.llvm.org/extra/clang-tidy/checks/modernize-deprecated-headers.html>`_ check
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 8af60124e02..14ea9e96e7b 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -67,6 +67,7 @@ Clang-Tidy Checks
misc-non-copyable-objects
misc-pointer-and-integral-operation
misc-sizeof-container
+ misc-sizeof-expression
misc-static-assert
misc-string-integer-assignment
misc-string-literal-with-embedded-nul
diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc-sizeof-expression.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-sizeof-expression.rst
new file mode 100644
index 00000000000..4b92ae7ce01
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc-sizeof-expression.rst
@@ -0,0 +1,137 @@
+.. title:: clang-tidy - misc-sizeof-expression
+
+misc-sizeof-expression
+======================
+
+The check finds usages of ``sizeof`` expressions which are most likely errors.
+
+The ``sizeof`` operator yields the size (in bytes) of its operand, which may be
+an expression or the parenthesized name of a type. Misuse of this operator may
+be leading to errors and possible software vulnerabilities.
+
+
+Suspicious usage of 'sizeof(K)'
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+A common mistake is to query the ``sizeof`` of an integer literal. This is
+equivalent to query the size of its type (probably ``int``). The intent of the
+programmer was probably to simply get the integer and not its size.
+
+.. code:: c++
+
+ #define BUFLEN 42
+ char buf[BUFLEN];
+ memset(buf, 0, sizeof(BUFLEN)); // sizeof(42) ==> sizeof(int)
+
+
+Suspicious usage of 'sizeof(this)'
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+The ``this`` keyword is evaluated to a pointer to an object of a given type.
+The expression ``sizeof(this)`` is returning the size of a pointer. The
+programmer most likely wanted the size of the object and not the size of the
+pointer.
+
+.. code:: c++
+
+ class Point {
+ [...]
+ size_t size() { return sizeof(this); } // should probably be sizeof(*this)
+ [...]
+ };
+
+
+Suspicious usage of 'sizeof(char*)'
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+There is a subtle difference between declaring a string literal with
+``char* A = ""`` and ``char A[] = ""``. The first case has the type ``char*``
+instead of the aggregate type ``char[]``. Using ``sizeof`` on an object declared
+with ``char*`` type is returning the size of a pointer instead of the number of
+characters (bytes) in the string literal.
+
+.. code:: c++
+
+ const char* kMessage = "Hello World!"; // const char kMessage[] = "...";
+ void getMessage(char* buf) {
+ memcpy(buf, kMessage, sizeof(kMessage)); // sizeof(char*)
+ }
+
+
+Suspicious usage of 'sizeof(A*)'
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+A common mistake is to compute the size of a pointer instead of its pointee.
+These cases may occur because of explicit cast or implicit conversion.
+
+.. code:: c++
+
+ int A[10];
+ memset(A, 0, sizeof(A + 0));
+
+ struct Point point;
+ memset(point, 0, sizeof(&point));
+
+
+Suspicious usage of 'sizeof(...)/sizeof(...)'
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Dividing ``sizeof`` expressions is typically used to retrieve the number of
+elements of an aggregate. This check warns on incompatible or suspicious cases.
+
+In the following example, the entity has 10-bytes and is incompatible with the
+type ``int`` which has 4 bytes.
+
+.. code:: c++
+
+ char buf[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; // sizeof(buf) => 10
+ void getMessage(char* dst) {
+ memcpy(dst, buf, sizeof(buf) / sizeof(int)); // sizeof(int) => 4 [incompatible sizes]
+ }
+
+
+In the following example, the expression ``sizeof(Values)`` is returning the
+size of ``char*``. One can easily be fooled by its declaration, but in parameter
+declaration the size '10' is ignored and the function is receiving a ``char*``.
+
+.. code:: c++
+
+ char OrderedValues[10] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
+ return CompareArray(char Values[10]) {
+ return memcmp(OrderedValues, Values, sizeof(Values)) == 0; // sizeof(Values) ==> sizeof(char*) [implicit cast to char*]
+ }
+
+
+Suspicious 'sizeof' by 'sizeof' expression
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Multiplying ``sizeof`` expressions typically makes no sense and is probably a
+logic error. In the following example, the programmer used ``*`` instead of
+``/``.
+
+.. code:: c++
+
+ const char kMessage[] = "Hello World!";
+ void getMessage(char* buf) {
+ memcpy(buf, kMessage, sizeof(kMessage) * sizeof(char)); // sizeof(kMessage) / sizeof(char)
+ }
+
+
+This check may trigger on code using the arraysize macro. The following code is
+working correctly but should be simplified by using only the ``sizeof``
+operator.
+
+.. code:: c++
+
+ extern Object objects[100];
+ void InitializeObjects() {
+ memset(objects, 0, arraysize(objects) * sizeof(Object)); // sizeof(objects)
+ }
+
+
+Suspicious usage of 'sizeof(sizeof(...))'
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Getting the ``sizeof`` of a ``sizeof`` makes no sense and is typically an error
+hidden through macros.
+
+.. code:: c++
+
+ #define INT_SZ sizeof(int)
+ int buf[] = { 42 };
+ void getInt(int* dst) {
+ memcpy(dst, buf, sizeof(INT_SZ)); // sizeof(sizeof(int)) is suspicious.
+ }
diff --git a/clang-tools-extra/test/clang-tidy/misc-sizeof-expression.cpp b/clang-tools-extra/test/clang-tidy/misc-sizeof-expression.cpp
new file mode 100644
index 00000000000..de315e389de
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/misc-sizeof-expression.cpp
@@ -0,0 +1,186 @@
+// RUN: %check_clang_tidy %s misc-sizeof-expression %t
+
+class C {
+ int size() { return sizeof(this); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(this)'
+};
+
+#define LEN 8
+
+int X;
+extern int A[10];
+extern short B[10];
+
+#pragma pack(1)
+struct S { char a, b, c; };
+
+int Test1(const char* ptr) {
+ int sum = 0;
+ sum += sizeof(LEN);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+ sum += sizeof(LEN + 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+ sum += sizeof(sum, LEN);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+ sum += sizeof(sizeof(X));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+ sum += sizeof(LEN + sizeof(X));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+ sum += sizeof(LEN + LEN + sizeof(X));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+ sum += sizeof(LEN + (LEN + sizeof(X)));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+ sum += sizeof(LEN + -sizeof(X));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+ sum += sizeof(LEN + - + -sizeof(X));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+ sum += sizeof(char) / sizeof(char);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+ sum += sizeof(A) / sizeof(S);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+ sum += sizeof(char) / sizeof(int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+ sum += sizeof(char) / sizeof(A);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+ sum += sizeof(B[0]) / sizeof(A);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+ sum += sizeof(ptr) / sizeof(char);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+ sum += sizeof(ptr) / sizeof(ptr[0]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+ sum += sizeof(ptr) / sizeof(char*);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+ sum += sizeof(ptr) / sizeof(void*);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+ sum += sizeof(ptr) / sizeof(const void volatile*);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+ sum += sizeof(ptr) / sizeof(char);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+ sum += sizeof(ptr) / sizeof(ptr[0]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+ sum += sizeof(int) * sizeof(char);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' multiplication
+ sum += sizeof(ptr) * sizeof(ptr[0]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' multiplication
+ sum += sizeof(int) * (2 * sizeof(char));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' multiplication
+ sum += (2 * sizeof(char)) * sizeof(int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' multiplication
+ if (sizeof(A) < 0x100000) sum += 42;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: suspicious comparison of 'sizeof(expr)' to a constant
+ if (sizeof(A) <= 0xFFFFFFFEU) sum += 42;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: suspicious comparison of 'sizeof(expr)' to a constant
+ return sum;
+}
+
+typedef char MyChar;
+typedef const MyChar MyConstChar;
+
+int CE0 = sizeof sizeof(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE1 = sizeof +sizeof(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE2 = sizeof sizeof(const char*);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE3 = sizeof sizeof(const volatile char* const*);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE4 = sizeof sizeof(MyConstChar);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+
+int Test2(MyConstChar* A) {
+ int sum = 0;
+ sum += sizeof(MyConstChar) / sizeof(char);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+ sum += sizeof(MyConstChar) / sizeof(MyChar);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+ sum += sizeof(A[0]) / sizeof(char);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+ return sum;
+}
+
+template <int T>
+int Foo() { int A[T]; return sizeof(T); }
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: suspicious usage of 'sizeof(K)'
+template <typename T>
+int Bar() { T A[5]; return sizeof(A[0]) / sizeof(T); }
+// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+int Test3() { return Foo<42>() + Bar<char>(); }
+
+static const char* kABC = "abc";
+static const wchar_t* kDEF = L"def";
+int Test4(const char A[10]) {
+ int sum = 0;
+ sum += sizeof(kABC);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(char*)'
+ sum += sizeof(kDEF);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(char*)'
+ return sum;
+}
+
+int Test5() {
+ typedef int Array10[10];
+
+ struct MyStruct {
+ Array10 arr;
+ Array10* ptr;
+ };
+ typedef const MyStruct TMyStruct;
+
+ static TMyStruct kGlocalMyStruct = {};
+ static TMyStruct volatile * kGlocalMyStructPtr = &kGlocalMyStruct;
+
+ MyStruct S;
+ Array10 A10;
+
+ int sum = 0;
+ sum += sizeof(&S.arr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ sum += sizeof(&kGlocalMyStruct.arr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ sum += sizeof(&kGlocalMyStructPtr->arr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ sum += sizeof(S.arr + 0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ sum += sizeof(+ S.arr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ sum += sizeof((int*)S.arr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+
+ sum += sizeof(S.ptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ sum += sizeof(kGlocalMyStruct.ptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ sum += sizeof(kGlocalMyStructPtr->ptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+
+ sum += sizeof(&kGlocalMyStruct);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ sum += sizeof(&S);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+ sum += sizeof(&A10);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate
+
+ return sum;
+}
+
+int ValidExpressions() {
+ int A[] = {1, 2, 3, 4};
+ static const char str[] = "hello";
+ static const char* ptr[] { "aaa", "bbb", "ccc" };
+ int sum = 0;
+ if (sizeof(A) < 10)
+ sum += sizeof(A);
+ sum += sizeof(int);
+ sum += sizeof(A[sizeof(A) / sizeof(int)]);
+ sum += sizeof(&A[sizeof(A) / sizeof(int)]);
+ sum += sizeof(sizeof(0)); // Special case: sizeof size_t.
+ sum += sizeof(void*);
+ sum += sizeof(void const *);
+ sum += sizeof(void const *) / 4;
+ sum += sizeof(str);
+ sum += sizeof(str) / sizeof(char);
+ sum += sizeof(str) / sizeof(str[0]);
+ sum += sizeof(ptr) / sizeof(ptr[0]);
+ sum += sizeof(ptr) / sizeof(*(ptr));
+ return sum;
+}
OpenPOWER on IntegriCloud