python · vstinner · Oct 7, 2019 · Oct 7, 2019
diff --git a/Include/cpython/unicodeobject.h b/Include/cpython/unicodeobject.h
@@ -246,6 +246,10 @@ typedef struct {
     } data;                     /* Canonical, smallest-form Unicode buffer */
 } PyUnicodeObject;
 
+PyAPI_FUNC(int) _PyUnicode_CheckConsistency(
+    PyObject *op,
+    int check_content);
+
 /* Fast access macros */
 #define PyUnicode_WSTR_LENGTH(op) \
     (PyUnicode_IS_COMPACT_ASCII(op) ?                  \

diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h
@@ -11,7 +11,6 @@ extern "C" {
 #include "pycore_pystate.h"   /* _PyRuntime.gc */
 
 PyAPI_FUNC(int) _PyType_CheckConsistency(PyTypeObject *type);
-PyAPI_FUNC(int) _PyUnicode_CheckConsistency(PyObject *op, int check_content);
 PyAPI_FUNC(int) _PyDict_CheckConsistency(PyObject *mp, int check_content);
 
 /* Tell the GC to track this object.

diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h
@@ -155,8 +155,9 @@ PyAPI_FUNC(int) _PyMem_SetDefaultAllocator(
     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.
+   (uninitialized), newly freed or NULL (is equal to zero).
+
+   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 (0xCD) and newly freed memory
@@ -166,11 +167,13 @@ static inline int _PyMem_IsPtrFreed(void *ptr)
 {
     uintptr_t value = (uintptr_t)ptr;
 #if SIZEOF_VOID_P == 8
-    return (value == (uintptr_t)0xCDCDCDCDCDCDCDCD
+    return (value == 0
+            || value == (uintptr_t)0xCDCDCDCDCDCDCDCD
             || value == (uintptr_t)0xDDDDDDDDDDDDDDDD
             || value == (uintptr_t)0xFDFDFDFDFDFDFDFD);
 #elif SIZEOF_VOID_P == 4
-    return (value == (uintptr_t)0xCDCDCDCD
+    return (value == 0
+            || value == (uintptr_t)0xCDCDCDCD
             || value == (uintptr_t)0xDDDDDDDD
             || value == (uintptr_t)0xFDFDFDFD);
 #else

diff --git a/Include/unicodeobject.h b/Include/unicodeobject.h
@@ -1032,16 +1032,6 @@ PyAPI_FUNC(int) PyUnicode_IsIdentifier(PyObject *s);
 
 /* === Characters Type APIs =============================================== */
 
-#if defined(Py_DEBUG) && !defined(Py_LIMITED_API)
-PyAPI_FUNC(int) _PyUnicode_CheckConsistency(
-    PyObject *op,
-    int check_content);
-#elif !defined(NDEBUG)
-/* For asserts that call _PyUnicode_CheckConsistency(), which would
- * otherwise be a problem when building with asserts but without Py_DEBUG. */
-#define _PyUnicode_CheckConsistency(op, check_content) PyUnicode_Check(op)
-#endif
-
 #ifndef Py_LIMITED_API
 #  define Py_CPYTHON_UNICODEOBJECT_H
 #  include  "cpython/unicodeobject.h"

diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
@@ -719,6 +719,9 @@ def check_pyobject_is_freed(self, func_name):
         ''')
         assert_python_ok('-c', code, PYTHONMALLOC=self.PYTHONMALLOC)
 
+    def test_pyobject_null_is_freed(self):
+        self.check_pyobject_is_freed('check_pyobject_null_is_freed')
+
     def test_pyobject_uninitialized_is_freed(self):
         self.check_pyobject_is_freed('check_pyobject_uninitialized_is_freed')
 

diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py
@@ -985,16 +985,19 @@ def test_refcount_errors(self):
             br'gcmodule\.c:[0-9]+: gc_decref: Assertion "gc_get_refs\(g\) > 0" failed.')
         self.assertRegex(stderr,
             br'refcount is too small')
+        # "address : 0x7fb5062efc18"
+        # "address : 7FB5062EFC18"
+        address_regex = br'[0-9a-fA-Fx]+'
         self.assertRegex(stderr,
-            br'object  : \[1, 2, 3\]')
+            br'object address  : ' + address_regex)
         self.assertRegex(stderr,
-            br'type    : list')
+            br'object refcount : 1')
         self.assertRegex(stderr,
-            br'refcount: 1')
-        # "address : 0x7fb5062efc18"
-        # "address : 7FB5062EFC18"
+            br'object type     : ' + address_regex)
+        self.assertRegex(stderr,
+            br'object type name: list')
         self.assertRegex(stderr,
-            br'address : [0-9a-fA-Fx]+')
+            br'object repr     : \[1, 2, 3\]')
 
 
 class GCTogglingTests(unittest.TestCase):

diff --git a/Misc/NEWS.d/next/C API/2019-10-07-17-15-09.bpo-36389.hFX_jD.rst b/Misc/NEWS.d/next/C API/2019-10-07-17-15-09.bpo-36389.hFX_jD.rst
@@ -0,0 +1,3 @@
+The ``_PyObject_CheckConsistency()`` function is now also available in release
+mode. For example, it can be used to debug a crash in the ``visit_decref()``
+function of the GC.
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
@@ -4611,6 +4611,14 @@ test_pyobject_is_freed(const char *test_name, PyObject *op)
 }
 
 
+static PyObject*
+check_pyobject_null_is_freed(PyObject *self, PyObject *Py_UNUSED(args))
+{
+    PyObject *op = NULL;
+    return test_pyobject_is_freed("check_pyobject_null_is_freed", op);
+}
+
+
 static PyObject*
 check_pyobject_uninitialized_is_freed(PyObject *self, PyObject *Py_UNUSED(args))
 {
@@ -5475,6 +5483,7 @@ static PyMethodDef TestMethods[] = {
     {"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},
+    {"check_pyobject_null_is_freed", check_pyobject_null_is_freed, METH_NOARGS},
     {"check_pyobject_uninitialized_is_freed", check_pyobject_uninitialized_is_freed, METH_NOARGS},
     {"check_pyobject_forbidden_bytes_is_freed", check_pyobject_forbidden_bytes_is_freed, METH_NOARGS},
     {"check_pyobject_freed_is_freed", check_pyobject_freed_is_freed, METH_NOARGS},

diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c
@@ -375,7 +375,6 @@ update_refs(PyGC_Head *containers)
 static int
 visit_decref(PyObject *op, void *data)
 {
    assert(op != NULL);
     _PyObject_ASSERT(op, !_PyObject_IsFreed(op));
 
     if (PyObject_IS_GC(op)) {

diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c
@@ -3206,10 +3206,10 @@ _PyBytesWriter_GetSize(_PyBytesWriter *writer, char *str)
     return str - start;
 }
 
-Py_LOCAL_INLINE(void)
+#ifndef NDEBUG
+Py_LOCAL_INLINE(int)
 _PyBytesWriter_CheckConsistency(_PyBytesWriter *writer, char *str)
 {
-#ifdef Py_DEBUG
     char *start, *end;
 
     if (writer->use_small_buffer) {
@@ -3239,15 +3239,16 @@ _PyBytesWriter_CheckConsistency(_PyBytesWriter *writer, char *str)
     end = start + writer->allocated;
     assert(str != NULL);
     assert(start <= str && str <= end);
-#endif
+    return 1;
 }
+#endif
 
 void*
 _PyBytesWriter_Resize(_PyBytesWriter *writer, void *str, Py_ssize_t size)
 {
     Py_ssize_t allocated, pos;
 
-    _PyBytesWriter_CheckConsistency(writer, str);
+    assert(_PyBytesWriter_CheckConsistency(writer, str));
     assert(writer->allocated < size);
 
     allocated = size;
@@ -3303,7 +3304,7 @@ _PyBytesWriter_Resize(_PyBytesWriter *writer, void *str, Py_ssize_t size)
     writer->allocated = allocated;
 
     str = _PyBytesWriter_AsString(writer) + pos;
-    _PyBytesWriter_CheckConsistency(writer, str);
+    assert(_PyBytesWriter_CheckConsistency(writer, str));
     return str;
 
 error:
@@ -3316,7 +3317,7 @@ _PyBytesWriter_Prepare(_PyBytesWriter *writer, void *str, Py_ssize_t size)
 {
     Py_ssize_t new_min_size;
 
-    _PyBytesWriter_CheckConsistency(writer, str);
+    assert(_PyBytesWriter_CheckConsistency(writer, str));
     assert(size >= 0);
 
     if (size == 0) {
@@ -3377,7 +3378,7 @@ _PyBytesWriter_Finish(_PyBytesWriter *writer, void *str)
     Py_ssize_t size;
     PyObject *result;
 
-    _PyBytesWriter_CheckConsistency(writer, str);
+    assert(_PyBytesWriter_CheckConsistency(writer, str));
 
     size = _PyBytesWriter_GetSize(writer, str);
     if (size == 0 && !writer->use_bytearray) {

diff --git a/Objects/dictobject.c b/Objects/dictobject.c
@@ -459,23 +459,26 @@ static PyObject *empty_values[1] = { NULL };
 int
 _PyDict_CheckConsistency(PyObject *op, int check_content)
 {
-#ifndef NDEBUG
-    _PyObject_ASSERT(op, PyDict_Check(op));
+#define CHECK(expr) \
+    do { if (!(expr)) { _PyObject_ASSERT_FAILED_MSG(op, Py_STRINGIFY(expr)); } } while (0)
+
+    assert(op != NULL);
+    CHECK(PyDict_Check(op));
     PyDictObject *mp = (PyDictObject *)op;
 
     PyDictKeysObject *keys = mp->ma_keys;
     int splitted = _PyDict_HasSplitTable(mp);
     Py_ssize_t usable = USABLE_FRACTION(keys->dk_size);
 
-    _PyObject_ASSERT(op, 0 <= mp->ma_used && mp->ma_used <= usable);
-    _PyObject_ASSERT(op, IS_POWER_OF_2(keys->dk_size));
-    _PyObject_ASSERT(op, 0 <= keys->dk_usable && keys->dk_usable <= usable);
-    _PyObject_ASSERT(op, 0 <= keys->dk_nentries && keys->dk_nentries <= usable);
-    _PyObject_ASSERT(op, keys->dk_usable + keys->dk_nentries <= usable);
+    CHECK(0 <= mp->ma_used && mp->ma_used <= usable);
+    CHECK(IS_POWER_OF_2(keys->dk_size));
+    CHECK(0 <= keys->dk_usable && keys->dk_usable <= usable);
+    CHECK(0 <= keys->dk_nentries && keys->dk_nentries <= usable);
+    CHECK(keys->dk_usable + keys->dk_nentries <= usable);
 
     if (!splitted) {
         /* combined table */
-        _PyObject_ASSERT(op, keys->dk_refcnt == 1);
+        CHECK(keys->dk_refcnt == 1);
     }
 
     if (check_content) {
@@ -484,7 +487,7 @@ _PyDict_CheckConsistency(PyObject *op, int check_content)
 
         for (i=0; i < keys->dk_size; i++) {
             Py_ssize_t ix = dictkeys_get_index(keys, i);
-            _PyObject_ASSERT(op, DKIX_DUMMY <= ix && ix <= usable);
+            CHECK(DKIX_DUMMY <= ix && ix <= usable);
         }
 
         for (i=0; i < usable; i++) {
@@ -494,32 +497,33 @@ _PyDict_CheckConsistency(PyObject *op, int check_content)
             if (key != NULL) {
                 if (PyUnicode_CheckExact(key)) {
                     Py_hash_t hash = ((PyASCIIObject *)key)->hash;
-                    _PyObject_ASSERT(op, hash != -1);
-                    _PyObject_ASSERT(op, entry->me_hash == hash);
+                    CHECK(hash != -1);
+                    CHECK(entry->me_hash == hash);
                 }
                 else {
                     /* test_dict fails if PyObject_Hash() is called again */
-                    _PyObject_ASSERT(op, entry->me_hash != -1);
+                    CHECK(entry->me_hash != -1);
                 }
                 if (!splitted) {
-                    _PyObject_ASSERT(op, entry->me_value != NULL);
+                    CHECK(entry->me_value != NULL);
                 }
             }
 
             if (splitted) {
-                _PyObject_ASSERT(op, entry->me_value == NULL);
+                CHECK(entry->me_value == NULL);
             }
         }
 
         if (splitted) {
             /* splitted table */
             for (i=0; i < mp->ma_used; i++) {
-                _PyObject_ASSERT(op, mp->ma_values[i] != NULL);
+                CHECK(mp->ma_values[i] != NULL);
             }
         }
     }
-#endif
     return 1;
+
+#undef CHECK
 }