fix: Narrow conditions for load_value to give invalid address

docs/advanced/classes.rst

@@ -1427,4 +1427,12 @@ You can do that using ``py::custom_type_setup``:
    cls.def("size", &ContainerOwnsPythonObjects::size);
    cls.def("clear", &ContainerOwnsPythonObjects::clear);
 
+.. note::
+
+   The ``py::detail::is_holder_constructed()`` guards above are required. During garbage
+   collection, ``tp_traverse`` and ``tp_clear`` may be handed an instance whose C++ value has
+   not been constructed yet -- for example one created with ``__new__`` before ``__init__``
+   has run. Casting such an instance raises ``ValueError``, and an exception must not be
+   allowed to escape either of these slots.
+
 .. versionadded:: 2.8

include/pybind11/cast.h

@@ -2179,6 +2179,12 @@ class argument_loader {
 private:
     static bool load_impl_sequence(function_call &, index_sequence<>) { return true; }
 
+    template <size_t I>
+    bool load_one(function_call &call) {
+        loader_life_support::argument_load_guard guard(I == 0);
+        return std::get<I>(argcasters).load(call.args[I], call.args_convert[I]);
+    }
+
     template <size_t... Is>
     bool load_impl_sequence(function_call &call, index_sequence<Is...>) {
         PYBIND11_WARNING_PUSH
@@ -2187,11 +2193,11 @@ class argument_loader {
         PYBIND11_WARNING_DISABLE_GCC("-Warray-bounds")
 #endif
 #ifdef __cpp_fold_expressions
-        if ((... || !std::get<Is>(argcasters).load(call.args[Is], call.args_convert[Is]))) {
+        if ((... || !load_one<Is>(call))) {
             return false;
         }
 #else
-        for (bool r : {std::get<Is>(argcasters).load(call.args[Is], call.args_convert[Is])...}) {
+        for (bool r : {load_one<Is>(call)...}) {
             if (!r) {
                 return false;
             }
@@ -2203,6 +2209,7 @@ class argument_loader {
 
     template <typename Return, typename Func, size_t... Is, typename Guard>
     Return call_impl(Func &&f, index_sequence<Is...>, Guard &&) && {
+        loader_life_support::old_style_init_call_guard old_style_init_guard;
         return std::forward<Func>(f)(cast_op<Args>(std::move(std::get<Is>(argcasters)))...);
     }
 

include/pybind11/detail/common.h

@@ -18,16 +18,16 @@
 // See also: https://github.com/python/cpython/blob/HEAD/Include/patchlevel.h
 /* -- start version constants -- */
 #define PYBIND11_VERSION_MAJOR 3
-#define PYBIND11_VERSION_MINOR 1
+#define PYBIND11_VERSION_MINOR 2
 #define PYBIND11_VERSION_MICRO 0
 // ALPHA = 0xA, BETA = 0xB, GAMMA = 0xC (release candidate), FINAL = 0xF (stable release)
 // - The release level is set to "alpha" for development versions.
 //   Use 0xA0 (LEVEL=0xA, SERIAL=0) for development versions.
 // - For stable releases, set the serial to 0.
-#define PYBIND11_VERSION_RELEASE_LEVEL PY_RELEASE_LEVEL_FINAL
+#define PYBIND11_VERSION_RELEASE_LEVEL PY_RELEASE_LEVEL_ALPHA
 #define PYBIND11_VERSION_RELEASE_SERIAL 0
 // String version of (micro, release level, release serial), e.g.: 0a0, 0b1, 0rc1, 0
-#define PYBIND11_VERSION_PATCH 0
+#define PYBIND11_VERSION_PATCH 0a0
 /* -- end version constants -- */
 
 #if !defined(Py_PACK_FULL_VERSION)
@@ -632,7 +632,9 @@ struct nonsimple_values_and_holders {
     uint8_t *status;
 };
 
-/// The 'instance' type which needs to be standard layout (need to be able to use 'offsetof')
+/// The 'instance' type which needs to be standard layout (need to be able to use 'offsetof').
+/// Changes to this struct or to the semantics of its members require incrementing
+/// `PYBIND11_INTERNALS_VERSION`.
 struct instance {
     PyObject_HEAD
     /// Storage for pointers and holder; see simple_layout, below, for a description
@@ -676,6 +678,8 @@ struct instance {
     bool has_patients : 1;
     /// If true, this Python object needs to be kept alive for the lifetime of the C++ value.
     bool is_alias : 1;
+    /// For simple layout, tracks whether a constructor is currently constructing the C++ value.
+    bool simple_value_constructing : 1;
 
     /// Initializes all of the above type/values/holders data (but not the instance values
     /// themselves)
@@ -693,6 +697,7 @@ struct instance {
     /// Bit values for the non-simple status flags
     static constexpr uint8_t status_holder_constructed = 1;
     static constexpr uint8_t status_instance_registered = 2;
+    static constexpr uint8_t status_value_constructing = 4;
 };
 
 static_assert(std::is_standard_layout<instance>::value,

include/pybind11/detail/internals.h

@@ -39,11 +39,11 @@
 /// further ABI-incompatible changes may be made before the ABI is officially
 /// changed to the new version.
 #ifndef PYBIND11_INTERNALS_VERSION
-#    define PYBIND11_INTERNALS_VERSION 12
+#    define PYBIND11_INTERNALS_VERSION 13
 #endif
 
-#if PYBIND11_INTERNALS_VERSION < 12
-#    error "PYBIND11_INTERNALS_VERSION 12 is the minimum for all platforms for pybind11 v3.1.0"
+#if PYBIND11_INTERNALS_VERSION < 13
+#    error "PYBIND11_INTERNALS_VERSION 13 is the minimum for all platforms for pybind11 v3.2.0"
 #endif
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)

include/pybind11/detail/type_caster_base.h

@@ -9,6 +9,7 @@
 
 #pragma once
 
+#include <pybind11/critical_section.h>
 #include <pybind11/gil.h>
 #include <pybind11/pytypes.h>
 #include <pybind11/trampoline_self_life_support.h>
@@ -61,9 +62,37 @@ class loader_life_support {
     loader_life_support *parent = nullptr;
     std::unordered_set<PyObject *> keep_alive;
 
+    // Old-style placement-new constructors need raw storage while loading their `self`
+    // argument. Keep it private to the exact overload candidate until its C++ callable returns.
+    value_and_holder *old_style_init_self = nullptr;
+    void *old_style_init_storage = nullptr;
+    bool old_style_init_self_load_allowed = false;
+    bool old_style_init_self_load_claimed = false;
+
+    static bool is_same_value_and_holder(const value_and_holder &lhs,
+                                         const value_and_holder &rhs) {
+        return lhs.inst == rhs.inst && lhs.vh == rhs.vh;
+    }
+
+    void cleanup_old_style_init_storage() {
+        if (old_style_init_storage == nullptr) {
+            return;
+        }
+        auto v_h = *old_style_init_self;
+        scoped_critical_section lock(
+            handle(reinterpret_cast<PyObject *>(old_style_init_self->inst)));
+        if (v_h.value_ptr() != nullptr) {
+            pybind11_fail("loader_life_support: old-style constructor storage collision");
+        }
+        v_h.value_ptr() = old_style_init_storage;
+        old_style_init_storage = nullptr;
+        v_h.type->dealloc(v_h); // Frees the storage and nulls the value pointer.
+    }
+
 public:
     /// A new patient frame is created when a function is entered
-    loader_life_support() {
+    explicit loader_life_support(value_and_holder *old_style_init_self = nullptr)
+        : old_style_init_self(old_style_init_self) {
         auto &frame = tls_current_frame();
         parent = frame;
         frame = this;
@@ -76,11 +105,118 @@ class loader_life_support {
             pybind11_fail("loader_life_support: internal error");
         }
         frame = parent;
+        cleanup_old_style_init_storage();
         for (auto *item : keep_alive) {
             Py_DECREF(item);
         }
     }
 
+    /// Restricts the special old-style constructor permission to argument zero of the current
+    /// candidate. A nested bound call has its own loader frame and cannot inherit this permission.
+    class argument_load_guard {
+    public:
+        explicit argument_load_guard(bool is_first_argument) {
+            auto *current = tls_current_frame();
+            if (is_first_argument && current != nullptr && current->old_style_init_self != nullptr
+                && !current->old_style_init_self_load_claimed) {
+                frame = current;
+                frame->old_style_init_self_load_allowed = true;
+            }
+        }
+        ~argument_load_guard() {
+            if (frame != nullptr) {
+                frame->old_style_init_self_load_allowed = false;
+            }
+        }
+        argument_load_guard(const argument_load_guard &) = delete;
+        argument_load_guard &operator=(const argument_load_guard &) = delete;
+
+    private:
+        loader_life_support *frame = nullptr;
+    };
+
+    /// Some legacy `__setstate__` implementations accept `self` as `py::object` and perform the
+    /// typed cast inside the C++ callable. At that point all other arguments have finished
+    /// loading, so granting the same exact, one-shot permission is safe. Reentrant bound calls
+    /// still get a separate loader frame.
+    class old_style_init_call_guard {
+    public:
+        old_style_init_call_guard() {
+            auto *current = tls_current_frame();
+            if (current != nullptr && current->old_style_init_self != nullptr
+                && !current->old_style_init_self_load_claimed) {
+                frame = current;
+                frame->old_style_init_self_load_allowed = true;
+            }
+        }
+        ~old_style_init_call_guard() {
+            if (frame != nullptr) {
+                frame->old_style_init_self_load_allowed = false;
+            }
+        }
+        old_style_init_call_guard(const old_style_init_call_guard &) = delete;
+        old_style_init_call_guard &operator=(const old_style_init_call_guard &) = delete;
+
+    private:
+        loader_life_support *frame = nullptr;
+    };
+
+    /// Claims and allocates the private storage for the exact old-style constructor `self` load.
+    /// The permission is consumed before invoking a potentially user-defined operator new.
+    static bool try_reserve_old_style_init_storage(value_and_holder &v_h,
+                                                   const type_info *type,
+                                                   void *&value) {
+        auto *frame = tls_current_frame();
+        if (frame == nullptr || !frame->old_style_init_self_load_allowed
+            || frame->old_style_init_self_load_claimed || frame->old_style_init_self == nullptr
+            || !is_same_value_and_holder(v_h, *frame->old_style_init_self)
+            || v_h.value_ptr() != nullptr) {
+            return false;
+        }
+
+        frame->old_style_init_self_load_claimed = true;
+        frame->old_style_init_self_load_allowed = false;
+        if (type->operator_new) {
+            frame->old_style_init_storage = type->operator_new(type->type_size);
+        } else {
+#if defined(__cpp_aligned_new)
+            if (type->type_align > __STDCPP_DEFAULT_NEW_ALIGNMENT__) {
+                frame->old_style_init_storage
+                    = ::operator new(type->type_size, std::align_val_t(type->type_align));
+            } else {
+                frame->old_style_init_storage = ::operator new(type->type_size);
+            }
+#else
+            frame->old_style_init_storage = ::operator new(type->type_size);
+#endif
+        }
+        if (frame->old_style_init_storage == nullptr) {
+            throw std::bad_alloc();
+        }
+        value = frame->old_style_init_storage;
+        return true;
+    }
+
+    /// Publishes a successfully placement-constructed value and immediately finalizes its holder.
+    /// This runs after the C++ callable, but before return-value conversion and post-call
+    /// policies.
+    static void complete_old_style_init() {
+        auto *frame = tls_current_frame();
+        if (frame == nullptr || frame->old_style_init_storage == nullptr) {
+            return;
+        }
+
+        auto v_h = *frame->old_style_init_self;
+        scoped_critical_section lock(handle(reinterpret_cast<PyObject *>(v_h.inst)));
+        if (!v_h.value_constructing() || v_h.value_ptr() != nullptr) {
+            pybind11_fail("loader_life_support: invalid old-style constructor commit");
+        }
+        v_h.value_ptr() = frame->old_style_init_storage;
+        frame->old_style_init_storage = nullptr;
+        v_h.type->init_instance(v_h.inst, nullptr);
+        v_h.set_value_constructing(false);
+    }
+
     /// Keep `h` alive until the current patient frame is destroyed, if there is one.
     /// Returns false when called outside a bound function (no frame). Use this, rather
     /// than `add_patient`, when failing to register is acceptable because the caller
@@ -525,6 +661,7 @@ PYBIND11_NOINLINE void instance::allocate_layout() {
             = reinterpret_cast<std::uint8_t *>(&nonsimple.values_and_holders[flags_at]);
     }
     owned = true;
+    simple_value_constructing = false;
 }
 
 // NOLINTNEXTLINE(readability-make-member-function-const)
@@ -534,6 +671,59 @@ PYBIND11_NOINLINE void instance::deallocate_layout() {
     }
 }
 
+/// Marks the exact value slot targeted by a constructor. This state is never permission to load
+/// the value: every load is rejected until construction finishes, apart from the one-shot
+/// old-style constructor `self` permission maintained by `loader_life_support`.
+class instance_construction_scope {
+public:
+    explicit instance_construction_scope(value_and_holder *v_h) {
+        if (v_h == nullptr) {
+            return;
+        }
+        v_h_ = *v_h;
+        started_ = false;
+        scoped_critical_section lock(handle(reinterpret_cast<PyObject *>(v_h_.inst)));
+        if (v_h_.value_constructing()) {
+            return;
+        }
+        if (v_h_.instance_registered()) {
+            already_registered_ = true;
+            return;
+        }
+        value_was_null_ = v_h_.value_ptr() == nullptr;
+        v_h_.set_value_constructing();
+        started_ = true;
+    }
+    ~instance_construction_scope() {
+        if (!started_ || v_h_.inst == nullptr) {
+            return;
+        }
+
+        scoped_critical_section lock(handle(reinterpret_cast<PyObject *>(v_h_.inst)));
+        // A failed new-style constructor can have published a value without completing its
+        // holder. Preserve the existing cleanup guarantee for that case.
+        if (value_was_null_ && !v_h_.holder_constructed() && v_h_.value_ptr() != nullptr) {
+            if (v_h_.instance_registered()) {
+                deregister_instance(v_h_.inst, v_h_.value_ptr(), v_h_.type);
+                v_h_.set_instance_registered(false);
+            }
+            v_h_.type->dealloc(v_h_);
+        }
+        v_h_.set_value_constructing(false);
+    }
+    instance_construction_scope(const instance_construction_scope &) = delete;
+    instance_construction_scope &operator=(const instance_construction_scope &) = delete;
+
+    bool started() const { return started_; }
+    bool already_registered() const { return already_registered_; }
+
+private:
+    value_and_holder v_h_;
+    bool started_ = true;
+    bool already_registered_ = false;
+    bool value_was_null_ = false;
+};
+
 PYBIND11_NOINLINE bool isinstance_generic(handle obj, const std::type_info &tp) {
     handle type = detail::get_type_handle(tp, false);
     if (!type) {
@@ -1128,6 +1318,25 @@ class type_caster_generic {
 
     // Base methods for generic caster; there are overridden in copyable_holder_caster
     void load_value(value_and_holder &&v_h) {
+        scoped_critical_section lock(handle(reinterpret_cast<PyObject *>(v_h.inst)));
+
+        // A non-null value pointer is not sufficient while a constructor is running: old-style
+        // placement-new storage may exist before the C++ object's lifetime has begun. Only the
+        // exact argument-zero load of the current old-style constructor may access private raw
+        // storage; reentrant, cross-base, nested, and cross-thread loads must all fail.
+        if (v_h.value_constructing()) {
+            void *reserved_value = nullptr;
+            const auto *type = v_h.type ? v_h.type : typeinfo;
+            if (loader_life_support::try_reserve_old_style_init_storage(
+                    v_h, type, reserved_value)) {
+                value = reserved_value;
+                return;
+            }
+            throw value_error("Missing value for wrapped C++ type `"
+                              + clean_type_id(cpptype->name())
+                              + "`: Python instance is still being constructed.");
+        }
+
         if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
             smart_holder_type_caster_support::value_and_holder_helper v_h_helper;
             v_h_helper.loaded_v_h = v_h;
@@ -1138,22 +1347,12 @@ class type_caster_generic {
             }
         }
         auto *&vptr = v_h.value_ptr();
-        // Lazy allocation for unallocated values:
         if (vptr == nullptr) {
-            const auto *type = v_h.type ? v_h.type : typeinfo;
-            if (type->operator_new) {
-                vptr = type->operator_new(type->type_size);
-            } else {
-#if defined(__cpp_aligned_new)
-                if (type->type_align > __STDCPP_DEFAULT_NEW_ALIGNMENT__) {
-                    vptr = ::operator new(type->type_size, std::align_val_t(type->type_align));
-                } else {
-                    vptr = ::operator new(type->type_size);
-                }
-#else
-                vptr = ::operator new(type->type_size);
-#endif
-            }
+            throw value_error("Missing value for wrapped C++ type `"
+                              + clean_type_id(cpptype->name())
+                              + "`: Python instance is uninitialized: the C++ object was "
+                                "never constructed (`__init__()` was bypassed, e.g. by "
+                                "calling `__new__()` directly).");
         }
         value = vptr;
     }