llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Carlos Galvez (carlosgalvezp) <details> <summary>Changes</summary> To detect unsafe use of bit_cast that should be reinterpret_cast instead. Otherwise, bit_cast bypasses all checks done by compilers and linters. Fixes #<!-- -->106987 --- Full diff: https://github.com/llvm/llvm-project/pull/108083.diff 7 Files Affected: - (added) clang-tools-extra/clang-tidy/bugprone/BitCastPointersCheck.cpp (+30) - (added) clang-tools-extra/clang-tidy/bugprone/BitCastPointersCheck.h (+34) - (modified) clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp (+3) - (modified) clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt (+1) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6) - (added) clang-tools-extra/docs/clang-tidy/checks/bugprone/bit-cast-pointers.rst (+39) - (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/bit-cast-pointers.cpp (+38) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/BitCastPointersCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/BitCastPointersCheck.cpp new file mode 100644 index 00000000000000..4589a35a567552 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/BitCastPointersCheck.cpp @@ -0,0 +1,30 @@ +//===--- BitCastPointersCheck.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 "BitCastPointersCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +void BitCastPointersCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + callExpr(callee(functionDecl(allOf(hasName("::std::bit_cast"), + hasAnyTemplateArgument(refersToType( + qualType(isAnyPointer()))))))) + .bind("x"), + this); +} + +void BitCastPointersCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *MatchedDecl = Result.Nodes.getNodeAs<CallExpr>("x")) + diag(MatchedDecl->getBeginLoc(), "do not use std::bit_cast on pointers; use reinterpret_cast instead"); +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/BitCastPointersCheck.h b/clang-tools-extra/clang-tidy/bugprone/BitCastPointersCheck.h new file mode 100644 index 00000000000000..2083979e527fd3 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/BitCastPointersCheck.h @@ -0,0 +1,34 @@ +//===--- BitCastPointersCheck.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_BITCASTPOINTERSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_BITCASTPOINTERSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Warns about usage of ``std::bit_cast`` when either the input or output types +/// are pointers. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/bit-cast-pointers.html +class BitCastPointersCheck : public ClangTidyCheck { +public: + BitCastPointersCheck(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.CPlusPlus20; + } +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_BITCASTPOINTERSCHECK_H diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 689eb92a3d8d17..931624224d784b 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 "BitCastPointersCheck.h" #include "BoolPointerImplicitConversionCheck.h" #include "BranchCloneCheck.h" #include "CastingThroughVoidCheck.h" @@ -108,6 +109,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-assignment-in-if-condition"); CheckFactories.registerCheck<BadSignalToKillThreadCheck>( "bugprone-bad-signal-to-kill-thread"); + CheckFactories.registerCheck<BitCastPointersCheck>( + "bugprone-bit-cast-pointers"); 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 cb0d8ae98bac58..55766b3d9bf91c 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 + BitCastPointersCheck.cpp BoolPointerImplicitConversionCheck.cpp BranchCloneCheck.cpp BugproneTidyModule.cpp diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 8d028f8863cb7a..61c3b0fba61c99 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -98,6 +98,12 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`bugprone-bit-cast-pointers + <clang-tidy/checks/bugprone/bit-cast-pointers>` check. + + Warns about usage of ``std::bit_cast`` when either the input or output types + are pointers. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/bit-cast-pointers.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/bit-cast-pointers.rst new file mode 100644 index 00000000000000..93b2caf224d6f9 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/bit-cast-pointers.rst @@ -0,0 +1,39 @@ +.. title:: clang-tidy - bugprone-bit-cast-pointers + +bugprone-bit-cast-pointers +========================== + +Warns about usage of ``std::bit_cast`` when either the input or output types +are pointers. + +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 violates 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 safe from Undefined Behavior. Compilers are able to optimize this copy and +generate identical assembly to the original ``reinterpret_cast`` version. + +Alternatively, if a cast between pointers is truly wanted, ``reinterpret_cast`` +should be used, to clearly convey intent and enable warnings from compilers and +linters, which should be addressed accordingly. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/bit-cast-pointers.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/bit-cast-pointers.cpp new file mode 100644 index 00000000000000..284149633049fb --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/bit-cast-pointers.cpp @@ -0,0 +1,38 @@ +// RUN: %check_clang_tidy -std=c++20-or-later %s bugprone-bit-cast-pointers %t + +namespace std +{ +template <typename To, typename From> +To bit_cast(From from) +{ + // Dummy implementation for the purpose of the check. + // We don't want to include <cstring> to get std::memcpy. + To to{}; + return to; +} +} + +void pointer2pointer() +{ + int x{}; + float bad = *std::bit_cast<float*>(&x); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use std::bit_cast on pointers; use reinterpret_cast instead [bugprone-bit-cast-pointers] + float good = *reinterpret_cast<float*>(&x); + float good2 = std::bit_cast<float>(x); +} + +void int2pointer() +{ + unsigned long long addr{}; + float* bad = std::bit_cast<float*>(addr); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use std::bit_cast on pointers; use reinterpret_cast instead [bugprone-bit-cast-pointers] + float* good = reinterpret_cast<float*>(addr); +} + +void pointer2int() +{ + int x{}; + auto bad = std::bit_cast<unsigned long long>(&x); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use std::bit_cast on pointers; use reinterpret_cast instead [bugprone-bit-cast-pointers] + auto good = reinterpret_cast<unsigned long long>(&x); +} `````````` </details> https://github.com/llvm/llvm-project/pull/108083 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
