bpo-36389: _PyObject_IsFreed() now also detects uninitialized memory (GH-12770)
Replace _PyMem_IsFreed() function with _PyMem_IsPtrFreed() inline
function. The function is now way more efficient, it became a simple
comparison on integers, rather than a short loop. It detects also
uninitialized bytes and "forbidden bytes" filled by debug hooks
on memory allocators.
Add unit tests on _PyObject_IsFreed().
diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h
index 1e7da87..78d457d 100644
--- a/Include/internal/pycore_pymem.h
+++ b/Include/internal/pycore_pymem.h
@@ -155,6 +155,31 @@
PyMemAllocatorDomain domain,
PyMemAllocatorEx *old_alloc);
+/* Heuristic checking if a pointer value is newly allocated
+ (uninitialized) or newly freed. The pointer is not dereferenced, only the
+ pointer value is checked.
+
+ The heuristic relies on the debug hooks on Python memory allocators which
+ fills newly allocated memory with CLEANBYTE (0xCB) and newly freed memory
+ with DEADBYTE (0xDB). Detect also "untouchable bytes" marked
+ with FORBIDDENBYTE (0xFB). */
+static inline int _PyMem_IsPtrFreed(void *ptr)
+{
+ uintptr_t value = (uintptr_t)ptr;
+#if SIZEOF_VOID_P == 8
+ return (value == (uintptr_t)0xCBCBCBCBCBCBCBCB
+ || value == (uintptr_t)0xDBDBDBDBDBDBDBDB
+ || value == (uintptr_t)0xFBFBFBFBFBFBFBFB
+ );
+#elif SIZEOF_VOID_P == 4
+ return (value == (uintptr_t)0xCBCBCBCB
+ || value == (uintptr_t)0xDBDBDBDB
+ || value == (uintptr_t)0xFBFBFBFB);
+#else
+# error "unknown pointer size"
+#endif
+}
+
#ifdef __cplusplus
}
#endif
diff --git a/Include/pymem.h b/Include/pymem.h
index 23457ad..93243f8 100644
--- a/Include/pymem.h
+++ b/Include/pymem.h
@@ -23,8 +23,6 @@
/* Try to get the allocators name set by _PyMem_SetupAllocators(). */
PyAPI_FUNC(const char*) _PyMem_GetAllocatorsName(void);
-
-PyAPI_FUNC(int) _PyMem_IsFreed(void *ptr, size_t size);
#endif /* !defined(Py_LIMITED_API) */
diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
index 7c68b2c..3cd39d4 100644
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -526,6 +526,29 @@
code = 'import _testcapi; _testcapi.pyobject_malloc_without_gil()'
self.check_malloc_without_gil(code)
+ def check_pyobject_is_freed(self, func):
+ code = textwrap.dedent('''
+ import gc, os, sys, _testcapi
+ # Disable the GC to avoid crash on GC collection
+ gc.disable()
+ obj = _testcapi.{func}()
+ error = (_testcapi.pyobject_is_freed(obj) == False)
+ # Exit immediately to avoid a crash while deallocating
+ # the invalid object
+ os._exit(int(error))
+ ''')
+ code = code.format(func=func)
+ assert_python_ok('-c', code, PYTHONMALLOC=self.PYTHONMALLOC)
+
+ def test_pyobject_is_freed_uninitialized(self):
+ self.check_pyobject_is_freed('pyobject_uninitialized')
+
+ def test_pyobject_is_freed_forbidden_bytes(self):
+ self.check_pyobject_is_freed('pyobject_forbidden_bytes')
+
+ def test_pyobject_is_freed_free(self):
+ self.check_pyobject_is_freed('pyobject_freed')
+
class PyMemMallocDebugTests(PyMemDebugTests):
PYTHONMALLOC = 'malloc_debug'
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index c515efe..1180b4b 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -4237,6 +4237,59 @@
static PyObject*
+pyobject_is_freed(PyObject *self, PyObject *op)
+{
+ int res = _PyObject_IsFreed(op);
+ return PyBool_FromLong(res);
+}
+
+
+static PyObject*
+pyobject_uninitialized(PyObject *self, PyObject *args)
+{
+ PyObject *op = (PyObject *)PyObject_Malloc(sizeof(PyObject));
+ if (op == NULL) {
+ return NULL;
+ }
+ /* Initialize reference count to avoid early crash in ceval or GC */
+ Py_REFCNT(op) = 1;
+ /* object fields like ob_type are uninitialized! */
+ return op;
+}
+
+
+static PyObject*
+pyobject_forbidden_bytes(PyObject *self, PyObject *args)
+{
+ /* Allocate an incomplete PyObject structure: truncate 'ob_type' field */
+ PyObject *op = (PyObject *)PyObject_Malloc(offsetof(PyObject, ob_type));
+ if (op == NULL) {
+ return NULL;
+ }
+ /* Initialize reference count to avoid early crash in ceval or GC */
+ Py_REFCNT(op) = 1;
+ /* ob_type field is after the memory block: part of "forbidden bytes"
+ when using debug hooks on memory allocatrs! */
+ return op;
+}
+
+
+static PyObject*
+pyobject_freed(PyObject *self, PyObject *args)
+{
+ PyObject *op = _PyObject_CallNoArg((PyObject *)&PyBaseObject_Type);
+ if (op == NULL) {
+ return NULL;
+ }
+ Py_TYPE(op)->tp_dealloc(op);
+ /* Reset reference count to avoid early crash in ceval or GC */
+ Py_REFCNT(op) = 1;
+ /* object memory is freed! */
+ return op;
+}
+
+
+static PyObject*
pyobject_malloc_without_gil(PyObject *self, PyObject *args)
{
char *buffer;
@@ -4907,6 +4960,10 @@
{"pymem_api_misuse", pymem_api_misuse, METH_NOARGS},
{"pymem_malloc_without_gil", pymem_malloc_without_gil, METH_NOARGS},
{"pymem_getallocatorsname", test_pymem_getallocatorsname, METH_NOARGS},
+ {"pyobject_is_freed", (PyCFunction)(void(*)(void))pyobject_is_freed, METH_O},
+ {"pyobject_uninitialized", pyobject_uninitialized, METH_NOARGS},
+ {"pyobject_forbidden_bytes", pyobject_forbidden_bytes, METH_NOARGS},
+ {"pyobject_freed", pyobject_freed, METH_NOARGS},
{"pyobject_malloc_without_gil", pyobject_malloc_without_gil, METH_NOARGS},
{"tracemalloc_track", tracemalloc_track, METH_VARARGS},
{"tracemalloc_untrack", tracemalloc_untrack, METH_VARARGS},
diff --git a/Objects/object.c b/Objects/object.c
index bd44aca..c9aa479 100644
--- a/Objects/object.c
+++ b/Objects/object.c
@@ -425,18 +425,17 @@
int
_PyObject_IsFreed(PyObject *op)
{
- uintptr_t ptr = (uintptr_t)op;
- if (_PyMem_IsFreed(&ptr, sizeof(ptr))) {
+ if (_PyMem_IsPtrFreed(op) || _PyMem_IsPtrFreed(op->ob_type)) {
return 1;
}
- int freed = _PyMem_IsFreed(&op->ob_type, sizeof(op->ob_type));
- /* ignore op->ob_ref: the value can have be modified
+ /* ignore op->ob_ref: its value can have be modified
by Py_INCREF() and Py_DECREF(). */
#ifdef Py_TRACE_REFS
- freed &= _PyMem_IsFreed(&op->_ob_next, sizeof(op->_ob_next));
- freed &= _PyMem_IsFreed(&op->_ob_prev, sizeof(op->_ob_prev));
+ if (_PyMem_IsPtrFreed(op->_ob_next) || _PyMem_IsPtrFreed(op->_ob_prev)) {
+ return 1;
+ }
#endif
- return freed;
+ return 0;
}
@@ -453,7 +452,7 @@
if (_PyObject_IsFreed(op)) {
/* It seems like the object memory has been freed:
don't access it to prevent a segmentation fault. */
- fprintf(stderr, "<freed object>\n");
+ fprintf(stderr, "<Freed object>\n");
return;
}
diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c
index 1c2a320..e919fad 100644
--- a/Objects/obmalloc.c
+++ b/Objects/obmalloc.c
@@ -1914,7 +1914,7 @@
/* Special bytes broadcast into debug memory blocks at appropriate times.
* Strings of these are unlikely to be valid addresses, floats, ints or
- * 7-bit ASCII.
+ * 7-bit ASCII. If modified, _PyMem_IsPtrFreed() should be updated as well.
*/
#undef CLEANBYTE
#undef DEADBYTE
@@ -2059,22 +2059,6 @@
}
-/* Heuristic checking if the memory has been freed. Rely on the debug hooks on
- Python memory allocators which fills the memory with DEADBYTE (0xDB) when
- memory is deallocated. */
-int
-_PyMem_IsFreed(void *ptr, size_t size)
-{
- unsigned char *bytes = ptr;
- for (size_t i=0; i < size; i++) {
- if (bytes[i] != DEADBYTE) {
- return 0;
- }
- }
- return 1;
-}
-
-
/* The debug free first checks the 2*SST bytes on each end for sanity (in
particular, that the FORBIDDENBYTEs with the api ID are still intact).
Then fills the original bytes with DEADBYTE.