-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: refcount bug involving trampoline functions with PyObject *
return type.
#5156
Merged
rwgk
merged 9 commits into
pybind:master
from
rwgk:trampoline_pyobject_ptr_return_fix_master
Jun 11, 2024
Merged
fix: refcount bug involving trampoline functions with PyObject *
return type.
#5156
rwgk
merged 9 commits into
pybind:master
from
rwgk:trampoline_pyobject_ptr_return_fix_master
Jun 11, 2024
Conversation
This file contains 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
…cast to avoid duplicating the type name [modernize-use-auto,-warnings-as-errors]
``` /__w/pybind11/pybind11/tests/test_type_caster_pyobject_ptr.cpp:23:13: error: definition of implicit copy constructor for 'WithPyObjectPtrReturn' is deprecated because it has a user-declared destructor [-Werror,-Wdeprecated] virtual ~WithPyObjectPtrReturn() = default; ^ ```
For completeness, the original failure was (using the Google-internal toolchain):
|
To ensure that the added test reproduces the original failure: Failure of the test added in this PR (without the production code changes; using the Google-internal toolchain):
|
rwgk
pushed a commit
to rwgk/pybind11clif
that referenced
this pull request
Jun 10, 2024
Ignoring the 3 failing Python 3.13 builds: The same failures appear under #3939. |
rwgk
changed the title
WIP: Fix refcount bug involving trampoline functions with
Fix refcount bug involving trampoline functions with Jun 10, 2024
PyObject *
return type.PyObject *
return type.
henryiii
approved these changes
Jun 11, 2024
Thanks @henryiii! |
henryiii
changed the title
Fix refcount bug involving trampoline functions with
fix: refcount bug involving trampoline functions with Jun 23, 2024
PyObject *
return type.PyObject *
return type.
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
The heap-use-after-free bug was discovered through PyCLIF-pybind11 testing with ASAN (see #5156 (comment)). The fix has two simple parts:
cast_safe()
specialization forPyObject *
.cast_ref
inPYBIND11_OVERRIDE_IMPL
(so thatcast_safe()
is used instead).These changes ensure that
in pybind11/functional.h does not incorrectly drop a Python reference count.
See also: PR #4601, which introduced
type_caster<PyObject>
but missed the corner case fixed by this PR.For easy reference: Google-internal global testing ID: OCL:641800583:BASE:641923381:1718036131555:271433f2
Suggested changelog entry:
A refcount bug (leading to heap-use-after-free) involving trampoline functions with ``PyObject *`` return type was fixed.