[clang-tidy] Create bugprone-bitwise-pointer-cast check (#108083)

To detect unsafe usages of casting a pointer to another via copying
the bytes from one into the other, either via std::bit_cast or via
memcpy. This is currently not caught by any other means.

Fixes #106987

---------

Co-authored-by: Carlos Gálvez <[email protected]>

Author: carlosgalvezp

clang-tools-extra/clang-tidy/bugprone/BitwisePointerCastCheck.cpp

@@ -0,0 +1,46 @@
+//===--- BitwisePointerCastCheck.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 "BitwisePointerCastCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+void BitwisePointerCastCheck::registerMatchers(MatchFinder *Finder) {
+  if (getLangOpts().CPlusPlus20) {
+    auto IsPointerType = refersToType(qualType(isAnyPointer()));
+    Finder->addMatcher(callExpr(hasDeclaration(functionDecl(allOf(
+                                    hasName("::std::bit_cast"),
+                                    hasTemplateArgument(0, IsPointerType),
+                                    hasTemplateArgument(1, IsPointerType)))))
+                           .bind("bit_cast"),
+                       this);
+  }
+
+  auto IsDoublePointerType =
+      hasType(qualType(pointsTo(qualType(isAnyPointer()))));
+  Finder->addMatcher(callExpr(hasArgument(0, IsDoublePointerType),
+                              hasArgument(1, IsDoublePointerType),
+                              hasDeclaration(functionDecl(hasName("::memcpy"))))
+                         .bind("memcpy"),
+                     this);
+}
+
+void BitwisePointerCastCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *Call = Result.Nodes.getNodeAs<CallExpr>("bit_cast"))
+    diag(Call->getBeginLoc(),
+         "do not use 'std::bit_cast' to cast between pointers")
+        << Call->getSourceRange();
+  else if (const auto *Call = Result.Nodes.getNodeAs<CallExpr>("memcpy"))
+    diag(Call->getBeginLoc(), "do not use 'memcpy' to cast between pointers")
+        << Call->getSourceRange();
+}
+
+} // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/bugprone/BitwisePointerCastCheck.h

@@ -0,0 +1,34 @@
+//===--- BitwisePointerCastCheck.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_BITWISEPOINTERCASTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_BITWISEPOINTERCASTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Warns about code that tries to cast between pointers by means of
+/// ``std::bit_cast`` or ``memcpy``.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/bitwise-pointer-cast.html
+class BitwisePointerCastCheck : public ClangTidyCheck {
+public:
+  BitwisePointerCastCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  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;
+  }
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_BITWISEPOINTERCASTCHECK_H

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

@@ -14,6 +14,7 @@
 #include "AssertSideEffectCheck.h"
 #include "AssignmentInIfConditionCheck.h"
 #include "BadSignalToKillThreadCheck.h"
+#include "BitwisePointerCastCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
 #include "BranchCloneCheck.h"
 #include "CastingThroughVoidCheck.h"
@@ -109,6 +110,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-assignment-in-if-condition");
     CheckFactories.registerCheck<BadSignalToKillThreadCheck>(
         "bugprone-bad-signal-to-kill-thread");
+    CheckFactories.registerCheck<BitwisePointerCastCheck>(
+        "bugprone-bitwise-pointer-cast");
     CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
         "bugprone-bool-pointer-implicit-conversion");
     CheckFactories.registerCheck<BranchCloneCheck>("bugprone-branch-clone");

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

