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

Calling PyList_GetItemRef after PyList_New segfaults #121403

Closed
lysnikolaou opened this issue Jul 5, 2024 · 5 comments
Closed

Calling PyList_GetItemRef after PyList_New segfaults #121403

lysnikolaou opened this issue Jul 5, 2024 · 5 comments
Labels
topic-C-API topic-free-threading type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@lysnikolaou
Copy link
Contributor

lysnikolaou commented Jul 5, 2024

Crash report

What happened?

While working on Pillow, I came across the following issue: I'm getting a crash when calling PyList_GetItemRef immediately after calling PyList_New since list item is initialized to NULL.

PyObject *list = PyList_New(1);
return PyList_GetItemRef(list, 0);

In the docs for PyList_New, we include the following note:

If len is greater than zero, the returned list object’s items are set to NULL. Thus you cannot use abstract API functions such as PySequence_SetItem() or expose the object to Python code before setting all items to a real object with PyList_SetItem().

Not sure if this includes calling PyList_GetItemRef.

The problem is there on both builds, cause they both eventually try to call some variation of Py_NewRef / _Py_NewRefWithLock. PyList_GetItem does not crash and returns NULL.

CPython versions tested on:

CPython main branch

Operating systems tested on:

macOS

Output from running 'python -VV' on the command line:

Python 3.13.0b2+ (heads/3.13:7302855, Jun 21 2024, 10:05:10) [Clang 15.0.0 (clang-1500.3.9.4)]

Linked PRs

@lysnikolaou lysnikolaou added type-crash A hard crash of the interpreter, possibly with a core dump topic-free-threading labels Jul 5, 2024
@gaogaotiantian
Copy link
Member

gaogaotiantian commented Jul 5, 2024

Maybe PyList_GetItemRef should check for the size of the list, instead of the limit?. Made a mistake. Personally I think this belongs to a misusage - trying to work with an uninitialized list.

@corona10
Copy link
Member

corona10 commented Jul 7, 2024

I think that we should update the documentation, @colesbury WDYT?

@vstinner
Copy link
Member

vstinner commented Jul 8, 2024

Thus you cannot use abstract API functions such as PySequence_SetItem() or expose the object to Python code before setting all items to a real object with PyList_SetItem().

Maybe the doc must only list functions which are safe to use: only PyList_SET_ITEM() is safe until the list is fully initialized.

@corona10
Copy link
Member

corona10 commented Jul 8, 2024

only PyList_SET_ITEM() is safe until the list is fully initialized.

+1

@colesbury
Copy link
Contributor

Yeah, I think we should update the docs to make the restrictions more clear instead of changing the behavior of PyList_GetItemRef.

corona10 added a commit to corona10/cpython that referenced this issue Jul 11, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 16, 2024
corona10 added a commit that referenced this issue Jul 16, 2024
… init (gh-121626) (gh-121827)

gh-121403: Add notes for PyList_GetXXX APIs about the need for init (gh-121626)
(cherry picked from commit 2bac2b8)

Co-authored-by: Donghee Na <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-C-API topic-free-threading type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Development

No branches or pull requests

6 participants