diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 824ebdfbd00dc..8ea24740b1e1d 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -17,6 +17,7 @@ #include "BoolPointerImplicitConversionCheck.h" #include "BranchCloneCheck.h" #include "CapturingThisInMemberVariableCheck.h" +#include "CastToStructCheck.h" #include "CastingThroughVoidCheck.h" #include "ChainedComparisonCheck.h" #include "ComparePointerToMemberVirtualFunctionCheck.h" @@ -125,6 +126,7 @@ class BugproneModule : public ClangTidyModule { "bugprone-capturing-this-in-member-variable"); CheckFactories.registerCheck( "bugprone-casting-through-void"); + CheckFactories.registerCheck("bugprone-cast-to-struct"); CheckFactories.registerCheck( "bugprone-chained-comparison"); CheckFactories.registerCheck( 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..0a18ae9b12547 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.cpp @@ -0,0 +1,103 @@ +//===--- 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 { + +namespace { + +AST_MATCHER(Type, charType) { + return Node.isCharType(); +} +AST_MATCHER(Type, unionType) { + return Node.isUnionType(); +} + +} + +CastToStructCheck::CastToStructCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IgnoredCasts( + utils::options::parseStringList(Options.get("IgnoredCasts", ""))) { + IgnoredCastsRegex.reserve(IgnoredCasts.size()); + for (const auto &Str : IgnoredCasts) { + std::string WholeWordRegex; + WholeWordRegex.reserve(Str.size() + 2); + WholeWordRegex.push_back('^'); + WholeWordRegex.append(Str); + WholeWordRegex.push_back('$'); + IgnoredCastsRegex.emplace_back(WholeWordRegex); + } +} + +void CastToStructCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoredCasts", + utils::options::serializeStringList(IgnoredCasts)); +} + +void CastToStructCheck::registerMatchers(MatchFinder *Finder) { + auto FromPointee = + qualType(hasUnqualifiedDesugaredType(type().bind("FromType")), + unless(voidType()), + unless(charType()), + unless(unionType())) + .bind("FromPointee"); + auto ToPointee = + qualType(hasUnqualifiedDesugaredType( + recordType(unless(hasDeclaration(recordDecl(isUnion())))) + .bind("ToType"))) + .bind("ToPointee"); + auto FromPtrType = qualType(pointsTo(FromPointee)).bind("FromPtr"); + auto ToPtrType = qualType(pointsTo(ToPointee)).bind("ToPtr"); + Finder->addMatcher(cStyleCastExpr(hasSourceExpression(hasType(FromPtrType)), + hasType(ToPtrType)) + .bind("CastExpr"), + this); +} + +void CastToStructCheck::check(const MatchFinder::MatchResult &Result) { + const auto *const FoundCastExpr = + Result.Nodes.getNodeAs("CastExpr"); + const auto *const FromPtr = Result.Nodes.getNodeAs("FromPtr"); + const auto *const ToPtr = Result.Nodes.getNodeAs("ToPtr"); + const auto *const FromType = Result.Nodes.getNodeAs("FromType"); + const auto *const ToType = Result.Nodes.getNodeAs("ToType"); + + if (FromType == ToType) + return; + + 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) { + FromMatch = Regex.match(FromName); + } else { + if (FromMatch && Regex.match(ToName)) + return true; + } + } + return false; + }; + + if (CheckNameIgnore(FromPtr->getAsString(), ToPtr->getAsString())) + 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..20bc9766c3262 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.h @@ -0,0 +1,38 @@ +//===--- 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.CPlusPlus; + } + +private: + std::vector IgnoredCasts; + std::vector IgnoredCastsRegex; +}; + +} // 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 + ` check. + + Finds casts from pointers to struct or scalar type to pointers to struct type. + - New :doc:`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..06fbd8adc4a66 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/cast-to-struct.rst @@ -0,0 +1,64 @@ +.. 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. Cast from a +scalar-type pointer (which points often to an array or memory block) to a +``struct`` type pointer can be unsafe for similar reasons. This check warns at +pointer 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) and ``char *`` types (which are used often as +pointers into data buffers). In addition, ``union`` types are excluded from the +check. It is possible to specify additional types to ignore. The check does not +take into account type compatibility or data layout, only the names of the +types. + +.. code-block:: c + + void test1(int *p) { + struct S1 *s; + s = (struct S1 *)p; // warn: 'int *' 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 + } + +Limitations +----------- + +The check does not run 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. + +Options +------- + +.. option:: IgnoredCasts + + Can contain a semicolon-separated list of type names that specify cast + 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. The parts between `;` characters are matched as regular + expressions over the whole type name. For example + `struct S1 \*;struct T1 \*;short \*;struct T1 \*` specifies that the check + does not produce warning for casts from ``struct S1 *`` to ``struct T1 *`` + and casts from ``short *`` to ``struct T1 *`` (the `*` character needs to be + escaped). The type name in the cast expression is matched without resolution + of ``typedef`` types. + + Default value of the option is an empty list. 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 `, "Yes" :doc:`bugprone-branch-clone `, :doc:`bugprone-capturing-this-in-member-variable `, + :doc:`bugprone-cast-to-struct `, :doc:`bugprone-casting-through-void `, :doc:`bugprone-chained-comparison `, :doc:`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..f1abcf57f2a3e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/cast-to-struct-ignore.c @@ -0,0 +1,63 @@ +// RUN: %check_clang_tidy %s bugprone-cast-to-struct %t -- \ +// RUN: -config="{CheckOptions: {bugprone-cast-to-struct.IgnoredCasts: 'int \*;struct S1 \*;TYPE \*;struct S2 \*;TYPE_P;struct S3 \*;struct S4 \*;struct S. \*'}}" + +struct S1 { +}; + +struct S2 { +}; + +struct S3 { +}; + +struct S4 { +}; + +typedef int TYPE; +typedef int * TYPE_P; +typedef unsigned char uchar; + +void test1(int *p0, TYPE *p1, TYPE_P p2) { + struct S1 *s1; + struct S4 *s4; + + s1 = (struct S1 *)p0; // no warning + s1 = (struct S1 *)p1; // CHECK-MESSAGES: warning: casting a 'TYPE *' (aka 'int *') pointer to a 'struct S1 *' + s1 = (struct S1 *)p2; // CHECK-MESSAGES: warning: casting a 'TYPE_P' (aka 'int *') pointer to a 'struct S1 *' + s4 = (struct S4 *)p0; // CHECK-MESSAGES: warning: casting a 'int *' pointer to a 'struct S4 *' +} + +void test2(int *p0, TYPE *p1, TYPE_P p2) { + struct S2 *s2; + struct S4 *s4; + + s2 = (struct S2 *)p0; // CHECK-MESSAGES: warning: casting a 'int *' pointer to a 'struct S2 *' + s2 = (struct S2 *)p1; // no warning + s2 = (struct S2 *)p2; // CHECK-MESSAGES: warning: casting a 'TYPE_P' (aka 'int *') pointer to a 'struct S2 *' + s4 = (struct S4 *)p1; // CHECK-MESSAGES: warning: casting a 'TYPE *' (aka 'int *') pointer to a 'struct S4 *' +} + +void test3(int *p0, TYPE *p1, TYPE_P p2) { + struct S3 *s3; + struct S4 *s4; + + s3 = (struct S3 *)p0; // CHECK-MESSAGES: warning: casting a 'int *' pointer to a 'struct S3 *' + s3 = (struct S3 *)p1; // CHECK-MESSAGES: warning: casting a 'TYPE *' (aka 'int *') pointer to a 'struct S3 *' + s3 = (struct S3 *)p2; // no warning + s4 = (struct S4 *)p2; // CHECK-MESSAGES: warning: casting a 'TYPE_P' (aka 'int *') pointer to a 'struct S4 *' +} + +void test_wildcard(struct S4 *p1, struct S1 *p2) { + struct S1 *s1; + struct S2 *s2; + s1 = (struct S1 *)p1; + s2 = (struct S2 *)p1; + s2 = (struct S2 *)p2; // CHECK-MESSAGES: warning: casting a 'struct S1 *' pointer to a 'struct S2 *' +} + +void test_default_ignore(void *p1, char *p2, uchar *p3) { + struct S4 *s4; + s4 = (struct S4 *)p1; + s4 = (struct S4 *)p2; + s4 = (struct S4 *)p3; +} 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..44921aaaf0be7 --- /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(short *p) { + return (struct S1 *)p; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: casting a 'short *' 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(short *p1, int_t *p2, int_ptr_t p3) { + TyS1 *a = (TyS1 *)p1; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: casting a 'short *' 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 'short *' 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(short *p1, union U1 *p2, TyPU1 p3) { + union U1 *a = (union U1 *)p1; + struct S1 *b = (struct S1 *)p2; + struct S1 *c = (struct S1 *)p3; +}