diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 0cb0924d445e5..ceb966c655d3a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -37,6 +37,7 @@ #include "MisplacedPointerArithmeticInAllocCheck.h" #include "MisplacedWideningCastCheck.h" #include "MoveForwardingReferenceCheck.h" +#include "MultiLevelImplicitPointerConversionCheck.h" #include "MultipleNewInOneExpressionCheck.h" #include "MultipleStatementMacroCheck.h" #include "NoEscapeCheck.h" @@ -140,6 +141,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-misplaced-widening-cast"); CheckFactories.registerCheck( "bugprone-move-forwarding-reference"); + CheckFactories.registerCheck( + "bugprone-multi-level-implicit-pointer-conversion"); CheckFactories.registerCheck( "bugprone-multiple-new-in-one-expression"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 4076e0d253584..4fa32aa61a4b0 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -33,6 +33,7 @@ add_clang_library(clangTidyBugproneModule MisplacedPointerArithmeticInAllocCheck.cpp MisplacedWideningCastCheck.cpp MoveForwardingReferenceCheck.cpp + MultiLevelImplicitPointerConversionCheck.cpp MultipleNewInOneExpressionCheck.cpp MultipleStatementMacroCheck.cpp NoEscapeCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.cpp new file mode 100644 index 0000000000000..4dd3cb57e6dd1 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.cpp @@ -0,0 +1,78 @@ +//===--- MultiLevelImplicitPointerConversionCheck.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 "MultiLevelImplicitPointerConversionCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +static unsigned getPointerLevel(const QualType &PtrType) { + if (!PtrType->isPointerType()) + return 0U; + + return 1U + getPointerLevel(PtrType->castAs()->getPointeeType()); +} + +namespace { + +AST_MATCHER(ImplicitCastExpr, isMultiLevelPointerConversion) { + const QualType TargetType = Node.getType() + .getCanonicalType() + .getNonReferenceType() + .getUnqualifiedType(); + const QualType SourceType = Node.getSubExpr() + ->getType() + .getCanonicalType() + .getNonReferenceType() + .getUnqualifiedType(); + + if (TargetType == SourceType) + return false; + + const unsigned TargetPtrLevel = getPointerLevel(TargetType); + if (0U == TargetPtrLevel) + return false; + + const unsigned SourcePtrLevel = getPointerLevel(SourceType); + if (0U == SourcePtrLevel) + return false; + + return SourcePtrLevel != TargetPtrLevel; +} + +} // namespace + +void MultiLevelImplicitPointerConversionCheck::registerMatchers( + MatchFinder *Finder) { + Finder->addMatcher( + implicitCastExpr(hasCastKind(CK_BitCast), isMultiLevelPointerConversion()) + .bind("expr"), + this); +} + +std::optional +MultiLevelImplicitPointerConversionCheck::getCheckTraversalKind() const { + return TK_AsIs; +} + +void MultiLevelImplicitPointerConversionCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MatchedExpr = Result.Nodes.getNodeAs("expr"); + QualType Target = MatchedExpr->getType().getDesugaredType(*Result.Context); + QualType Source = + MatchedExpr->getSubExpr()->getType().getDesugaredType(*Result.Context); + + diag(MatchedExpr->getExprLoc(), "multilevel pointer conversion from %0 to " + "%1, please use explicit cast") + << Source << Target; +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h b/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h new file mode 100644 index 0000000000000..13228145ff35f --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h @@ -0,0 +1,33 @@ +//===--- MultiLevelImplicitPointerConversionCheck.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_MULTILEVELIMPLICITPOINTERCONVERSIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MULTILEVELIMPLICITPOINTERCONVERSIONCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Detects implicit conversions between pointers of different levels of +/// indirection. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.html +class MultiLevelImplicitPointerConversionCheck : public ClangTidyCheck { +public: + MultiLevelImplicitPointerConversionCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + std::optional getCheckTraversalKind() const override; +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MULTILEVELIMPLICITPOINTERCONVERSIONCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index d22287d4effe4..4441473774b2c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -114,6 +114,12 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`bugprone-multi-level-implicit-pointer-conversion + ` check. + + Detects implicit conversions between pointers of different levels of + indirection. + - New :doc:`performance-enum-size ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.rst new file mode 100644 index 0000000000000..e6dc5c13aa025 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.rst @@ -0,0 +1,44 @@ +.. title:: clang-tidy - bugprone-multi-level-implicit-pointer-conversion + +bugprone-multi-level-implicit-pointer-conversion +================================================ + +Detects implicit conversions between pointers of different levels of +indirection. + +Conversions between pointer types of different levels of indirection can be +dangerous and may lead to undefined behavior, particularly if the converted +pointer is later cast to a type with a different level of indirection. +For example, converting a pointer to a pointer to an ``int`` (``int**``) to +a ``void*`` can result in the loss of information about the original level of +indirection, which can cause problems when attempting to use the converted +pointer. If the converted pointer is later cast to a type with a different +level of indirection and dereferenced, it may lead to access violations, +memory corruption, or other undefined behavior. + +Consider the following example: + +.. code-block:: c++ + + void foo(void* ptr); + + int main() { + int x = 42; + int* ptr = &x; + int** ptr_ptr = &ptr; + foo(ptr_ptr); // warning will trigger here + return 0; + } + +In this example, ``foo()`` is called with ``ptr_ptr`` as its argument. However, +``ptr_ptr`` is a ``int**`` pointer, while ``foo()`` expects a ``void*`` pointer. +This results in an implicit pointer level conversion, which could cause issues +if ``foo()`` dereferences the pointer assuming it's a ``int*`` pointer. + +Using an explicit cast is a recommended solution to prevent issues caused by +implicit pointer level conversion, as it allows the developer to explicitly +state their intention and show their reasoning for the type conversion. +Additionally, it is recommended that developers thoroughly check and verify the +safety of the conversion before using an explicit cast. This extra level of +caution can help catch potential issues early on in the development process, +improving the overall reliability and maintainability of the code. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 1804346da020e..73aad3c0315ab 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -103,6 +103,7 @@ Clang-Tidy Checks `bugprone-misplaced-pointer-arithmetic-in-alloc `_, "Yes" `bugprone-misplaced-widening-cast `_, `bugprone-move-forwarding-reference `_, "Yes" + `bugprone-multi-level-implicit-pointer-conversion `_, `bugprone-multiple-new-in-one-expression `_, `bugprone-multiple-statement-macro `_, `bugprone-no-escape `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/multi-level-implicit-pointer-conversion.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/multi-level-implicit-pointer-conversion.cpp new file mode 100644 index 0000000000000..7a56242e4202d --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/multi-level-implicit-pointer-conversion.cpp @@ -0,0 +1,65 @@ +// RUN: %check_clang_tidy %s bugprone-multi-level-implicit-pointer-conversion %t + +using OneStar = void*; +using OneStarFancy = OneStar; + +void takeFirstLevelVoidPtr(OneStar message); +void takeFirstLevelConstVoidPtr(const OneStarFancy message); +void takeFirstLevelConstVoidPtrConst(const void* const message); +void takeSecondLevelVoidPtr(void** message); + +void** getSecondLevelVoidPtr(); +void* getFirstLevelVoidPtr(); +int** getSecondLevelIntPtr(); +int* getFirstLevelIntPtr(); + +int table[5]; + +void test() +{ + void** secondLevelVoidPtr; + int* firstLevelIntPtr; + + // CHECK-MESSAGES: :[[@LINE+1]]:13: warning: multilevel pointer conversion from 'void **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion] + void* a = getSecondLevelVoidPtr(); + + void** b = getSecondLevelVoidPtr(); + void* c = getFirstLevelVoidPtr(); + + // CHECK-MESSAGES: :[[@LINE+1]]:13: warning: multilevel pointer conversion from 'int **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion] + void* d = getSecondLevelIntPtr(); + + takeFirstLevelVoidPtr(&table); + + takeFirstLevelVoidPtr(firstLevelIntPtr); + + takeFirstLevelVoidPtr(getFirstLevelIntPtr()); + + // CHECK-MESSAGES: :[[@LINE+1]]:25: warning: multilevel pointer conversion from 'void **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion] + takeFirstLevelVoidPtr(secondLevelVoidPtr); + + // CHECK-MESSAGES: :[[@LINE+1]]:30: warning: multilevel pointer conversion from 'void **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion] + takeFirstLevelConstVoidPtr(secondLevelVoidPtr); + + // CHECK-MESSAGES: :[[@LINE+1]]:35: warning: multilevel pointer conversion from 'void **' to 'const void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion] + takeFirstLevelConstVoidPtrConst(secondLevelVoidPtr); + + // CHECK-MESSAGES: :[[@LINE+1]]:35: warning: multilevel pointer conversion from 'void ***' to 'const void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion] + takeFirstLevelConstVoidPtrConst(&secondLevelVoidPtr); + + takeSecondLevelVoidPtr(secondLevelVoidPtr); + + // CHECK-MESSAGES: :[[@LINE+1]]:25: warning: multilevel pointer conversion from 'void **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion] + takeFirstLevelVoidPtr(getSecondLevelVoidPtr()); + + // CHECK-MESSAGES: :[[@LINE+1]]:30: warning: multilevel pointer conversion from 'void **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion] + takeFirstLevelConstVoidPtr(getSecondLevelVoidPtr()); + + // CHECK-MESSAGES: :[[@LINE+1]]:35: warning: multilevel pointer conversion from 'void **' to 'const void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion] + takeFirstLevelConstVoidPtrConst(getSecondLevelVoidPtr()); + + // CHECK-MESSAGES: :[[@LINE+1]]:25: warning: multilevel pointer conversion from 'int **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion] + takeFirstLevelVoidPtr(getSecondLevelIntPtr()); + + takeSecondLevelVoidPtr(getSecondLevelVoidPtr()); +}