Skip to content

Commit c9e69b5

Browse files
author
Carlos Gálvez
committed
[clang-tidy] Create bugprone-bit-cast-pointers check
To detect unsafe use of bit_cast. Otherwise, bit_cast bypasses all checks done by compilers and linters. Fixes #106987
1 parent b8b8fbe commit c9e69b5

File tree

7 files changed

+202
-0
lines changed

7 files changed

+202
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
//===--- BitCastPointersCheck.cpp - clang-tidy ----------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "BitCastPointersCheck.h"
10+
#include "clang/ASTMatchers/ASTMatchFinder.h"
11+
12+
using namespace clang::ast_matchers;
13+
14+
namespace clang::tidy::bugprone {
15+
16+
void BitCastPointersCheck::registerMatchers(MatchFinder *Finder) {
17+
auto IsPointerType = refersToType(qualType(isAnyPointer()));
18+
Finder->addMatcher(callExpr(hasDeclaration(functionDecl(allOf(
19+
hasName("::std::bit_cast"),
20+
hasTemplateArgument(0, IsPointerType),
21+
hasTemplateArgument(1, IsPointerType)))))
22+
.bind("bit_cast"),
23+
this);
24+
25+
auto IsDoublePointerType =
26+
hasType(qualType(pointsTo(qualType(isAnyPointer()))));
27+
Finder->addMatcher(callExpr(hasArgument(0, IsDoublePointerType),
28+
hasArgument(1, IsDoublePointerType),
29+
hasDeclaration(functionDecl(hasName("::memcpy"))))
30+
.bind("memcpy"),
31+
this);
32+
}
33+
34+
void BitCastPointersCheck::check(const MatchFinder::MatchResult &Result) {
35+
if (const auto *MatchedDecl = Result.Nodes.getNodeAs<CallExpr>("bit_cast"))
36+
diag(MatchedDecl->getBeginLoc(),
37+
"do not use std::bit_cast to cast between pointers")
38+
<< MatchedDecl->getSourceRange();
39+
if (const auto *MatchedDecl = Result.Nodes.getNodeAs<CallExpr>("memcpy"))
40+
diag(MatchedDecl->getBeginLoc(),
41+
"do not use memcpy to cast between pointers")
42+
<< MatchedDecl->getSourceRange();
43+
}
44+
45+
} // namespace clang::tidy::bugprone
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//===--- BitCastPointersCheck.h - clang-tidy --------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_BITCASTPOINTERSCHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_BITCASTPOINTERSCHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
14+
namespace clang::tidy::bugprone {
15+
16+
/// Warns about code that tries to cast between pointers by means of
17+
/// ``std::bit_cast`` or ``memcpy``.
18+
///
19+
/// For the user-facing documentation see:
20+
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/bit-cast-pointers.html
21+
class BitCastPointersCheck : public ClangTidyCheck {
22+
public:
23+
BitCastPointersCheck(StringRef Name, ClangTidyContext *Context)
24+
: ClangTidyCheck(Name, Context) {}
25+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
26+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
27+
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
28+
return LangOpts.CPlusPlus;
29+
}
30+
};
31+
32+
} // namespace clang::tidy::bugprone
33+
34+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_BITCASTPOINTERSCHECK_H

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

