pybind11
pybind11 copied to clipboard
Add tests for accessing uninitialized holders
Description
Add tests for #4549.
- #4549
Suggested changelog entry:
cc @rwgk @henryiii for code review
Hi @XuehaiPan, to set expectations: Unless there is a surprise, any fix will have to come from you.
-
I recommend that you reduce this PR as much as possible ("minimal reproducer"). Concretely, reduce
VectorOwns4PythonObjectsto justappendandsize, and maybe change the name to something shorter (e.g.VecObj). Also remove all things related to the faulthandler/multiprocessing, those are just distracting you from working on a fix. (Please add[skip ci]to your commits, to not trigger the CI. The main purpose of the PR is that we can all inspect and clone it as needed.) -
Please explain (like you would in documentation) why this is useful, and what exactly cannot be achieved with a normal
init:
m.VectorOwns4PythonObjects.__new__(m.VectorOwns4PythonObjects)
-
Next, maybe, try a minimal change: preempt the apparently non-deterministic (?) undefined behavior by null-initializing the holder (?), so that you reliably get a segfault when it is dereferenced.
-
Usually, from work like that I get ideas for real fixes.
BTW: You can help me following along by not force pushing.
I recommend never force pushing, just keep merging in from master (i.e. do not rebase). We'll squash-merge in the end anyway.
I strongly recommend rebasing to keep following master, and never, ever create a merge commit. There's an even an automated rebase button in the dropdown below. If you create merge commits, eventually the pull request becomes unmergable and it is horrible to try to pick out the actual changes. It also becomes really hard to see what you've changed and what change upstream; eventually GitHub can't display it either. I've had a had an important long-standing PR in pypa/build die because of this. I spend hours trying to pick out the actual changes before giving up. Linear history is really important in git!
Just don't rebase to change previous commits, only to keep the PR fresh. (PS: if I'm the primary reviewer, then you can change commits too, I don't mind, I prefer the PR to tell a story. But this is the balance we've agreed to for general PRs here.)
PS: Feel free to follow @rwgk's wishes for this PR to get his help, this is general advice since he gave general advice.