diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -36,6 +36,7 @@ #include "MisplacedPointerArithmeticInAllocCheck.h" #include "MisplacedWideningCastCheck.h" #include "MoveForwardingReferenceCheck.h" +#include "MultiLevelImplicitPointerConversionCheck.h" #include "MultipleStatementMacroCheck.h" #include "NoEscapeCheck.h" #include "NonZeroEnumToBoolConversionCheck.h" @@ -133,6 +134,8 @@ "bugprone-misplaced-widening-cast"); CheckFactories.registerCheck( "bugprone-move-forwarding-reference"); + CheckFactories.registerCheck( + "bugprone-multi-level-implicit-pointer-conversion"); CheckFactories.registerCheck( "bugprone-multiple-statement-macro"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -31,6 +31,7 @@ MisplacedPointerArithmeticInAllocCheck.cpp MisplacedWideningCastCheck.cpp MoveForwardingReferenceCheck.cpp + MultiLevelImplicitPointerConversionCheck.cpp MultipleStatementMacroCheck.cpp NoEscapeCheck.cpp NonZeroEnumToBoolConversionCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h b/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.h new file mode 100644 --- /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/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MultiLevelImplicitPointerConversionCheck.cpp new file mode 100644 --- /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 (!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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -106,6 +106,12 @@ New checks ^^^^^^^^^^ +- New :doc:`bugprone-multi-level-implicit-pointer-conversion + ` check. + + Detects implicit conversions between pointers of different levels of + indirection. + - New :doc:`bugprone-non-zero-enum-to-bool-conversion ` 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 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion.rst @@ -0,0 +1,45 @@ +.. 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 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -102,6 +102,7 @@ `bugprone-misplaced-pointer-arithmetic-in-alloc `_, "Yes" `bugprone-misplaced-widening-cast `_, `bugprone-move-forwarding-reference `_, "Yes" + `bugprone-multi-level-implicit-pointer-conversion `_, `bugprone-multiple-statement-macro `_, `bugprone-no-escape `_, `bugprone-non-zero-enum-to-bool-conversion `_, 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 --- /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()); +}