[clang-tools-extra] r266451 - [clang-tidy] Add new checker for suspicious sizeof expressions
Etienne Bergeron via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 15 09:36:00 PDT 2016
Author: etienneb
Date: Fri Apr 15 11:36:00 2016
New Revision: 266451
URL: http://llvm.org/viewvc/llvm-project?rev=266451&view=rev
Log:
[clang-tidy] Add new checker for suspicious sizeof expressions
Summary:
This check is finding suspicious cases of sizeof expression.
Sizeof expression is returning the size (in bytes) of a type or an
expression. Programmers often abuse or misuse this expression.
This checker is adding common set of patterns to detect some
of these bad constructs.
Some examples found by this checker:
R/packages/ifultools/ifultools/src/fra_neig.c
```
/* free buffer memory */
(void) mutil_free( dist_buff, sizeof( ctr * sizeof( double ) ) );
(void) mutil_free( nidx_buff, sizeof( ctr * sizeof( sint32 ) ) );
```
graphviz/v2_20_2/lib/common/utils.c
```
static Dtdisc_t mapDisc = {
offsetof(item, p),
sizeof(2 * sizeof(void *)),
offsetof(item, link),
(Dtmake_f) newItem,
(Dtfree_f) freeItem,
(Dtcompar_f) cmpItem,
NIL(Dthash_f),
NIL(Dtmemory_f),
NIL(Dtevent_f)
};
```
mDNSResponder/mDNSShared/dnsextd.c
```
context = ( TCPContext* ) malloc( sizeof( TCPContext ) );
require_action( context, exit, err = mStatus_NoMemoryErr; LogErr( "AcceptTCPConnection", "malloc" ) );
mDNSPlatformMemZero( context, sizeof( sizeof( TCPContext ) ) );
context->d = self;
```
Reviewers: alexfh
Subscribers: malcolm.parsons, Eugene.Zelenko, cfe-commits
Differential Revision: http://reviews.llvm.org/D19014
Added:
clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-expression.rst
clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-expression.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=266451&r1=266450&r2=266451&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Fri Apr 15 11:36:00 2016
@@ -23,6 +23,7 @@ add_clang_library(clangTidyMiscModule
NonCopyableObjects.cpp
PointerAndIntegralOperationCheck.cpp
SizeofContainerCheck.cpp
+ SizeofExpressionCheck.cpp
StaticAssertCheck.cpp
StringIntegerAssignmentCheck.cpp
StringLiteralWithEmbeddedNulCheck.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=266451&r1=266450&r2=266451&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Fri Apr 15 11:36:00 2016
@@ -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>(
Added: clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.cpp?rev=266451&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.cpp Fri Apr 15 11:36:00 2016
@@ -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
Added: clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.h?rev=266451&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.h Fri Apr 15 11:36:00 2016
@@ -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
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=266451&r1=266450&r2=266451&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Fri Apr 15 11:36:00 2016
@@ -108,6 +108,11 @@ identified. The improvements since the
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
<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
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=266451&r1=266450&r2=266451&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Fri Apr 15 11:36:00 2016
@@ -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
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-expression.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-expression.rst?rev=266451&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-expression.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-expression.rst Fri Apr 15 11:36:00 2016
@@ -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.
+ }
Added: clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-expression.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-expression.cpp?rev=266451&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-expression.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-expression.cpp Fri Apr 15 11:36:00 2016
@@ -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;
+}
More information about the cfe-commits
mailing list