etienneb updated this revision to Diff 53431.
etienneb added a comment.
more unittests
http://reviews.llvm.org/D19014
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/SizeofExpressionCheck.cpp
clang-tidy/misc/SizeofExpressionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-sizeof-expression.rst
test/clang-tidy/misc-sizeof-expression.cpp
Index: test/clang-tidy/misc-sizeof-expression.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-sizeof-expression.cpp
@@ -0,0 +1,183 @@
+// 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 multiplication of two 'sizeof'
+ sum += sizeof(ptr) * sizeof(ptr[0]);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+ sum += sizeof(int) * (2 * sizeof(char));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+ sum += (2 * sizeof(char)) * sizeof(int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+ 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";
+int Test4(const char A[10]) {
+ int sum = 0;
+ sum += sizeof(kABC);
+ // 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;
+}
Index: docs/clang-tidy/checks/misc-sizeof-expression.rst
===================================================================
--- /dev/null
+++ 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 it's type (probably ``int``). The intend of the
+programmer was probably to simply get the integer and not it's 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*)
+ }
+
+
+warning: suspicious usage of 'sizeof(A*)'
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+A common mistake is to compute the sizeof a pointer instead it's 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
+sub-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 expressions ``sizeof(Values)`` is returning the
+size of ``char*``. One who can be easily fooled by it's declaration, but in
+parameter declaraction 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 multiplation of two 'sizeof'
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Multiplying sizeof expressions typically make no sense and is probably an 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 only using 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.
+ }
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -65,6 +65,7 @@
misc-noexcept-move-constructor
misc-non-copyable-objects
misc-sizeof-container
+ misc-sizeof-expression
misc-static-assert
misc-string-integer-assignment
misc-string-literal-with-embedded-nul
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -97,6 +97,11 @@
Checks if an unused forward declaration is in a wrong namespace.
+- 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-misplaced-widening-cast
<http://clang.llvm.org/extra/clang-tidy/checks/misc-misplaced-widening-cast.html>`_ check
@@ -118,7 +123,7 @@
<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
Index: clang-tidy/misc/SizeofExpressionCheck.h
===================================================================
--- /dev/null
+++ 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
Index: clang-tidy/misc/SizeofExpressionCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/SizeofExpressionCheck.cpp
@@ -0,0 +1,263 @@
+//===--- 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_P(Expr, hasSizeOfAncestor, ast_matchers::internal::Matcher<Expr>,
+ InnerMatcher) {
+ const Expr *E = &Node;
+
+ ASTContext::DynTypedNodeList Parents = Finder->getASTContext().getParents(*E);
+ if (Parents.size() != 1)
+ return false;
+
+ E = Parents[0].get<Expr>();
+ if (!E)
+ return false;
+
+ if (InnerMatcher.matches(*E, Finder, Builder))
+ return true;
+
+ if (isa<CastExpr>(E) || isa<UnaryOperator>(E) || isa<BinaryOperator>(E) ||
+ isa<ParenExpr>(E)) {
+ ast_matchers::internal::Matcher<Expr> M = hasSizeOfAncestor(InnerMatcher);
+ return M.matches(*E, 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 point 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, unless(SizeOfZero),
+ hasSizeOfAncestor(SizeOfExpr.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 multiplication of two 'sizeof'");
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -29,6 +29,7 @@
#include "NoexceptMoveConstructorCheck.h"
#include "NonCopyableObjects.h"
#include "SizeofContainerCheck.h"
+#include "SizeofExpressionCheck.h"
#include "StaticAssertCheck.h"
#include "StringIntegerAssignmentCheck.h"
#include "StringLiteralWithEmbeddedNulCheck.h"
@@ -86,6 +87,8 @@
CheckFactories.registerCheck<NonCopyableObjectsCheck>(
"misc-non-copyable-objects");
CheckFactories.registerCheck<SizeofContainerCheck>("misc-sizeof-container");
+ CheckFactories.registerCheck<SizeofExpressionCheck>(
+ "misc-sizeof-expression");
CheckFactories.registerCheck<StaticAssertCheck>(
"misc-static-assert");
CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -21,6 +21,7 @@
NoexceptMoveConstructorCheck.cpp
NonCopyableObjects.cpp
SizeofContainerCheck.cpp
+ SizeofExpressionCheck.cpp
StaticAssertCheck.cpp
StringIntegerAssignmentCheck.cpp
StringLiteralWithEmbeddedNulCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
