bpo-9263: Dump Python object on GC assertion failure (GH-10062)

Changes:

* Add _PyObject_AssertFailed() function.
* Add _PyObject_ASSERT() and _PyObject_ASSERT_WITH_MSG() macros.
* gc_decref(): replace assert() with _PyObject_ASSERT_WITH_MSG() to
  dump the faulty object if the assertion fails.

_PyObject_AssertFailed() calls:

* _PyMem_DumpTraceback(): try to log the traceback where the object
  memory has been allocated if tracemalloc is enabled.
* _PyObject_Dump(): log repr(obj).
* Py_FatalError(): log the current Python traceback.

_PyObject_AssertFailed() uses _PyObject_IsFreed() heuristic to check
if the object memory has been freed by a debug hook on Python memory
allocators.

Initial patch written by David Malcolm.

Co-Authored-By: David Malcolm <[email protected]>

Author: vstinner

Include/object.h

@@ -1105,6 +1105,53 @@ PyAPI_FUNC(void)
 _PyObject_DebugTypeStats(FILE *out);
 #endif /* ifndef Py_LIMITED_API */
 
+
+#ifndef Py_LIMITED_API
+/* Define a pair of assertion macros:
+   _PyObject_ASSERT_WITH_MSG() and _PyObject_ASSERT().
+
+   These work like the regular C assert(), in that they will abort the
+   process with a message on stderr if the given condition fails to hold,
+   but compile away to nothing if NDEBUG is defined.
+
+   However, before aborting, Python will also try to call _PyObject_Dump() on
+   the given object.  This may be of use when investigating bugs in which a
+   particular object is corrupt (e.g. buggy a tp_visit method in an extension
+   module breaking the garbage collector), to help locate the broken objects.
+
+   The WITH_MSG variant allows you to supply an additional message that Python
+   will attempt to print to stderr, after the object dump. */
+#ifdef NDEBUG
+   /* No debugging: compile away the assertions: */
+#  define _PyObject_ASSERT_WITH_MSG(obj, expr, msg) ((void)0)
+#else
+   /* With debugging: generate checks: */
+#  define _PyObject_ASSERT_WITH_MSG(obj, expr, msg)     \
+     ((expr)                                           \
+      ? (void)(0)                                      \
+      : _PyObject_AssertFailed((obj),                  \
+                               (msg),                  \
+                               Py_STRINGIFY(expr),     \
+                               __FILE__,               \
+                               __LINE__,               \
+                               __func__))
+#endif
+
+#define _PyObject_ASSERT(obj, expr) _PyObject_ASSERT_WITH_MSG(obj, expr, NULL)
+
+/* Declare and define _PyObject_AssertFailed() even when NDEBUG is defined,
+   to avoid causing compiler/linker errors when building extensions without
+   NDEBUG against a Python built with NDEBUG defined. */
+PyAPI_FUNC(void) _PyObject_AssertFailed(
+    PyObject *obj,
+    const char *msg,
+    const char *expr,
+    const char *file,
+    int line,
+    const char *function);
+#endif /* ifndef Py_LIMITED_API */
+
+
 #ifdef __cplusplus
 }
 #endif

Include/pymem.h

