Check the GIL in PyObject_Malloc()

Issue #26558: The debug hook of PyObject_Malloc() now checks that the GIL is
held when the function is called.

Doc/c-api/memory.rst

@@ -341,10 +341,13 @@ Customize Memory Allocators
    Newly allocated memory is filled with the byte ``0xCB``, freed memory is
    filled with the byte ``0xDB``. Additional checks:
 
-   - detect API violations, ex: :c:func:`PyObject_Free` called on a buffer
+   - Detect API violations, ex: :c:func:`PyObject_Free` called on a buffer
      allocated by :c:func:`PyMem_Malloc`
-   - detect write before the start of the buffer (buffer underflow)
-   - detect write after the end of the buffer (buffer overflow)
+   - Detect write before the start of the buffer (buffer underflow)
+   - Detect write after the end of the buffer (buffer overflow)
+   - Check that the :term:`GIL <global interpreter lock>` is held when
+     allocator functions of the :c:data:`PYMEM_DOMAIN_OBJ` domain (ex:
+     :c:func:`PyObject_Malloc`) are called
 
    These hooks are installed by default if Python is compiled in debug
    mode. The :envvar:`PYTHONMALLOC` environment variable can be used to install

Doc/whatsnew/3.6.rst

@@ -117,6 +117,9 @@ compiled in release mode using ``PYTHONMALLOC=debug``. Effects of debug hooks:
   :c:func:`PyMem_Malloc`.
 * Detect write before the start of the buffer (buffer underflow)
 * Detect write after the end of the buffer (buffer overflow)
+* Check that the :term:`GIL <global interpreter lock>` is held when allocator
+  functions of the :c:data:`PYMEM_DOMAIN_OBJ` domain (ex:
+  :c:func:`PyObject_Malloc`) are called
 
 See the :c:func:`PyMem_SetupDebugHooks` function for debug hooks on Python
 memory allocators.

Lib/test/test_capi.py

@@ -441,6 +441,7 @@ class EmbeddingTests(unittest.TestCase):
         self.maxDiff = None
         self.assertEqual(out.strip(), expected_output)
 
+
 class SkipitemTest(unittest.TestCase):
 
     def test_skipitem(self):
@@ -558,14 +559,15 @@ class Test_testcapi(unittest.TestCase):
                     test()
 
 
-class MallocTests(unittest.TestCase):
-    ENV = 'debug'
+class PyMemDebugTests(unittest.TestCase):
+    PYTHONMALLOC = 'debug'
     # '0x04c06e0' or '04C06E0'
     PTR_REGEX = r'(?:0x)?[0-9a-fA-F]+'
 
     def check(self, code):
         with support.SuppressCrashReport():
-            out = assert_python_failure('-c', code, PYTHONMALLOC=self.ENV)
+            out = assert_python_failure('-c', code,
+                                        PYTHONMALLOC=self.PYTHONMALLOC)
         stderr = out.err
         return stderr.decode('ascii', 'replace')
 
@@ -598,20 +600,30 @@ class MallocTests(unittest.TestCase):
         regex = regex.format(ptr=self.PTR_REGEX)
         self.assertRegex(out, regex)
 
+    def test_pyobject_malloc_without_gil(self):
+        # Calling PyObject_Malloc() without holding the GIL must raise an
+        # error in debug mode.
+        code = 'import _testcapi; _testcapi.pyobject_malloc_without_gil()'
+        out = self.check(code)
+        expected = ('Fatal Python error: Python memory allocator called '
+                    'without holding the GIL')
+        self.assertIn(expected, out)
+
 
-class MallocDebugTests(MallocTests):
-    ENV = 'malloc_debug'
+class PyMemMallocDebugTests(PyMemDebugTests):
+    PYTHONMALLOC = 'malloc_debug'
 
 
 @unittest.skipUnless(sysconfig.get_config_var('WITH_PYMALLOC') == 1,
                      'need pymalloc')
-class PymallocDebugTests(MallocTests):
-    ENV = 'pymalloc_debug'
+class PyMemPymallocDebugTests(PyMemDebugTests):
+    PYTHONMALLOC = 'pymalloc_debug'
 
 
 @unittest.skipUnless(Py_DEBUG, 'need Py_DEBUG')
-class DefaultMallocDebugTests(MallocTests):
-    ENV = ''
+class PyMemDefaultTests(PyMemDebugTests):
+    # test default allocator of Python compiled in debug mode
+    PYTHONMALLOC = ''
 
 
 if __name__ == "__main__":

Misc/NEWS

@@ -10,6 +10,10 @@ Release date: tba
 Core and Builtins
 -----------------
 
+- Issue #26558: The debug hooks on Python memory allocator
+  :c:func:`PyObject_Malloc` now detect when functions are called without
+  holding the GIL.
+
 - Issue #26516: Add :envvar`PYTHONMALLOC` environment variable to set the
   Python memory allocators and/or install debug hooks.
 

Modules/_testcapimodule.c

@@ -3643,6 +3643,20 @@ pymem_api_misuse(PyObject *self, PyObject *args)
     Py_RETURN_NONE;
 }
 
