[clang-tidy] Added bugprone-multi-level-implicit-pointer-conversion c…

…heck

Detects implicit conversions between pointers of different levels of
indirection.

Reviewed By: xgupta

Differential Revision: https://reviews.llvm.org/D149084

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<MoveForwardingReferenceCheck>(
         "bugprone-move-forwarding-reference");
+    CheckFactories.registerCheck<MultiLevelImplicitPointerConversionCheck>(
+        "bugprone-multi-level-implicit-pointer-conversion");
     CheckFactories.registerCheck<MultipleNewInOneExpressionCheck>(
         "bugprone-multiple-new-in-one-expression");
     CheckFactories.registerCheck<MultipleStatementMacroCheck>(

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

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<PointerType>()->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<TraversalKind>
+MultiLevelImplicitPointerConversionCheck::getCheckTraversalKind() const {
+  return TK_AsIs;
+}
+
+void MultiLevelImplicitPointerConversionCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedExpr = Result.Nodes.getNodeAs<ImplicitCastExpr>("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

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<TraversalKind> getCheckTraversalKind() const override;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MULTILEVELIMPLICITPOINTERCONVERSIONCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -114,6 +114,12 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
+- New :doc:`bugprone-multi-level-implicit-pointer-conversion
+  <clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion>` check.
+
+  Detects implicit conversions between pointers of different levels of
+  indirection.
+
 - New :doc:`performance-enum-size
   <clang-tidy/checks/performance/enum-size>` check.
 

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.

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -103,6 +103,7 @@ Clang-Tidy Checks
    `bugprone-misplaced-pointer-arithmetic-in-alloc <bugprone/misplaced-pointer-arithmetic-in-alloc.html>`_, "Yes"
    `bugprone-misplaced-widening-cast <bugprone/misplaced-widening-cast.html>`_,
    `bugprone-move-forwarding-reference <bugprone/move-forwarding-reference.html>`_, "Yes"
+   `bugprone-multi-level-implicit-pointer-conversion <bugprone/multi-level-implicit-pointer-conversion.html>`_,
    `bugprone-multiple-new-in-one-expression <bugprone/multiple-new-in-one-expression.html>`_,
    `bugprone-multiple-statement-macro <bugprone/multiple-statement-macro.html>`_,
    `bugprone-no-escape <bugprone/no-escape.html>`_,

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());
+}