Files
pybind11/tests/test_class_sh_mi_thunks.py
Ralf W. Grosse-Kunstleve 4dc33d6524 Fix smart_holder multiple/virtual inheritance bugs in shared_ptr and unique_ptr to-Python conversions (#5836)
* ChatGPT-generated diamond virtual-inheritance test case.

* Report "virtual base at offset 0" but don't skip test.

* Remove Left/Right virtual default dtors, to resolve clang-tidy errors:

```
/__w/pybind11/pybind11/tests/test_class_sh_mi_thunks.cpp:44:13: error: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override,-warnings-as-errors]
   44 |     virtual ~Left() = default;
      |     ~~~~~~~ ^
      |                     override
/__w/pybind11/pybind11/tests/test_class_sh_mi_thunks.cpp:48:13: error: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override,-warnings-as-errors]
   48 |     virtual ~Right() = default;
      |     ~~~~~~~ ^
      |                      override
```

* Add assert(ptr) in register_instance_impl, deregister_instance_impl

* Proper bug fix

* Also exercise smart_holder_from_unique_ptr

* [skip ci] ChatGPT-generated bug fix: smart_holder::from_unique_ptr()

* Exception-safe ownership transfer from unique_ptr to shared_ptr

ChatGPT:

* shared_ptr’s ctor can throw (control-block alloc). Using get() keeps unique_ptr owning the memory if that happens, so no leak.

* Only after the shared_ptr is successfully constructed do you release(), transferring ownership exactly once.

* [skip ci] Rename alias_ptr to mi_subobject_ptr to distinguish from trampoline code (which often uses the term "alias", too)

* [skip ci] Also exercise smart_holder::from_raw_ptr_take_ownership

* [skip ci] Add st.first comments (generated by ChatGPT)

* [skip ci] Copy and extend (raw_ptr, unique_ptr) reproducer from PR #5796

* Some polishing: comments, add back Left/Right dtors for consistency within test_class_sh_mi_thunks.cpp

* explicitly default copy/move for VBase to silence -Wdeprecated-copy-with-dtor

* Resolve clang-tidy error:

```
/__w/pybind11/pybind11/tests/test_class_sh_mi_thunks.cpp:67:5: error: 'auto ptr' can be declared as 'auto *ptr' [readability-qualified-auto,-warnings-as-errors]
   67 |     auto ptr = new Diamond;
      |     ^~~~
      |     auto *
```

* Expand comment in `smart_holder::from_unique_ptr()`

* Better Left/Right padding to make it more likely that we avoid "all at offset 0". Clarify comment.

* Give up on `alignas(16)` to resolve MSVC warning:

```
       "D:\a\pybind11\pybind11\build\ALL_BUILD.vcxproj" (default target) (1) ->
       "D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj" (default target) (13) ->
       (ClCompile target) ->
         D:\a\pybind11\pybind11\tests\test_class_sh_mi_thunks.cpp(70,17): warning C4316: 'test_class_sh_mi_thunks::Diamond': object allocated on the heap may not be aligned 16 [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj]
         D:\a\pybind11\pybind11\tests\test_class_sh_mi_thunks.cpp(80,43): warning C4316: 'test_class_sh_mi_thunks::Diamond': object allocated on the heap may not be aligned 16 [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj]
         C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.44.35207\include\memory(2913,46): warning C4316: 'std::_Ref_count_obj2<_Ty>': object allocated on the heap may not be aligned 16 [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj]
       C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.44.35207\include\memory(2913,46): warning C4316:         with [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj]
       C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.44.35207\include\memory(2913,46): warning C4316:         [ [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj]
       C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.44.35207\include\memory(2913,46): warning C4316:             _Ty=test_class_sh_mi_thunks::Diamond [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj]
       C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.44.35207\include\memory(2913,46): warning C4316:         ] [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj]
         D:\a\pybind11\pybind11\include\pybind11\detail\init.h(77,21): warning C4316: 'test_class_sh_mi_thunks::Diamond': object allocated on the heap may not be aligned 16 [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj]
```

The warning came from alignas(16) making Diamond over-aligned, while regular new/make_shared aren’t guaranteed to return 16-byte aligned memory on MSVC (hence C4316). I’ve removed the explicit alignment and switched to asymmetric payload sizes (char[4] vs char[24]), which still nudges MI layout without relying on over-alignment. This keeps the test goal and eliminates the warning across all MSVC builds. If we ever want to stress over-alignment explicitly, we can add aligned operator new/delete under __cpp_aligned_new, but that’s more than we need here.

* Rename test_virtual_base_at_offset_0() → test_virtual_base_not_at_offset_0() and replace pytest.skip() with assert. Add helpful comment for future maintainers.
2025-10-01 11:21:47 -07:00

105 lines
3.3 KiB
Python

from __future__ import annotations
import pytest
from pybind11_tests import class_sh_mi_thunks as m
def test_ptrdiff_drvd_base0():
ptrdiff = m.ptrdiff_drvd_base0()
# A failure here does not (necessarily) mean that there is a bug, but that
# test_class_sh_mi_thunks is not exercising what it is supposed to.
# If this ever fails on some platforms: use pytest.skip()
# If this ever fails on all platforms: don't know, seems extremely unlikely.
assert ptrdiff != 0
@pytest.mark.parametrize(
"vec_size_fn",
[
m.vec_size_base0_raw_ptr,
m.vec_size_base0_shared_ptr,
],
)
@pytest.mark.parametrize(
"get_fn",
[
m.get_drvd_as_base0_raw_ptr,
m.get_drvd_as_base0_shared_ptr,
m.get_drvd_as_base0_unique_ptr,
],
)
def test_get_vec_size_raw_shared(get_fn, vec_size_fn):
obj = get_fn()
assert vec_size_fn(obj) == 5
@pytest.mark.parametrize(
"get_fn", [m.get_drvd_as_base0_raw_ptr, m.get_drvd_as_base0_unique_ptr]
)
def test_get_vec_size_unique(get_fn):
obj = get_fn()
assert m.vec_size_base0_unique_ptr(obj) == 5
with pytest.raises(ValueError, match="Python instance was disowned"):
m.vec_size_base0_unique_ptr(obj)
def test_get_shared_vec_size_unique():
obj = m.get_drvd_as_base0_shared_ptr()
with pytest.raises(ValueError) as exc_info:
m.vec_size_base0_unique_ptr(obj)
assert (
str(exc_info.value) == "Cannot disown external shared_ptr (load_as_unique_ptr)."
)
def test_virtual_base_not_at_offset_0():
# This test ensures that the Diamond fixture actually exercises a non-zero
# virtual-base subobject offset on our supported platforms/ABIs.
#
# If this assert ever fails on some platform/toolchain, please adjust the
# C++ fixture so the virtual base is *not* at offset 0:
# - Keep VBase non-empty.
# - Make Left and Right non-empty and asymmetrically sized and, if
# needed, nudge with a modest alignment.
# - The goal is to achieve a non-zero address delta between `Diamond*`
# and `static_cast<VBase*>(Diamond*)`.
#
# Rationale: certain smart_holder features are exercised only when the
# registered subobject address differs from the most-derived object start,
# so this check guards test efficacy across compilers.
addrs = m.diamond_addrs()
assert addrs.as_vbase - addrs.as_self != 0, (
"Diamond VBase at offset 0 on this platform; to ensure test efficacy, "
"tweak fixtures (VBase/Left/Right) to ensure non-zero subobject offset."
)
@pytest.mark.parametrize(
"make_fn",
[
m.make_diamond_as_vbase_raw_ptr, # exercises smart_holder::from_raw_ptr_take_ownership
m.make_diamond_as_vbase_shared_ptr, # exercises smart_holder_from_shared_ptr
m.make_diamond_as_vbase_unique_ptr, # exercises smart_holder_from_unique_ptr
],
)
def test_make_diamond_as_vbase(make_fn):
# Added under PR #5836
vb = make_fn()
assert vb.ping() == 7
@pytest.mark.parametrize(
"clone_fn",
[
m.Tiger.clone_raw_ptr,
m.Tiger.clone_shared_ptr,
m.Tiger.clone_unique_ptr,
],
)
def test_animal_cat_tiger(clone_fn):
# Based on Animal-Cat-Tiger reproducer under PR #5796
tiger = m.Tiger()
cloned = clone_fn(tiger)
assert isinstance(cloned, m.Tiger)