Skip to content
2 changes: 2 additions & 0 deletions clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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>(
Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ add_clang_library(clangTidyBugproneModule STATIC
BugproneTidyModule.cpp
CapturingThisInMemberVariableCheck.cpp
CastingThroughVoidCheck.cpp
CastToStructCheck.cpp
ChainedComparisonCheck.cpp
ComparePointerToMemberVirtualFunctionCheck.cpp
CopyConstructorInitCheck.cpp
Expand Down
82 changes: 82 additions & 0 deletions clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
//===--- CastToStructCheck.cpp - clang-tidy -------------------------------===//
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New style:

Suggested change
//===--- 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),
IgnoredCasts(
utils::options::parseStringList(Options.get("IgnoredCasts", ""))) {}

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(hasDeclaration(recordDecl(isUnion()))))
.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<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 (FromType == ToType)
return;

const std::string FromName = FromPointee->getAsString();
const std::string ToName = ToPointee->getAsString();
llvm::Regex FromR;
llvm::Regex ToR;
for (auto [Idx, Str] : llvm::enumerate(IgnoredCasts)) {
if (Idx % 2 == 0) {
FromR = llvm::Regex(Str);
} else {
ToR = llvm::Regex(Str);
if (FromR.match(FromName) && ToR.match(ToName))
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
37 changes: 37 additions & 0 deletions clang-tools-extra/clang-tidy/bugprone/CastToStructCheck.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//===--- CastToStructCheck.h - clang-tidy -----------------------*- C++ -*-===//
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Suggested change
//===--- 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Suggested change
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/cast-to-struct.html
/// https://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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check should also be active in C++ (and it should include C++ tests using C-style cast and reinterpret_cast

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to limit to C, probably not bad idea would be to check if Cplusplus flag is not set.

}

private:
std::vector<llvm::StringRef> IgnoredCasts;
};

} // namespace clang::tidy::bugprone

#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CASTTOSTRUCTCHECK_H
5 changes: 5 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


- New :doc:`bugprone-invalid-enum-default-initialization
<clang-tidy/checks/bugprone/invalid-enum-default-initialization>` check.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
.. 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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. 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). 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(char *p) {
struct S1 *s;
s = (struct S1 *)p; // warn: 'char *' is converted to 'struct S1 *'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
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 run only on `C` code.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C99 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does "C" not mean "all versions of C"? But it is not relevant any more, this section is removed. The check runs on any code that is not C++. I am not sure if it is useful for ObjC.


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. For example `char;Type1;char;Type2` specifies that the
check does not produce warning for casts from ``char *`` to ``Type1 *`` and
casts from ``char *`` to ``Type2 *``. The list entries can be regular
expressions. The type name in the cast expression is matched without
resolution of type aliases like ``typedef``. Default value is empty list.
(Casts from ``void *`` are ignored always regardless of this list.)
1 change: 1 addition & 0 deletions clang-tools-extra/docs/clang-tidy/checks/list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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>`,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// RUN: %check_clang_tidy %s bugprone-cast-to-struct %t -- \
// RUN: -config="{CheckOptions: {bugprone-cast-to-struct.IgnoredCasts: 'char;S1;int;Other*'}}"

struct S1 {
int a;
};

struct S2 {
char a;
};

struct OtherS {
int a;
int b;
};

void test1(char *p1, int *p2) {
struct S1 *s1;
s1 = (struct S1 *)p1;
struct S2 *s2;
s2 = (struct S2 *)p1;
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: casting a 'char *' pointer to a 'struct S2 *'
s2 = (struct S2 *)p2;
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: casting a 'int *' pointer to a 'struct S2 *'
struct OtherS *s3;
s3 = (struct OtherS *)p2;
s3 = (struct OtherS *)p1;
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: casting a 'char *' pointer to a 'struct OtherS *'
}

struct S2 *test_void_is_always_ignored(void *p) {
return (struct S2 *)p;
}
Original file line number Diff line number Diff line change
@@ -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;
}
Loading