[clang-tidy] Add check 'bugprone-cast-to-struct' #153428
Conversation
|
@llvm/pr-subscribers-clang-tidy Author: Balázs Kéri (balazske) ChangesThis check is based on the static analyzer checker Full diff: https://github.com/llvm/llvm-project/pull/153428.diff 9 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 824ebdfbd00dc..5a1573eb34ee3 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -18,6 +18,7 @@
#include "BranchCloneCheck.h"
#include "CapturingThisInMemberVariableCheck.h"
#include "CastingThroughVoidCheck.h"
+#include "CastToStructCheck.h"
#include "ChainedComparisonCheck.h"
#include "ComparePointerToMemberVirtualFunctionCheck.h"
#include "CopyConstructorInitCheck.h"
@@ -125,6 +126,7 @@ class BugproneModule : public ClangTidyModule {
"bugprone-capturing-this-in-member-variable");
CheckFactories.registerCheck<CastingThroughVoidCheck>(
"bugprone-casting-through-void");
+ CheckFactories.registerCheck<CastToStructCheck>("bugprone-cast-to-struct");
CheckFactories.registerCheck<ChainedComparisonCheck>(
"bugprone-chained-comparison");
CheckFactories.registerCheck<ComparePointerToMemberVirtualFunctionCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 59928e5e47a09..c2bd0d1e3456c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -14,6 +14,7 @@ add_clang_library(clangTidyBugproneModule STATIC
BugproneTidyModule.cpp
CapturingThisInMemberVariableCheck.cpp
CastingThroughVoidCheck.cpp
+ CastToStructCheck.cpp
ChainedComparisonCheck.cpp
ComparePointerToMemberVirtualFunctionCheck.cpp
CopyConstructorInitCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.cpp
new file mode 100644
index 0000000000000..374736d48d1c8
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.cpp
@@ -0,0 +1,82 @@
+//===--- CastToStructCheck.cpp - clang-tidy -------------------------------===//
+//
+// 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 "CastToStructCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+CastToStructCheck::CastToStructCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ IgnoredFunctions(
+ utils::options::parseStringList(Options.get("IgnoredFunctions", ""))),
+ IgnoredFromTypes(
+ utils::options::parseStringList(Options.get("IgnoredFromTypes", ""))),
+ IgnoredToTypes(
+ utils::options::parseStringList(Options.get("IgnoredToTypes", ""))) {}
+
+void CastToStructCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IgnoredFunctions",
+ utils::options::serializeStringList(IgnoredFunctions));
+ Options.store(Opts, "IgnoredFromTypes",
+ utils::options::serializeStringList(IgnoredFromTypes));
+ Options.store(Opts, "IgnoredToTypes",
+ utils::options::serializeStringList(IgnoredToTypes));
+}
+
+void CastToStructCheck::registerMatchers(MatchFinder *Finder) {
+ auto FromPointee =
+ qualType(hasUnqualifiedDesugaredType(type().bind("FromType")),
+ unless(qualType(matchers::matchesAnyListedTypeName(
+ IgnoredFromTypes, false))))
+ .bind("FromPointee");
+ auto ToPointee =
+ qualType(hasUnqualifiedDesugaredType(recordType().bind("ToType")),
+ unless(qualType(
+ matchers::matchesAnyListedTypeName(IgnoredToTypes, false))))
+ .bind("ToPointee");
+ auto FromPtrType = qualType(pointsTo(FromPointee)).bind("FromPtr");
+ auto ToPtrType = qualType(pointsTo(ToPointee)).bind("ToPtr");
+ Finder->addMatcher(
+ cStyleCastExpr(hasSourceExpression(hasType(FromPtrType)),
+ hasType(ToPtrType),
+ unless(hasAncestor(functionDecl(
+ matchers::matchesAnyListedName(IgnoredFunctions)))))
+ .bind("CastExpr"),
+ this);
+}
+
+void CastToStructCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *const FoundCastExpr =
+ Result.Nodes.getNodeAs<CStyleCastExpr>("CastExpr");
+ const auto *const FromPtr = Result.Nodes.getNodeAs<QualType>("FromPtr");
+ const auto *const ToPtr = Result.Nodes.getNodeAs<QualType>("ToPtr");
+ const auto *const FromPointee =
+ Result.Nodes.getNodeAs<QualType>("FromPointee");
+ const auto *const ToPointee = Result.Nodes.getNodeAs<QualType>("ToPointee");
+ const auto *const FromType = Result.Nodes.getNodeAs<Type>("FromType");
+ const auto *const ToType = Result.Nodes.getNodeAs<RecordType>("ToType");
+ if (!FromPointee || !ToPointee)
+ return;
+ if (FromType->isVoidType() || FromType->isUnionType() ||
+ ToType->isUnionType())
+ return;
+ if (FromType == ToType)
+ return;
+ diag(FoundCastExpr->getExprLoc(),
+ "casting a %0 pointer to a "
+ "%1 pointer and accessing a field can lead to memory "
+ "access errors or data corruption")
+ << *FromPtr << *ToPtr;
+}
+
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.h b/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.h
new file mode 100644
index 0000000000000..9e6c868824d23
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.h
@@ -0,0 +1,39 @@
+//===--- CastToStructCheck.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_CASTTOSTRUCTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CASTTOSTRUCTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Finds casts from pointers to struct or scalar type to pointers to struct
+/// type.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/cast-to-struct.html
+class CastToStructCheck : public ClangTidyCheck {
+public:
+ CastToStructCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.C99;
+ }
+
+private:
+ std::vector<llvm::StringRef> IgnoredFunctions;
+ std::vector<llvm::StringRef> IgnoredFromTypes;
+ std::vector<llvm::StringRef> IgnoredToTypes;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CASTTOSTRUCTCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 187aae2ec8c90..a13072123b9ef 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -112,6 +112,11 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
+- New :doc:`bugprone-cast-to-struct
+ <clang-tidy/checks/bugprone/cast-to-struct>` check.
+
+ Finds casts from pointers to struct or scalar type to pointers to struct type.
+
- New :doc:`bugprone-invalid-enum-default-initialization
<clang-tidy/checks/bugprone/invalid-enum-default-initialization>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/cast-to-struct.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/cast-to-struct.rst
new file mode 100644
index 0000000000000..79864935d3ea1
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/cast-to-struct.rst
@@ -0,0 +1,54 @@
+.. title:: clang-tidy - bugprone-cast-to-struct
+
+bugprone-cast-to-struct
+=======================
+
+Finds casts from pointers to struct or scalar type to pointers to struct type.
+
+Casts between pointers to different structs can be unsafe because it is possible
+to access uninitialized or undefined data after the cast. There may be issues
+with type compatibility or data alignment. Cast from a pointer to a scalar type
+(which points often to an array or memory block) to a `struct` type pointer can
+be unsafe for similar reasons. This check warns at casts from any non-`struct`
+type to a `struct` type. No warning is produced at cast from type `void *` (this
+is the usual way of allocating memory with `malloc`-like functions). It is
+possible to specify additional types to ignore by the check. In addition,
+`union` types are completely excluded from the check. The check does not take
+into account type compatibility or data layout, only the names of the types.
+
+.. code-block:: c
+
+ void test1(char *p) {
+ struct S1 *s;
+ s = (struct S1 *)p; // warn: 'char *' is converted to 'struct S1 *'
+ }
+
+ void test2(struct S1 *p) {
+ struct S2 *s;
+ s = (struct S2 *)p; // warn: 'struct S1 *' is converted to 'struct S2 *'
+ }
+
+ void test3(void) {
+ struct S1 *s;
+ s = (struct S1 *)calloc(1, sizeof(struct S1)); // no warning
+ }
+
+Options
+-------
+
+.. option:: IgnoredFromTypes
+
+ Semicolon-separated list of types for which the checker should not warn if
+ encountered at cast source. Can contain regular expressions. The `*`
+ character (for pointer type) is not needed in the type names.
+
+.. option:: IgnoredToTypes
+
+ Semicolon-separated list of types for which the checker should not warn if
+ encountered at cast destination. Can contain regular expressions. The `*`
+ character (for pointer type) is not needed in the type names.
+
+.. option:: IgnoredFunctions
+
+ List of function names from which the checker should produce no warnings. Can
+ contain regular expressions.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index b6444eb3c9aec..5f8ae4a6eabb7 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -85,6 +85,7 @@ Clang-Tidy Checks
:doc:`bugprone-bool-pointer-implicit-conversion <bugprone/bool-pointer-implicit-conversion>`, "Yes"
:doc:`bugprone-branch-clone <bugprone/branch-clone>`,
:doc:`bugprone-capturing-this-in-member-variable <bugprone/capturing-this-in-member-variable>`,
+ :doc:`bugprone-cast-to-struct <bugprone/cast-to-struct>`,
:doc:`bugprone-casting-through-void <bugprone/casting-through-void>`,
:doc:`bugprone-chained-comparison <bugprone/chained-comparison>`,
:doc:`bugprone-compare-pointer-to-member-virtual-function <bugprone/compare-pointer-to-member-virtual-function>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct-ignore.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct-ignore.c
new file mode 100644
index 0000000000000..1cc4744220f4a
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct-ignore.c
@@ -0,0 +1,40 @@
+// RUN: %check_clang_tidy -check-suffixes=FUNC %s bugprone-cast-to-struct %t -- \
+// RUN: -config="{CheckOptions: {bugprone-cast-to-struct.IgnoredFunctions: 'ignored_f$'}}"
+// RUN: %check_clang_tidy -check-suffixes=FROM-TY %s bugprone-cast-to-struct %t -- \
+// RUN: -config="{CheckOptions: {bugprone-cast-to-struct.IgnoredFromTypes: 'int'}}"
+// RUN: %check_clang_tidy -check-suffixes=TO-TY %s bugprone-cast-to-struct %t -- \
+// RUN: -config="{CheckOptions: {bugprone-cast-to-struct.IgnoredToTypes: 'IgnoredType'}}"
+
+struct IgnoredType {
+ int a;
+};
+
+struct OtherType {
+ int a;
+ int b;
+};
+
+void ignored_f(char *p) {
+ struct OtherType *p1;
+ p1 = (struct OtherType *)p;
+ // CHECK-MESSAGES-FROM-TY: :[[@LINE-1]]:8: warning: casting a 'char *' pointer to a 'struct OtherType *' pointer and accessing a field can lead to memory access errors or data corruption
+ // CHECK-MESSAGES-TO-TY: :[[@LINE-2]]:8: warning: casting a 'char *' pointer to a 'struct OtherType *' pointer and accessing a field can lead to memory access errors or data corruption
+}
+
+void ignored_from_type(int *p) {
+ struct OtherType *p1;
+ p1 = (struct OtherType *)p;
+ // CHECK-MESSAGES-FUNC: :[[@LINE-1]]:8: warning: casting a 'int *' pointer to a 'struct OtherType *' pointer and accessing a field can lead to memory access errors or data corruption
+ // CHECK-MESSAGES-TO-TY: :[[@LINE-2]]:8: warning: casting a 'int *' pointer to a 'struct OtherType *' pointer and accessing a field can lead to memory access errors or data corruption
+}
+
+void ignored_to_type(char *p) {
+ struct IgnoredType *p1;
+ p1 = (struct IgnoredType *)p;
+ // CHECK-MESSAGES-FUNC: :[[@LINE-1]]:8: warning: casting a 'char *' pointer to a 'struct IgnoredType *' pointer and accessing a field can lead to memory access errors or data corruption
+ // CHECK-MESSAGES-FROM-TY: :[[@LINE-2]]:8: warning: casting a 'char *' pointer to a 'struct IgnoredType *' pointer and accessing a field can lead to memory access errors or data corruption
+}
+
+struct OtherType *test_void_is_always_ignored(void *p) {
+ return (struct OtherType *)p;
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct.c
new file mode 100644
index 0000000000000..cfabf69351f39
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct.c
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s bugprone-cast-to-struct %t
+
+struct S1 {
+ int a;
+};
+
+struct S2 {
+ char a;
+};
+
+union U1 {
+ int a;
+ char b;
+};
+
+union U2 {
+ struct S1 a;
+ char b;
+};
+
+typedef struct S1 TyS1;
+typedef struct S1 *TyPS1;
+
+typedef union U1 *TyPU1;
+
+typedef int int_t;
+typedef int * int_ptr_t;
+
+struct S1 *test_simple(char *p) {
+ return (struct S1 *)p;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: casting a 'char *' pointer to a 'struct S1 *' pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+ struct S1 *s;
+ int i;
+ s = (struct S1 *)&i;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: casting a 'int *' pointer to a 'struct S1 *' pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+}
+
+struct S1 *test_cast_from_void(void *p) {
+ return (struct S1 *)p;
+}
+
+struct S1 *test_cast_from_struct(struct S2 *p) {
+ return (struct S1 *)p;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: casting a 'struct S2 *' pointer to a 'struct S1 *' pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+}
+
+TyPS1 test_cast_from_similar(struct S1 *p) {
+ return (TyPS1)p;
+}
+
+void test_typedef(char *p1, int_t *p2, int_ptr_t p3) {
+ TyS1 *a = (TyS1 *)p1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: casting a 'char *' pointer to a 'TyS1 *' (aka 'struct S1 *') pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+ TyPS1 b = (TyPS1)p1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: casting a 'char *' pointer to a 'TyPS1' (aka 'struct S1 *') pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+ struct S1 *c = (struct S1 *)p2;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: casting a 'int_t *' (aka 'int *') pointer to a 'struct S1 *' pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+ struct S1 *d = (struct S1 *)p3;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: casting a 'int_ptr_t' (aka 'int *') pointer to a 'struct S1 *' pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+}
+
+void test_union(char *p1, union U1 *p2, TyPU1 p3) {
+ union U1 *a = (union U1 *)p1;
+ struct S1 *b = (struct S1 *)p2;
+ struct S1 *c = (struct S1 *)p3;
+}
|
|
@llvm/pr-subscribers-clang-tools-extra Author: Balázs Kéri (balazske) ChangesThis check is based on the static analyzer checker Full diff: https://github.com/llvm/llvm-project/pull/153428.diff 9 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 824ebdfbd00dc..5a1573eb34ee3 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -18,6 +18,7 @@
#include "BranchCloneCheck.h"
#include "CapturingThisInMemberVariableCheck.h"
#include "CastingThroughVoidCheck.h"
+#include "CastToStructCheck.h"
#include "ChainedComparisonCheck.h"
#include "ComparePointerToMemberVirtualFunctionCheck.h"
#include "CopyConstructorInitCheck.h"
@@ -125,6 +126,7 @@ class BugproneModule : public ClangTidyModule {
"bugprone-capturing-this-in-member-variable");
CheckFactories.registerCheck<CastingThroughVoidCheck>(
"bugprone-casting-through-void");
+ CheckFactories.registerCheck<CastToStructCheck>("bugprone-cast-to-struct");
CheckFactories.registerCheck<ChainedComparisonCheck>(
"bugprone-chained-comparison");
CheckFactories.registerCheck<ComparePointerToMemberVirtualFunctionCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 59928e5e47a09..c2bd0d1e3456c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -14,6 +14,7 @@ add_clang_library(clangTidyBugproneModule STATIC
BugproneTidyModule.cpp
CapturingThisInMemberVariableCheck.cpp
CastingThroughVoidCheck.cpp
+ CastToStructCheck.cpp
ChainedComparisonCheck.cpp
ComparePointerToMemberVirtualFunctionCheck.cpp
CopyConstructorInitCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.cpp
new file mode 100644
index 0000000000000..374736d48d1c8
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.cpp
@@ -0,0 +1,82 @@
+//===--- CastToStructCheck.cpp - clang-tidy -------------------------------===//
+//
+// 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 "CastToStructCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+CastToStructCheck::CastToStructCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ IgnoredFunctions(
+ utils::options::parseStringList(Options.get("IgnoredFunctions", ""))),
+ IgnoredFromTypes(
+ utils::options::parseStringList(Options.get("IgnoredFromTypes", ""))),
+ IgnoredToTypes(
+ utils::options::parseStringList(Options.get("IgnoredToTypes", ""))) {}
+
+void CastToStructCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IgnoredFunctions",
+ utils::options::serializeStringList(IgnoredFunctions));
+ Options.store(Opts, "IgnoredFromTypes",
+ utils::options::serializeStringList(IgnoredFromTypes));
+ Options.store(Opts, "IgnoredToTypes",
+ utils::options::serializeStringList(IgnoredToTypes));
+}
+
+void CastToStructCheck::registerMatchers(MatchFinder *Finder) {
+ auto FromPointee =
+ qualType(hasUnqualifiedDesugaredType(type().bind("FromType")),
+ unless(qualType(matchers::matchesAnyListedTypeName(
+ IgnoredFromTypes, false))))
+ .bind("FromPointee");
+ auto ToPointee =
+ qualType(hasUnqualifiedDesugaredType(recordType().bind("ToType")),
+ unless(qualType(
+ matchers::matchesAnyListedTypeName(IgnoredToTypes, false))))
+ .bind("ToPointee");
+ auto FromPtrType = qualType(pointsTo(FromPointee)).bind("FromPtr");
+ auto ToPtrType = qualType(pointsTo(ToPointee)).bind("ToPtr");
+ Finder->addMatcher(
+ cStyleCastExpr(hasSourceExpression(hasType(FromPtrType)),
+ hasType(ToPtrType),
+ unless(hasAncestor(functionDecl(
+ matchers::matchesAnyListedName(IgnoredFunctions)))))
+ .bind("CastExpr"),
+ this);
+}
+
+void CastToStructCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *const FoundCastExpr =
+ Result.Nodes.getNodeAs<CStyleCastExpr>("CastExpr");
+ const auto *const FromPtr = Result.Nodes.getNodeAs<QualType>("FromPtr");
+ const auto *const ToPtr = Result.Nodes.getNodeAs<QualType>("ToPtr");
+ const auto *const FromPointee =
+ Result.Nodes.getNodeAs<QualType>("FromPointee");
+ const auto *const ToPointee = Result.Nodes.getNodeAs<QualType>("ToPointee");
+ const auto *const FromType = Result.Nodes.getNodeAs<Type>("FromType");
+ const auto *const ToType = Result.Nodes.getNodeAs<RecordType>("ToType");
+ if (!FromPointee || !ToPointee)
+ return;
+ if (FromType->isVoidType() || FromType->isUnionType() ||
+ ToType->isUnionType())
+ return;
+ if (FromType == ToType)
+ return;
+ diag(FoundCastExpr->getExprLoc(),
+ "casting a %0 pointer to a "
+ "%1 pointer and accessing a field can lead to memory "
+ "access errors or data corruption")
+ << *FromPtr << *ToPtr;
+}
+
+} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.h b/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.h
new file mode 100644
index 0000000000000..9e6c868824d23
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.h
@@ -0,0 +1,39 @@
+//===--- CastToStructCheck.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_CASTTOSTRUCTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CASTTOSTRUCTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Finds casts from pointers to struct or scalar type to pointers to struct
+/// type.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/cast-to-struct.html
+class CastToStructCheck : public ClangTidyCheck {
+public:
+ CastToStructCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.C99;
+ }
+
+private:
+ std::vector<llvm::StringRef> IgnoredFunctions;
+ std::vector<llvm::StringRef> IgnoredFromTypes;
+ std::vector<llvm::StringRef> IgnoredToTypes;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CASTTOSTRUCTCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 187aae2ec8c90..a13072123b9ef 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -112,6 +112,11 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
+- New :doc:`bugprone-cast-to-struct
+ <clang-tidy/checks/bugprone/cast-to-struct>` check.
+
+ Finds casts from pointers to struct or scalar type to pointers to struct type.
+
- New :doc:`bugprone-invalid-enum-default-initialization
<clang-tidy/checks/bugprone/invalid-enum-default-initialization>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/cast-to-struct.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/cast-to-struct.rst
new file mode 100644
index 0000000000000..79864935d3ea1
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/cast-to-struct.rst
@@ -0,0 +1,54 @@
+.. title:: clang-tidy - bugprone-cast-to-struct
+
+bugprone-cast-to-struct
+=======================
+
+Finds casts from pointers to struct or scalar type to pointers to struct type.
+
+Casts between pointers to different structs can be unsafe because it is possible
+to access uninitialized or undefined data after the cast. There may be issues
+with type compatibility or data alignment. Cast from a pointer to a scalar type
+(which points often to an array or memory block) to a `struct` type pointer can
+be unsafe for similar reasons. This check warns at casts from any non-`struct`
+type to a `struct` type. No warning is produced at cast from type `void *` (this
+is the usual way of allocating memory with `malloc`-like functions). It is
+possible to specify additional types to ignore by the check. In addition,
+`union` types are completely excluded from the check. The check does not take
+into account type compatibility or data layout, only the names of the types.
+
+.. code-block:: c
+
+ void test1(char *p) {
+ struct S1 *s;
+ s = (struct S1 *)p; // warn: 'char *' is converted to 'struct S1 *'
+ }
+
+ void test2(struct S1 *p) {
+ struct S2 *s;
+ s = (struct S2 *)p; // warn: 'struct S1 *' is converted to 'struct S2 *'
+ }
+
+ void test3(void) {
+ struct S1 *s;
+ s = (struct S1 *)calloc(1, sizeof(struct S1)); // no warning
+ }
+
+Options
+-------
+
+.. option:: IgnoredFromTypes
+
+ Semicolon-separated list of types for which the checker should not warn if
+ encountered at cast source. Can contain regular expressions. The `*`
+ character (for pointer type) is not needed in the type names.
+
+.. option:: IgnoredToTypes
+
+ Semicolon-separated list of types for which the checker should not warn if
+ encountered at cast destination. Can contain regular expressions. The `*`
+ character (for pointer type) is not needed in the type names.
+
+.. option:: IgnoredFunctions
+
+ List of function names from which the checker should produce no warnings. Can
+ contain regular expressions.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index b6444eb3c9aec..5f8ae4a6eabb7 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -85,6 +85,7 @@ Clang-Tidy Checks
:doc:`bugprone-bool-pointer-implicit-conversion <bugprone/bool-pointer-implicit-conversion>`, "Yes"
:doc:`bugprone-branch-clone <bugprone/branch-clone>`,
:doc:`bugprone-capturing-this-in-member-variable <bugprone/capturing-this-in-member-variable>`,
+ :doc:`bugprone-cast-to-struct <bugprone/cast-to-struct>`,
:doc:`bugprone-casting-through-void <bugprone/casting-through-void>`,
:doc:`bugprone-chained-comparison <bugprone/chained-comparison>`,
:doc:`bugprone-compare-pointer-to-member-virtual-function <bugprone/compare-pointer-to-member-virtual-function>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct-ignore.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct-ignore.c
new file mode 100644
index 0000000000000..1cc4744220f4a
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct-ignore.c
@@ -0,0 +1,40 @@
+// RUN: %check_clang_tidy -check-suffixes=FUNC %s bugprone-cast-to-struct %t -- \
+// RUN: -config="{CheckOptions: {bugprone-cast-to-struct.IgnoredFunctions: 'ignored_f$'}}"
+// RUN: %check_clang_tidy -check-suffixes=FROM-TY %s bugprone-cast-to-struct %t -- \
+// RUN: -config="{CheckOptions: {bugprone-cast-to-struct.IgnoredFromTypes: 'int'}}"
+// RUN: %check_clang_tidy -check-suffixes=TO-TY %s bugprone-cast-to-struct %t -- \
+// RUN: -config="{CheckOptions: {bugprone-cast-to-struct.IgnoredToTypes: 'IgnoredType'}}"
+
+struct IgnoredType {
+ int a;
+};
+
+struct OtherType {
+ int a;
+ int b;
+};
+
+void ignored_f(char *p) {
+ struct OtherType *p1;
+ p1 = (struct OtherType *)p;
+ // CHECK-MESSAGES-FROM-TY: :[[@LINE-1]]:8: warning: casting a 'char *' pointer to a 'struct OtherType *' pointer and accessing a field can lead to memory access errors or data corruption
+ // CHECK-MESSAGES-TO-TY: :[[@LINE-2]]:8: warning: casting a 'char *' pointer to a 'struct OtherType *' pointer and accessing a field can lead to memory access errors or data corruption
+}
+
+void ignored_from_type(int *p) {
+ struct OtherType *p1;
+ p1 = (struct OtherType *)p;
+ // CHECK-MESSAGES-FUNC: :[[@LINE-1]]:8: warning: casting a 'int *' pointer to a 'struct OtherType *' pointer and accessing a field can lead to memory access errors or data corruption
+ // CHECK-MESSAGES-TO-TY: :[[@LINE-2]]:8: warning: casting a 'int *' pointer to a 'struct OtherType *' pointer and accessing a field can lead to memory access errors or data corruption
+}
+
+void ignored_to_type(char *p) {
+ struct IgnoredType *p1;
+ p1 = (struct IgnoredType *)p;
+ // CHECK-MESSAGES-FUNC: :[[@LINE-1]]:8: warning: casting a 'char *' pointer to a 'struct IgnoredType *' pointer and accessing a field can lead to memory access errors or data corruption
+ // CHECK-MESSAGES-FROM-TY: :[[@LINE-2]]:8: warning: casting a 'char *' pointer to a 'struct IgnoredType *' pointer and accessing a field can lead to memory access errors or data corruption
+}
+
+struct OtherType *test_void_is_always_ignored(void *p) {
+ return (struct OtherType *)p;
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct.c
new file mode 100644
index 0000000000000..cfabf69351f39
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct.c
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s bugprone-cast-to-struct %t
+
+struct S1 {
+ int a;
+};
+
+struct S2 {
+ char a;
+};
+
+union U1 {
+ int a;
+ char b;
+};
+
+union U2 {
+ struct S1 a;
+ char b;
+};
+
+typedef struct S1 TyS1;
+typedef struct S1 *TyPS1;
+
+typedef union U1 *TyPU1;
+
+typedef int int_t;
+typedef int * int_ptr_t;
+
+struct S1 *test_simple(char *p) {
+ return (struct S1 *)p;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: casting a 'char *' pointer to a 'struct S1 *' pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+ struct S1 *s;
+ int i;
+ s = (struct S1 *)&i;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: casting a 'int *' pointer to a 'struct S1 *' pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+}
+
+struct S1 *test_cast_from_void(void *p) {
+ return (struct S1 *)p;
+}
+
+struct S1 *test_cast_from_struct(struct S2 *p) {
+ return (struct S1 *)p;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: casting a 'struct S2 *' pointer to a 'struct S1 *' pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+}
+
+TyPS1 test_cast_from_similar(struct S1 *p) {
+ return (TyPS1)p;
+}
+
+void test_typedef(char *p1, int_t *p2, int_ptr_t p3) {
+ TyS1 *a = (TyS1 *)p1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: casting a 'char *' pointer to a 'TyS1 *' (aka 'struct S1 *') pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+ TyPS1 b = (TyPS1)p1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: casting a 'char *' pointer to a 'TyPS1' (aka 'struct S1 *') pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+ struct S1 *c = (struct S1 *)p2;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: casting a 'int_t *' (aka 'int *') pointer to a 'struct S1 *' pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+ struct S1 *d = (struct S1 *)p3;
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: casting a 'int_ptr_t' (aka 'int *') pointer to a 'struct S1 *' pointer and accessing a field can lead to memory access errors or data corruption [bugprone-cast-to-struct]
+}
+
+void test_union(char *p1, union U1 *p2, TyPU1 p3) {
+ union U1 *a = (union U1 *)p1;
+ struct S1 *b = (struct S1 *)p2;
+ struct S1 *c = (struct S1 *)p3;
+}
|
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions c,cpp,h -- clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.cpp clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.h clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct-ignore.c clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct.c clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
View the diff from clang-format here.diff --git a/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.cpp
index 0a18ae9b1..f6913b0e8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.cpp
@@ -17,14 +17,10 @@ namespace clang::tidy::bugprone {
namespace {
-AST_MATCHER(Type, charType) {
- return Node.isCharType();
-}
-AST_MATCHER(Type, unionType) {
- return Node.isUnionType();
-}
+AST_MATCHER(Type, charType) { return Node.isCharType(); }
+AST_MATCHER(Type, unionType) { return Node.isUnionType(); }
-}
+} // namespace
CastToStructCheck::CastToStructCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
@@ -49,9 +45,7 @@ void CastToStructCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
void CastToStructCheck::registerMatchers(MatchFinder *Finder) {
auto FromPointee =
qualType(hasUnqualifiedDesugaredType(type().bind("FromType")),
- unless(voidType()),
- unless(charType()),
- unless(unionType()))
+ unless(voidType()), unless(charType()), unless(unionType()))
.bind("FromPointee");
auto ToPointee =
qualType(hasUnqualifiedDesugaredType(
@@ -77,7 +71,8 @@ void CastToStructCheck::check(const MatchFinder::MatchResult &Result) {
if (FromType == ToType)
return;
- auto CheckNameIgnore = [this](const std::string &FromName, const std::string &ToName) {
+ auto CheckNameIgnore = [this](const std::string &FromName,
+ const std::string &ToName) {
bool FromMatch = false;
for (auto [Idx, Regex] : llvm::enumerate(IgnoredCastsRegex)) {
if (Idx % 2 == 0) {
|
| - New :doc:`bugprone-cast-to-struct | ||
| <clang-tidy/checks/bugprone/cast-to-struct>` check. | ||
|
|
||
| Finds casts from pointers to struct or scalar type to pointers to struct type. |
There was a problem hiding this comment.
| Finds casts from pointers to struct or scalar type to pointers to struct type. | |
| Finds casts from pointers to ``struct`` or scalar type to pointers to ``struct`` type. |
There was a problem hiding this comment.
Here I do not want to use the word "struct" as a C keyword, instead to describe the type created with struct keyword, in this case I think the code style is not needed.
| bugprone-cast-to-struct | ||
| ======================= | ||
|
|
||
| Finds casts from pointers to struct or scalar type to pointers to struct type. |
There was a problem hiding this comment.
| Finds casts from pointers to struct or scalar type to pointers to struct type. | |
| Finds casts from pointers to ``struct`` or scalar type to pointers to ``struct`` type. |
| Casts between pointers to different structs can be unsafe because it is possible | ||
| to access uninitialized or undefined data after the cast. There may be issues | ||
| with type compatibility or data alignment. Cast from a pointer to a scalar type | ||
| (which points often to an array or memory block) to a `struct` type pointer can |
There was a problem hiding this comment.
Please use double back-ticks for language constructs. Same in other places.
|
|
||
| Semicolon-separated list of types for which the checker should not warn if | ||
| encountered at cast source. Can contain regular expressions. The `*` | ||
| character (for pointer type) is not needed in the type names. |
There was a problem hiding this comment.
Please add default value. Same for other options.
|
|
||
| .. option:: IgnoredFromTypes | ||
|
|
||
| Semicolon-separated list of types for which the checker should not warn if |
There was a problem hiding this comment.
| Semicolon-separated list of types for which the checker should not warn if | |
| Semicolon-separated list of types for which the check should not warn if |
Same in other places.
| void registerMatchers(ast_matchers::MatchFinder *Finder) override; | ||
| void check(const ast_matchers::MatchFinder::MatchResult &Result) override; | ||
| bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { | ||
| return LangOpts.C99; |
There was a problem hiding this comment.
This check should also be active in C++ (and it should include C++ tests using C-style cast and reinterpret_cast
There was a problem hiding this comment.
I think in C++ the problem can be handled in different way. C-style cast is discouraged in C++, so all C style casts should be converted to other kind of cast (this is job of another checker, probably it exists already). The reinterpret_cast indicates already by syntax that it is a potentially dangerous operation and many uses of reinterpret_cast would be likely found by the checker (if it checks for it in similar way), so a checker is not much more than find all uses of reinterpret_cast. It is possible to check exact data layout but probably still many false positives would appear. Additionally in C++ the casting rules are more complicated at least because inheritance.
There was a problem hiding this comment.
Could we then add "Limitations" that this check does not run on C++ code. Similar to how https://clang.llvm.org/extra/clang-tidy/checks/misc/const-correctness.html states that it does not run on C code.
There was a problem hiding this comment.
If you want to limit to C, probably not bad idea would be to check if Cplusplus flag is not set.
| Finds casts from pointers to struct or scalar type to pointers to struct type. | ||
|
|
||
| Casts between pointers to different structs can be unsafe because it is possible | ||
| to access uninitialized or undefined data after the cast. There may be issues |
There was a problem hiding this comment.
I think we should simply state it's undefined behavior without entering into details about what a specific compiler may or may not do in practice.
| encountered at cast destination. Can contain regular expressions. The `*` | ||
| character (for pointer type) is not needed in the type names. | ||
|
|
||
| .. option:: IgnoredFunctions |
There was a problem hiding this comment.
I think we should remove this option, as it can be easily abused. If one needs to perform these dangerous operations (despite them being UB), they should have a big NOLINT suppression with a long comment explaining why they are choosing to deviate and why it's safe.
I feel similarly about the other two options, but not as strongly ;)
There was a problem hiding this comment.
Agree, other options make sense that types are used in many-many places, but functions don't tend to have duplicate names.
There was a problem hiding this comment.
I added the ignored function option to remove false positives coming from system headers like xmmintrin.h from many functions like this:
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_loadu_ps(const float *__p)
{
struct __loadu_ps {
__m128_u __v;
} __attribute__((__packed__, __may_alias__));
return ((const struct __loadu_ps*)__p)->__v; // warning: cast from 'const float *' to 'const struct __loadu_ps *'
}Probably the type name __loadu_ps or similar can be enough, or the case can be detected in other way (check for specific function attributes like nodebug which is defined in __DEFAULT_FN_ATTRS can work for this case). I wanted to have a solution to remove similar warnings. Sometimes such casts appear in specific macros, then a similar option for macro names would be needed. Or just do not emit warnings if it comes from system header.
There was a problem hiding this comment.
Or just do not emit warnings if it comes from system header.
Yes, we should skip warnings in system headers. You might want to try rebasing on my patch and see if you still get any (even though clang-tidy should already be filtering those out by default).
There was a problem hiding this comment.
These warning must be when only when --system-headers is enabled. Also, you could use unless(isExpansionInSystemHeader()) in matchers.
I feel a bit at unease reading this. Why can't we improve the existing check, instead of duplicating it? |
It could be that all path-sensitive checks should live in |
| if (FromType->isVoidType() || FromType->isUnionType() || | ||
| ToType->isUnionType()) |
There was a problem hiding this comment.
Should be placed inside AST matchers for readability and speedup
| if (!FromPointee || !ToPointee) | ||
| return; |
There was a problem hiding this comment.
We expect both to be matched, so could we write assert(FromPointee && ToPointee)
The reason was that this check is not path-sensitive and these should be put into |
@carlosgalvezp This commit doesn't intend to duplicate the existing check, the planned follow-up is that the unfinished low-quality Originally we planned to improve that
@balazske The original check ( The Static Analyzer checkers can access dynamic type information if they use the "usual" path-sensitive framework (which follows execution step by step and can guess the dynamic type or memory layout based on that); but the existing code of Also note that the dynamic type information of the Static Analyzer is not too reliable, and this check is specific to C where dynamic types are less relevant and less well-defined (compared to C++), so I don't think that it would be a fruitful direction of development. |
Thanks for the clarification, it was indeed written |
|
I have improved the documentation and changed the options. Now there is a single option to specify type pairs (cast-from and cast-to types) to ignore, this is more flexible than the previous way. |
|
Would it make sense to extend the warning to any pointer cast, not only |
I feel that it is reasonable to limit this to |
At least name should be generic now. |
|
|
||
| The check does run only on `C` code. | ||
|
|
||
| C-style casts are discouraged in `C++` and should be converted to more type-safe |
There was a problem hiding this comment.
| C-style casts are discouraged in `C++` and should be converted to more type-safe | |
| C-style casts are discouraged in C++ and should be converted to more type-safe |
| types to ignore. The list should contain pairs of type names in a way that | ||
| the first type is the "from" type, the second is the "to" type in a cast | ||
| expression. The types in a pair and the pairs itself are separated by | ||
| ``;`` characters. For example ``char;Type1;char;Type2`` specifies that the |
There was a problem hiding this comment.
| ``;`` characters. For example ``char;Type1;char;Type2`` specifies that the | |
| `;` characters. For example `char;Type1;char;Type2` specifies that the |
clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct-ignore.c
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Consider changing name, to: bugprone-suspicious-cast-to-struct-pointer.
When I first read name of check I immediately had this construction in mind:
struct A { ... };
struct B : A { ... };
const A a;
const B& b = (B)a;
Current name is too generic.
- Consider extending this to C++.
- Consider ignoring char/u8* types by default. Consider supporting typedefs (you may need to resolve them 1 by 1).
- Consider ignoring system headers.
- Consider using TK_IgnoreUnlessSpelledInSource explicitly.
| Limitations | ||
| ----------- | ||
|
|
||
| The check does run only on `C` code. |
| C-style casts are discouraged in C++ and should be converted to more type-safe | ||
| casts. The ``reinterpreted_cast`` is used for the most unsafe cases and | ||
| indicates by itself a potentially dangerous operation. Additionally, inheritance | ||
| and dynamic types would make such a check less useful. |
There was a problem hiding this comment.
No point to mention, as this is check for C.
| void registerMatchers(ast_matchers::MatchFinder *Finder) override; | ||
| void check(const ast_matchers::MatchFinder::MatchResult &Result) override; | ||
| bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { | ||
| return LangOpts.C99; |
There was a problem hiding this comment.
If you want to limit to C, probably not bad idea would be to check if Cplusplus flag is not set.
|
|
||
| void test1(char *p) { | ||
| struct S1 *s; | ||
| s = (struct S1 *)p; // warn: 'char *' is converted to 'struct S1 *' |
There was a problem hiding this comment.
Not necessary good example, as char* or alias like u8* would be usually used as "buffer" in most of APIs. In such case it would create huge number of false-positives at start.
|
|
||
| diag(FoundCastExpr->getExprLoc(), | ||
| "casting a %0 pointer to a " | ||
| "%1 pointer and accessing a field can lead to memory " |
There was a problem hiding this comment.
you don't know if someone just did cast or did cast and access, then no point to mention access here.
There was a problem hiding this comment.
I think the problem is only if the new pointer is used for read or write (what is likely after the cast), this is why it is mentioned.
It was already considered in #153428 (comment).
For the ignore
This is done by default in clang-tidy.
|
|
|
|
I want to change the behavior of the ignore list to only match the type names "as written in the code" (without any resolution of |
This check is based on the static analyzer checker
alpha.core.CastToStruct. That checker does not look very useful in the current form and can be moved to a non path-sensitive checker. This new check does nearly the same thing, but has the "warn only for casts to bigger struct" condition removed, and ignored types can be added by configuration.