[Python-checkins] bpo-9263: Dump Python object on GC assertion failure (GH-10062)
Victor Stinner
webhook-mailer at python.org
Thu Oct 25 11:31:16 EDT 2018
https://github.com/python/cpython/commit/626bff856840f471e98ec627043f780c111a063d
commit: 626bff856840f471e98ec627043f780c111a063d
branch: master
author: Victor Stinner <vstinner at redhat.com>
committer: GitHub <noreply at github.com>
date: 2018-10-25T17:31:10+02:00
summary:
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 <dmalcolm at redhat.com>
files:
M Include/object.h
M Include/pymem.h
M Lib/test/test_gc.py
M Modules/_tracemalloc.c
M Modules/gcmodule.c
M Objects/object.c
diff --git a/Include/object.h b/Include/object.h
index 8b2afc2bc5be..4a49609c72c3 100644
--- a/Include/object.h
+++ b/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
diff --git a/Include/pymem.h b/Include/pymem.h
index e993628a21ee..19f0c8a2d9bf 100644
--- a/Include/pymem.h
+++ b/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.
diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py
index 8d806db3ba57..16b22422528f 100644
--- a/Lib/test/test_gc.py
+++ b/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()
diff --git a/Modules/_tracemalloc.c b/Modules/_tracemalloc.c
index d736b240fc2d..1005f2ea4d87 100644
--- a/Modules/_tracemalloc.c
+++ b/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)
diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c
index 7cddabafbc46..2e19fe4b3646 100644
--- a/Modules/gcmodule.c
+++ b/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;
diff --git a/Objects/object.c b/Objects/object.c
index 825607104526..2252f9834756 100644
--- a/Objects/object.c
+++ b/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. */
More information about the Python-checkins
mailing list