@@ -196,7 +196,7 @@ PyAPI_FUNC(void) PyMem_SetAllocator(PyMemAllocatorDomain domain,
 
    The function does nothing if Python is not compiled is debug mode. */
 PyAPI_FUNC(void) PyMem_SetupDebugHooks(void);
-#endif
+#endif   /* Py_LIMITED_API */
 
 #ifdef Py_BUILD_CORE
 /* Set the memory allocator of the specified domain to the default.

Lib/test/test_gc.py

@@ -1,14 +1,17 @@
 import unittest
 from test.support import (verbose, refcount_test, run_unittest,
                           strip_python_stderr, cpython_only, start_threads,
-                          temp_dir, requires_type_collecting, TESTFN, unlink)
+                          temp_dir, requires_type_collecting, TESTFN, unlink,
+                          import_module)
 from test.support.script_helper import assert_python_ok, make_script
 
+import gc
 import sys
+import sysconfig
+import textwrap
+import threading
 import time
-import gc
 import weakref
-import threading
 
 try:
     from _testcapi import with_tp_del
@@ -62,6 +65,14 @@ def __init__(self, partner=None):
     def __tp_del__(self):
         pass
 
+if sysconfig.get_config_vars().get('PY_CFLAGS', ''):
+    BUILD_WITH_NDEBUG = ('-DNDEBUG' in sysconfig.get_config_vars()['PY_CFLAGS'])
+else:
+    # Usually, sys.gettotalrefcount() is only present if Python has been
+    # compiled in debug mode. If it's missing, expect that Python has
+    # been released in release mode: with NDEBUG defined.
+    BUILD_WITH_NDEBUG = (not hasattr(sys, 'gettotalrefcount'))
+
 ### Tests
 ###############################################################################
 
@@ -878,6 +889,58 @@ def test_collect_garbage(self):
         self.assertEqual(len(gc.garbage), 0)
 
 
+    @unittest.skipIf(BUILD_WITH_NDEBUG,
+                     'built with -NDEBUG')
+    def test_refcount_errors(self):
+        self.preclean()
+        # Verify the "handling" of objects with broken refcounts
+
+        # Skip the test if ctypes is not available
+        import_module("ctypes")
+
+        import subprocess
+        code = textwrap.dedent('''
+            from test.support import gc_collect, SuppressCrashReport
+
+            a = [1, 2, 3]
+            b = [a]
+
+            # Avoid coredump when Py_FatalError() calls abort()
+            SuppressCrashReport().__enter__()
+
+            # Simulate the refcount of "a" being too low (compared to the
+            # references held on it by live data), but keeping it above zero
+            # (to avoid deallocating it):
+            import ctypes
+            ctypes.pythonapi.Py_DecRef(ctypes.py_object(a))
+
+            # The garbage collector should now have a fatal error
+            # when it reaches the broken object
+            gc_collect()
+        ''')
+        p = subprocess.Popen([sys.executable, "-c", code],
+                             stdout=subprocess.PIPE,
+                             stderr=subprocess.PIPE)
+        stdout, stderr = p.communicate()
+        p.stdout.close()
+        p.stderr.close()
+        # Verify that stderr has a useful error message:
+        self.assertRegex(stderr,
+            br'gcmodule\.c:[0-9]+: gc_decref: Assertion "gc_get_refs\(g\) > 0" failed.')
+        self.assertRegex(stderr,
+            br'refcount is too small')
+        self.assertRegex(stderr,
+            br'object  : \[1, 2, 3\]')
+        self.assertRegex(stderr,
+            br'type    : list')
+        self.assertRegex(stderr,
+            br'refcount: 1')
+        # "address : 0x7fb5062efc18"
+        # "address : 7FB5062EFC18"
+        self.assertRegex(stderr,
+            br'address : [0-9a-fA-Fx]+')
+
+
 class GCTogglingTests(unittest.TestCase):
     def setUp(self):
         gc.enable()

Modules/_tracemalloc.c

@@ -1436,10 +1436,12 @@ _tracemalloc__get_object_traceback(PyObject *module, PyObject *obj)
     traceback_t *traceback;
 
     type = Py_TYPE(obj);
-    if (PyType_IS_GC(type))
+    if (PyType_IS_GC(type)) {
         ptr = (void *)((char *)obj - sizeof(PyGC_Head));
-    else
+    }
+    else {
         ptr = (void *)obj;
+    }
 
     traceback = tracemalloc_get_traceback(DEFAULT_DOMAIN, (uintptr_t)ptr);
     if (traceback == NULL)

Modules/gcmodule.c

@@ -62,6 +62,12 @@ module gc
 // most gc_list_* functions for it.
 #define NEXT_MASK_UNREACHABLE  (1)
 
+/* Get an object's GC head */
+#define AS_GC(o) ((PyGC_Head *)(o)-1)
+
+/* Get the object given the GC head */
+#define FROM_GC(g) ((PyObject *)(((PyGC_Head *)g)+1))
+
 static inline int
 gc_is_collecting(PyGC_Head *g)
 {
@@ -98,16 +104,12 @@ gc_reset_refs(PyGC_Head *g, Py_ssize_t refs)
 static inline void
 gc_decref(PyGC_Head *g)
 {
-    assert(gc_get_refs(g) > 0);
+    _PyObject_ASSERT_WITH_MSG(FROM_GC(g),
+                              gc_get_refs(g) > 0,
+                              "refcount is too small");
     g->_gc_prev -= 1 << _PyGC_PREV_SHIFT;
 }
 
-/* Get an object's GC head */
-#define AS_GC(o) ((PyGC_Head *)(o)-1)
-
-/* Get the object given the GC head */
-#define FROM_GC(g) ((PyObject *)(((PyGC_Head *)g)+1))
-
 /* Python string to use if unhandled exception occurs */
 static PyObject *gc_str = NULL;
 

Objects/object.c

@@ -10,6 +10,9 @@
 extern "C" {
 #endif
 
+/* Defined in tracemalloc.c */
+extern void _PyMem_DumpTraceback(int fd, const void *ptr);
+
 _Py_IDENTIFIER(Py_Repr);
 _Py_IDENTIFIER(__bytes__);
 _Py_IDENTIFIER(__dir__);
@@ -2212,6 +2215,55 @@ _PyTrash_thread_destroy_chain(void)
     --tstate->trash_delete_nesting;
 }
 
+
+void
+_PyObject_AssertFailed(PyObject *obj, const char *msg, const char *expr,
+                       const char *file, int line, const char *function)
+{
+    fprintf(stderr,
+            "%s:%d: %s: Assertion \"%s\" failed",
+            file, line, function, expr);
+    fflush(stderr);
+
+    if (msg) {
+        fprintf(stderr, "; %s.\n", msg);
+    }
+    else {
+        fprintf(stderr, ".\n");
+    }
+    fflush(stderr);
+
+    if (obj == NULL) {
+        fprintf(stderr, "<NULL object>\n");
+    }
+    else if (_PyObject_IsFreed(obj)) {
+        /* It seems like the object memory has been freed:
+           don't access it to prevent a segmentation fault. */
+        fprintf(stderr, "<Freed object>\n");
+    }
+    else {
+        /* Diplay the traceback where the object has been allocated.
+           Do it before dumping repr(obj), since repr() is more likely
+           to crash than dumping the traceback. */
+        void *ptr;
+        PyTypeObject *type = Py_TYPE(obj);
+        if (PyType_IS_GC(type)) {
+            ptr = (void *)((char *)obj - sizeof(PyGC_Head));
+        }
+        else {
+            ptr = (void *)obj;
+        }
+        _PyMem_DumpTraceback(fileno(stderr), ptr);
+
+        /* This might succeed or fail, but we're about to abort, so at least
+           try to provide any extra info we can: */
+        _PyObject_Dump(obj);
+    }
+    fflush(stderr);
+
+    Py_FatalError("_PyObject_AssertFailed");
+}
+
 #ifndef Py_TRACE_REFS
 /* For Py_LIMITED_API, we need an out-of-line version of _Py_Dealloc.
    Define this here, so we can undefine the macro. */