bpo-35059: Convert _PyObject_GC_TRACK() to inline function (GH-10643)

vstinner · web-flow · commit 271753a27aca · 2018-11-22T01:02:54.000+01:00
* 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.
diff --git 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
@@ -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
@@ -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
