Skip to content

Commit 83f71d8

Browse files
authored
fix: detect virtual inheritance in add_base to prevent pointer offset crash (#6017)
Virtual inheritance places the base subobject at a dynamic offset, but load_impl Case 2a uses reinterpret_cast which assumes a fixed offset. This caused segfaults when dispatching inherited methods through virtual bases (e.g. SftVirtDerived2::name()). Add an is_static_downcastable SFINAE trait that detects whether static_cast<Derived*>(Base*) is valid. When it is not (virtual inheritance), set multiple_inheritance = true in add_base to force the implicit_casts path, which correctly adjusts pointers at runtime. Remove the workaround .def("name", &SftVirtDerived2::name) from test_smart_ptr.cpp that was papering over the issue. Made-with: Cursor
1 parent 524d72b commit 83f71d8

File tree

3 files changed

+18
-4
lines changed

3 files changed

+18
-4
lines changed

include/pybind11/detail/common.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,17 @@ struct is_instantiation<Class, Class<Us...>> : std::true_type {};
10391039
template <typename T>
10401040
using is_shared_ptr = is_instantiation<std::shared_ptr, T>;
10411041

1042+
/// Detects whether static_cast<Derived*>(Base*) is valid, i.e. the inheritance is non-virtual.
1043+
/// Used to detect virtual bases: if this is false, pointer adjustments require the implicit_casts
1044+
/// chain rather than reinterpret_cast.
1045+
template <typename Base, typename Derived, typename = void>
1046+
struct is_static_downcastable : std::false_type {};
1047+
template <typename Base, typename Derived>
1048+
struct is_static_downcastable<Base,
1049+
Derived,
1050+
void_t<decltype(static_cast<Derived *>(std::declval<Base *>()))>>
1051+
: std::true_type {};
1052+
10421053
/// Check if T looks like an input iterator
10431054
template <typename T, typename = void>
10441055
struct is_input_iterator : std::false_type {};

include/pybind11/pybind11.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2416,6 +2416,13 @@ class class_ : public detail::generic_type {
24162416
rec.add_base(typeid(Base), [](void *src) -> void * {
24172417
return static_cast<Base *>(reinterpret_cast<type *>(src));
24182418
});
2419+
// Virtual inheritance means the base subobject is at a dynamic offset,
2420+
// so the reinterpret_cast shortcut in load_impl Case 2a is invalid.
2421+
// Force the MI path (implicit_casts) for correct pointer adjustment.
2422+
// Detection: static_cast<Derived*>(Base*) is ill-formed for virtual bases.
2423+
if PYBIND11_MAYBE_CONSTEXPR (!detail::is_static_downcastable<Base, type>::value) {
2424+
rec.multiple_inheritance = true;
2425+
}
24192426
}
24202427

24212428
template <typename Base, detail::enable_if_t<!is_base<Base>::value, int> = 0>

tests/test_smart_ptr.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -553,10 +553,6 @@ TEST_SUBMODULE(smart_ptr, m) {
553553
py::class_<SftVirtDerived2, SftVirtDerived, std::shared_ptr<SftVirtDerived2>>(
554554
m, "SftVirtDerived2")
555555
.def(py::init<>(&SftVirtDerived2::create))
556-
// TODO: Remove this once inherited methods work through virtual bases.
557-
// Without it, d2.name() segfaults because pybind11 uses an incorrect
558-
// pointer offset when dispatching through the virtual inheritance chain.
559-
.def("name", &SftVirtDerived2::name)
560556
.def("call_name", &SftVirtDerived2::call_name, py::arg("d2"));
561557

562558
// test_move_only_holder

0 commit comments

Comments
 (0)