Detect and fail if using mismatched holders

This adds a check when registering a class or a function with a holder
return that the same wrapped type hasn't been previously seen using a
different holder type.

This fixes pybind#1138 by detecting the failure; currently attempting to use
two different holder types (e.g. a unique_ptr<T> and shared_ptr<T>) in
difference places can segfault because we don't have any type safety on
the holder instances.

Author: jagerman

include/pybind11/cast.h

@@ -1607,6 +1607,33 @@ template <typename base, typename holder> struct is_holder_type :
 template <typename base, typename deleter> struct is_holder_type<base, std::unique_ptr<base, deleter>> :
     std::true_type {};
 
+template <typename holder> using is_holder = any_of<
+    is_template_base_of<move_only_holder_caster, make_caster<holder>>,
+    is_template_base_of<copyable_holder_caster, make_caster<holder>>>;
+
+template <typename holder>
+void check_for_holder_mismatch(enable_if_t<!is_holder<holder>::value, int> = 0) {}
+template <typename holder>
+void check_for_holder_mismatch(enable_if_t<is_holder<holder>::value, int> = 0) {
+    using iholder = intrinsic_t<holder>;
+    using base_type = decltype(*holder_helper<iholder>::get(std::declval<iholder>()));
+    auto &holder_typeinfo = typeid(iholder);
+    auto ins = get_internals().holders_seen.emplace(typeid(base_type), &holder_typeinfo);
+
+    auto debug = type_id<base_type>();
+    if (!ins.second && !same_type(*ins.first->second, holder_typeinfo)) {
+#ifdef NDEBUG
+        pybind11_fail("Mismatched holders detected (compile in debug mode for details)");
+#else
+        std::string seen_holder_name(ins.first->second->name());
+        detail::clean_type_id(seen_holder_name);
+        pybind11_fail("Mismatched holders detected: "
+                " attempting to use holder type " + type_id<iholder>() + ", but " + type_id<base_type>() +
+                " was already seen using holder type " + seen_holder_name);
+#endif
+    }
+}
+
 template <typename T> struct handle_type_name { static constexpr auto name = _<T>(); };
 template <> struct handle_type_name<bytes> { static constexpr auto name = _(PYBIND11_BYTES_NAME); };
 template <> struct handle_type_name<int_> { static constexpr auto name = _("int"); };

include/pybind11/detail/internals.h

@@ -97,6 +97,7 @@ struct internals {
     type_map<type_info *> registered_types_cpp; // std::type_index -> pybind11's type information
     std::unordered_map<PyTypeObject *, std::vector<type_info *>> registered_types_py; // PyTypeObject* -> base type_info(s)
     std::unordered_multimap<const void *, instance*> registered_instances; // void * -> instance*
+    type_map<const std::type_info *> holders_seen; // type -> seen holder type (to detect holder conflicts)
     std::unordered_set<std::pair<const PyObject *, const char *>, override_hash> inactive_override_cache;
     type_map<std::vector<bool (*)(PyObject *, void *&)>> direct_conversions;
     std::unordered_map<const PyObject *, std::vector<PyObject *>> patients;
@@ -150,7 +151,7 @@ struct type_info {
 };
 
 /// Tracks the `internals` and `type_info` ABI version independent of the main library version
-#define PYBIND11_INTERNALS_VERSION 4
+#define PYBIND11_INTERNALS_VERSION 5
 
 /// On MSVC, debug and release builds are not ABI-compatible!
 #if defined(_MSC_VER) && defined(_DEBUG)

include/pybind11/pybind11.h

@@ -157,6 +157,9 @@ class cpp_function : public function {
         static_assert(expected_num_args<Extra...>(sizeof...(Args), cast_in::has_args, cast_in::has_kwargs),
                       "The number of argument annotations does not match the number of function arguments");
 
+        // Fail if we've previously seen a different holder around the held type
+        detail::check_for_holder_mismatch<Return>();
+
         /* Dispatch code which converts function arguments and performs the actual function call */
         rec->impl = [](function_call &call) -> handle {
             cast_in args_converter;
@@ -1221,6 +1224,9 @@ class class_ : public detail::generic_type {
                 none_of<std::is_same<multiple_inheritance, Extra>...>::value), // no multiple_inheritance attr
             "Error: multiple inheritance bases must be specified via class_ template options");
 
+        // Fail if we've previously seen a different holder around the type
+        detail::check_for_holder_mismatch<holder_type>();
+
         type_record record;
         record.scope = scope;
         record.name = name;

tests/test_smart_ptr.cpp

@@ -41,7 +41,7 @@ template <typename T> class huge_unique_ptr {
     uint64_t padding[10];
 public:
     huge_unique_ptr(T *p) : ptr(p) {};
-    T *get() { return ptr.get(); }
+    T *get() const { return ptr.get(); }
 };
 PYBIND11_DECLARE_HOLDER_TYPE(T, huge_unique_ptr<T>);