@@ -8,6 +8,7 @@ add_clang_library(clangTidyBugproneModule
   AssertSideEffectCheck.cpp
   AssignmentInIfConditionCheck.cpp
   BadSignalToKillThreadCheck.cpp
+  BitwisePointerCastCheck.cpp
   BoolPointerImplicitConversionCheck.cpp
   BranchCloneCheck.cpp
   BugproneTidyModule.cpp

clang-tools-extra/docs/ReleaseNotes.rst

@@ -109,6 +109,12 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
+- New :doc:`bugprone-bitwise-pointer-cast
+  <clang-tidy/checks/bugprone/bitwise-pointer-cast>` check.
+
+  Warns about code that tries to cast between pointers by means of
+  ``std::bit_cast`` or ``memcpy``.
+
 - New :doc:`bugprone-tagged-union-member-count
   <clang-tidy/checks/bugprone/tagged-union-member-count>` check.
 

clang-tools-extra/docs/clang-tidy/checks/bugprone/bitwise-pointer-cast.rst

@@ -0,0 +1,52 @@
+.. title:: clang-tidy - bugprone-bitwise-pointer-cast
+
+bugprone-bitwise-pointer-cast
+=============================
+
+Warns about code that tries to cast between pointers by means of
+``std::bit_cast`` or ``memcpy``.
+
+The motivation is that ``std::bit_cast`` is advertised as the safe alternative
+to type punning via ``reinterpret_cast`` in modern C++. However, one should not
+blindly replace ``reinterpret_cast`` with ``std::bit_cast``, as follows:
+
+.. code-block:: c++
+
+    int x{};
+    -float y = *reinterpret_cast<float*>(&x);
+    +float y = *std::bit_cast<float*>(&x);
+
+The drop-in replacement behaves exactly the same as ``reinterpret_cast``, and
+Undefined Behavior is still invoked. ``std::bit_cast`` is copying the bytes of
+the input pointer, not the pointee, into an output pointer of a different type,
+which may violate the strict aliasing rules. However, simply looking at the
+code, it looks "safe", because it uses ``std::bit_cast`` which is advertised as
+safe.
+
+The solution to safe type punning is to apply ``std::bit_cast`` on value types,
+not on pointer types:
+
+.. code-block:: c++
+
+    int x{};
+    float y = std::bit_cast<float>(x);
+
+This way, the bytes of the input object are copied into the output object, which
+is much safer. Do note that Undefined Behavior can still occur, if there is no
+value of type ``To`` corresponding to the value representation produced.
+Compilers may be able to optimize this copy and generate identical assembly to
+the original ``reinterpret_cast`` version.
+
+Code before C++20 may backport ``std::bit_cast`` by means of ``memcpy``, or
+simply call ``memcpy`` directly, which is equally problematic. This is also
+detected by this check:
+
+.. code-block:: c++
+
+    int* x{};
+    float* y{};
+    std::memcpy(&y, &x, sizeof(x));
+
+Alternatively, if a cast between pointers is truly wanted, ``reinterpret_cast``
+should be used, to clearly convey the intent and enable warnings from compilers
+and linters, which should be addressed accordingly.

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

@@ -81,6 +81,7 @@ Clang-Tidy Checks
    :doc:`bugprone-assert-side-effect <bugprone/assert-side-effect>`,
    :doc:`bugprone-assignment-in-if-condition <bugprone/assignment-in-if-condition>`,
    :doc:`bugprone-bad-signal-to-kill-thread <bugprone/bad-signal-to-kill-thread>`,
+   :doc:`bugprone-bitwise-pointer-cast <bugprone/bitwise-pointer-cast>`,
    :doc:`bugprone-bool-pointer-implicit-conversion <bugprone/bool-pointer-implicit-conversion>`, "Yes"
    :doc:`bugprone-branch-clone <bugprone/branch-clone>`,
    :doc:`bugprone-casting-through-void <bugprone/casting-through-void>`,

clang-tools-extra/test/clang-tidy/checkers/bugprone/bitwise-pointer-cast-cxx20.cpp

@@ -0,0 +1,63 @@
+// RUN: %check_clang_tidy -std=c++20 %s bugprone-bitwise-pointer-cast %t
+
+void memcpy(void* to, void* dst, unsigned long long size)
+{
+  // Dummy implementation for the purpose of the test
+}
+
+namespace std
+{
+template <typename To, typename From>
+To bit_cast(From from)
+{
+  // Dummy implementation for the purpose of the test
+  To to{};
+  return to;
+}
+
+using ::memcpy;
+}
+
+void pointer2pointer()
+{
+  int x{};
+  float bad = *std::bit_cast<float*>(&x); // UB, but looks safe due to std::bit_cast
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use 'std::bit_cast' to cast between pointers [bugprone-bitwise-pointer-cast]
+  float good = std::bit_cast<float>(x);   // Well-defined
+
+  using IntPtr = int*;
+  using FloatPtr = float*;
+  IntPtr x2{};
+  float bad2 = *std::bit_cast<FloatPtr>(x2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not use 'std::bit_cast' to cast between pointers [bugprone-bitwise-pointer-cast]
+}
+
+void pointer2pointer_memcpy()
+{
+  int x{};
+  int* px{};
+  float y{};
+  float* py{};
+
+  memcpy(&py, &px, sizeof(px));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'memcpy' to cast between pointers [bugprone-bitwise-pointer-cast]
+  std::memcpy(&py, &px, sizeof(px));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'memcpy' to cast between pointers [bugprone-bitwise-pointer-cast]
+
+  std::memcpy(&y, &x, sizeof(x));
+}
+
+// Pointer-integer conversions are allowed by this check
+void int2pointer()
+{
+  unsigned long long addr{};
+  float* p = std::bit_cast<float*>(addr);
+  std::memcpy(&p, &addr, sizeof(addr));
+}
+
+void pointer2int()
+{
+  float* p{};
+  auto addr = std::bit_cast<unsigned long long>(p);
+  std::memcpy(&addr, &p, sizeof(p));
+}

clang-tools-extra/test/clang-tidy/checkers/bugprone/bitwise-pointer-cast.cpp

@@ -0,0 +1,41 @@
+// RUN: %check_clang_tidy %s bugprone-bitwise-pointer-cast %t
+
+void memcpy(void* to, void* dst, unsigned long long size)
+{
+  // Dummy implementation for the purpose of the test
+}
+
+namespace std
+{
+using ::memcpy;
+}
+
+void pointer2pointer()
+{
+  int x{};
+  int* px{};
+  float y{};
+  float* py{};
+
+  memcpy(&py, &px, sizeof(px));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'memcpy' to cast between pointers [bugprone-bitwise-pointer-cast]
+  std::memcpy(&py, &px, sizeof(px));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use 'memcpy' to cast between pointers [bugprone-bitwise-pointer-cast]
+
+  std::memcpy(&y, &x, sizeof(x));
+}
+
+// Pointer-integer conversions are allowed by this check
+void int2pointer()
+{
+  unsigned long long addr{};
+  float* p{};
+  std::memcpy(&p, &addr, sizeof(addr));
+}
+
+void pointer2int()
+{
+  unsigned long long addr{};
+  float* p{};
+  std::memcpy(&addr, &p, sizeof(p));
+}