[clang-tools-extra] [clang-tidy] Create bugprone-bitwise-pointer-cast check (PR #108083)
Carlos Galvez via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 2 11:45:08 PDT 2024
https://github.com/carlosgalvezp updated https://github.com/llvm/llvm-project/pull/108083
>From 1b1d54e0ce0d0bc4250ff045840b0a0a7bac59a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20G=C3=A1lvez?= <carlos.galvez at zenseact.com>
Date: Tue, 10 Sep 2024 13:46:51 +0000
Subject: [PATCH] [clang-tidy] Create bugprone-bitwise-pointer-cast check
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.use of bit_cast. This is currently not caught by any other
means.
Fixes #106987
---
.../bugprone/BitwisePointerCastCheck.cpp | 44 +++++++++++++
.../bugprone/BitwisePointerCastCheck.h | 34 +++++++++++
.../bugprone/BugproneTidyModule.cpp | 3 +
.../clang-tidy/bugprone/CMakeLists.txt | 1 +
clang-tools-extra/docs/ReleaseNotes.rst | 8 ++-
.../checks/bugprone/bitwise-pointer-cast.rst | 52 ++++++++++++++++
.../docs/clang-tidy/checks/list.rst | 1 +
.../bugprone/bitwise-pointer-cast.cpp | 61 +++++++++++++++++++
8 files changed, 203 insertions(+), 1 deletion(-)
create mode 100644 clang-tools-extra/clang-tidy/bugprone/BitwisePointerCastCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/bugprone/BitwisePointerCastCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/bitwise-pointer-cast.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/bitwise-pointer-cast.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/BitwisePointerCastCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/BitwisePointerCastCheck.cpp
new file mode 100644
index 00000000000000..3a361a22ed9b73
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/BitwisePointerCastCheck.cpp
@@ -0,0 +1,44 @@
+//===--- 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) {
+ 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
diff --git a/clang-tools-extra/clang-tidy/bugprone/BitwisePointerCastCheck.h b/clang-tools-extra/clang-tidy/bugprone/BitwisePointerCastCheck.h
new file mode 100644
index 00000000000000..1515519b3c9fda
--- /dev/null
+++ b/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
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 642f025359b1d1..9120c4b6c0d9ae 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/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");
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 9f7ecb9623c539..24fc5f23249c0d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/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
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 702a8d2a3576b9..6e5c08e1398ae4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -103,6 +103,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.
@@ -168,7 +174,7 @@ Changes in existing checks
- Improved :doc:`modernize-avoid-c-arrays
<clang-tidy/checks/modernize/avoid-c-arrays>` check to suggest using ``std::span``
- as a replacement for parameters of incomplete C array type in C++20 and
+ as a replacement for parameters of incomplete C array type in C++20 and
``std::array`` or ``std::vector`` before C++20.
- Improved :doc:`modernize-min-max-use-initializer-list
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/bitwise-pointer-cast.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/bitwise-pointer-cast.rst
new file mode 100644
index 00000000000000..ac58654421a0ae
--- /dev/null
+++ b/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.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index e3dfabba8fad14..d76466d480d39c 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/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>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/bitwise-pointer-cast.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/bitwise-pointer-cast.cpp
new file mode 100644
index 00000000000000..2fcac62586ae57
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/bitwise-pointer-cast.cpp
@@ -0,0 +1,61 @@
+// 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
+{
+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);
+}
+
+void pointer2int()
+{
+ float* p{};
+ auto addr = std::bit_cast<unsigned long long>(p);
+}
More information about the cfe-commits
mailing list