Skip to content

Commit c9c8a40

Browse files
gh-118909: Fix crash freeing tp_doc allocated with PyObject_Malloc()
type_dealloc() always freed a heap type's tp_doc with PyMem_Free(), but some C extensions (e.g. older pybind11 and nanobind versions) allocate it directly with PyObject_Malloc() instead, relying on CPython to free it. The two allocator domains share the same underlying allocator in a release build, so this went unnoticed, but a build with debug allocator hooks enabled (Py_DEBUG, or PYTHONMALLOC=debug) tags each domain's blocks and aborts when a block is freed with the mismatched domain. Detect which allocator was actually used, from the tag debug builds write before the returned pointer, and free with the matching function as a backwards-compatibility fallback. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent e620377 commit c9c8a40

5 files changed

Lines changed: 99 additions & 1 deletion

File tree

Doc/c-api/typeobj.rst

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,6 +1565,15 @@ and :c:data:`PyType_Type` effectively act as defaults.)
15651565
type object. This is exposed as the :attr:`~type.__doc__` attribute on the
15661566
type and instances of the type.
15671567

1568+
For a heap type (:c:macro:`Py_TPFLAGS_HEAPTYPE`), if this field is set
1569+
directly rather than through the :c:data:`Py_tp_doc` slot, the string
1570+
must be allocated with :c:func:`PyMem_Malloc`, since CPython frees it
1571+
with :c:func:`PyMem_Free` when the type is deallocated. (As a
1572+
backwards-compatibility fallback, a string allocated with
1573+
:c:func:`PyObject_Malloc` is also detected and freed correctly, but
1574+
this fallback is not guaranteed and may be removed in a future
1575+
version -- see :gh:`118909`.)
1576+
15681577
**Inheritance:**
15691578

15701579
This field is *not* inherited by subtypes.

Lib/test/test_capi/test_mem.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,17 @@ def test_pyobject_malloc_without_gil(self):
9292
code = 'import _testcapi; _testcapi.pyobject_malloc_without_gil()'
9393
self.check_malloc_without_gil(code)
9494