+3
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "AssertSideEffectCheck.h"
1515
#include "AssignmentInIfConditionCheck.h"
1616
#include "BadSignalToKillThreadCheck.h"
17+
#include "BitCastPointersCheck.h"
1718
#include "BoolPointerImplicitConversionCheck.h"
1819
#include "BranchCloneCheck.h"
1920
#include "CastingThroughVoidCheck.h"
@@ -108,6 +109,8 @@ class BugproneModule : public ClangTidyModule {
108109
"bugprone-assignment-in-if-condition");
109110
CheckFactories.registerCheck<BadSignalToKillThreadCheck>(
110111
"bugprone-bad-signal-to-kill-thread");
112+
CheckFactories.registerCheck<BitCastPointersCheck>(
113+
"bugprone-bit-cast-pointers");
111114
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
112115
"bugprone-bool-pointer-implicit-conversion");
113116
CheckFactories.registerCheck<BranchCloneCheck>("bugprone-branch-clone");

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

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ add_clang_library(clangTidyBugproneModule
88
AssertSideEffectCheck.cpp
99
AssignmentInIfConditionCheck.cpp
1010
BadSignalToKillThreadCheck.cpp
11+
BitCastPointersCheck.cpp
1112
BoolPointerImplicitConversionCheck.cpp
1213
BranchCloneCheck.cpp
1314
BugproneTidyModule.cpp

clang-tools-extra/docs/ReleaseNotes.rst

+6
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@ Improvements to clang-tidy
9898
New checks
9999
^^^^^^^^^^
100100

101+
- New :doc:`bugprone-bit-cast-pointers
102+
<clang-tidy/checks/bugprone/bit-cast-pointers>` check.
103+
104+
Warns about code that tries to cast between pointers by means of
105+
``std::bit_cast`` or ``memcpy``.
106+
101107
New check aliases
102108
^^^^^^^^^^^^^^^^^
103109

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
.. title:: clang-tidy - bugprone-bit-cast-pointers
2+
3+
bugprone-bit-cast-pointers
4+
==========================
5+
6+
Warns about code that tries to cast between pointers by means of
7+
``std::bit_cast`` or ``memcpy``.
8+
9+
The motivation is that ``std::bit_cast`` is advertised as the safe alternative
10+
to type punning via ``reinterpret_cast`` in modern C++. However, one should not
11+
blindly replace ``reinterpret_cast`` with ``std::bit_cast``, as follows:
12+
13+
.. code-block:: c++
14+
15+
int x{};
16+
-float y = *reinterpret_cast<float*>(&x);
17+
+float y = *std::bit_cast<float*>(&x);
18+
19+
The drop-in replacement behaves exactly the same as ``reinterpret_cast``, and
20+
Undefined Behavior is still invoked. ``std::bit_cast`` is copying the bytes of
21+
the input pointer, not the pointee, into an output pointer of a different type,
22+
which may violate the strict aliasing rules. However, simply looking at the
23+
code, it looks "safe", because it uses ``std::bit_cast`` which is advertised as
24+
safe.
25+
26+
The solution to safe type punning is to apply ``std::bit_cast`` on value types,
27+
not on pointer types:
28+
29+
.. code-block:: c++
30+
31+
int x{};
32+
float y = std::bit_cast<float>(x);
33+
34+
This way, the bytes of the input object are copied into the output object, which
35+
is much safer. Do note that Undefined Behavior can still occur, if there is no
36+
value of type ``To`` corresponding to the value representation produced.
37+
Compilers may be able to optimize this copy and generate identical assembly to
38+
the original ``reinterpret_cast`` version.
39+
40+
Code before C++20 may backport ``std::bit_cast`` by means of ``memcpy``, or
41+
simply call ``memcpy`` directly, which is equally problematic. This is also
42+
detected by this check:
43+
44+
.. code-block:: c++
45+
46+
int* x{};
47+
float* y{};
48+
std::memcpy(&y, &x, sizeof(x));
49+
50+
Alternatively, if a cast between pointers is truly wanted, ``reinterpret_cast``
51+
should be used, to clearly convey the intent and enable warnings from compilers
52+
and linters, which should be addressed accordingly.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// RUN: %check_clang_tidy %s bugprone-bit-cast-pointers %t
2+
3+
void memcpy(void* to, void* dst, unsigned long long size)
4+
{
5+
// Dummy implementation for the purpose of the test
6+
}
7+
8+
namespace std
9+
{
10+
template <typename To, typename From>
11+
To bit_cast(From from)
12+
{
13+
// Dummy implementation for the purpose of the test
14+
To to{};
15+
return to;
16+
}
17+
18+
using ::memcpy;
19+
}
20+
21+
void pointer2pointer()
22+
{
23+
int x{};
24+
float bad = *std::bit_cast<float*>(&x); // UB, but looks safe due to std::bit_cast
25+
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use std::bit_cast to cast between pointers [bugprone-bit-cast-pointers]
26+
float good = std::bit_cast<float>(x); // Well-defined
27+
28+
using IntPtr = int*;
29+
using FloatPtr = float*;
30+
IntPtr x2{};
31+
float bad2 = *std::bit_cast<FloatPtr>(x2);
32+
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not use std::bit_cast to cast between pointers [bugprone-bit-cast-pointers]
33+
}
34+
35+
void pointer2pointer_memcpy()
36+
{
37+
int x{};
38+
int* px{};
39+
float y{};
40+
float* py{};
41+
42+
memcpy(&py, &px, sizeof(px));
43+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use memcpy to cast between pointers [bugprone-bit-cast-pointers]
44+
std::memcpy(&py, &px, sizeof(px));
45+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not use memcpy to cast between pointers [bugprone-bit-cast-pointers]
46+
47+
std::memcpy(&y, &x, sizeof(x));
48+
}
49+
50+
// Pointer-integer conversions are allowed by this check
51+
void int2pointer()
52+
{
53+
unsigned long long addr{};
54+
float* p = std::bit_cast<float*>(addr);
55+
}
56+
57+
void pointer2int()
58+
{
59+
float* p{};
60+
auto addr = std::bit_cast<unsigned long long>(p);
61+
}

0 commit comments

Comments
 (0)