Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -21,6 +21,7 @@ #include "ForwardDeclarationNamespaceCheck.h" #include "ForwardingReferenceOverloadCheck.h" #include "InaccurateEraseCheck.h" +#include "IncorrectPointerCastCheck.h" #include "IncorrectRoundingsCheck.h" #include "IntegerDivisionCheck.h" #include "LambdaFunctionNameCheck.h" @@ -82,6 +83,8 @@ "bugprone-forwarding-reference-overload"); CheckFactories.registerCheck( "bugprone-inaccurate-erase"); + CheckFactories.registerCheck( + "bugprone-incorrect-pointer-cast"); CheckFactories.registerCheck( "bugprone-incorrect-roundings"); CheckFactories.registerCheck( Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -13,6 +13,7 @@ ForwardDeclarationNamespaceCheck.cpp ForwardingReferenceOverloadCheck.cpp InaccurateEraseCheck.cpp + IncorrectPointerCastCheck.cpp IncorrectRoundingsCheck.cpp IntegerDivisionCheck.cpp LambdaFunctionNameCheck.cpp Index: clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.h @@ -0,0 +1,43 @@ +//===--- IncorrectPointerCastCheck.h - clang-tidy ---------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTPOINTERCASTCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTPOINTERCASTCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Warns for cases when pointer is casted and the destination type is +/// incompatible with the original 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/bugprone-incorrect-pointer-cast.html +class IncorrectPointerCastCheck : public ClangTidyCheck { +public: + IncorrectPointerCastCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IgnoreReinterpretCast(Options.get("IgnoreReinterpretCast", 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 do not warn when reinterpret cast is + /// used. The default is false. + const bool IgnoreReinterpretCast; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTPOINTERCASTCHECK_H Index: clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.cpp @@ -0,0 +1,142 @@ +//===--- IncorrectPointerCastCheck.cpp - clang-tidy -------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#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 bugprone { + +void IncorrectPointerCastCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoreReinterpretCast", IgnoreReinterpretCast); +} + +void IncorrectPointerCastCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(cStyleCastExpr(hasCastKind(CK_BitCast), + unless(isInTemplateInstantiation())) + .bind("cast"), + this); + if (!IgnoreReinterpretCast) { + Finder->addMatcher( + cxxReinterpretCastExpr(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 bool HasCXXStyle = isa(castExpr); + + const QualType SrcType = castExpr->getSubExpr()->getType(); + const QualType DestType = castExpr->getType(); + + if (!SrcType->isPointerType() || !DestType->isPointerType()) + return; + + if (SrcType->isDependentType() || DestType->isDependentType()) + return; + + const QualType SrcPointedType = SrcType->getPointeeType(); + const QualType DestPointedType = DestType->getPointeeType(); + + if (SrcPointedType->isIncompleteType() || DestPointedType->isIncompleteType()) + return; + + // -Wcast-align already warns for C-style casts increasing alignment + // requirements + const CharUnits SrcWidth = Context.getTypeSizeInChars(SrcPointedType); + const CharUnits DestWidth = Context.getTypeSizeInChars(DestPointedType); + if (SrcWidth < DestWidth && HasCXXStyle) { + diag(castExpr->getBeginLoc(), + "cast from %0 to %1 may lead to access memory based on invalid memory " + "layout; new type is wider (%2) than the original type (%3)") + << SrcPointedType << DestPointedType + << static_cast(SrcWidth.getQuantity()) + << static_cast(DestWidth.getQuantity()); + return; + } + + // -Wcast-align already warns for C-style casts increasing alignment + // requirements + const CharUnits SrcAlign = Context.getTypeAlignInChars(SrcPointedType); + const CharUnits DestAlign = Context.getTypeAlignInChars(DestPointedType); + if (SrcAlign < DestAlign && HasCXXStyle) { + diag(castExpr->getBeginLoc(), + "cast from %0 to %1 may lead to access memory based on invalid " + "memory layout; new type is more strictly aligned (%2) than the " + "original type (%3)") + << SrcPointedType << DestPointedType + << static_cast(SrcAlign.getQuantity()) + << static_cast(DestAlign.getQuantity()); + return; + } + + // make sure both pointee are structs + if (!SrcPointedType->isStructureType() || !DestPointedType->isStructureType()) + return; + + bool FieldsAreSame = true; + const auto *SrcTypeRecordDecl = SrcPointedType->getAsRecordDecl(); + const auto *DestTypeRecordDecl = DestPointedType->getAsRecordDecl(); + auto SrcIterator = SrcTypeRecordDecl->field_begin(); + auto DestIterator = DestTypeRecordDecl->field_begin(); + + for (RecordDecl::field_iterator SrcEnd = SrcTypeRecordDecl->field_end(), + DestEnd = DestTypeRecordDecl->field_end(); + SrcIterator != SrcEnd && DestIterator != DestEnd; + ++SrcIterator, ++DestIterator) { + const FieldDecl &SrcField = **SrcIterator; + const FieldDecl &DestField = **DestIterator; + if (SrcField.getType() != DestField.getType()) { + FieldsAreSame = false; + break; + } + } + + if (!FieldsAreSame) { + const auto *SrcRecDecl = (*SrcIterator)->getParent(); + const auto *DestRecDecl = (*DestIterator)->getParent(); + + diag(castExpr->getBeginLoc(), + "cast from %0 to %1 may lead to access memory based on invalid " + "memory layout; struct members are incompatible") + << SrcPointedType << DestPointedType; + diag((*SrcIterator)->getBeginLoc(), + "first incompatible member of %0 is declared here; at %1 bit offset " + "with %2 bit width", + DiagnosticIDs::Note) + << SrcRecDecl->getName() + << static_cast( + Context.getASTRecordLayout(SrcRecDecl) + .getFieldOffset(SrcIterator->getFieldIndex())) + << static_cast(SrcWidth.getQuantity()); + diag((*DestIterator)->getBeginLoc(), + "first incompatible member of %0 is declared here; at %1 bit offset " + "with %2 bit width", + DiagnosticIDs::Note) + << DestRecDecl->getName() + << static_cast( + Context.getASTRecordLayout(DestRecDecl) + .getFieldOffset(DestIterator->getFieldIndex())) + << static_cast(DestWidth.getQuantity()); + return; + } +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -77,6 +77,13 @@ Checks for cases where addition should be performed in the ``absl::Time`` domain. +- New :doc:`bugprone-incorrect-pointer-cast + ` check + + Warns for cases when pointer is casted and the destination type is + incompatible with the original type. This may lead to access memory + based on invalid memory layout. + - New :doc:`abseil-duration-conversion-cast ` check. Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-incorrect-pointer-cast.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/bugprone-incorrect-pointer-cast.rst @@ -0,0 +1,75 @@ +.. title:: clang-tidy - bugprone-incorrect-pointer-cast + +bugprone-incorrect-pointer-cast +=============================== + +Warns for cases when pointer is casted and the destination type is wider than the +original type. +For example `char` vs `int`, `long` vs `char` etc. +Also warns for cases when the layout of the destination type is different from the +original one, like different structs, `int` vs `float`/`double`, +different signedness. + +Allows pointer casts if the destination type is "part" of the original type. +Which means the original type contains the members of the destination type +strictly in order. + +Highly recommended to use `-Wcast-align` whith this check which will cover some +other cases of alignment requirement violations via casts. + +Options +------- + +.. option:: IgnoreReinterpretCast + + This option can be configured to do not warn when reinterpret cast is used. + Disabled by default but this option might be useful on code bases where + `reinterpret_cast` is used carefully. + +Examples +------- + +Cast char pointer to integer pointer. +Check will warn because of cast to a wider type. + +.. code-block:: c++ + + char c = 'a'; + int *i = reinterpret_cast(&c); + i = (int *)&c; // It won't warn becouse '-Wcast-align' warns instead. + +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 with `reinterpret_cast`. +If the `IgnoreReinterpretCast` option is `0`, check will warn for these +kind of casts. + +.. code-block:: c++ + + char c = 'x'; + int *i = reinterpret_cast(&c); + Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -46,6 +46,7 @@ bugprone-forward-declaration-namespace bugprone-forwarding-reference-overload bugprone-inaccurate-erase + bugprone-incorrect-pointer-cast bugprone-incorrect-roundings bugprone-integer-division bugprone-lambda-function-name Index: clang-tools-extra/test/clang-tidy/bugprone-incorrect-pointer-cast.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/bugprone-incorrect-pointer-cast.cpp @@ -0,0 +1,205 @@ +// RUN: %check_clang_tidy %s bugprone-incorrect-pointer-cast %t -- \ +// RUN: -config="{CheckOptions: [{key: bugprone-incorrect-pointer-cast.WarnForDifferentSignedness, value: 1}]}" -- + +bool b; +char __attribute__((aligned(4))) a[16]; + +struct S0 { + char a[16]; +}; + +struct S01 { + char __attribute__((aligned(4))) a[16]; + struct S0 __attribute__((aligned(4))) s0; +}; + +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; +}; + +struct __attribute__((aligned(16))) SAligned { + char buffer[16]; +}; + +void initDouble(double *d) { + *d = 0.5; +} + +void castCharToInt(void) { + char c = 'x'; + b = (int *)&c; // -Wcast-align already warns for this + b = reinterpret_cast(&c); // -Wcast-align does NOT warn for this + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: cast from 'char' to 'int' may lead to access memory based on invalid memory layout; new type is wider (1) than the original type (4) [bugprone-incorrect-pointer-cast] +} + +void castCharToSort(char c) { + b = (short *)&c; // -Wcast-align already warns for this + b = reinterpret_cast(&c); // -Wcast-align does NOT warn for this + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: cast from 'char' to 'short' may lead to access memory based on invalid memory layout; new type is wider (1) than the original type (2) [bugprone-incorrect-pointer-cast] +} + +void castShortToInt(short s) { + b = (int *)&s; // -Wcast-align already warns for this + b = reinterpret_cast(&s); // -Wcast-align does NOT warn for this + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: cast from 'short' to 'int' may lead to access memory based on invalid memory layout; new type is wider (2) than the original type (4) [bugprone-incorrect-pointer-cast] +} + +void castWideCharToLong(wchar_t wc) { + b = (long *)&wc; // -Wcast-align already warns for this + b = reinterpret_cast(&wc); // -Wcast-align does NOT warn for this + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: cast from 'wchar_t' to 'long' may lead to access memory based on invalid memory layout; new type is wider (4) than the original type (8) [bugprone-incorrect-pointer-cast] +} + +void castFloatToDouble(float f) { + initDouble((double *)&f); // -Wcast-align already warns for this + initDouble(reinterpret_cast(&f)); // -Wcast-align does NOT warn for this + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: cast from 'float' to 'double' may lead to access memory based on invalid memory layout; new type is wider (4) than the original type (8) [bugprone-incorrect-pointer-cast] +} + +void castToS2(char *data, unsigned offset) { + b = (struct S2 *)(data + offset); // -Wcast-align already warns for this + b = reinterpret_cast(data + offset); // -Wcast-align does NOT warn for this + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: cast from 'char' to 'struct S2' may lead to access memory based on invalid memory layout; new type is wider (1) than the original type (4) [bugprone-incorrect-pointer-cast] +} + +void castS3ToS1(struct S3 s3) { + b = (struct S1 *)&s3; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: cast from 'struct S3' to 'struct S1' may lead to access memory based on invalid memory layout; struct members are incompatible [bugprone-incorrect-pointer-cast] + // CHECK-MESSAGES: :27:3: note: first incompatible member of S3 is declared here; at 64 bit offset with 16 bit width + // CHECK-MESSAGES: :18:3: note: first incompatible member of S1 is declared here; at 32 bit offset with 8 bit width + + b = reinterpret_cast(&s3); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: cast from 'struct S3' to 'struct S1' may lead to access memory based on invalid memory layout; struct members are incompatible [bugprone-incorrect-pointer-cast] + // CHECK-MESSAGES: :27:3: note: first incompatible member of S3 is declared here; at 64 bit offset with 16 bit width + // CHECK-MESSAGES: :18:3: note: first incompatible member of S1 is declared here; at 32 bit offset with 8 bit width +} + +void castS4ToS5(struct S4 s4) { + b = (struct S5 *)&s4; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: cast from 'struct S4' to 'struct S5' may lead to access memory based on invalid memory layout; struct members are incompatible [bugprone-incorrect-pointer-cast] + // CHECK-MESSAGES: :31:3: note: first incompatible member of S4 is declared here; at 0 bit offset with 16 bit width + // CHECK-MESSAGES: :36:3: note: first incompatible member of S5 is declared here; at 0 bit offset with 16 bit width + + b = reinterpret_cast(&s4); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: cast from 'struct S4' to 'struct S5' may lead to access memory based on invalid memory layout; struct members are incompatible [bugprone-incorrect-pointer-cast] + // CHECK-MESSAGES: :31:3: note: first incompatible member of S4 is declared here; at 0 bit offset with 16 bit width + // CHECK-MESSAGES: :36:3: note: first incompatible member of S5 is declared here; at 0 bit offset with 16 bit width +} + +void castULongToLong(unsigned long ul) { + b = (long *)&ul; + b = reinterpret_cast(&ul); +} + +void castIntToUInt(int i) { + b = (unsigned int *)&i; + b = reinterpret_cast(&i); +} + +void castToAlignedStruct(char *P) { + b = (struct SAligned *)P; // -Wcast-align already warns for this + b = reinterpret_cast(P); // -Wcast-align does NOT warn for this + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: cast from 'char' to 'struct SAligned' may lead to access memory based on invalid memory layout; new type is wider (1) than the original type (16) [bugprone-incorrect-pointer-cast] +} + +void castCharToIntWithReinterpretCast(void) { + char c = 'x'; + b = reinterpret_cast(&c); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: cast from 'char' to 'int' may lead to access memory based on invalid memory layout; new type is wider (1) than the original type (4) [bugprone-incorrect-pointer-cast] +} + +void TestDifferentAlignment() { + struct S01 s; + int *i = (int *)s.a; // ok, s.a is aligned to 4 bytes just like and 'int *' + i = (int *)&s.s0; // same + i = (int *)a; // same +} + +// negatives +void castIntToFloat(int i) { + float *f = (float *)&i; +} + +void castCharToChar(char *p) { + char *c = (char *)p; +} + +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; +} + +void IncompleteType(char *P) { + struct B *b = (struct B *)P; +} + +// Casts from void* are a special case. +void CastFromVoidPointer(void *P) { + char *a = (char *)P; + short *b = (short *)P; + int *c = (int *)P; + + const volatile void *P2 = P; + char *d = (char *)P2; + short *e = (short *)P2; + int *f = (int *)P2; + + const char *g = (const char *)P2; + const short *h = (const short *)P2; + const int *i = (const int *)P2; + + const volatile char *j = (const volatile char *)P2; + const volatile short *k = (const volatile short *)P2; + const volatile int *l = (const volatile int *)P2; +} + +typedef int (*FnTy)(void); +unsigned int func(void); + +FnTy testFunc(void) { + return (FnTy)&func; +} + +struct W; +void function3(struct W *v) { + int *i = (int *)v; + struct W *u = (struct W *)i; +}