Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -1,6 +1,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyMiscModule + IncorrectPointerCastCheck.cpp MisplacedConstCheck.cpp UnconventionalAssignOperatorCheck.cpp DefinitionsInHeadersCheck.cpp Index: clang-tidy/misc/IncorrectPointerCastCheck.h =================================================================== --- /dev/null +++ clang-tidy/misc/IncorrectPointerCastCheck.h @@ -0,0 +1,46 @@ +//===--- IncorrectPointerCastCheck.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_INCORRECTPOINTERCASTCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_INCORRECTPOINTERCASTCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace misc { + +/// Warns for cases when pointer is cast and the pointed to type is incompatible +/// with allocated memory area type. This may lead to access memory based on +/// invalid memory layout. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/misc-incorrect-pointer-cast.html +class IncorrectPointerCastCheck : public ClangTidyCheck { +public: + IncorrectPointerCastCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + WarnForDifferentSignedness( + Options.get("WarnForDifferentSignedness", false)) {} + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + /// This option can be configured to warn when the pointed to type signedness + /// is different from the allocated type. The default is false because this + /// option might be noisy on some code bases. + const bool WarnForDifferentSignedness; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_INCORRECTPOINTERCASTCHECK_H Index: clang-tidy/misc/IncorrectPointerCastCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/misc/IncorrectPointerCastCheck.cpp @@ -0,0 +1,83 @@ +//===--- IncorrectPointerCastCheck.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 "IncorrectPointerCastCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/RecordLayout.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace misc { + +void IncorrectPointerCastCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "WarnForDifferentSignedness", WarnForDifferentSignedness); +} + +void IncorrectPointerCastCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(cStyleCastExpr(hasCastKind(CK_BitCast), + unless(isInTemplateInstantiation())) + .bind("cast"), + this); +} + +void IncorrectPointerCastCheck::check(const MatchFinder::MatchResult &Result) { + const ASTContext &Context = *Result.Context; + const auto *CastExpr = Result.Nodes.getNodeAs("cast"); + const QualType SrcType = CastExpr->getSubExpr()->getType()->getPointeeType(); + const QualType DestType = CastExpr->getType()->getPointeeType(); + + if (SrcType->isStructureType() && DestType->isStructureType()) { + const auto *SrcTypeCXXRecordDecl = SrcType->getAsCXXRecordDecl(); + const auto *DestTypeCXXRecordDecl = DestType->getAsCXXRecordDecl(); + bool FieldsAreSame = true; + + for (RecordDecl::field_iterator + SrcIterator = SrcTypeCXXRecordDecl->field_begin(), + SrcEnd = SrcTypeCXXRecordDecl->field_end(), + DestIterator = DestTypeCXXRecordDecl->field_begin(), + DestEnd = DestTypeCXXRecordDecl->field_end(); + DestIterator != DestEnd && FieldsAreSame; + ++SrcIterator, ++DestIterator) { + const FieldDecl &SrcField = **SrcIterator; + const FieldDecl &DestField = **DestIterator; + if (SrcField.getType() != DestField.getType() || SrcIterator == SrcEnd) { + FieldsAreSame = false; + } + + if (!FieldsAreSame) { + diag(CastExpr->getLocStart(), + "cast from %0 to %1 may lead to access memory based on invalid " + "memory layout; struct members are incompatible") + << SrcType << DestType; + } + } + } else if (Context.getIntWidth(SrcType) < Context.getIntWidth(DestType)) { + diag(CastExpr->getLocStart(), + "cast from %0 to %1 may lead to access memory based on invalid memory " + "layout; pointed to type is wider than the allocated type") + << DestType << SrcType; + } else if (WarnForDifferentSignedness && + Context.getIntWidth(SrcType) == Context.getIntWidth(DestType)) { + if ((SrcType->isSignedIntegerType() && DestType->isUnsignedIntegerType()) || + (SrcType->isUnsignedIntegerType() && DestType->isSignedIntegerType())) { + diag(CastExpr->getLocStart(), + "cast from %0 to %1 may lead to access memory based on invalid " + "memory layout; different signedness types") + << SrcType << DestType; + } + } +} + +} // namespace misc +} // namespace tidy +} // namespace clang Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -11,6 +11,7 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "DefinitionsInHeadersCheck.h" +#include "IncorrectPointerCastCheck.h" #include "MisplacedConstCheck.h" #include "NewDeleteOverloadsCheck.h" #include "NonCopyableObjects.h" @@ -30,6 +31,8 @@ class MiscModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "misc-incorrect-pointer-cast"); CheckFactories.registerCheck("misc-misplaced-const"); CheckFactories.registerCheck( "misc-unconventional-assign-operator"); Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -32,6 +32,13 @@ Improvements to clang-tidy -------------------------- +- New :doc:`misc-incorrect-pointer-cast + ` check + + Warns for cases when pointer is cast and the pointed to type is + incompatible with allocated memory area type. This may lead to access memory + based on invalid memory layout. + - The checks profiling info can now be stored as JSON files for futher post-processing and analysis. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -153,6 +153,7 @@ llvm-namespace-comment llvm-twine-local misc-definitions-in-headers + misc-incorrect-pointer-cast misc-misplaced-const misc-new-delete-overloads misc-non-copyable-objects Index: docs/clang-tidy/checks/misc-incorrect-pointer-cast.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/misc-incorrect-pointer-cast.rst @@ -0,0 +1,70 @@ +.. title:: clang-tidy - misc-incorrect-pointer-cast + +misc-incorrect-pointer-cast +=========================== + +Warns for cases when pointer is cast and the pointed to type is wider than the +allocated type. +For example char vs integer, long vs char etc. +Also warns for cases when the pointed to type layout is different from the +allocated type layout, like different structs, integer vs float/double, +different signedness. + +Allows pointer casts if the pointed to struct type is "part" of the allocated +type. +Which means the allocated type contains the pointed to type member by member. + +Options +------- + +.. option:: WarnForDifferentSignedness + + This option can be configured to warn when the pointed to type signedness + is different from the allocated type. + Disabled by default because this option might be noisy on some code bases. + +Examles +------- + +Cast char pointer to integer pointer. +Check will warn because of cast to a wider type. + +.. code-block:: c++ + + char c = 'a'; + int *i = (int *)&c; + +Cast between structs. +Check will allow to cast to a narrower struct if it is part of the source struct +member by member. + +.. code-block:: c++ + + struct S1 { + int a; + }; + + struct S2 { + int a; + double b; + }; + + struct S3 { + double y; + long x; + }; + + struct S2 s2; + struct S1 *s1 = (struct S1 *)&s2; // Won't warn. Struct "S2" contains struct + // "S2" member by member. + struct S3 *s3 = (struct S3 *)&s2; // Warning because of different type + // layout. + +Cast between different signedness types. +If the `WarnForDifferentSignedness` option is `1`, check will warn for these +kind of casts. + +.. code-block:: c++ + + unsigned int u; + int i = (int *)&u; Index: test/clang-tidy/misc-incorrect-pointer-cast.cpp =================================================================== --- /dev/null +++ test/clang-tidy/misc-incorrect-pointer-cast.cpp @@ -0,0 +1,108 @@ +// RUN: %check_clang_tidy %s misc-incorrect-pointer-cast %t -- \ +// RUN: -config="{CheckOptions: [{key: misc-incorrect-pointer-cast.WarnForDifferentSignedness, value: 1}]}" -- + +struct S1 { + int a; + int b; +}; + +struct S2 { + int a; +}; + +struct S3 { + int a; + double b; +}; + +struct S4 { + int x; + double y; +}; + +struct S5 { + double y; + int x; +}; + +void initDouble(double *d) { + *d = 0.5; +} + +void castCharToSort() { + char c; + short *i = (short *)&c; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: cast from 'short' to 'char' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast] +} + +void castShortToInt() { + short s; + int *i = (int *)&s; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: cast from 'int' to 'short' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast] +} + +void castWideCharToLong() { + wchar_t wc; + long *f = (long *)&wc; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: cast from 'long' to 'wchar_t' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast] +} + +void castFloatToDouble() { + float f; + initDouble((double *)&f); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: cast from 'double' to 'float' may lead to access memory based on invalid memory layout; pointed to type is wider than the allocated type [misc-incorrect-pointer-cast] +} + +void castS3ToS1() { + struct S3 s3; + struct S1 *s1 = (struct S1 *)&s3; + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: cast from 'struct S3' to 'struct S1' may lead to access memory based on invalid memory layout; struct members are incompatible [misc-incorrect-pointer-cast] +} + +void castS4ToS5() { + struct S4 s4; + struct S5 *s5 = (struct S5 *)&s4; + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: cast from 'struct S4' to 'struct S5' may lead to access memory based on invalid memory layout; struct members are incompatible [misc-incorrect-pointer-cast] +} + +void castULongToLong() { + unsigned long ul; + long *l = (long *)&ul; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: cast from 'unsigned long' to 'long' may lead to access memory based on invalid memory layout; different signedness types [misc-incorrect-pointer-cast] +} + +void castIntToUInt() { + int i; + unsigned int *ui = (unsigned int *)&i; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: cast from 'int' to 'unsigned int' may lead to access memory based on invalid memory layout; different signedness types [misc-incorrect-pointer-cast] +} + +// negatives +void castIntToFloat() { + int i; + float *f = (float *)&i; +} + +void castShortToChar() { + short s; + char *c = (char *)&s; +} + +void initInt(int *i) { + *i = 1; +} + +void castIntToInt() { + int i; + initInt(&i); +} + +void castS1ToS2() { + struct S1 s1; + struct S2 *s2 = (struct S2 *)&s1; +} + +void castS4ToS3() { + struct S4 s4; + struct S3 *s3 = (struct S3 *)&s4; +}