pybind/pybind11
1
0
Fork 0
mirror of https://github.com/pybind/pybind11.git synced 2026-04-19 22:39:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

Eliminate cross-DSO RTTI reliance in smart_holder functionality (for platforms like macOS). (#5728)

Browse Source 
* Revert PR #5700 production code change (pybind11/detail/struct_smart_holder.h).

```
git checkout b19489145b2c7a117138632d624809dfb3b380bb~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.
This commit is contained in:
Ralf W. Grosse-Kunstleve
2025-06-17 12:14:50 -07:00
committed by GitHub
parent e2f86af216
commit 365d41a4ba
9 changed files with 135 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
tests/pure_cpp/smart_holder_poc.h
View File

@@ -36,17 +36,17 @@ T *as_raw_ptr_release_ownership(smart_holder &hld,
const char *context = "as_raw_ptr_release_ownership") {
hld.ensure_can_release_ownership(context);
T *raw_ptr = hld.as_raw_ptr_unowned<T>();
hld.release_ownership();
hld.release_ownership(get_guarded_delete);
return raw_ptr;
}
template <typename T, typename D = std::default_delete<T>>
std::unique_ptr<T, D> as_unique_ptr(smart_holder &hld) {
static const char *context = "as_unique_ptr";
hld.ensure_compatible_rtti_uqp_del<T, D>(context);
hld.ensure_compatible_uqp_del<T, D>(context);
hld.ensure_use_count_1(context);
T *raw_ptr = hld.as_raw_ptr_unowned<T>();
hld.release_ownership();
hld.release_ownership(get_guarded_delete);
// KNOWN DEFECT (see PR #4850): Does not copy the deleter.
return std::unique_ptr<T, D>(raw_ptr);
}

12
tests/pure_cpp/smart_holder_poc_test.cpp
View File

@@ -14,6 +14,7 @@
#define CATCH_CONFIG_MAIN
#include "catch.hpp"
using pybind11::memory::guarded_delete;
using pybind11::memory::smart_holder;
namespace poc = pybind11::memory::smart_holder_poc;
@@ -160,11 +161,12 @@ TEST_CASE("from_raw_ptr_take_ownership+as_shared_ptr", "[S]") {
TEST_CASE("from_raw_ptr_take_ownership+disown+reclaim_disowned", "[S]") {
auto hld = smart_holder::from_raw_ptr_take_ownership(new int(19));
std::unique_ptr<int> new_owner(hld.as_raw_ptr_unowned<int>());
hld.disown();
hld.disown(pybind11::memory::get_guarded_delete);
REQUIRE(poc::as_lvalue_ref<int>(hld) == 19);
REQUIRE(*new_owner == 19);
hld.reclaim_disowned(); // Manually veriified: without this, clang++ -fsanitize=address reports
// "detected memory leaks".
// Manually verified: without this, clang++ -fsanitize=address reports
// "detected memory leaks".
hld.reclaim_disowned(pybind11::memory::get_guarded_delete);
// NOLINTNEXTLINE(bugprone-unused-return-value)
(void) new_owner.release(); // Manually verified: without this, clang++ -fsanitize=address
// reports "attempting double-free".
@@ -175,7 +177,7 @@ TEST_CASE("from_raw_ptr_take_ownership+disown+reclaim_disowned", "[S]") {
TEST_CASE("from_raw_ptr_take_ownership+disown+release_disowned", "[S]") {
auto hld = smart_holder::from_raw_ptr_take_ownership(new int(19));
std::unique_ptr<int> new_owner(hld.as_raw_ptr_unowned<int>());
hld.disown();
hld.disown(pybind11::memory::get_guarded_delete);
REQUIRE(poc::as_lvalue_ref<int>(hld) == 19);
REQUIRE(*new_owner == 19);
hld.release_disowned();
@@ -187,7 +189,7 @@ TEST_CASE("from_raw_ptr_take_ownership+disown+ensure_is_not_disowned", "[E]") {
auto hld = smart_holder::from_raw_ptr_take_ownership(new int(19));
hld.ensure_is_not_disowned(context); // Does not throw.
std::unique_ptr<int> new_owner(hld.as_raw_ptr_unowned<int>());
hld.disown();
hld.disown(pybind11::memory::get_guarded_delete);
REQUIRE_THROWS_WITH(hld.ensure_is_not_disowned(context),
"Holder was disowned already (test_case).");
}