GH-127705: Add debug mode for _PyStackRefs inspired by HPy debug mode

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,113 @@ 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);
+PyAPI_FUNC(void) _Py_stackref_record_borrow(_PyStackRef ref, 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 })
+
+#define LAST_PREDEFINED_STACKREF_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, const char *filename, int linenumber)
+{
+ _Py_stackref_record_borrow(ref, filename, linenumber);
+ return _Py_stackref_get_object(ref);
+}
+
+#define PyStackRef_AsPyObjectBorrow(REF) _PyStackRef_AsPyObjectBorrow((REF), __FILE__, __LINE__)
+
+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 +310,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 \

Misc/NEWS.d/next/Core_and_Builtins/2024-12-20-12-25-16.gh-issue-127705.WmCz1z.rst

@@ -0,0 +1,4 @@
+Adds stackref debugging when ``Py_STACKREF_DEBUG`` is set. Finds all
+double-closes and leaks, logging the origin and last borrow.
+
+Inspired by HPy's debug mode. https://docs.hpyproject.org/en/latest/debug-mode.html

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/bytecodes.c

@@ -679,7 +679,7 @@ dummy_func(
  assert(Py_REFCNT(left_o) >= 2);
  PyStackRef_CLOSE(left);
  DEAD(left);
- PyObject *temp = PyStackRef_AsPyObjectBorrow(*target_local);
+ PyObject *temp = PyStackRef_AsPyObjectSteal(*target_local);
  PyUnicode_Append(&temp, right_o);
  *target_local = PyStackRef_FromPyObjectSteal(temp);
  PyStackRef_CLOSE_SPECIALIZED(right, _PyUnicode_ExactDealloc);
@@ -4476,17 +4476,17 @@ dummy_func(
 
  op(_DO_CALL_FUNCTION_EX, (func_st, unused, callargs_st, kwargs_st if (oparg & 1) -- result)){
  PyObject *func = PyStackRef_AsPyObjectBorrow(func_st);
- PyObject *callargs = PyStackRef_AsPyObjectBorrow(callargs_st);
- PyObject *kwargs = PyStackRef_AsPyObjectBorrow(kwargs_st);
 
  // DICT_MERGE is called before this opcode if there are kwargs.
  // It converts all dict subtypes in kwargs into regular dicts.
- assert(kwargs == NULL || PyDict_CheckExact(kwargs));
- assert(PyTuple_CheckExact(callargs));
  EVAL_CALL_STAT_INC_IF_FUNCTION(EVAL_CALL_FUNCTION_EX, func);
  PyObject *result_o;
  assert(!_PyErr_Occurred(tstate));
  if (opcode == INSTRUMENTED_CALL_FUNCTION_EX){
+ PyObject *callargs = PyStackRef_AsPyObjectBorrow(callargs_st);
+ PyObject *kwargs = PyStackRef_AsPyObjectBorrow(kwargs_st);
+ assert(kwargs == NULL || PyDict_CheckExact(kwargs));
+ assert(PyTuple_CheckExact(callargs));
  PyObject *arg = PyTuple_GET_SIZE(callargs) > 0 ?
  PyTuple_GET_ITEM(callargs, 0) : &_PyInstrumentation_MISSING;
  int err = _Py_call_instrumentation_2args(
@@ -4517,7 +4517,10 @@ dummy_func(
  if (Py_TYPE(func) == &PyFunction_Type &&
  tstate->interp->eval_frame == NULL &&
  ((PyFunctionObject *)func)->vectorcall == _PyFunction_Vectorcall){
+ PyObject *callargs = PyStackRef_AsPyObjectSteal(callargs_st);
  assert(PyTuple_CheckExact(callargs));
+ PyObject *kwargs = PyStackRef_IsNull(kwargs_st) ? NULL : PyStackRef_AsPyObjectSteal(kwargs_st);
+ assert(kwargs == NULL || PyDict_CheckExact(kwargs));
  Py_ssize_t nargs = PyTuple_GET_SIZE(callargs);
  int code_flags = ((PyCodeObject *)PyFunction_GET_CODE(func))->co_flags;
  PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : Py_NewRef(PyFunction_GET_GLOBALS(func));
@@ -4535,6 +4538,10 @@ dummy_func(
  frame->return_offset = 1;
  DISPATCH_INLINED(new_frame);
  }
+ PyObject *callargs = PyStackRef_AsPyObjectBorrow(callargs_st);
+ assert(PyTuple_CheckExact(callargs));
+ PyObject *kwargs = PyStackRef_AsPyObjectBorrow(kwargs_st);
+ assert(kwargs == NULL || PyDict_CheckExact(kwargs));
  result_o = PyObject_Call(func, callargs, kwargs);
  }
  PyStackRef_XCLOSE(kwargs_st);

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;
@@ -1810,27 +1810,48 @@ _PyEvalFramePushAndInit_Ex(PyThreadState *tstate, _PyStackRef func,
 {
  bool has_dict = (kwargs != NULL && PyDict_GET_SIZE(kwargs) > 0);
  PyObject *kwnames = NULL;
- PyObject *const *newargs;
+ _PyStackRef *newargs;
+ PyObject *const *object_array = NULL;
+ _PyStackRef stack_array[8];
  if (has_dict){
-newargs = _PyStack_UnpackDict(tstate, _PyTuple_ITEMS(callargs), nargs, kwargs, &kwnames);
- if (newargs == NULL){
+object_array = _PyStack_UnpackDict(tstate, _PyTuple_ITEMS(callargs), nargs, kwargs, &kwnames);
+ if (object_array == NULL){
  PyStackRef_CLOSE(func);
  goto error;
  }
+ size_t total_args = nargs + PyDict_GET_SIZE(kwargs);
+ assert(sizeof(PyObject *) == sizeof(_PyStackRef));
+ newargs = (_PyStackRef *)object_array;
+ for (size_t i = 0; i < total_args; i++){
+ newargs[i] = PyStackRef_FromPyObjectSteal(object_array[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){
+ newargs = stack_array;
+ }
+ else{
+ newargs = PyMem_Malloc(sizeof(_PyStackRef) *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++){
+ newargs[i] = PyStackRef_FromPyObjectNew(PyTuple_GET_ITEM(callargs, i));
  }
  }
  _PyInterpreterFrame *new_frame = _PyEvalFramePushAndInit(
  tstate, func, locals,
-(_PyStackRef const *)newargs, nargs, kwnames, previous
+ newargs, nargs, kwnames, previous
  );
  if (has_dict){
- _PyStack_UnpackDict_FreeNoDecRef(newargs, kwnames);
+ _PyStack_UnpackDict_FreeNoDecRef(object_array, 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 +1871,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)