Add (slow) stack ref debug mode to locate stackref leaks and double frees.

Author: markshannon

Include/internal/pycore_interp.h

@@ -34,6 +34,7 @@ extern "C" {
 #include "pycore_optimizer.h"     // _PyOptimizerObject
 #include "pycore_obmalloc.h"      // struct _obmalloc_state
 #include "pycore_qsbr.h"          // struct _qsbr_state
+#include "pycore_stackref.h"      // Py_STACKREF_DEBUG
 #include "pycore_tstate.h"        // _PyThreadStateImpl
 #include "pycore_tuple.h"         // struct _Py_tuple_state
 #include "pycore_uniqueid.h"      // struct _Py_unique_id_pool
@@ -285,6 +286,11 @@ struct _is {
     _PyThreadStateImpl _initial_thread;
     // _initial_thread should be the last field of PyInterpreterState.
     // See https://github.com/python/cpython/issues/127117.
+
+#if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG)
+    uint64_t next_stackref;
+    _Py_hashtable_t *stackref_debug_table;
+#endif
 };
 
 

Include/internal/pycore_stackref.h

@@ -4,6 +4,9 @@
 extern "C" {
 #endif
 
+// Define this to get precise tracking of stackrefs.
+#define Py_STACKREF_DEBUG 1
+
 #ifndef Py_BUILD_CORE
 #  error "this header requires Py_BUILD_CORE define"
 #endif
@@ -49,6 +52,105 @@ extern "C" {
    CPython refcounting operations on it!
 */
 
+
+#if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG)
+
+typedef union _PyStackRef {
+    uint64_t index;
+} _PyStackRef;
+
+#define Py_TAG_BITS 0
+
+PyAPI_FUNC(PyObject *) _Py_stackref_get_object(_PyStackRef ref);
+PyAPI_FUNC(PyObject *) _Py_stackref_close(_PyStackRef ref);
+PyAPI_FUNC(_PyStackRef) _Py_stackref_create(PyObject *obj, const char *filename, int linenumber);
+extern void _Py_stackref_associate(PyInterpreterState *interp, PyObject *obj, _PyStackRef ref);
+
+static const _PyStackRef PyStackRef_NULL = { .index = 0 };
+
+#define PyStackRef_None ((_PyStackRef){ .index = 1 } )
+#define PyStackRef_False ((_PyStackRef){ .index = 2 })
+#define PyStackRef_True ((_PyStackRef){ .index = 3 })
+
+static inline int
+PyStackRef_IsNull(_PyStackRef ref)
+{
+    return ref.index == 0;
+}
+
+static inline int
+PyStackRef_IsTrue(_PyStackRef ref)
+{
+    return _Py_stackref_get_object(ref) == Py_True;
+}
+
+static inline int
+PyStackRef_IsFalse(_PyStackRef ref)
+{
+    return _Py_stackref_get_object(ref) == Py_False;
+}
+
+static inline int
+PyStackRef_IsNone(_PyStackRef ref)
+{
+    return _Py_stackref_get_object(ref) == Py_None;
+}
+
+static inline PyObject *
+PyStackRef_AsPyObjectBorrow(_PyStackRef ref)
+{
+    return _Py_stackref_get_object(ref);
+}
+
+static inline PyObject *
+PyStackRef_AsPyObjectSteal(_PyStackRef ref)
+{
+    return _Py_stackref_close(ref);
+}
+
+static inline _PyStackRef
+_PyStackRef_FromPyObjectNew(PyObject *obj, const char *filename, int linenumber)
+{
+    Py_INCREF(obj);
+    return _Py_stackref_create(obj, filename, linenumber);
+}
+#define PyStackRef_FromPyObjectNew(obj) _PyStackRef_FromPyObjectNew(_PyObject_CAST(obj), __FILE__, __LINE__)
+
+static inline _PyStackRef
+_PyStackRef_FromPyObjectSteal(PyObject *obj, const char *filename, int linenumber)
+{
+    return _Py_stackref_create(obj, filename, linenumber);
+}
+#define PyStackRef_FromPyObjectSteal(obj) _PyStackRef_FromPyObjectSteal(_PyObject_CAST(obj), __FILE__, __LINE__)
+
+static inline _PyStackRef
+_PyStackRef_FromPyObjectImmortal(PyObject *obj, const char *filename, int linenumber)
+{
+    assert(_Py_IsImmortal(obj));
+    return _Py_stackref_create(obj, filename, linenumber);
+}
+#define PyStackRef_FromPyObjectImmortal(obj) _PyStackRef_FromPyObjectImmortal(_PyObject_CAST(obj), __FILE__, __LINE__)
+
+static inline void
+PyStackRef_CLOSE(_PyStackRef ref)
+{
+    PyObject *obj = _Py_stackref_close(ref);
+    Py_DECREF(obj);
+}
+
+static inline _PyStackRef
+_PyStackRef_DUP(_PyStackRef ref, const char *filename, int linenumber)
+{
+    PyObject *obj = _Py_stackref_get_object(ref);
+    Py_INCREF(obj);
+    return _Py_stackref_create(obj, filename, linenumber);
+}
+#define PyStackRef_DUP(REF) _PyStackRef_DUP(REF, __FILE__, __LINE__)
+
+#define PyStackRef_CLOSE_SPECIALIZED(stackref, dealloc) PyStackRef_CLOSE(stackref)
+
+#else
+
 typedef union _PyStackRef {
     uintptr_t bits;
 } _PyStackRef;
@@ -200,12 +302,15 @@ static const _PyStackRef PyStackRef_NULL = { .bits = 0 };
 #define PyStackRef_IsTrue(ref) (PyStackRef_AsPyObjectBorrow(ref) == Py_True)
 #define PyStackRef_IsFalse(ref) (PyStackRef_AsPyObjectBorrow(ref) == Py_False)
 
+#endif
+
 // Converts a PyStackRef back to a PyObject *, converting the
 // stackref to a new reference.
 #define PyStackRef_AsPyObjectNew(stackref) Py_NewRef(PyStackRef_AsPyObjectBorrow(stackref))
 
 #define PyStackRef_TYPE(stackref) Py_TYPE(PyStackRef_AsPyObjectBorrow(stackref))
 
+
 #define PyStackRef_CLEAR(op) \
     do { \
         _PyStackRef *_tmp_op_ptr = &(op); \

Makefile.pre.in

@@ -488,6 +488,7 @@ PYTHON_OBJS=	\
 		Python/qsbr.o \
 		Python/bootstrap_hash.o \
 		Python/specialize.o \
+		Python/stackrefs.o \
 		Python/structmember.o \
 		Python/symtable.o \
 		Python/sysmodule.o \

Objects/frameobject.c

@@ -179,9 +179,9 @@ framelocalsproxy_setitem(PyObject *self, PyObject *key, PyObject *value)
         if (kind == CO_FAST_FREE) {
             // The cell was set when the frame was created from
             // the function's closure.
-            assert(oldvalue.bits != 0 && PyCell_Check(PyStackRef_AsPyObjectBorrow(oldvalue)));
+            assert(!PyStackRef_IsNull(oldvalue) && PyCell_Check(PyStackRef_AsPyObjectBorrow(oldvalue)));
             cell = PyStackRef_AsPyObjectBorrow(oldvalue);
-        } else if (kind & CO_FAST_CELL && oldvalue.bits != 0) {
+        } else if (kind & CO_FAST_CELL && !PyStackRef_IsNull(oldvalue)) {
             PyObject *as_obj = PyStackRef_AsPyObjectBorrow(oldvalue);
             if (PyCell_Check(as_obj)) {
                 cell = as_obj;

Python/ceval.c

@@ -164,7 +164,7 @@ dump_stack(_PyInterpreterFrame *frame, _PyStackRef *stack_pointer)
             PyErr_Clear();
         }
         // Don't call __repr__(), it might recurse into the interpreter.
-        printf("<%s at %p>", Py_TYPE(obj)->tp_name, (void *)(ptr->bits));
+        printf("<%s at %p>", Py_TYPE(obj)->tp_name, PyStackRef_AsPyObjectBorrow(*ptr));
     }
     printf("]\n");
     fflush(stdout);
@@ -805,7 +805,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
 
 
 
-#ifdef Py_DEBUG
+#if defined(Py_DEBUG) && !defined(Py_STACKREF_DEBUG)
     /* Set these to invalid but identifiable values for debugging. */
     entry_frame.f_funcobj = (_PyStackRef){.bits = 0xaaa0};
     entry_frame.f_locals = (PyObject*)0xaaa1;
@@ -1817,12 +1817,27 @@ _PyEvalFramePushAndInit_Ex(PyThreadState *tstate, _PyStackRef func,
             PyStackRef_CLOSE(func);
             goto error;
         }
+        size_t total_args = nargs + PyDict_GET_SIZE(kwargs);
+        for (size_t i = 0; i < total_args; i++) {
+            ((_PyStackRef *)newargs)[i] = PyStackRef_FromPyObjectSteal(newargs[i]);
+        }
     }
     else {
-        newargs = &PyTuple_GET_ITEM(callargs, 0);
-        /* We need to incref all our args since the new frame steals the references. */
-        for (Py_ssize_t i = 0; i < nargs; ++i) {
-            Py_INCREF(PyTuple_GET_ITEM(callargs, i));
+        if (nargs <= 8) {
+            PyObject *stack_array[8];
+            newargs = stack_array;
+        }
+        else {
+            newargs = PyMem_Malloc(sizeof(PyObject *) *nargs);
+            if (newargs == NULL) {
+                PyErr_NoMemory();
+                PyStackRef_CLOSE(func);
+                goto error;
+            }
+        }
+        /* We need to create a new reference for all our args since the new frame steals them. */
+        for (Py_ssize_t i = 0; i < nargs; i++) {
+            ((_PyStackRef *)newargs)[i] = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(callargs, i));
         }
     }
     _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit(
@@ -1832,6 +1847,9 @@ _PyEvalFramePushAndInit_Ex(PyThreadState *tstate, _PyStackRef func,
     if (has_dict) {
         _PyStack_UnpackDict_FreeNoDecRef(newargs, kwnames);
     }
+    else if (nargs > 8) {
+       PyMem_Free((void *)newargs);
+    }
     /* No need to decref func here because the reference has been stolen by
        _PyEvalFramePushAndInit.
     */
@@ -1850,21 +1868,39 @@ _PyEval_Vector(PyThreadState *tstate, PyFunctionObject *func,
                PyObject* const* args, size_t argcount,
                PyObject *kwnames)
 {
+    size_t total_args = argcount;
+    if (kwnames) {
+        total_args += PyTuple_GET_SIZE(kwnames);
+    }
+    _PyStackRef stack_array[8];
+    _PyStackRef *arguments;
+    if (total_args <= 8) {
+        arguments = stack_array;
+    }
+    else {
+        arguments = PyMem_Malloc(sizeof(_PyStackRef) * total_args);
+        if (arguments == NULL) {
+            return PyErr_NoMemory();
+        }
+    }
     /* _PyEvalFramePushAndInit consumes the references
      * to func, locals and all its arguments */
     Py_XINCREF(locals);
     for (size_t i = 0; i < argcount; i++) {
-        Py_INCREF(args[i]);
+        arguments[i] = PyStackRef_FromPyObjectNew(args[i]);
     }
     if (kwnames) {
         Py_ssize_t kwcount = PyTuple_GET_SIZE(kwnames);
         for (Py_ssize_t i = 0; i < kwcount; i++) {
-            Py_INCREF(args[i+argcount]);
+            arguments[i+argcount] = PyStackRef_FromPyObjectNew(args[i+argcount]);
         }
     }
     _PyInterpreterFrame *frame = _PyEvalFramePushAndInit(
         tstate, PyStackRef_FromPyObjectNew(func), locals,
-        (_PyStackRef const *)args, argcount, kwnames, NULL);
+        arguments, argcount, kwnames, NULL);
+    if (total_args > 8) {
+        PyMem_Free(arguments);
+    }
     if (frame == NULL) {
         return NULL;
     }

Python/ceval_macros.h

@@ -450,7 +450,7 @@ do { \
 /* How much scratch space to give stackref to PyObject* conversion. */
 #define MAX_STACKREF_SCRATCH 10
 
-#ifdef Py_GIL_DISABLED
+#if defined(Py_GIL_DISABLED) || defined(Py_STACKREF_DEBUG)
 #define STACKREFS_TO_PYOBJECTS(ARGS, ARG_COUNT, NAME) \
     /* +1 because vectorcall might use -1 to write self */ \
     PyObject *NAME##_temp[MAX_STACKREF_SCRATCH+1]; \
@@ -461,7 +461,7 @@ do { \
     assert(NAME != NULL);
 #endif
 
-#ifdef Py_GIL_DISABLED
+#if defined(Py_GIL_DISABLED) || defined(Py_STACKREF_DEBUG)
 #define STACKREFS_TO_PYOBJECTS_CLEANUP(NAME) \
     /* +1 because we +1 previously */ \
     _PyObjectArray_Free(NAME - 1, NAME##_temp);
@@ -470,7 +470,7 @@ do { \
     (void)(NAME);
 #endif
 
-#ifdef Py_GIL_DISABLED
+#if defined(Py_GIL_DISABLED) || defined(Py_STACKREF_DEBUG)
 #define CONVERSION_FAILED(NAME) ((NAME) == NULL)
 #else
 #define CONVERSION_FAILED(NAME) (0)

Python/pystate.c

@@ -19,6 +19,7 @@
 #include "pycore_pymem.h"         // _PyMem_SetDefaultAllocator()
 #include "pycore_pystate.h"
 #include "pycore_runtime_init.h"  // _PyRuntimeState_INIT
+#include "pycore_stackref.h"      // Py_STACKREF_DEBUG
 #include "pycore_obmalloc.h"      // _PyMem_obmalloc_state_on_heap()
 #include "pycore_uniqueid.h"      // _PyObject_FinalizePerThreadRefcounts()
 
@@ -663,6 +664,23 @@ init_interpreter(PyInterpreterState *interp,
         /* Fix the self-referential, statically initialized fields. */
         interp->dtoa = (struct _dtoa_state)_dtoa_state_INIT(interp);
     }
+#if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG)
+    interp->next_stackref = 1;
+    _Py_hashtable_allocator_t alloc = {
+        .malloc = malloc,
+        .free = free,
+    };
+    interp->stackref_debug_table = _Py_hashtable_new_full(
+        _Py_hashtable_hash_ptr,
+        _Py_hashtable_compare_direct,
+        NULL,
+        NULL,
+        &alloc
+    );
+    _Py_stackref_associate(interp, Py_None, PyStackRef_None);
+    _Py_stackref_associate(interp, Py_False, PyStackRef_False);
+    _Py_stackref_associate(interp, Py_True, PyStackRef_True);
+#endif
 
     interp->_initialized = 1;
     return _PyStatus_OK();
@@ -768,6 +786,11 @@ PyInterpreterState_New(void)
     return interp;
 }
 
+#if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG)
+extern void
+_Py_stackref_report_leaks(PyInterpreterState *interp);
+#endif
+
 static void
 interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)
 {
@@ -877,6 +900,12 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)
     Py_CLEAR(interp->sysdict);
     Py_CLEAR(interp->builtins);
 
+#if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG)
+    _Py_stackref_report_leaks(interp);
+    _Py_hashtable_destroy(interp->stackref_debug_table);
+    interp->stackref_debug_table = NULL;
+#endif
+
     if (tstate->interp == interp) {
         /* We are now safe to fix tstate->_status.cleared. */
         // XXX Do this (much) earlier?