95+
def test_pyobject_malloc_tp_doc(self):
96+
# gh-118909: tp_doc allocated with PyObject_Malloc() (as some
97+
# extensions do) must still be freeable when the type is
98+
# deallocated, even with debug allocator hooks enabled.
99+
assert_python_ok(
100+
'-c', 'import _testcapi; _testcapi.test_pyobject_malloc_tp_doc()',
101+
PYTHONMALLOC=self.PYTHONMALLOC,
102+
MALLOC_CONF="junk:false",
103+
MALLOC_OPTIONS="j",
104+
)
105+
95106
def check_pyobject_is_freed(self, func_name):
96107
code = textwrap.dedent(f'''
97108
import gc, os, sys, _testinternalcapi
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a crash in debug builds (or with :envvar:`PYTHONMALLOC=debug <PYTHONMALLOC>`)
2+
when deallocating a heap type whose :c:member:`~PyTypeObject.tp_doc` was
3+
allocated with :c:func:`PyObject_Malloc` instead of :c:func:`PyMem_Malloc`,
4+
as some C extensions do.

Modules/_testcapi/heaptype.c

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,46 @@ create_heapctype_with_none_bases_slot(PyObject *self, PyObject *Py_UNUSED(ignore
578578
}
579579

580580

581+
static PyType_Slot NoDocSlots_slots[] = {
582+
{0, 0},
583+
};
584+
585+
static PyType_Spec NoDocSlots_spec = {
586+
.name = "_testcapi.PyObjectMallocDocType",
587+
.basicsize = sizeof(PyObject),
588+
.flags = Py_TPFLAGS_DEFAULT,
589+
.slots = NoDocSlots_slots,
590+
};
591+
592+
static PyObject *
593+
test_pyobject_malloc_tp_doc(PyObject *self, PyObject *Py_UNUSED(ignored))
594+
{
595+
/* Regression test for gh-118909: some C extensions (e.g. older
596+
* pybind11/nanobind versions) allocate tp_doc with PyObject_Malloc()
597+
* directly instead of going through PyType_FromSpec()'s Py_tp_doc
598+
* slot, relying on CPython to free it when the type is deallocated.
599+
* Make sure that still works when debug allocator hooks are
600+
* enabled (PYTHONMALLOC=debug). */
601+
PyObject *type = PyType_FromSpec(&NoDocSlots_spec);
602+
if (type == NULL) {
603+
return NULL;
604+
}
605+
assert(((PyTypeObject *)type)->tp_doc == NULL);
606+
607+
static const char doc[] = "some docstring";
608+
char *tp_doc = PyObject_Malloc(sizeof(doc));
609+
if (tp_doc == NULL) {
610+
Py_DECREF(type);
611+
return PyErr_NoMemory();
612+
}
613+
memcpy(tp_doc, doc, sizeof(doc));
614+
((PyTypeObject *)type)->tp_doc = tp_doc;
615+
616+
Py_DECREF(type); // triggers type_dealloc(), which frees tp_doc
617+
Py_RETURN_NONE;
618+
}
619+
620+
581621
static PyMethodDef TestMethods[] = {
582622
{"pytype_fromspec_meta", pytype_fromspec_meta, METH_O},
583623
{"test_type_from_ephemeral_spec", test_type_from_ephemeral_spec, METH_NOARGS},
@@ -598,6 +638,7 @@ static PyMethodDef TestMethods[] = {
598638
{"pytype_getmodulebytoken", pytype_getmodulebytoken, METH_VARARGS},
599639
{"create_heapctype_with_none_bases_slot",
600640
create_heapctype_with_none_bases_slot, METH_NOARGS},
641+
{"test_pyobject_malloc_tp_doc", test_pyobject_malloc_tp_doc, METH_NOARGS},
601642
{NULL},
602643
};
603644

Objects/typeobject.c

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "pycore_object_alloc.h" // _PyObject_MallocWithType()
1717
#include "pycore_pyatomic_ft_wrappers.h"
1818
#include "pycore_pyerrors.h" // _PyErr_Occurred()
19+
#include "pycore_pymem.h" // _PyMem_DebugEnabled()
1920
#include "pycore_pystate.h" // _PyThreadState_GET()
2021
#include "pycore_slots.h" // _PySlotIterator_Init
2122
#include "pycore_symtable.h" // _Py_Mangle()
@@ -6857,6 +6858,38 @@ _PyTypes_FiniCachedDescriptors(PyInterpreterState *interp)
68576858
}
68586859

68596860

6861+
/* Free a heap type's tp_doc.
6862+
*
6863+
* tp_doc is documented as being allocated by the type's creator, and
6864+
* historically some C extensions (e.g. older versions of pybind11 and
6865+
* nanobind) set it directly with PyObject_Malloc(), relying on CPython
6866+
* to free it here. gh-114574 switched CPython's own allocation of
6867+
* tp_doc from PyObject_Malloc() to PyMem_Malloc(). The two domains
6868+
* share the same underlying allocator in a release build, so this
6869+
* didn't matter in practice, but a build with the debug allocator
6870+
* hooks enabled (Py_DEBUG, or PYTHONMALLOC=debug) tags each domain's
6871+
* blocks and aborts if a block is freed with the wrong one.
6872+
*
6873+
* Detect which allocator was actually used, from the tag debug builds
6874+
* write just before the returned pointer, and free with the matching
6875+
* function so extensions written against the old contract keep
6876+
* working. This is a backwards-compatibility fallback, not a stable
6877+
* API: extensions should allocate tp_doc with PyMem_Malloc() (see
6878+
* gh-118909).
6879+
*/
6880+
static void
6881+
type_free_tp_doc(char *tp_doc)
6882+
{
6883+
if (tp_doc != NULL && _PyMem_DebugEnabled()) {
6884+
char api_id = ((char *)tp_doc)[-(Py_ssize_t)SIZEOF_SIZE_T];
6885+
if (api_id == 'o') {
6886+
PyObject_Free(tp_doc);
6887+
return;
6888+
}
6889+
}
6890+
PyMem_Free(tp_doc);
6891+
}
6892+
68606893
static void
68616894
type_dealloc(PyObject *self)
68626895
{
@@ -6910,7 +6943,7 @@ type_dealloc(PyObject *self)
69106943
/* A type's tp_doc is heap allocated, unlike the tp_doc slots
69116944
* of most other objects. It's okay to cast it to char *.
69126945
*/
6913-
PyMem_Free((char *)type->tp_doc);
6946+
type_free_tp_doc((char *)type->tp_doc);
69146947

69156948
PyHeapTypeObject *et = (PyHeapTypeObject *)type;
69166949
Py_XDECREF(et->ht_name);

0 commit comments

Comments
 (0)