steakhal updated this revision to Diff 202913.
steakhal marked 4 inline comments as done.
steakhal added a comment.
- Removed different signess related parts.
- Don't warn for the casts which are already covered by '-Wcast-align' check.
- Improved the diagnostic messages:
- Now adds notes for the first incompatible members of structs.
- Added alignment, size and type information to most of the warning messages.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D48866/new/
https://reviews.llvm.org/D48866
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.cpp
clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone-incorrect-pointer-cast.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/bugprone-incorrect-pointer-cast.cpp
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<int *>(&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<short *>(&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<int *>(&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<long *>(&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<double *>(&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<struct S2 *>(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<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
+}
+
+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<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
+}
+
+void castULongToLong(unsigned long ul) {
+ b = (long *)&ul;
+ b = reinterpret_cast<long *>(&ul);
+}
+
+void castIntToUInt(int i) {
+ b = (unsigned int *)&i;
+ b = reinterpret_cast<unsigned int *>(&i);
+}
+
+void castToAlignedStruct(char *P) {
+ b = (struct SAligned *)P; // -Wcast-align already warns for this
+ b = reinterpret_cast<struct SAligned *>(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<int *>(&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;
+}
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/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<int *>(&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<int *>(&c);
+
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
+ <clang-tidy/checks/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
<clang-tidy/checks/abseil-duration-conversion-cast>` check.
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<CastExpr>("cast");
+ const bool HasCXXStyle = isa<CXXReinterpretCastExpr>(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<unsigned>(SrcWidth.getQuantity())
+ << static_cast<unsigned>(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<unsigned>(SrcAlign.getQuantity())
+ << static_cast<unsigned>(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<unsigned>(
+ Context.getASTRecordLayout(SrcRecDecl)
+ .getFieldOffset(SrcIterator->getFieldIndex()))
+ << static_cast<unsigned>(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<unsigned>(
+ Context.getASTRecordLayout(DestRecDecl)
+ .getFieldOffset(DestIterator->getFieldIndex()))
+ << static_cast<unsigned>(DestWidth.getQuantity());
+ return;
+ }
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
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/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<InaccurateEraseCheck>(
"bugprone-inaccurate-erase");
+ CheckFactories.registerCheck<IncorrectPointerCastCheck>(
+ "bugprone-incorrect-pointer-cast");
CheckFactories.registerCheck<IncorrectRoundingsCheck>(
"bugprone-incorrect-roundings");
CheckFactories.registerCheck<IntegerDivisionCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