+static PyObject*
+pyobject_malloc_without_gil(PyObject *self, PyObject *args)
+{
+    char *buffer;
+
+    Py_BEGIN_ALLOW_THREADS
+    buffer = PyObject_Malloc(10);
+    Py_END_ALLOW_THREADS
+
+    PyObject_Free(buffer);
+
+    Py_RETURN_NONE;
+}
+
 
 static PyMethodDef TestMethods[] = {
     {"raise_exception",         raise_exception,                 METH_VARARGS},
@@ -3827,6 +3841,7 @@ static PyMethodDef TestMethods[] = {
     {"get_recursion_depth", get_recursion_depth, METH_NOARGS},
     {"pymem_buffer_overflow", pymem_buffer_overflow, METH_NOARGS},
     {"pymem_api_misuse", pymem_api_misuse, METH_NOARGS},
+    {"pyobject_malloc_without_gil", pyobject_malloc_without_gil, METH_NOARGS},
     {NULL, NULL} /* sentinel */
 };
 

Objects/obmalloc.c

@@ -16,10 +16,15 @@
 #define uptr    Py_uintptr_t
 
 /* Forward declaration */
+static void* _PyMem_DebugRawMalloc(void *ctx, size_t size);
+static void* _PyMem_DebugRawCalloc(void *ctx, size_t nelem, size_t elsize);
+static void* _PyMem_DebugRawRealloc(void *ctx, void *ptr, size_t size);
+static void _PyMem_DebugRawFree(void *ctx, void *p);
+
 static void* _PyMem_DebugMalloc(void *ctx, size_t size);
 static void* _PyMem_DebugCalloc(void *ctx, size_t nelem, size_t elsize);
-static void _PyMem_DebugFree(void *ctx, void *p);
 static void* _PyMem_DebugRealloc(void *ctx, void *ptr, size_t size);
+static void _PyMem_DebugFree(void *ctx, void *p);
 
 static void _PyObject_DebugDumpAddress(const void *p);
 static void _PyMem_DebugCheckAddress(char api_id, const void *p);
@@ -173,11 +178,14 @@ static struct {
     {'o', {NULL, PYOBJ_FUNCS}}
     };
 
-#define PYDBG_FUNCS _PyMem_DebugMalloc, _PyMem_DebugCalloc, _PyMem_DebugRealloc, _PyMem_DebugFree
+#define PYRAWDBG_FUNCS \
+    _PyMem_DebugRawMalloc, _PyMem_DebugRawCalloc, _PyMem_DebugRawRealloc, _PyMem_DebugRawFree
+#define PYDBG_FUNCS \
+    _PyMem_DebugMalloc, _PyMem_DebugCalloc, _PyMem_DebugRealloc, _PyMem_DebugFree
 
 static PyMemAllocatorEx _PyMem_Raw = {
 #ifdef Py_DEBUG
-    &_PyMem_Debug.raw, PYDBG_FUNCS
+    &_PyMem_Debug.raw, PYRAWDBG_FUNCS
 #else
     NULL, PYRAW_FUNCS
 #endif
@@ -185,7 +193,7 @@ static PyMemAllocatorEx _PyMem_Raw = {
 
 static PyMemAllocatorEx _PyMem = {
 #ifdef Py_DEBUG
-    &_PyMem_Debug.mem, PYDBG_FUNCS
+    &_PyMem_Debug.mem, PYRAWDBG_FUNCS
 #else
     NULL, PYMEM_FUNCS
 #endif
@@ -260,6 +268,7 @@ _PyMem_SetupAllocators(const char *opt)
 #undef PYRAW_FUNCS
 #undef PYMEM_FUNCS
 #undef PYOBJ_FUNCS
+#undef PYRAWDBG_FUNCS
 #undef PYDBG_FUNCS
 
 static PyObjectArenaAllocator _PyObject_Arena = {NULL,
@@ -296,27 +305,28 @@ PyMem_SetupDebugHooks(void)
 {
     PyMemAllocatorEx alloc;
 
-    /* hooks already installed */
-    if (_PyMem_DebugEnabled())
-        return;
-
-    alloc.malloc = _PyMem_DebugMalloc;
-    alloc.calloc = _PyMem_DebugCalloc;
-    alloc.realloc = _PyMem_DebugRealloc;
-    alloc.free = _PyMem_DebugFree;
+    alloc.malloc = _PyMem_DebugRawMalloc;
+    alloc.calloc = _PyMem_DebugRawCalloc;
+    alloc.realloc = _PyMem_DebugRawRealloc;
+    alloc.free = _PyMem_DebugRawFree;
 
-    if (_PyMem_Raw.malloc != _PyMem_DebugMalloc) {
+    if (_PyMem_Raw.malloc != _PyMem_DebugRawMalloc) {
         alloc.ctx = &_PyMem_Debug.raw;
         PyMem_GetAllocator(PYMEM_DOMAIN_RAW, &_PyMem_Debug.raw.alloc);
         PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &alloc);
     }
 
-    if (_PyMem.malloc != _PyMem_DebugMalloc) {
+    if (_PyMem.malloc != _PyMem_DebugRawMalloc) {
         alloc.ctx = &_PyMem_Debug.mem;
         PyMem_GetAllocator(PYMEM_DOMAIN_MEM, &_PyMem_Debug.mem.alloc);
         PyMem_SetAllocator(PYMEM_DOMAIN_MEM, &alloc);
     }
 
+    alloc.malloc = _PyMem_DebugMalloc;
+    alloc.calloc = _PyMem_DebugCalloc;
+    alloc.realloc = _PyMem_DebugRealloc;
+    alloc.free = _PyMem_DebugFree;
+
     if (_PyObject.malloc != _PyMem_DebugMalloc) {
         alloc.ctx = &_PyMem_Debug.obj;
         PyMem_GetAllocator(PYMEM_DOMAIN_OBJ, &_PyMem_Debug.obj.alloc);
@@ -1869,7 +1879,7 @@ p[2*S+n+S: 2*S+n+2*S]
 */
 
 static void *
-_PyMem_DebugAlloc(int use_calloc, void *ctx, size_t nbytes)
+_PyMem_DebugRawAlloc(int use_calloc, void *ctx, size_t nbytes)
 {
     debug_alloc_api_t *api = (debug_alloc_api_t *)ctx;
     uchar *p;           /* base address of malloc'ed block */
@@ -1906,18 +1916,18 @@ _PyMem_DebugAlloc(int use_calloc, void *ctx, size_t nbytes)
 }
 
 static void *
-_PyMem_DebugMalloc(void *ctx, size_t nbytes)
+_PyMem_DebugRawMalloc(void *ctx, size_t nbytes)
 {
-    return _PyMem_DebugAlloc(0, ctx, nbytes);
+    return _PyMem_DebugRawAlloc(0, ctx, nbytes);
 }
 
 static void *
-_PyMem_DebugCalloc(void *ctx, size_t nelem, size_t elsize)
+_PyMem_DebugRawCalloc(void *ctx, size_t nelem, size_t elsize)
 {
     size_t nbytes;
     assert(elsize == 0 || nelem <= PY_SSIZE_T_MAX / elsize);
     nbytes = nelem * elsize;
-    return _PyMem_DebugAlloc(1, ctx, nbytes);
+    return _PyMem_DebugRawAlloc(1, ctx, nbytes);
 }
 
 /* The debug free first checks the 2*SST bytes on each end for sanity (in
@@ -1926,7 +1936,7 @@ _PyMem_DebugCalloc(void *ctx, size_t nelem, size_t elsize)
    Then calls the underlying free.
 */
 static void
-_PyMem_DebugFree(void *ctx, void *p)
+_PyMem_DebugRawFree(void *ctx, void *p)
 {
     debug_alloc_api_t *api = (debug_alloc_api_t *)ctx;
     uchar *q = (uchar *)p - 2*SST;  /* address returned from malloc */
@@ -1943,7 +1953,7 @@ _PyMem_DebugFree(void *ctx, void *p)
 }
 
 static void *
-_PyMem_DebugRealloc(void *ctx, void *p, size_t nbytes)
+_PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes)
 {
     debug_alloc_api_t *api = (debug_alloc_api_t *)ctx;
     uchar *q = (uchar *)p, *oldq;
@@ -1953,7 +1963,7 @@ _PyMem_DebugRealloc(void *ctx, void *p, size_t nbytes)
     int i;
 
     if (p == NULL)
-        return _PyMem_DebugAlloc(0, ctx, nbytes);
+        return _PyMem_DebugRawAlloc(0, ctx, nbytes);
 
     _PyMem_DebugCheckAddress(api->api_id, p);
     bumpserialno();
@@ -1996,6 +2006,44 @@ _PyMem_DebugRealloc(void *ctx, void *p, size_t nbytes)
     return q;
 }
 
+static void
+_PyMem_DebugCheckGIL(void)
+{
+#ifdef WITH_THREAD
+    if (!PyGILState_Check())
+        Py_FatalError("Python memory allocator called "
+                      "without holding the GIL");
+#endif
+}
+
+static void *
+_PyMem_DebugMalloc(void *ctx, size_t nbytes)
+{
+    _PyMem_DebugCheckGIL();
+    return _PyMem_DebugRawMalloc(ctx, nbytes);
+}
+
+static void *
+_PyMem_DebugCalloc(void *ctx, size_t nelem, size_t elsize)
+{
+    _PyMem_DebugCheckGIL();
+    return _PyMem_DebugRawCalloc(ctx, nelem, elsize);
+}
+
+static void
+_PyMem_DebugFree(void *ctx, void *ptr)
+{
+    _PyMem_DebugCheckGIL();
+    return _PyMem_DebugRawFree(ctx, ptr);
+}
+
+static void *
+_PyMem_DebugRealloc(void *ctx, void *ptr, size_t nbytes)
+{
+    _PyMem_DebugCheckGIL();
+    return _PyMem_DebugRawRealloc(ctx, ptr, nbytes);
+}
+
 /* Check the forbidden bytes on both ends of the memory allocated for p.
  * If anything is wrong, print info to stderr via _PyObject_DebugDumpAddress,
  * and call Py_FatalError to kill the program.