Skip to content
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

Check to see if object is None before traversing #2921

Merged
merged 2 commits into from
Jan 27, 2023

Conversation

neachdainn
Copy link
Contributor

Closes #2915

When using the C API directly, the intended way to call visitproc is via the Py_VISIT macro, which checks to see that the provided pointer is not null before passing it along to visitproc. Because PyO3 isn't using the macro, it needs to manually check that the pointer isn't null. Without this check, calling visit.call(&obj) where let obj = None; will segfault.

@neachdainn
Copy link
Contributor Author

I read through the contributing guide and I think I did the newsfragment correctly. If it isn't right, feel free to either let me know what I did wrong or just do whatever needs to be done to fix it.

@neachdainn
Copy link
Contributor Author

The failed CI jobs do not seem related to my changes, so I don't plan on addressing them.

@mejrs
Copy link
Member

mejrs commented Jan 26, 2023

Can you add a test? Your example in #2915 should do nicely. You can put it in a test function similar to https://github.com/PyO3/pyo3/pull/2914/files#diff-e28c611e920921574b15af9f361787212c273d1b4327337593639e619176bd6f

The failed CI jobs do not seem related to my changes, so I don't plan on addressing them.

That's just because some diagnostics have changed in today's new Rust version. If you rebase after #2922 they should pass.

@davidhewitt
Copy link
Member

Looks good to me, thanks - I think rebasing will fix CI (new rust release).

@davidhewitt
Copy link
Member

davidhewitt commented Jan 26, 2023

@mejrs is right, a test would be appreciated 😂

Closes PyO3#2915

When using the C API directly, the intended way to call `visitproc` is
via the `Py_VISIT` macro, which checks to see that the provided pointer
is not null before passing it along to `visitproc`. Because PyO3 isn't
using the macro, it needs to manually check that the pointer isn't null.
Without this check, calling `visit.call(&obj)` where `let obj = None;`
will segfault.
@neachdainn
Copy link
Contributor Author

Rebased and unit test added.

Since we're just testing a bug during traversal, we don't actually have
to reap the object, we just have to make a reference cycle so that it's
traverse method is called.
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@davidhewitt
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 27, 2023

Build succeeded:

@bors bors bot merged commit 083dd5f into PyO3:main Jan 27, 2023
@neachdainn neachdainn deleted the 2915-segfault branch January 27, 2023 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PyVisit::call segfaults when passed None
3 participants