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.gal...@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); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
