Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[smart_holder] type_caster ODR guard #4022

Merged
merged 85 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
85 commits
Select commit Hold shift + click to select a range
c84f381
Insert type_caster_odr_guard<> (an empty struct to start with).
Jun 11, 2022
5c7e77c
Add odr_guard_registry() used in type_caster_odr_guard() default cons…
Jun 12, 2022
d1960a1
Add minimal_real_caster (from PR #3862) to test_async, test_buffers
Jun 12, 2022
5095069
VERY MESSY SNAPSHOT of WIP, this was the starting point for cl/454658…
Jun 14, 2022
9a84890
Restore original test_async, test_buffers from current smart_holder HEAD
Jun 20, 2022
c148a6b
Copy from cl/454991845 snapshot Jun 14, 5:08 PM
Jun 20, 2022
3718516
Cleanup of tests. Systematically insert `if (make_caster<T>::translat…
Jun 20, 2022
5553043
Small simplification of odr_guard_impl()
Jun 20, 2022
1522f57
WIP
Jun 21, 2022
e06518d
Add PYBIND11_SOURCE_FILE_LINE macro.
Jun 21, 2022
3a95ae1
Replace PYBIND11_TYPE_CASTER_UNIQUE_IDENTIFIER with PYBIND11_TYPE_CAS…
Jun 21, 2022
24d450b
Add more PYBIND11_DETAIL_TYPE_CASTER_ACCESS_TRANSLATION_UNIT_LOCAL; r…
Jun 21, 2022
0f9bb4c
load_type fixes & follow-on cleanup
Jun 21, 2022
b8876ac
Strip ./ from source_file_line
Jun 21, 2022
ca5708a
Add new tests to CMakeLists.txt, disable PYBIND11_WERROR
Jun 21, 2022
a598fe6
Replace C++17 syntax. Compiles with Debian clang 13 C++11 mode, but f…
Jun 21, 2022
0feb0be
Show C++ version along with ODR VIOLATION DETECTED message.
Jun 21, 2022
47c4e79
Add source_file_line_basename()
Jun 21, 2022
d390918
Introduce PYBIND11_TYPE_CASTER_ODR_GUARD_ON (but not set automatically).
Jun 21, 2022
e515940
Minor cleanup.
Jun 21, 2022
15db5e5
Set PYBIND11_TYPE_CASTER_ODR_GUARD_ON automatically.
Jun 21, 2022
a8144d9
Resolve clang-tidy error.
Jun 21, 2022
1bf2577
Compatibility with old compilers.
Jun 21, 2022
cfd98a7
Fix off-by-one in source_file_line_basename()
Jun 22, 2022
502f3cb
Report PYBIND11_INTERNALS_ID & C++ Version from pytest_configure()
Jun 22, 2022
369a390
Restore use of PYBIND11_WERROR
Jun 22, 2022
a34771a
Move cpp_version_in_use() from cast.h to pybind11_tests.cpp
Jun 22, 2022
6a6eb6c
define PYBIND11_DETAIL_ODR_GUARD_IMPL_THROW_DISABLED true in test_odr…
Jun 22, 2022
ed4b50b
IWYU cleanup of detail/type_caster_odr_guard.h
Jun 22, 2022
3d064fc
Replace `throw err;` to resolve clang-tidy error.
Jun 22, 2022
590171e
Add new header filename to CMakeLists.txt, test_files.py
Jun 22, 2022
5aaf96a
Experiment: Try any C++17 compiler.
Jun 22, 2022
5d45055
Fix ifdef for pragma GCC diagnostic.
Jun 22, 2022
1acc9d0
type_caster_odr_guard_impl() cleanup
Jun 22, 2022
2e6e833
Move type_caster_odr_guard to type_caster_odr_guard.h
Jun 22, 2022
61a0bb8
Rename test_odr_guard* to test_type_caster_odr_guard*
Jun 22, 2022
ec8b8b6
Remove comments that are (now) more distracting than helpful.
Jun 22, 2022
21fc6b3
Mark tu_local_no_data_always_false operator bool as explicit (clang-t…
Jun 22, 2022
d2bafae
New PYBIND11_TYPE_CASTER_ODR_GUARD_STRICT option (current on by defau…
Jun 22, 2022
1263ce9
Add test_type_caster_odr_registry_values(), test_type_caster_odr_viol…
Jun 22, 2022
262998b
Report UNEXPECTED: test_type_caster_odr_guard_2.cpp prevailed (but do…
Jun 22, 2022
3cc2c9c
Apply clang-tidy suggestion.
Jun 22, 2022
b10fc2e
Attempt to handle valgrind behavior.
Jun 22, 2022
5737b37
Another attempt to handle valgrind behavior.
Jun 22, 2022
0c0f322
Yet another attempt to handle valgrind behavior.
Jun 22, 2022
0d2bf26
Trying a new direction: show compiler info & std for UNEXPECTED: type…
Jun 23, 2022
491c2c7
compiler_info MSVC fix. num_violations == 0 condition.
Jun 23, 2022
d8280df
assert pybind11_tests.compiler_info is not None
Jun 23, 2022
4e13032
Introduce `make_caster_intrinsic<T>`, to be able to undo the 2 change…
Jun 25, 2022
3fc4833
Add test for stl.h / stl_bind.h mix.
Jun 26, 2022
3ea3700
Eliminate need for `PYBIND11_DETAIL_TYPE_CASTER_ACCESS_TRANSLATION_UN…
Jun 28, 2022
11e858d
tu_local_descr with src_loc experiment
Jun 28, 2022
4b6d3b8
clang-tidy suggested fixes
Jun 28, 2022
37e583b
Use source_file_line_from_sloc in type_caster_odr_guard_registry
Jun 29, 2022
9ba32bd
Disable type_caster ODR guard for __INTEL_COMPILER (see comment). Als…
Jun 29, 2022
38c2565
Add missing include (discovered via google-internal testing).
Jul 1, 2022
44966da
Work `scr_loc` into `descr`
Jul 1, 2022
68b155a
Use `TypeCasterType::name.sloc` instead of `source_file_line.sloc`
Jul 1, 2022
11adace
Fix small oversight (src_loc::here() -> src_loc{nullptr, 0}).
Jul 1, 2022
ae38889
Remove PYBIND11_DETAIL_TYPE_CASTER_ACCESS_TRANSLATION_UNIT_LOCAL macr…
Jul 2, 2022
13076e4
Remove PYBIND11_TYPE_CASTER_SOURCE_FILE_LINE macro completely. Some s…
Jul 2, 2022
70f645f
Merge branch 'smart_holder' into odr_guard_sh
Jul 10, 2022
75c6598
Minor tweaks looking at the PR with a fresh eye.
Jul 11, 2022
bd0dbd7
src_loc comments
Jul 11, 2022
2a14ee6
Add new test_descr_src_loc & and fix descr.h `concat()` `src_loc` bug…
Jul 11, 2022
38688ec
Some more work on source code comments.
Jul 12, 2022
4dabffc
Merge branch 'smart_holder' into odr_guard_sh
Jul 14, 2022
02ac969
Fully document the ODR violations in the ODR guard itself and introdu…
Jul 14, 2022
671c2ce
Update comment (incl. mention of deadsnakes known to not work as inte…
Jul 14, 2022
696b80a
Use no-destructor idiom for type_caster_odr_guard_registry, as sugges…
Jul 15, 2022
5168116
Fix clang-tidy error: 'auto reg' can be declared as 'auto *reg' [read…
Jul 15, 2022
31e8ac5
WIP
Jul 18, 2022
09b027b
Revert "WIP" (tu_local_no_data_always_false_base experiment).
Jul 20, 2022
acd5a8e
Change `PYBIND11_TYPE_CASTER_ODR_GUARD_ON` to `PYBIND11_ENABLE_TYPE_C…
Jul 20, 2022
a30c4be
Improved `#if` determining `PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD`, b…
Jul 20, 2022
ba5c623
Make `descr::sloc` `const`, as suggested by @rainwoodman
Jul 20, 2022
f5b0946
Rename macro to `PYBIND11_DETAIL_TYPE_CASTER_ODR_GUARD_IMPL_DEBUG`, a…
Jul 20, 2022
fd5a298
Tweak comments some more (add "white hat hacker" analogy).
Jul 20, 2022
6182137
Bring back `PYBIND11_CPP17` in determining `PYBIND11_ENABLE_TYPE_CAST…
Jul 20, 2022
95ff3d2
Try another workaround for `__has_builtin`-related breakages (https:/…
Jul 20, 2022
0c27340
Remove `defined(__has_builtin)` and subconditions.
Jul 20, 2022
f930544
Update "known to not work" expectation in test and comment.
Jul 20, 2022
87adbbf
`pytest.skip` `num_violations == 0` only `#ifdef __NO_INLINE__` (irre…
Jul 20, 2022
f6de990
Systematically change all new `#ifdef` to `#if defined` (review sugge…
Jul 21, 2022
31a2da3
Bring back MSVC comment that got lost while experimenting.
Jul 21, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ set(PYBIND11_HEADERS
include/pybind11/detail/smart_holder_sfinae_hooks_only.h
include/pybind11/detail/smart_holder_type_casters.h
include/pybind11/detail/type_caster_base.h
include/pybind11/detail/type_caster_odr_guard.h
include/pybind11/detail/typeid.h
include/pybind11/attr.h
include/pybind11/buffer_info.h
Expand Down
19 changes: 16 additions & 3 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "detail/descr.h"
#include "detail/smart_holder_sfinae_hooks_only.h"
#include "detail/type_caster_base.h"
#include "detail/type_caster_odr_guard.h"
#include "detail/typeid.h"
#include "pytypes.h"

Expand Down Expand Up @@ -44,8 +45,20 @@ class type_caster_for_class_ : public type_caster_base<T> {};
template <typename type, typename SFINAE = void>
class type_caster : public type_caster_for_class_<type> {};

#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD)

template <typename type>
using make_caster_for_intrinsic = type_caster_odr_guard<type, type_caster<type>>;

#else

template <typename type>
using make_caster = type_caster<intrinsic_t<type>>;
using make_caster_for_intrinsic = type_caster<type>;

#endif

template <typename type>
using make_caster = make_caster_for_intrinsic<intrinsic_t<type>>;

template <typename T>
struct type_uses_smart_holder_type_caster {
Expand Down Expand Up @@ -1035,8 +1048,8 @@ struct return_value_policy_override<
};

// Basic python -> C++ casting; throws if casting fails
template <typename T, typename SFINAE>
type_caster<T, SFINAE> &load_type(type_caster<T, SFINAE> &conv, const handle &handle) {
template <typename T>
make_caster_for_intrinsic<T> &load_type(make_caster_for_intrinsic<T> &conv, const handle &handle) {
static_assert(!detail::is_pyobject<T>::value,
"Internal error: type_caster should only be used for C++ types");
if (!conv.load(handle, true)) {
Expand Down
153 changes: 127 additions & 26 deletions include/pybind11/detail/descr.h
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright (c) 2022 The Pybind Development Team.
// All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
/*
pybind11/detail/descr.h: Helper type for concatenating type signatures at compile time

Expand All @@ -20,21 +23,102 @@ PYBIND11_NAMESPACE_BEGIN(detail)
# define PYBIND11_DESCR_CONSTEXPR const
#endif

// struct src_loc below is to support type_caster_odr_guard.h
// (see https://github.com/pybind/pybind11/pull/4022).
// The ODR guard creates ODR violations itself (see WARNING below & in type_caster_odr_guard.h),
// but is currently the only tool available.
// The ODR is useful to know *for sure* what is safe and what is not, but that is only a
// subset of what actually works in practice, in a specific environment. The implementation
// here exploits the gray area (similar to a white hat hacker).
// The dedicated test_type_caster_odr_guard_1, test_type_caster_odr_guard_2 pair of unit tests
// passes reliably on almost all platforms that meet the compiler requirements (C++17, C++20),
// except one (gcc 9.4.0 debug build).
// In the pybind11 unit tests we want to test the ODR guard in as many environments as possible,
// but it is NOT recommended to enable the guard in regular builds, production, or
// debug. The guard is meant to be used similar to a sanitizer, to check for type_caster ODR
// violations in binaries that are otherwise already fully tested and assumed to be healthy.
//
// * MSVC 2017 does not support __builtin_FILE(), __builtin_LINE().
// * Intel 2021.6.0.20220226 (g++ 9.4 mode) __builtin_LINE() is unreliable
// (line numbers vary between translation units).
#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD_IF_AVAILABLE) \
&& !defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD) && defined(PYBIND11_CPP17) \
&& !defined(__INTEL_COMPILER) \
&& (!defined(_MSC_VER) || _MSC_VER >= 1920) // MSVC 2019 or newer.
# define PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD
#endif

#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD)

// Not using std::source_location because:
// 1. "It is unspecified whether the copy/move constructors and the copy/move
// assignment operators of source_location are trivial and/or constexpr."
// (https://en.cppreference.com/w/cpp/utility/source_location).
// 2. A matching no-op stub is needed (below) to avoid code duplication.
struct src_loc {
const char *file;
unsigned line;

constexpr src_loc(const char *file, unsigned line) : file(file), line(line) {}

static constexpr src_loc here(const char *file = __builtin_FILE(),
unsigned line = __builtin_LINE()) {
return src_loc(file, line);
}

constexpr src_loc if_known_or(const src_loc &other) const {
if (file != nullptr) {
return *this;
}
return other;
}
};

#else

// No-op stub, to avoid code duplication, expected to be optimized out completely.
struct src_loc {
constexpr src_loc(const char *, unsigned) {}

static constexpr src_loc here(const char * = nullptr, unsigned = 0) {
return src_loc(nullptr, 0);
}

constexpr src_loc if_known_or(const src_loc &) const { return *this; }
};

#endif

#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD)
namespace { // WARNING: This creates an ODR violation in the ODR guard itself,
// but we do not have any alternative at the moment.
// The ODR violation here is a difference in constexpr between multiple TUs.
// All definitions have the same data layout, the only difference is the
// text const char* pointee (the pointees are identical in value),
// src_loc const char* file pointee (the pointees are different in value),
// src_loc unsigned line value.
// See also: Comment above; WARNING in type_caster_odr_guard.h
#endif

/* Concatenate type signatures at compile time */
template <size_t N, typename... Ts>
struct descr {
char text[N + 1]{'\0'};
const src_loc sloc;

constexpr descr() = default;
explicit constexpr descr(src_loc sloc) : sloc(sloc) {}
// NOLINTNEXTLINE(google-explicit-constructor)
constexpr descr(char const (&s)[N + 1]) : descr(s, make_index_sequence<N>()) {}
constexpr descr(char const (&s)[N + 1], src_loc sloc = src_loc::here())
: descr(s, make_index_sequence<N>(), sloc) {}

template <size_t... Is>
constexpr descr(char const (&s)[N + 1], index_sequence<Is...>) : text{s[Is]..., '\0'} {}
constexpr descr(char const (&s)[N + 1], index_sequence<Is...>, src_loc sloc = src_loc::here())
: text{s[Is]..., '\0'}, sloc(sloc) {}

template <typename... Chars>
// NOLINTNEXTLINE(google-explicit-constructor)
constexpr descr(char c, Chars... cs) : text{c, static_cast<char>(cs)..., '\0'} {}
constexpr descr(src_loc sloc, char c, Chars... cs)
: text{c, static_cast<char>(cs)..., '\0'}, sloc(sloc) {}

static constexpr std::array<const std::type_info *, sizeof...(Ts) + 1> types() {
return {{&typeid(Ts)..., nullptr}};
Expand All @@ -47,7 +131,8 @@ constexpr descr<N1 + N2, Ts1..., Ts2...> plus_impl(const descr<N1, Ts1...> &a,
index_sequence<Is1...>,
index_sequence<Is2...>) {
PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(b);
return {a.text[Is1]..., b.text[Is2]...};
return descr<N1 + N2, Ts1..., Ts2...>{
a.sloc.if_known_or(b.sloc), a.text[Is1]..., b.text[Is2]...};
}

template <size_t N1, size_t N2, typename... Ts1, typename... Ts2>
Expand All @@ -57,27 +142,33 @@ constexpr descr<N1 + N2, Ts1..., Ts2...> operator+(const descr<N1, Ts1...> &a,
}

template <size_t N>
constexpr descr<N - 1> const_name(char const (&text)[N]) {
return descr<N - 1>(text);
constexpr descr<N - 1> const_name(char const (&text)[N], src_loc sloc = src_loc::here()) {
return descr<N - 1>(text, sloc);
}
constexpr descr<0> const_name(char const (&)[1], src_loc sloc = src_loc::here()) {
return descr<0>(sloc);
}
constexpr descr<0> const_name(char const (&)[1]) { return {}; }

template <size_t Rem, size_t... Digits>
struct int_to_str : int_to_str<Rem / 10, Rem % 10, Digits...> {};
template <size_t... Digits>
struct int_to_str<0, Digits...> {
// WARNING: This only works with C++17 or higher.
static constexpr auto digits = descr<sizeof...(Digits)>(('0' + Digits)...);
// src_loc not tracked (not needed in this situation, at least at the moment).
static constexpr auto digits
= descr<sizeof...(Digits)>(src_loc{nullptr, 0}, ('0' + Digits)...);
};

// Ternary description (like std::conditional)
template <bool B, size_t N1, size_t N2>
constexpr enable_if_t<B, descr<N1 - 1>> const_name(char const (&text1)[N1], char const (&)[N2]) {
return const_name(text1);
constexpr enable_if_t<B, descr<N1 - 1>>
const_name(char const (&text1)[N1], char const (&)[N2], src_loc sloc = src_loc::here()) {
return const_name(text1, sloc);
}
template <bool B, size_t N1, size_t N2>
constexpr enable_if_t<!B, descr<N2 - 1>> const_name(char const (&)[N1], char const (&text2)[N2]) {
return const_name(text2);
constexpr enable_if_t<!B, descr<N2 - 1>>
const_name(char const (&)[N1], char const (&text2)[N2], src_loc sloc = src_loc::here()) {
return const_name(text2, sloc);
}

template <bool B, typename T1, typename T2>
Expand All @@ -91,12 +182,13 @@ constexpr enable_if_t<!B, T2> const_name(const T1 &, const T2 &d) {

template <size_t Size>
auto constexpr const_name() -> remove_cv_t<decltype(int_to_str<Size / 10, Size % 10>::digits)> {
// src_loc not tracked (not needed in this situation, at least at the moment).
return int_to_str<Size / 10, Size % 10>::digits;
}

template <typename Type>
constexpr descr<1, Type> const_name() {
return {'%'};
constexpr descr<1, Type> const_name(src_loc sloc = src_loc::here()) {
return {sloc, '%'};
}

// If "_" is defined as a macro, py::detail::_ cannot be provided.
Expand All @@ -106,16 +198,18 @@ constexpr descr<1, Type> const_name() {
#ifndef _
# define PYBIND11_DETAIL_UNDERSCORE_BACKWARD_COMPATIBILITY
template <size_t N>
constexpr descr<N - 1> _(char const (&text)[N]) {
return const_name<N>(text);
constexpr descr<N - 1> _(char const (&text)[N], src_loc sloc = src_loc::here()) {
return const_name<N>(text, sloc);
}
template <bool B, size_t N1, size_t N2>
constexpr enable_if_t<B, descr<N1 - 1>> _(char const (&text1)[N1], char const (&text2)[N2]) {
return const_name<B, N1, N2>(text1, text2);
constexpr enable_if_t<B, descr<N1 - 1>>
_(char const (&text1)[N1], char const (&text2)[N2], src_loc sloc = src_loc::here()) {
return const_name<B, N1, N2>(text1, text2, sloc);
}
template <bool B, size_t N1, size_t N2>
constexpr enable_if_t<!B, descr<N2 - 1>> _(char const (&text1)[N1], char const (&text2)[N2]) {
return const_name<B, N1, N2>(text1, text2);
constexpr enable_if_t<!B, descr<N2 - 1>>
_(char const (&text1)[N1], char const (&text2)[N2], src_loc sloc = src_loc::here()) {
return const_name<B, N1, N2>(text1, text2, sloc);
}
template <bool B, typename T1, typename T2>
constexpr enable_if_t<B, T1> _(const T1 &d1, const T2 &d2) {
Expand All @@ -128,15 +222,16 @@ constexpr enable_if_t<!B, T2> _(const T1 &d1, const T2 &d2) {

template <size_t Size>
auto constexpr _() -> remove_cv_t<decltype(int_to_str<Size / 10, Size % 10>::digits)> {
// src_loc not tracked (not needed in this situation, at least at the moment).
return const_name<Size>();
}
template <typename Type>
constexpr descr<1, Type> _() {
return const_name<Type>();
constexpr descr<1, Type> _(src_loc sloc = src_loc::here()) {
return const_name<Type>(sloc);
}
#endif // #ifndef _

constexpr descr<0> concat() { return {}; }
constexpr descr<0> concat(src_loc sloc = src_loc::here()) { return descr<0>{sloc}; }

template <size_t N, typename... Ts>
constexpr descr<N, Ts...> concat(const descr<N, Ts...> &descr) {
Expand All @@ -146,13 +241,19 @@ constexpr descr<N, Ts...> concat(const descr<N, Ts...> &descr) {
template <size_t N, typename... Ts, typename... Args>
constexpr auto concat(const descr<N, Ts...> &d, const Args &...args)
-> decltype(std::declval<descr<N + 2, Ts...>>() + concat(args...)) {
return d + const_name(", ") + concat(args...);
// Ensure that src_loc of existing descr is used.
return d + const_name(", ", src_loc{nullptr, 0}) + concat(args...);
}

template <size_t N, typename... Ts>
constexpr descr<N + 2, Ts...> type_descr(const descr<N, Ts...> &descr) {
return const_name("{") + descr + const_name("}");
// Ensure that src_loc of existing descr is used.
return const_name("{", src_loc{nullptr, 0}) + descr + const_name("}");
}

#if defined(PYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD)
} // namespace
#endif

PYBIND11_NAMESPACE_END(detail)
PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
Loading