From c4aec3628b6effcf205d70ce7d80f69847954b66 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 14 Mar 2016 22:26:53 +0100 Subject: [PATCH] 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 | 9 ++-- Doc/whatsnew/3.6.rst | 3 ++ Lib/test/test_capi.py | 30 +++++++++---- Misc/NEWS | 4 ++ Modules/_testcapimodule.c | 15 +++++++ Objects/obmalloc.c | 92 +++++++++++++++++++++++++++++---------- 6 files changed, 119 insertions(+), 34 deletions(-) diff --git a/Doc/c-api/memory.rst b/Doc/c-api/memory.rst index fe1cd5f3156..25867d9b876 100644 --- a/Doc/c-api/memory.rst +++ b/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 ` 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 diff --git a/Doc/whatsnew/3.6.rst b/Doc/whatsnew/3.6.rst index 588826b5599..b644a5ccda4 100644 --- a/Doc/whatsnew/3.6.rst +++ b/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 ` 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. diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 6c940ef1808..1de19a02f34 100644 --- a/Lib/test/test_capi.py +++ b/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__": diff --git a/Misc/NEWS b/Misc/NEWS index 852be069527..5233a43eb29 100644 --- a/Misc/NEWS +++ b/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. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index babffc4d671..b3d8818ac67 100644 --- a/Modules/_testcapimodule.c +++ b/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 */ }; diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index e4bd8ac6abe..f526b1ff209 100644 --- a/Objects/obmalloc.c +++ b/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_DebugRawMalloc; + alloc.calloc = _PyMem_DebugRawCalloc; + alloc.realloc = _PyMem_DebugRawRealloc; + alloc.free = _PyMem_DebugRawFree; - alloc.malloc = _PyMem_DebugMalloc; - alloc.calloc = _PyMem_DebugCalloc; - alloc.realloc = _PyMem_DebugRealloc; - alloc.free = _PyMem_DebugFree; - - 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.