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>