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

Do not allocate non-PyObjects with PyObject_Malloc() #114569

Closed
Tracked by #108219
colesbury opened this issue Jan 25, 2024 · 4 comments · Fixed by #114574, #114587 or #114690
Closed
Tracked by #108219

Do not allocate non-PyObjects with PyObject_Malloc() #114569

colesbury opened this issue Jan 25, 2024 · 4 comments · Fixed by #114574, #114587 or #114690
Labels

Comments

@colesbury
Copy link
Contributor

colesbury commented Jan 25, 2024

Feature or enhancement

The free-threaded build requires that Python objects -- and only Python objects -- be allocated through the Python object allocation APIs (like PyObject_Malloc() or PyType_GenericNew()) 1.

There are a few places internally 2 that use PyObject_Malloc() for non Python objects. We should switch those call sites to use PyMem_Malloc()/Free() instead.

Note that there is not a significant difference between using PyObject_Malloc() and PyMem_Malloc() in the default build. Both calls use obmalloc under the hood, so switching from one to the other should not matter for the default build.

Here are some examples, but this list may not be exhaustive:

ctx->u.rep = (SRE_REPEAT*) PyObject_Malloc(sizeof(*ctx->u.rep));

self->extra = PyObject_Malloc(sizeof(ElementObjectExtra));

children = PyObject_Realloc(self->extra->children,
size * sizeof(PyObject*));

https://github.com/python/cpython/blob/main/Modules/mathmodule.c#L2573
static XML_Memory_Handling_Suite ExpatMemoryHandler = {
PyObject_Malloc, PyObject_Realloc, PyObject_Free};

new->ob_bytes = PyObject_Malloc(alloc);

char *result = (char *)PyObject_Malloc((input_length + 1) * sizeof(char));

Linked PRs

Footnotes

  1. See https://peps.python.org/pep-0703/#backwards-compatibility:~:text=Non%2DPython%20objects%20must%20not%20be%20allocated%20through%20those%20APIs

  2. @DinoV fixed the use in dictobject.c in https://github.com/python/cpython/pull/114543

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Jan 26, 2024

There is some strange usage in unicodeobject.c as well; I did not look closely at it yet. Perhaps it is ok, perhaps not.

@colesbury
Copy link
Contributor Author

Yeah, it looks like a two of those should use PyMem as well, but not the struct encoding_map one -- that's a Python object.

char *cache = PyObject_Malloc(len + 1);

data = PyObject_Malloc((length + 1) * char_size);

@erlend-aasland
Copy link
Contributor

Did we catch all of them?

@colesbury
Copy link
Contributor Author

Yeah, looks like it!

aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
…114574)

Fix usage in Modules, Objects, and Parser subdirectories.
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
…114574)

Fix usage in Modules, Objects, and Parser subdirectories.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment