From 626bff856840f471e98ec627043f780c111a063d Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 25 Oct 2018 17:31:10 +0200 Subject: [PATCH] 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 --- Include/object.h | 47 ++++++++++++++++++++++++++++ Include/pymem.h | 2 +- Lib/test/test_gc.py | 71 +++++++++++++++++++++++++++++++++++++++--- Modules/_tracemalloc.c | 6 ++-- Modules/gcmodule.c | 16 +++++----- Objects/object.c | 52 +++++++++++++++++++++++++++++++ 6 files changed, 180 insertions(+), 14 deletions(-) diff --git a/Include/object.h b/Include/object.h index 8b2afc2bc5b..4a49609c72c 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 e993628a21e..19f0c8a2d9b 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 8d806db3ba5..16b22422528 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 sys -import time import gc -import weakref +import sys +import sysconfig +import textwrap import threading +import time +import weakref try: from _testcapi import with_tp_del @@ -62,6 +65,14 @@ class Uncollectable(object): 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 @@ class GCCallbackTests(unittest.TestCase): 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 d736b240fc2..1005f2ea4d8 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 7cddabafbc4..2e19fe4b364 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 82560710452..2252f983475 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, "\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, "\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. */