From 271753a27aca2e13275f0827080b915fb438107a Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 22 Nov 2018 01:02:54 +0100 Subject: bpo-35059: Convert _PyObject_GC_TRACK() to inline function (GH-10643) * Add _PyObject_ASSERT_FROM() and _PyObject_ASSERT_FAILED_MSG() macros. * PyObject_GC_Track() now calls _PyObject_ASSERT_FAILED_MSG(), instead of Py_FatalError(), if the object is already tracked, to dump more information on error. * _PyObject_GC_TRACK() no longer checks if the object is already tracked at runtime, use an assertion instead for best performances; PyObject_GC_Track() still checks at runtime. * pycore_object.h now includes pycore_pystate.h. * Convert _PyObject_GC_TRACK() and _PyObject_GC_UNTRACK() macros to inline functions. --- Include/internal/pycore_object.h | 70 ++++++++++++++++++++++++++-------------- Include/object.h | 27 +++++++++------- Modules/gcmodule.c | 13 ++++---- 3 files changed, 68 insertions(+), 42 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index a263834..347ab68 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -8,6 +8,8 @@ extern "C" { # error "this header requires Py_BUILD_CORE or Py_BUILD_CORE_BUILTIN defined" #endif +#include "pycore_pystate.h" /* _PyRuntime */ + /* Tell the GC to track this object. * * NB: While the object is tracked by the collector, it must be safe to call the @@ -19,36 +21,56 @@ extern "C" { * * The PyObject_GC_Track() function is the public version of this macro. */ -#define _PyObject_GC_TRACK(o) do { \ - PyGC_Head *g = _Py_AS_GC(o); \ - if (g->_gc_next != 0) { \ - Py_FatalError("GC object already tracked"); \ - } \ - assert((g->_gc_prev & _PyGC_PREV_MASK_COLLECTING) == 0); \ - PyGC_Head *last = (PyGC_Head*)(_PyRuntime.gc.generation0->_gc_prev); \ - _PyGCHead_SET_NEXT(last, g); \ - _PyGCHead_SET_PREV(g, last); \ - _PyGCHead_SET_NEXT(g, _PyRuntime.gc.generation0); \ - _PyRuntime.gc.generation0->_gc_prev = (uintptr_t)g; \ - } while (0); +static inline void _PyObject_GC_TRACK_impl(const char *filename, int lineno, + PyObject *op) +{ + _PyObject_ASSERT_FROM(op, !_PyObject_GC_IS_TRACKED(op), + "object already tracked by the garbage collector", + filename, lineno, "_PyObject_GC_TRACK"); + + PyGC_Head *gc = _Py_AS_GC(op); + _PyObject_ASSERT_FROM(op, + (gc->_gc_prev & _PyGC_PREV_MASK_COLLECTING) == 0, + "object is in generation which is garbage collected", + filename, lineno, "_PyObject_GC_TRACK"); + + PyGC_Head *last = (PyGC_Head*)(_PyRuntime.gc.generation0->_gc_prev); + _PyGCHead_SET_NEXT(last, gc); + _PyGCHead_SET_PREV(gc, last); + _PyGCHead_SET_NEXT(gc, _PyRuntime.gc.generation0); + _PyRuntime.gc.generation0->_gc_prev = (uintptr_t)gc; +} + +#define _PyObject_GC_TRACK(op) \ + _PyObject_GC_TRACK_impl(__FILE__, __LINE__, (PyObject *)(op)) /* Tell the GC to stop tracking this object. * - * Internal note: This may be called while GC. So _PyGC_PREV_MASK_COLLECTING must - * be cleared. But _PyGC_PREV_MASK_FINALIZED bit is kept. + * Internal note: This may be called while GC. So _PyGC_PREV_MASK_COLLECTING + * must be cleared. But _PyGC_PREV_MASK_FINALIZED bit is kept. + * + * The object must be tracked by the GC. * * The PyObject_GC_UnTrack() function is the public version of this macro. */ -#define _PyObject_GC_UNTRACK(o) do { \ - PyGC_Head *g = _Py_AS_GC(o); \ - PyGC_Head *prev = _PyGCHead_PREV(g); \ - PyGC_Head *next = _PyGCHead_NEXT(g); \ - assert(next != NULL); \ - _PyGCHead_SET_NEXT(prev, next); \ - _PyGCHead_SET_PREV(next, prev); \ - g->_gc_next = 0; \ - g->_gc_prev &= _PyGC_PREV_MASK_FINALIZED; \ - } while (0); +static inline void _PyObject_GC_UNTRACK_impl(const char *filename, int lineno, + PyObject *op) +{ + _PyObject_ASSERT_FROM(op, _PyObject_GC_IS_TRACKED(op), + "object not tracked by the garbage collector", + filename, lineno, "_PyObject_GC_UNTRACK"); + + PyGC_Head *gc = _Py_AS_GC(op); + PyGC_Head *prev = _PyGCHead_PREV(gc); + PyGC_Head *next = _PyGCHead_NEXT(gc); + _PyGCHead_SET_NEXT(prev, next); + _PyGCHead_SET_PREV(next, prev); + gc->_gc_next = 0; + gc->_gc_prev &= _PyGC_PREV_MASK_FINALIZED; +} + +#define _PyObject_GC_UNTRACK(op) \ + _PyObject_GC_UNTRACK_impl(__FILE__, __LINE__, (PyObject *)(op)) #ifdef __cplusplus } diff --git a/Include/object.h b/Include/object.h index 0d84d36..5947b79 100644 --- a/Include/object.h +++ b/Include/object.h @@ -1136,7 +1136,7 @@ _PyObject_DebugTypeStats(FILE *out); #ifndef Py_LIMITED_API /* Define a pair of assertion macros: - _PyObject_ASSERT_WITH_MSG() and _PyObject_ASSERT(). + _PyObject_ASSERT_FROM(), _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, @@ -1151,21 +1151,24 @@ _PyObject_DebugTypeStats(FILE *out); 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) +# define _PyObject_ASSERT_FROM(obj, expr, msg, filename, lineno, func) \ + ((void)0) #else /* With debugging: generate checks: */ -# define _PyObject_ASSERT_WITH_MSG(obj, expr, msg) \ - ((expr) \ - ? (void)(0) \ - : _PyObject_AssertFailed((obj), \ - Py_STRINGIFY(expr), \ - (msg), \ - __FILE__, \ - __LINE__, \ - __func__)) +# define _PyObject_ASSERT_FROM(obj, expr, msg, filename, lineno, func) \ + ((expr) \ + ? (void)(0) \ + : _PyObject_AssertFailed((obj), Py_STRINGIFY(expr), \ + (msg), (filename), (lineno), (func))) #endif -#define _PyObject_ASSERT(obj, expr) _PyObject_ASSERT_WITH_MSG(obj, expr, NULL) +#define _PyObject_ASSERT_WITH_MSG(obj, expr, msg) \ + _PyObject_ASSERT_FROM(obj, expr, msg, __FILE__, __LINE__, __func__) +#define _PyObject_ASSERT(obj, expr) \ + _PyObject_ASSERT_WITH_MSG(obj, expr, NULL) + +#define _PyObject_ASSERT_FAILED_MSG(obj, msg) \ + _PyObject_AssertFailed((obj), NULL, (msg), __FILE__, __LINE__, __func__) /* Declare and define _PyObject_AssertFailed() even when NDEBUG is defined, to avoid causing compiler/linker errors when building extensions without diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index 2cbf738..506ae19 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -1846,15 +1846,16 @@ _PyGC_Dump(PyGC_Head *g) /* extension modules might be compiled with GC support so these functions must always be available */ -#undef PyObject_GC_Track -#undef PyObject_GC_UnTrack -#undef PyObject_GC_Del -#undef _PyObject_GC_Malloc - void PyObject_GC_Track(void *op) { - _PyObject_GC_TRACK(op); + PyObject *obj = (PyObject *)op; + if (_PyObject_GC_IS_TRACKED(op)) { + _PyObject_ASSERT_FAILED_MSG(op, + "object already tracked " + "by the garbage collector"); + } + _PyObject_GC_TRACK(obj); } void -- cgit v0.12