Skip to content

Commit 365d41a

Browse files
authored
Eliminate cross-DSO RTTI reliance in smart_holder functionality (for platforms like macOS). (#5728)
* Revert PR #5700 production code change (pybind11/detail/struct_smart_holder.h). ``` git checkout b194891~1 include/pybind11/detail/struct_smart_holder.h ``` * Introduce `get_internals().get_memory_guarded_delete()` * [skip ci] Only pass around `memory::get_guarded_delete` function pointer. * [skip ci] Change a variable name for internal consistency. Add 3 x NOTE: PYBIND11_INTERNALS_VERSION needs to be bumped if changes are made to this struct. * Add comment: get_internals().get_memory_guarded_delete does not need with_internals() * Traverse all DSOs to find memory::guarded_delete with matching RTTI. * Add nullptr check to dynamic_cast overload. Suggested by ChatGPT for these reasons: * Prevents runtime RTTI lookups on nullptr. * Helps avoid undefined behavior if users pass in nulls from failed casts or optional paths. * Ensures consistent return value semantics and no accidental access to vtable. * Improve smart_holder unique_ptr deleter compatibility checks across DSOs: * Replace RTTI-based detection of std::default_delete<T> with a constexpr check to avoid RTTI reliance * Add type_info_equal_across_dso_boundaries() fallback using type_info::name() for RTTI equality across macOS DSOs * Rename related flags and functions for clarity (e.g., builtin → std_default) * Improves ABI robustness and clarity of ownership checks in smart_holder * Trivial renaming for internal consistency: builtin_delete → std_default_delete * Add get_trampoline_self_life_support to detail::type_info (passes local testing). * Polish previous commit slightly. * [skip ci] Store memory::get_guarded_delete in `detail::type_info` instead of `detail::internals` (no searching across DSOs required). * Revert change suggested by ChatGPT. After double-checking, ChatGPT agrees this isn't needed. * Minor polishing.
1 parent e2f86af commit 365d41a

File tree

9 files changed

+135
-89
lines changed

9 files changed

+135
-89
lines changed

include/pybind11/attr.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include "detail/common.h"
1414
#include "cast.h"
15+
#include "trampoline_self_life_support.h"
1516

1617
#include <functional>
1718

@@ -312,6 +313,12 @@ struct type_record {
312313
/// Function pointer to class_<..>::dealloc
313314
void (*dealloc)(detail::value_and_holder &) = nullptr;
314315

316+
/// Function pointer for casting alias class (aka trampoline) pointer to
317+
/// trampoline_self_life_support pointer. Sidesteps cross-DSO RTTI issues
318+
/// on platforms like macOS (see PR #5728 for details).
319+
get_trampoline_self_life_support_fn get_trampoline_self_life_support
320+
= [](void *) -> trampoline_self_life_support * { return nullptr; };
321+
315322
/// List of base classes of the newly created type
316323
list bases;
317324

include/pybind11/cast.h

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -980,7 +980,7 @@ struct copyable_holder_caster<
980980

981981
explicit operator std::shared_ptr<type> &() {
982982
if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
983-
shared_ptr_storage = sh_load_helper.load_as_shared_ptr(value);
983+
shared_ptr_storage = sh_load_helper.load_as_shared_ptr(typeinfo, value);
984984
}
985985
return shared_ptr_storage;
986986
}
@@ -989,7 +989,8 @@ struct copyable_holder_caster<
989989
if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
990990
// Reusing shared_ptr code to minimize code complexity.
991991
shared_ptr_storage
992-
= sh_load_helper.load_as_shared_ptr(value,
992+
= sh_load_helper.load_as_shared_ptr(typeinfo,
993+
value,
993994
/*responsible_parent=*/nullptr,
994995
/*force_potentially_slicing_shared_ptr=*/true);
995996
}
@@ -1019,7 +1020,8 @@ struct copyable_holder_caster<
10191020
copyable_holder_caster loader;
10201021
loader.load(responsible_parent, /*convert=*/false);
10211022
assert(loader.typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder);
1022-
return loader.sh_load_helper.load_as_shared_ptr(loader.value, responsible_parent);
1023+
return loader.sh_load_helper.load_as_shared_ptr(
1024+
loader.typeinfo, loader.value, responsible_parent);
10231025
}
10241026

