fix: Narrow conditions for `load_value` to give invalid address (#6157)
* fix: Guard against using a uninitialized value after `__new__` allocating python object
fixes: #6153
Objects initialized with `cls.__new__(cls)` (`cls` is a pybind11 bound
type). Will not have the C++ object allocated. When hitting `load_value`
storage is allocated but not initialized, calling a virtual method will
load a garbage vptr and segfault. This is similar to #2152, but the
guard in metaclass `__call__` is not triggered when using `__new__`.
Protect against giving a pointer to garbage in all cases except the
`__init__` + `__setstate__` path.
Authored with claude
* fix: free lazily allocated storage on failed init and only permit lazy allocation for old-style constructors
If an old-style placement-new `__init__`/`__setstate__` failed after
`self` was loaded, the lazily allocated storage stayed behind with a
null-holder instance, so the uninitialized-value guard never fired again
and later use read uninitialized memory. `instance_construction_scope`
now tracks the constructor's `value_and_holder` and frees storage that
was lazily allocated during a construction that did not complete.
Also arm the scope only when the overload chain contains an old-style
constructor. New-style constructors receive `self` directly and never
need lazy allocation, so reentrant loads of the half-built instance now
raise `ValueError` instead of handing out uninitialized storage.
Assisted-by: ClaudeCode:claude-fable-5
Claude-Session: https://claude.ai/code/session_01TQXCSykMn5EL7sc6VgTUTC
* fix: isolate old-style constructor storage
Track construction per value-and-holder, grant a one-shot loader-frame permission only to the exact legacy constructor self conversion, and keep its raw storage private until the native callback returns.
Reject reentrant, nested, cross-base, and cross-thread loads while preserving overload fallback, failure cleanup, pickle setstate callbacks, and repeated initialization behavior.
* test: skip constructor thread test on Emscripten
* fix: bump internals version to 13
The new detail::instance construction state has cross-DSO semantics that internals-v12 modules do not understand. Isolate the incompatible domains for v3.2.0 and document that future structural or semantic instance changes require another bump.
* Revert "fix: bump internals version to 13"
This reverts commit 14e32ae23af529df8d82681c2d3064884b259a3c.
* fix: recover from legacy constructor storage collisions
* test: fix collision subprocess imports
* refactor: simplify old-style constructor storage tracking
The loader frame already identifies the constructor candidate, so the
one-shot `self` permission only needs a frame match and a claimed flag.
This removes both argument guard classes, the changes to cast.h, and the
per-call TLS lookups they added.
Also:
- Hoist deallocate_instance_value to a detail free function and use it
from instance_construction_scope.
- Take the dispatcher's constructor lock before the construction scope and
drop the nested critical sections it made redundant.
- Keep the non-constructor path inline: the loader destructor checks for
storage before the out-of-line cleanup, and the construction scope
defaults to not started.
- Commit old-style storage once in cpp_function::initialize, gated on
is_constructor.
- Share one __index__ probe across the reentrancy tests and turn the
subprocess script into a plain function.
Assisted-by: ClaudeCode:claude-fable-5-1
* test: reject later self alias during old-style init
* fix: restrict old-style constructor self permission to self's own load phase
The one-shot `self` permission granted by `loader_life_support` matched only the
frame and the value slot, not the *phase* of the load. The slot is identified by
the instance, so any later argument that aliases the same, still-unconstructed
`self` matched too, consumed the reservation, and reached C++ over raw storage.
Track the frame's phase instead: authorize the load of positional argument 0
(the typed-`self` variant) and casts performed from within the C++ callable (the
legacy `py::object`-self variant), and deny every load during conversion of
positional arguments >= 1. `argument_loader` reports the argument index, and the
dispatcher flips the frame to the callable phase once loading is done. The frame
pointer is resolved once in the dispatcher, where `is_constructor` is known, so
no non-constructor call pays a thread-local lookup.
This restores the restriction that 89a5f72e dropped, re-enabling the regression
test added in 7a7e9f34, and adds three more tests. Both new negative tests
assert the decisive observable rather than only that the callback was entered,
and both use types chosen so that a build where the guard has regressed reports
an assertion failure instead of crashing during teardown:
- A later argument typed as a *base* of the class under construction. It shares
the value slot, so it matched, and the reservation was then sized from the
base's `type_info`: 16 bytes for a 144-byte derived object. Because the claim
is one-shot it also denied `self` its own storage, so the callback could not
placement-new at all; the value was nevertheless committed, and destroying it
ran a virtual destructor over never-constructed memory. The test asserts that
no reservation was made at the base's size.
- The still-unconstructed `self` reached through a container argument. Here
`stl.h`'s element caster copy-constructs, so the read of uninitialized memory
happens inside pybind11 and no binding author can guard against it. The test
counts copy constructions whose source was raw storage and asserts zero; the
instrumented copy constructor does not read its source, so the test itself
performs no uninitialized read.
- A positive test pinning the case that must keep working: a later argument that
is a different, already-constructed instance of the same class.
Assisted-by: ClaudeCode:claude-opus-5
* style: pre-commit fixes
* style: clang-tidy fixes
The Clang-Tidy job failed on two `modernize-use-default-member-init`
diagnostics in tests/test_class.cpp, both introduced by this branch:
tests/test_class.cpp:167:18: error: use default member initializer
for 'payload' [modernize-use-default-member-init,-warnings-as-errors]
tests/test_class.cpp:177:9: error: use default member initializer
for 'value' [modernize-use-default-member-init,-warnings-as-errors]
Applied exactly the replacements clang-tidy emitted:
- `AliasStealDerived::payload` gains a `{}` default member initializer
and drops `payload{}` from the constructor initializer list. Both
value-initialize the array.
- `ContainerAliasItem::value` gains a `{-1}` default member initializer
and the copy constructor drops `value(-1)`. The converting constructor
keeps `value(v)`, which overrides the default, so both constructors
still produce the values the container-alias test asserts on.
No behavior change; clang-tidy is not installed locally, so the fix-its
were transcribed from the CI diagnostic rather than auto-applied, and
the translation unit was compiled clean at -std=c++17 with the CI
warning set.
Assisted-by: ClaudeCode:claude-opus-5
* fix: GraalPY exceptions
Both GraalPy jobs failed on the same assertion in the legacy-v12
collision test:
tests/test_class.py:469: assert stats() == (3, 3, constructed + 1, constructed + 1)
That is the final check, reached after `del obj` and two `gc.collect()`
calls. GraalPy is not refcounted and does not guarantee finalization
from `gc.collect()`, so the destruction counters lag and the assertion
fails while every preceding assertion in the loop passes.
Gate only that assertion behind `if not env.GRAALPY:`. This follows the
existing convention in the suite, where GC-timing-dependent checks are
exempted on GraalPy with the same "Cannot reliably trigger GC" reason
(test_call_policies.py, test_callbacks.py,
test_class_sh_trampoline_shared_ptr_cpp_arg.py, and others).
Nothing this branch introduces stops being tested on GraalPy. The gated
line only observes ordinary teardown of a normal, fully constructed
retry object. The rollback properties the test exists for are pinned by
the assertions above it, which still run everywhere: after rollback both
collision allocations are freed with no spurious destruction, and after
the retry exactly one allocation is live with the private value's
destructor having run neither early nor twice.
Assisted-by: ClaudeCode:claude-opus-5
* test: pin the two gaps identified in the load-phase review
Adds coverage for the two limitations called out in the review of the
load-phase restriction. Both tests pass, pinning today's behavior; both
fail against `master`'s headers, which is what makes them meaningful.
test_old_style_init_value_error_hides_later_overload
Two old-style candidates take the same two Python arguments. The
first one's argument 1 is the `self` alias that the construction
guard rejects; the second matches the same call and constructs.
The guard reports rejection with `value_error`, and only
`reference_cast_error` becomes PYBIND11_TRY_NEXT_OVERLOAD, so the
throw escapes the overload loop and the second candidate is never
attempted.
Verified counterfactual, same test files built against master's
headers: master reaches the second candidate and constructs
(`entered == ["second candidate entered"]`); here the call raises
ValueError with `entered == []`.
Note this is a new trigger for pre-existing behavior rather than a
new behavior: master's casters already throw `value_error` from load
paths with the same non-fallthrough consequence.
test_old_style_init_callable_phase_grant_is_not_self_specific
While the callable runs, the one-shot grant is keyed on the value
slot, not on the `self` handle, so a cast of `stash[0]` claims the
reservation and the genuine `self` cast then fails. Narrowing the
grant to "a cast of the `self` object" would not close this: the
claiming cast targets the same Python object as `self`, so the two
are indistinguishable at cast time.
Verified counterfactual: on master both casts succeed
(`["stash cast claimed the reservation", "self cast succeeded"]`)
because every load lazily allocates. The one-shot reservation is
therefore a narrowing of master's behavior, and this gap is the
residue rather than a regression.
Neither callback inspects the reference it obtains over storage whose
lifetime has not begun, so the tests themselves stay free of undefined
behavior. Both verify the object is still retryable afterwards.
Assisted-by: ClaudeCode:claude-opus-5
* perf: only test the old-style frame pointer where the phase can change
Addresses the review suggestion to stop paying the null check once per
argument.
The literal form suggested, `I == 0 &&`, is not safe: `begin_argument_load`
is what moves the frame from `self_argument` to `later_argument`, so
skipping it for arguments 1 and up leaves the phase at `self_argument` for
the whole argument list. That re-opens exactly the hole 955cb193 closed. It
regresses four tests:
test_old_style_init_does_not_authorize_later_self_alias
test_old_style_init_does_not_authorize_base_typed_later_alias
test_old_style_init_does_not_authorize_self_alias_inside_container
test_old_style_init_value_error_hides_later_overload
Gate on `I < 2` instead. The phase only changes at argument 0 and argument
1; from argument 2 on it is already `later_argument`, so those arguments
need no call and no test. Two checks per call rather than one, but it is
the minimum that preserves the invariant. All 55 tests pass.
`I` is a template parameter, so no `if constexpr` is needed and none can be
used: pybind11 still supports C++11 and `if constexpr` is a C++17
extension there. A plain `if` on a constant condition already folds
completely. clang -O2, the `I == 5` instantiation of a reduction of this
function tail-calls straight through with no pointer test emitted, while
`I == 0` and `I == 1` keep theirs.
MSVC C4127 (constant conditional) is already disabled file-wide at the top
of cast.h, and the header compiles clean at -std=c++11/14/17/20 with
-Wall -Wextra -Wpedantic -Wconversion -Werror.
Assisted-by: ClaudeCode:claude-opus-5
* docs: describe status_value_constructing with the other status bits
The non-simple layout comment enumerated status_holder_constructed and
status_instance_registered but not status_value_constructing, which was
added alongside them. Addresses the review comment on that block.
Also states what the bit means for readers of the value pointer: while it
is set, the pointer must not be treated as denoting a live C++ object.
That is the invariant the rest of this change depends on, and the status
byte is where someone will look for it.
Comment-only. Longest line is 94 columns, within the 99-column limit, so
clang-format does not reflow it.
Assisted-by: ClaudeCode:claude-opus-5
* style: pre-commit fixes
* style: clang-tidy/format
* fix: narrow uninitialized-instance guard to direct misuse
Return to the minimal scope needed for #6153: ordinary loads of a wrapper with no constructed C++ value raise ValueError, while overload chains containing deprecated placement-new constructors retain their historical lazy allocation. Failed old-style construction also frees lazily allocated storage so the object remains guarded and retryable.
Remove the expanded per-value construction protocol, including private candidate storage, argument and callable phases, cross-thread locking, and stale-v12 collision recovery. Those mechanisms attempted to make deprecated placement-new construction safe under reentry rather than fixing direct __new__ misuse.
Accordingly, remove tests requiring special handling for later arguments (both distinct instances and self aliases), base-typed and container aliases, nested initialization, Python multiple-inheritance bases, concurrent access, mixed old/new overloads, and old-style __setstate__ reentry. Also remove tests pinning protocol-specific overload fallthrough, callable-phase grants, and legacy-v12 collision cleanup.
Keep focused coverage for direct __new__ misuse, reentry during new-style construction, deprecated __init__/__setstate__ compatibility, failed-construction cleanup, and successful retry.
* test: characterize old-style reentrant load limitation
Keep one pointer-only probe for the historical broad lazy-allocation window. It records the known hazard that a reentrant load can expose a pointer to unconstructed storage, without inspecting or dereferencing that storage.
The cleanup and retry assertions remain in place so the behavior retained by the minimal fix stays covered.
* docs: document deprecated placement-new limitations
Explain that the compatibility window spans an entire constructor overload chain and can expose unconstructed storage during reentrant conversion or callbacks, nested initialization, Python multiple inheritance, or concurrent access.
Recommend new-style constructor and pickle APIs, and add source-level references at the compatibility flag and scope so future changes encounter the accepted limitations and rationale before attempting to narrow the window.
---------
Co-authored-by: Henry Schreiner <henryfs@princeton.edu>
Co-authored-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Co-authored-by: Andrew M. James <ajames@openteams.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst
index 2954411..a5de5b1 100644
--- a/docs/advanced/classes.rst
+++ b/docs/advanced/classes.rst
@@ -875,6 +875,12 @@
rules as the single-argument version of ``py::init()``. The return type can be
a value, pointer or holder type. See :ref:`custom_constructors` for details.
+Calling ``__new__`` directly creates the Python wrapper without constructing its C++ value.
+Outside deprecated placement-new constructor dispatch, passing such an uninitialized wrapper to
+bound C++ code raises ``ValueError``. Calling its ``__init__`` or a pickle-generated
+``__setstate__`` can still finish construction normally. See :ref:`old_style_placement_new` for
+the compatibility behavior and safety limitations of deprecated placement-new callbacks.
+
An instance can now be pickled as follows:
.. code-block:: python
@@ -1427,4 +1433,12 @@
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
diff --git a/docs/upgrade.rst b/docs/upgrade.rst
index 966a319..236494a 100644
--- a/docs/upgrade.rst
+++ b/docs/upgrade.rst
@@ -381,6 +381,8 @@
}
+.. _old_style_placement_new:
+
New API for defining custom constructors and pickling functions
---------------------------------------------------------------
@@ -408,6 +410,25 @@
// or: return Foo(...); // return by value (move constructor)
}));
+.. warning::
+
+ Deprecated placement-new ``__init__`` and ``__setstate__`` callbacks receive access to raw
+ storage before the C++ object's lifetime begins. For compatibility, pybind11 retains this
+ behavior for the complete constructor overload chain whenever the chain contains such a
+ callback. This compatibility feature has important caveats: other loads triggered during
+ argument conversion or callback execution may receive a C++ pointer to the storage even
+ though no C++ object has been constructed there yet. Accessing the storage through such
+ a pointer as though it contained a live C++ object results in undefined behavior.
+
+ Consequently, until placement-new completes, the binding must not otherwise load or inspect
+ the instance as a C++ object. Unsafe access can occur through reentrant argument conversion
+ or callback code, nested initialization, another C++ base in a Python multiple-inheritance
+ instance, or concurrent access. Mixing old- and new-style constructor overloads does not
+ narrow the window. Such access may treat unconstructed storage as a live object and result
+ in undefined behavior. To avoid these hazards, use ``py::init()`` factories and
+ ``py::pickle()`` for new bindings, and migrate existing placement-new callbacks wherever
+ practical.
+
Mirroring the custom constructor changes, ``py::pickle()`` is now the preferred
way to get and set object state. See :ref:`pickling` for details.
diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h
index 9f8b3b3..7738b3f 100644
--- a/include/pybind11/detail/common.h
+++ b/include/pybind11/detail/common.h
@@ -676,6 +676,14 @@
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;
+ /// If true, this instance is being dispatched through a constructor chain containing a
+ /// deprecated old-style placement-new `__init__`/`__setstate__`. Such chains retain the
+ /// historical ability to lazily allocate C++ value storage. This is an instance-wide
+ /// compatibility marker, not per-value construction state or a synchronization mechanism.
+ /// Its intentionally retained safety limitations are documented under
+ /// `old_style_placement_new` in `docs/upgrade.rst` and referenced from
+ /// `docs/advanced/classes.rst`.
+ bool old_style_init_active : 1;
/// Initializes all of the above type/values/holders data (but not the instance values
/// themselves)
diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h
index 82bfa0b..65b6bd0 100644
--- a/include/pybind11/detail/type_caster_base.h
+++ b/include/pybind11/detail/type_caster_base.h
@@ -525,6 +525,7 @@
= reinterpret_cast<std::uint8_t *>(&nonsimple.values_and_holders[flags_at]);
}
owned = true;
+ old_style_init_active = false;
}
// NOLINTNEXTLINE(readability-make-member-function-const)
@@ -534,6 +535,45 @@
}
}
+/// RAII helper preserving lazy value allocation for a constructor chain containing a deprecated
+/// old-style placement-new `__init__`/`__setstate__`. Passing `nullptr` makes this a no-op. The
+/// compatibility window covers the whole chain and all value slots in the Python instance; it does
+/// not attempt to distinguish the old-style `self` load from reentrant, later-argument,
+/// cross-base, nested, or concurrent loads. Nesting restores the previous state but is not made
+/// safe by this scope.
+/// Before narrowing this window, review `old_style_placement_new` in `docs/upgrade.rst` and its
+/// reference from `docs/advanced/classes.rst`: the broad scope preserves historical behavior,
+/// with documented reentrancy, multiple-inheritance, nesting, and concurrency limitations.
+///
+/// If construction fails (the holder was never constructed) after storage was lazily allocated
+/// inside this scope, the destructor frees that storage and resets the value pointer, so that the
+/// uninitialized-value guard in `load_value()` stays effective for later uses of the instance.
+class old_style_init_scope {
+public:
+ explicit old_style_init_scope(value_and_holder *v_h) : v_h_{v_h} {
+ if (v_h_ != nullptr) {
+ was_active_ = v_h_->inst->old_style_init_active;
+ value_was_null_ = v_h_->value_ptr() == nullptr;
+ v_h_->inst->old_style_init_active = true;
+ }
+ }
+ ~old_style_init_scope() {
+ if (v_h_ != nullptr) {
+ v_h_->inst->old_style_init_active = was_active_;
+ if (value_was_null_ && !v_h_->holder_constructed() && v_h_->value_ptr() != nullptr) {
+ v_h_->type->dealloc(*v_h_); // Frees the storage and nulls the value pointer.
+ }
+ }
+ }
+ old_style_init_scope(const old_style_init_scope &) = delete;
+ old_style_init_scope &operator=(const old_style_init_scope &) = delete;
+
+private:
+ value_and_holder *v_h_;
+ bool was_active_ = 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) {
@@ -1140,6 +1180,20 @@
auto *&vptr = v_h.value_ptr();
// Lazy allocation for unallocated values:
if (vptr == nullptr) {
+ // Lazy allocation exists only to support the deprecated old-style placement-new
+ // `__init__`/`__setstate__` idiom, which is handed a reference to uninitialized
+ // storage and constructs the C++ value into it. In any other context a null value
+ // pointer means the C++ object was never constructed -- e.g. the instance was created
+ // with `__new__()`, bypassing `__init__()` -- and handing out a pointer to
+ // uninitialized memory from here is undefined behavior (typically a segfault on the
+ // first virtual call). Fail loudly instead.
+ if (!v_h.inst->old_style_init_active) {
+ 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).");
+ }
const auto *type = v_h.type ? v_h.type : typeinfo;
if (type->operator_new) {
vptr = type->operator_new(type->type_size);
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
index c57312d..e9c0e90 100644
--- a/include/pybind11/pybind11.h
+++ b/include/pybind11/pybind11.h
@@ -1001,6 +1001,25 @@
}
}
+ // While a constructor chain containing an old-style placement-new
+ // `__init__`/`__setstate__` runs, `type_caster_generic::load_value()` is permitted to
+ // lazily allocate storage for the C++ value that the constructor is about to construct
+ // into. New-style constructors never load `self` through a type caster (it is injected
+ // directly below), so the scope stays disarmed for chains that contain only new-style
+ // constructors and loading a not-yet-constructed instance remains an error even while they
+ // run. The scope also frees storage that was lazily allocated by a constructor call that
+ // then failed.
+ detail::value_and_holder *lazily_allocatable_v_h = nullptr;
+ if (overloads->is_constructor) {
+ for (const function_record *fr = overloads; fr != nullptr; fr = fr->next) {
+ if (!fr->is_new_style_constructor) {
+ lazily_allocatable_v_h = &self_value_and_holder;
+ break;
+ }
+ }
+ }
+ detail::old_style_init_scope old_style_init_guard(lazily_allocatable_v_h);
+
try {
// We do this in two passes: in the first pass, we load arguments with `convert=false`;
// in the second, we allow conversion (except for arguments with an explicit
diff --git a/tests/test_class.cpp b/tests/test_class.cpp
index e520f29..21ff617 100644
--- a/tests/test_class.cpp
+++ b/tests/test_class.cpp
@@ -77,6 +77,27 @@
test_class::pr5396_forward_declared_class::ForwardClass>::value,
"");
+// test_new_bypasses_init
+struct NewNoInit {
+ int m_data;
+ explicit NewNoInit(int data) : m_data(data) {}
+ NewNoInit(const NewNoInit &) = default;
+ virtual ~NewNoInit() = default;
+ int data() const { return m_data; }
+ // Virtual on purpose: using a not-yet-constructed instance reads the vtable pointer out of
+ // uninitialized storage, which segfaults rather than merely returning a garbage value.
+ virtual int v_data() const { return m_data; }
+};
+
+// test_failed_old_style_init_does_not_leave_lazy_storage
+struct OldStyleInit {
+ int m_data;
+ explicit OldStyleInit(int data) : m_data(data) {}
+ virtual ~OldStyleInit() = default;
+ int data() const { return m_data; }
+ virtual int v_data() const { return m_data; }
+};
+
TEST_SUBMODULE(class_, m) {
m.def("obj_class_name", [](py::handle obj) { return py::detail::obj_class_name(obj.ptr()); });
@@ -597,6 +618,38 @@
m.def("return_universal_recipient", []() -> test_class::ConvertibleFromAnything {
return test_class::ConvertibleFromAnything{};
});
+
+ py::class_<NewNoInit>(m, "NewNoInit")
+ .def(py::init<int>())
+ .def("data", &NewNoInit::data)
+ .def("v_data", &NewNoInit::v_data)
+ .def(py::pickle([](const NewNoInit &p) { return py::make_tuple(p.m_data); },
+ [](const py::tuple &t) {
+ if (t.size() != 1) {
+ throw std::runtime_error("Invalid state!");
+ }
+ return NewNoInit(t[0].cast<int>());
+ }));
+
+ py::class_<OldStyleInit> old_style_init(m, "OldStyleInit");
+ ignoreOldStyleInitWarnings([&old_style_init]() {
+ old_style_init
+ .def("__init__",
+ [](OldStyleInit &self, int x) {
+ if (x < 0) {
+ throw std::runtime_error("negative data");
+ }
+ new (&self) OldStyleInit(x);
+ })
+ .def("__setstate__", [](const py::object &self, int x) {
+ auto &typed_self = self.cast<OldStyleInit &>();
+ new (&typed_self) OldStyleInit(x);
+ });
+ });
+ old_style_init.def("data", &OldStyleInit::data).def("v_data", &OldStyleInit::v_data);
+ // This probe intentionally does not dereference the pointer. It documents the narrow scope of
+ // this fix without itself reading storage before an OldStyleInit lifetime has begun.
+ m.def("expose_old_style_init_pointer", [](OldStyleInit *value) { return value != nullptr; });
}
template <int N>
diff --git a/tests/test_class.py b/tests/test_class.py
index 201c7e3..645b799 100644
--- a/tests/test_class.py
+++ b/tests/test_class.py
@@ -1,6 +1,7 @@
from __future__ import annotations
import gc
+import pickle
import sys
from unittest import mock
@@ -251,6 +252,115 @@
assert msg(exc_info.value) == expected
+def test_new_bypasses_init():
+ """`__new__` allocates the Python object but not the C++ one; using the instance before
+ `__init__` has run must raise instead of segfaulting."""
+
+ class PythonDerived(m.NewNoInit):
+ pass
+
+ for cls in (m.NewNoInit, PythonDerived):
+ obj = cls.__new__(cls)
+ for use in (obj.data, obj.v_data, obj.__getstate__):
+ with pytest.raises(ValueError) as exc_info:
+ use()
+ assert "Python instance is uninitialized" in str(exc_info.value)
+ assert "NewNoInit" in str(exc_info.value)
+
+ # Calling `__init__()` is the sanctioned way to finish an object made with `__new__()`.
+ obj.__init__(42)
+ assert obj.data() == 42
+ assert obj.v_data() == 42
+
+
+def test_new_then_setstate():
+ """`__new__` must not be blocked: pickle relies on it, and `__setstate__` finishes the
+ object off. This walks the protocol by hand, then checks the real thing."""
+ real_obj = m.NewNoInit(42)
+ assert real_obj.data() == 42
+ state = real_obj.__getstate__()
+
+ obj = m.NewNoInit.__new__(m.NewNoInit) # NEWOBJ
+ obj.__setstate__(state) # BUILD
+ assert obj.data() == 42
+ assert obj.v_data() == 42
+
+ for protocol in range(2, pickle.HIGHEST_PROTOCOL + 1):
+ assert pickle.loads(pickle.dumps(m.NewNoInit(7), protocol)).v_data() == 7
+
+
+def test_failed_old_style_init_does_not_leave_lazy_storage():
+ """If an old-style placement-new `__init__` throws before constructing the value, the
+ lazily allocated storage must not linger: later use must still raise, not segfault."""
+ obj = m.OldStyleInit.__new__(m.OldStyleInit)
+ with pytest.raises(RuntimeError, match="negative data"):
+ obj.__init__(-1)
+
+ # The failed __init__ already lazily allocated storage for `self`, so without cleanup the
+ # uninitialized-instance guard never fires again and this reads a garbage vtable pointer.
+ with pytest.raises(ValueError, match="uninitialized"):
+ obj.v_data()
+
+ # A successful retry is still allowed.
+ obj.__init__(42)
+ assert obj.v_data() == 42
+
+
+def test_old_style_setstate_remains_supported():
+ """Deprecated placement-new `__setstate__` may still obtain storage inside its callback."""
+ obj = m.OldStyleInit.__new__(m.OldStyleInit)
+ obj.__setstate__(43)
+ assert obj.data() == 43
+
+
+def test_old_style_init_reentrant_load_current_limitation():
+ """The historical broad lazy-allocation window retained during an old-style constructor
+ chain can expose a pointer to unconstructed storage through a reentrant load."""
+ obj = m.OldStyleInit.__new__(m.OldStyleInit)
+ seen = {}
+
+ class LoadOnIndex:
+ def __index__(self):
+ # This pointer-only probe deliberately does not inspect or dereference the storage.
+ seen["exposed"] = m.expose_old_style_init_pointer(obj)
+ raise TypeError("stop the constructor")
+
+ with pytest.raises(TypeError):
+ obj.__init__(LoadOnIndex())
+
+ assert seen == {"exposed": True}
+
+ # Failure cleanup removes the raw storage, so subsequent ordinary loads are rejected and a
+ # normal initialization retry remains possible.
+ with pytest.raises(ValueError, match="uninitialized"):
+ m.expose_old_style_init_pointer(obj)
+ obj.__init__(44)
+ assert obj.data() == 44
+
+
+def test_reentrant_load_during_new_style_init():
+ """New-style constructors never need lazy allocation, so passing the half-built instance
+ to another bound function while `__init__` runs must raise, not hand out garbage."""
+ obj = m.NewNoInit.__new__(m.NewNoInit)
+ seen = {}
+
+ class Evil:
+ def __index__(self):
+ # Runs during int conversion of a pure new-style constructor. Its chain has no
+ # compatibility window for lazy allocation.
+ try:
+ seen["data"] = obj.data()
+ except ValueError as exc:
+ seen["error"] = exc
+ raise TypeError("stop the constructor")
+
+ with pytest.raises(TypeError):
+ obj.__init__(Evil())
+
+ assert "data" not in seen, f"handed out uninitialized storage: {seen['data']!r}"
+ assert "error" in seen
+
+
@pytest.mark.parametrize(
"mock_return_value", [None, (1, 2, 3), m.Pet("Polly", "parrot"), m.Dog("Molly")]
)
