Repository navigation
Conversation
optional_caster and variant_caster destroyed their inner value caster when load() returned, but std::reference_wrapper<T> stores a reference into that caster's own storage when T is materialized from Python (e.g. std::string, Eigen matrix). The stored reference then dangled, so the first use during the bound call was a heap-use-after-free (pybind#5637). Hold the inner caster(s) as members so they outlive the loaded value for the duration of the bound call.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes #5637.
std::optional<std::reference_wrapper<T>>segfaults when used as a function argument, even though the plainstd::reference_wrapper<T>andstd::optional<T>forms both work.optional_caster::load()built its inner value caster as a frame-local, emplaced the loaded value into the optional, then let that local go out of scope when the function returned. For areference_wrapperthe loaded value is a reference into the inner caster's own storage, which is exactly where theTlives whenever it has to be materialized from Python (anEigen::MatrixXd, astd::string, an arithmetic value); once the inner caster was destroyed that reference dangled, so the first read inside the bound call was a heap-use-after-free. I reproduced it under AddressSanitizer and found the same shape invariant_caster, whose per-alternative casters were also frame-locals. The fix keeps the inner caster(s) alive for as long as the composite caster lives, which is the duration of the bound call:optional_casternow holds its inner caster as a member, andvariant_casterholds astd::tupleof its alternative casters and indexes into it. The new tests round-trip a long, heap-allocatedstd::stringthroughstd::optional<std::reference_wrapper<std::string>>andstd::variant<int, std::reference_wrapper<std::string>>; both fail on master and pass with this change, and the rest of thetest_stlsuite stays green.Suggested changelog entry:
std::optionalorstd::variantofstd::reference_wrapperas an argument, where the referenced value is materialized by its caster.