10251027
protected:
@@ -1240,20 +1242,20 @@ struct move_only_holder_caster<
12401242

12411243
explicit operator std::unique_ptr<type, deleter>() {
12421244
if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
1243-
return sh_load_helper.template load_as_unique_ptr<deleter>(value);
1245+
return sh_load_helper.template load_as_unique_ptr<deleter>(typeinfo, value);
12441246
}
12451247
pybind11_fail("Expected to be UNREACHABLE: " __FILE__ ":" PYBIND11_TOSTRING(__LINE__));
12461248
}
12471249

12481250
explicit operator const std::unique_ptr<type, deleter> &() {
12491251
if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
12501252
// Get shared_ptr to ensure that the Python object is not disowned elsewhere.
1251-
shared_ptr_storage = sh_load_helper.load_as_shared_ptr(value);
1253+
shared_ptr_storage = sh_load_helper.load_as_shared_ptr(typeinfo, value);
12521254
// Build a temporary unique_ptr that is meant to never expire.
12531255
unique_ptr_storage = std::shared_ptr<std::unique_ptr<type, deleter>>(
12541256
new std::unique_ptr<type, deleter>{
12551257
sh_load_helper.template load_as_const_unique_ptr<deleter>(
1256-
shared_ptr_storage.get())},
1258+
typeinfo, shared_ptr_storage.get())},
12571259
[](std::unique_ptr<type, deleter> *ptr) {
12581260
if (!ptr) {
12591261
pybind11_fail("FATAL: `const std::unique_ptr<T, D> &` was disowned "

include/pybind11/detail/internals.h

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212
#include <pybind11/conduit/pybind11_platform_abi_id.h>
1313
#include <pybind11/gil_simple.h>
1414
#include <pybind11/pytypes.h>
15+
#include <pybind11/trampoline_self_life_support.h>
1516

1617
#include "common.h"
18+
#include "struct_smart_holder.h"
1719

1820
#include <atomic>
1921
#include <exception>
@@ -35,11 +37,11 @@
3537
/// further ABI-incompatible changes may be made before the ABI is officially
3638
/// changed to the new version.
3739
#ifndef PYBIND11_INTERNALS_VERSION
38-
# define PYBIND11_INTERNALS_VERSION 10
40+
# define PYBIND11_INTERNALS_VERSION 11
3941
#endif
4042

41-
#if PYBIND11_INTERNALS_VERSION < 10
42-
# error "PYBIND11_INTERNALS_VERSION 10 is the minimum for all platforms for pybind11v3."
43+
#if PYBIND11_INTERNALS_VERSION < 11
44+
# error "PYBIND11_INTERNALS_VERSION 11 is the minimum for all platforms for pybind11v3."
4345
#endif
4446

4547
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
@@ -308,6 +310,12 @@ struct type_info {
308310
void *(*operator_new)(size_t);
309311
void (*init_instance)(instance *, const void *);
310312
void (*dealloc)(value_and_holder &v_h);
313+
314+
// Cross-DSO-safe function pointers, to sidestep cross-DSO RTTI issues
315+
// on platforms like macOS (see PR #5728 for details):
316+
memory::get_guarded_delete_fn get_memory_guarded_delete = memory::get_guarded_delete;
317+
get_trampoline_self_life_support_fn get_trampoline_self_life_support = nullptr;
318+
311319
std::vector<PyObject *(*) (PyObject *, PyTypeObject *)> implicit_conversions;
312320
std::vector<std::pair<const std::type_info *, void *(*) (void *)>> implicit_casts;
313321
std::vector<bool (*)(PyObject *, void *&)> *direct_conversions;

include/pybind11/detail/struct_smart_holder.h

Lines changed: 59 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ High-level aspects:
5050

5151
#include "pybind11_namespace_macros.h"
5252

53+
#include <cstring>
5354
#include <functional>
5455
#include <memory>
5556
#include <stdexcept>
@@ -58,19 +59,6 @@ High-level aspects:
5859
#include <typeinfo>
5960
#include <utility>
6061

61-
// IMPORTANT: This code block must stay BELOW the #include <stdexcept> above.
62-
// This is only required on some builds with libc++ (one of three implementations
63-
// in
64-
// https://github.com/llvm/llvm-project/blob/a9b64bb3180dab6d28bf800a641f9a9ad54d2c0c/libcxx/include/typeinfo#L271-L276
65-
// require it)
66-
#if !defined(PYBIND11_EXPORT_GUARDED_DELETE)
67-
# if defined(_LIBCPP_VERSION) && !defined(WIN32) && !defined(_WIN32)
68-
# define PYBIND11_EXPORT_GUARDED_DELETE __attribute__((visibility("default")))
69-
# else
70-
# define PYBIND11_EXPORT_GUARDED_DELETE
71-
# endif
72-
#endif
73-
7462
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
7563
PYBIND11_NAMESPACE_BEGIN(memory)
7664

@@ -91,7 +79,8 @@ static constexpr bool type_has_shared_from_this(const void *) {
9179
return false;
9280
}
9381

94-
struct PYBIND11_EXPORT_GUARDED_DELETE guarded_delete {
82+
struct guarded_delete {
83+
// NOTE: PYBIND11_INTERNALS_VERSION needs to be bumped if changes are made to this struct.
9584
std::weak_ptr<void> released_ptr; // Trick to keep the smart_holder memory footprint small.
9685
std::function<void(void *)> del_fun; // Rare case.
9786
void (*del_ptr)(void *); // Common case.
@@ -113,26 +102,33 @@ struct PYBIND11_EXPORT_GUARDED_DELETE guarded_delete {
113102
}
114103
};
115104

105+
inline guarded_delete *get_guarded_delete(const std::shared_ptr<void> &ptr) {
106+
return std::get_deleter<guarded_delete>(ptr);
107+
}
108+
109+
using get_guarded_delete_fn = guarded_delete *(*) (const std::shared_ptr<void> &);
110+
116111
template <typename T, typename std::enable_if<std::is_destructible<T>::value, int>::type = 0>
117-
inline void builtin_delete_if_destructible(void *raw_ptr) {
112+
inline void std_default_delete_if_destructible(void *raw_ptr) {
118113
std::default_delete<T>{}(static_cast<T *>(raw_ptr));
119114
}
120115

121116
template <typename T, typename std::enable_if<!std::is_destructible<T>::value, int>::type = 0>
122-
inline void builtin_delete_if_destructible(void *) {
117+
inline void std_default_delete_if_destructible(void *) {
123118
// This noop operator is needed to avoid a compilation error (for `delete raw_ptr;`), but
124119
// throwing an exception from a destructor will std::terminate the process. Therefore the
125120
// runtime check for lifetime-management correctness is implemented elsewhere (in
126121
// ensure_pointee_is_destructible()).
127122
}
128123

129124
template <typename T>
130-
guarded_delete make_guarded_builtin_delete(bool armed_flag) {
131-
return guarded_delete(builtin_delete_if_destructible<T>, armed_flag);
125+
guarded_delete make_guarded_std_default_delete(bool armed_flag) {
126+
return guarded_delete(std_default_delete_if_destructible<T>, armed_flag);
132127
}
133128

134129
template <typename T, typename D>
135130
struct custom_deleter {
131+
// NOTE: PYBIND11_INTERNALS_VERSION needs to be bumped if changes are made to this struct.
136132
D deleter;
137133
explicit custom_deleter(D &&deleter) : deleter{std::forward<D>(deleter)} {}
138134
void operator()(void *raw_ptr) { deleter(static_cast<T *>(raw_ptr)); }
@@ -144,17 +140,25 @@ guarded_delete make_guarded_custom_deleter(D &&uqp_del, bool armed_flag) {
144140
std::function<void(void *)>(custom_deleter<T, D>(std::forward<D>(uqp_del))), armed_flag);
145141
}
146142

147-
template <typename T>
148-
inline bool is_std_default_delete(const std::type_info &rtti_deleter) {
149-
return rtti_deleter == typeid(std::default_delete<T>)
150-
|| rtti_deleter == typeid(std::default_delete<T const>);
143+
template <typename T, typename D>
144+
constexpr bool uqp_del_is_std_default_delete() {
145+
return std::is_same<D, std::default_delete<T>>::value
146+
|| std::is_same<D, std::default_delete<T const>>::value;
147+
}
148+
149+
inline bool type_info_equal_across_dso_boundaries(const std::type_info &a,
150+
const std::type_info &b) {
151+
// RTTI pointer comparison may fail across DSOs (e.g., macOS libc++).
152+
// Fallback to name comparison, which is generally safe and ABI-stable enough for our use.
153+
return a == b || std::strcmp(a.name(), b.name()) == 0;
151154
}
152155

153156
struct smart_holder {
157+
// NOTE: PYBIND11_INTERNALS_VERSION needs to be bumped if changes are made to this struct.
154158
const std::type_info *rtti_uqp_del = nullptr;
155159
std::shared_ptr<void> vptr;
156160
bool vptr_is_using_noop_deleter : 1;
157-
bool vptr_is_using_builtin_delete : 1;
161+
bool vptr_is_using_std_default_delete : 1;
158162
bool vptr_is_external_shared_ptr : 1;
159163
bool is_populated : 1;
160164
bool is_disowned : 1;
@@ -166,7 +170,7 @@ struct smart_holder {
166170
smart_holder &operator=(const smart_holder &) = delete;
167171

168172
smart_holder()
169-
: vptr_is_using_noop_deleter{false}, vptr_is_using_builtin_delete{false},
173+
: vptr_is_using_noop_deleter{false}, vptr_is_using_std_default_delete{false},
170174
vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false} {}
171175

172176
bool has_pointee() const { return vptr != nullptr; }
@@ -191,7 +195,7 @@ struct smart_holder {
191195
}
192196
}
193197

194-
void ensure_vptr_is_using_builtin_delete(const char *context) const {
198+
void ensure_vptr_is_using_std_default_delete(const char *context) const {
195199
if (vptr_is_external_shared_ptr) {
196200
throw std::invalid_argument(std::string("Cannot disown external shared_ptr (")
197201
+ context + ").");
@@ -200,24 +204,26 @@ struct smart_holder {
200204
throw std::invalid_argument(std::string("Cannot disown non-owning holder (") + context
201205
+ ").");
202206
}
203-
if (!vptr_is_using_builtin_delete) {
207+
if (!vptr_is_using_std_default_delete) {
204208
throw std::invalid_argument(std::string("Cannot disown custom deleter (") + context
205209
+ ").");
206210
}
207211
}
208212

209213
template <typename T, typename D>
210-
void ensure_compatible_rtti_uqp_del(const char *context) const {
211-
const std::type_info *rtti_requested = &typeid(D);
214+
void ensure_compatible_uqp_del(const char *context) const {
212215
if (!rtti_uqp_del) {
213-
if (!is_std_default_delete<T>(*rtti_requested)) {
216+
if (!uqp_del_is_std_default_delete<T, D>()) {
214217
throw std::invalid_argument(std::string("Missing unique_ptr deleter (") + context
215218
+ ").");
216219
}
217-
ensure_vptr_is_using_builtin_delete(context);
218-
} else if (!(*rtti_requested == *rtti_uqp_del)
219-
&& !(vptr_is_using_builtin_delete
220-
&& is_std_default_delete<T>(*rtti_requested))) {
220+
ensure_vptr_is_using_std_default_delete(context);
221+
return;
222+
}
223+
if (uqp_del_is_std_default_delete<T, D>() && vptr_is_using_std_default_delete) {
224+
return;
225+
}
226+
if (!type_info_equal_across_dso_boundaries(typeid(D), *rtti_uqp_del)) {
221227
throw std::invalid_argument(std::string("Incompatible unique_ptr deleter (") + context
222228
+ ").");
223229
}
@@ -244,19 +250,20 @@ struct smart_holder {
244250
}
245251
}
246252

247-
void reset_vptr_deleter_armed_flag(bool armed_flag) const {
248-
auto *vptr_del_ptr = std::get_deleter<guarded_delete>(vptr);
249-
if (vptr_del_ptr == nullptr) {
253+
void reset_vptr_deleter_armed_flag(const get_guarded_delete_fn ggd_fn, bool armed_flag) const {
254+
auto *gd = ggd_fn(vptr);
255+
if (gd == nullptr) {
250256
throw std::runtime_error(
251257
"smart_holder::reset_vptr_deleter_armed_flag() called in an invalid context.");
252258
}
253-
vptr_del_ptr->armed_flag = armed_flag;
259+
gd->armed_flag = armed_flag;
254260
}
255261

256-
// Caller is responsible for precondition: ensure_compatible_rtti_uqp_del<T, D>() must succeed.
262+
// Caller is responsible for precondition: ensure_compatible_uqp_del<T, D>() must succeed.
257263
template <typename T, typename D>
258-
std::unique_ptr<D> extract_deleter(const char *context) const {
259-
const auto *gd = std::get_deleter<guarded_delete>(vptr);
264+
std::unique_ptr<D> extract_deleter(const char *context,
265+
const get_guarded_delete_fn ggd_fn) const {
266+
auto *gd = ggd_fn(vptr);
260267
if (gd && gd->use_del_fun) {
261268
const auto &custom_deleter_ptr = gd->del_fun.template target<custom_deleter<T, D>>();
262269
if (custom_deleter_ptr == nullptr) {
@@ -288,28 +295,28 @@ struct smart_holder {
288295
static smart_holder from_raw_ptr_take_ownership(T *raw_ptr, bool void_cast_raw_ptr = false) {
289296
ensure_pointee_is_destructible<T>("from_raw_ptr_take_ownership");
290297
smart_holder hld;
291-
auto gd = make_guarded_builtin_delete<T>(true);
298+
auto gd = make_guarded_std_default_delete<T>(true);
292299
if (void_cast_raw_ptr) {
293300
hld.vptr.reset(static_cast<void *>(raw_ptr), std::move(gd));
294301
} else {
295302
hld.vptr.reset(raw_ptr, std::move(gd));
296303
}
297-
hld.vptr_is_using_builtin_delete = true;
304+
hld.vptr_is_using_std_default_delete = true;
298305
hld.is_populated = true;
299306
return hld;
300307
}
301308

302309
// Caller is responsible for ensuring the complex preconditions
303310
// (see `smart_holder_type_caster_support::load_helper`).
304-
void disown() {
305-
reset_vptr_deleter_armed_flag(false);
311+
void disown(const get_guarded_delete_fn ggd_fn) {
312+
reset_vptr_deleter_armed_flag(ggd_fn, false);
306313
is_disowned = true;
307314
}
308315

309316
// Caller is responsible for ensuring the complex preconditions
310317
// (see `smart_holder_type_caster_support::load_helper`).
311-
void reclaim_disowned() {
312-
reset_vptr_deleter_armed_flag(true);
318+
void reclaim_disowned(const get_guarded_delete_fn ggd_fn) {
319+
reset_vptr_deleter_armed_flag(ggd_fn, true);
313320
is_disowned = false;
314321
}
315322

@@ -319,14 +326,14 @@ struct smart_holder {
319326

320327
void ensure_can_release_ownership(const char *context = "ensure_can_release_ownership") const {
321328
ensure_is_not_disowned(context);
322-
ensure_vptr_is_using_builtin_delete(context);
329+
ensure_vptr_is_using_std_default_delete(context);
323330
ensure_use_count_1(context);
324331
}
325332

326333
// Caller is responsible for ensuring the complex preconditions
327334
// (see `smart_holder_type_caster_support::load_helper`).
328-
void release_ownership() {
329-
reset_vptr_deleter_armed_flag(false);
335+
void release_ownership(const get_guarded_delete_fn ggd_fn) {
336+
reset_vptr_deleter_armed_flag(ggd_fn, false);
330337
release_disowned();
331338
}
332339

@@ -335,10 +342,10 @@ struct smart_holder {
335342
void *void_ptr = nullptr) {
336343
smart_holder hld;
337344
hld.rtti_uqp_del = &typeid(D);
338-
hld.vptr_is_using_builtin_delete = is_std_default_delete<T>(*hld.rtti_uqp_del);
345+
hld.vptr_is_using_std_default_delete = uqp_del_is_std_default_delete<T, D>();
339346
guarded_delete gd{nullptr, false};
340-
if (hld.vptr_is_using_builtin_delete) {
341-
gd = make_guarded_builtin_delete<T>(true);
347+
if (hld.vptr_is_using_std_default_delete) {
348+
gd = make_guarded_std_default_delete<T>(true);
342349
} else {
343350
gd = make_guarded_custom_deleter<T, D>(std::move(unq_ptr.get_deleter()), true);
344351
}

0 commit comments

Comments
 (0)