diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0298d0467fe35f..5b043a1309b1c7 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -241,6 +241,19 @@ jobs: # Cirrus used for upstream, macos-14 for forks. os-matrix: '["ghcr.io/cirruslabs/macos-runner:sonoma", "macos-14"]' + build_macos_free_threading_with_stackref_debug: + name: 'macOS (free-threading) (StackRef Debug)' + needs: check_source + if: needs.check_source.outputs.run_tests == 'true' + uses: ./.github/workflows/reusable-macos.yml + with: + config_hash: ${{ needs.check_source.outputs.config_hash }} + free-threading: true + stackref-debug: true + # Cirrus and macos-14 are M1. + # Cirrus used for upstream, macos-14 for forks. + os-matrix: '["ghcr.io/cirruslabs/macos-runner:sonoma", "macos-14"]' + build_ubuntu: name: 'Ubuntu' needs: check_source @@ -577,6 +590,7 @@ jobs: - check_generated_files - build_macos - build_macos_free_threading + - build_macos_free_threading_with_stackref_debug - build_ubuntu - build_ubuntu_free_threading - build_ubuntu_ssltests diff --git a/.github/workflows/reusable-macos.yml b/.github/workflows/reusable-macos.yml index f825d1a7b3f69a..6611b9c69292ae 100644 --- a/.github/workflows/reusable-macos.yml +++ b/.github/workflows/reusable-macos.yml @@ -8,6 +8,10 @@ on: required: false type: boolean default: false + stackref-debug: + required: false + type: boolean + default: false os-matrix: required: false type: string @@ -54,6 +58,7 @@ jobs: --config-cache \ --with-pydebug \ ${{ inputs.free-threading && '--disable-gil' || '' }} \ + ${{ inputs.stackref-debug && '--with-pystackrefdebug' || '' }} \ --prefix=/opt/python-dev \ --with-openssl="$(brew --prefix openssl@3.0)" - name: Build CPython @@ -61,4 +66,6 @@ jobs: - name: Display build info run: make pythoninfo - name: Tests - run: make test + # Stackref debug is 3.5x slower than normal CPython, + # so we only run it on a restricted subset of tests. + run: ${{ inputs.stackref-debug && './python.exe -m test test_typing' || 'make test' }} diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 4a83862ac13e26..9831a9c7ed4180 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -275,6 +275,10 @@ struct _is { /* the initial PyInterpreterState.threads.head */ _PyThreadStateImpl _initial_thread; Py_ssize_t _interactive_src_count; + +#if defined(Py_STACKREF_DEBUG) && defined(Py_GIL_DISABLED) + struct _Py_stackref_state stackref_state; +#endif }; diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 8d3d559814bfd9..249f3493fcf1fb 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -9,6 +9,7 @@ extern "C" { #endif #include "pycore_object_deferred.h" +#include "pycore_hashtable.h" #include @@ -52,42 +53,93 @@ typedef union _PyStackRef { uintptr_t bits; } _PyStackRef; +struct _Py_stackref_state { + PyMutex lock; + _Py_hashtable_t *entries; + size_t next_ref; +}; + +typedef enum _PyStackRef_OpKind { + BORROW, + STEAL, + NEW, +} _PyStackRef_OpKind; + +PyAPI_FUNC(PyObject *)_Py_stackref_to_object_transition(_PyStackRef stackref, _PyStackRef_OpKind op); +PyAPI_FUNC(_PyStackRef) _Py_object_to_stackref_transition(PyObject *obj, char tag, _PyStackRef_OpKind op); +PyAPI_FUNC(int) _PyStackRef_IsLive(_PyStackRef stackref); +PyAPI_FUNC(void) _PyStackRef_Close(_PyStackRef stackref); +PyAPI_FUNC(_PyStackRef) _PyStackRef_Dup(_PyStackRef stackref); #define Py_TAG_DEFERRED (1) #define Py_TAG_PTR (0) #define Py_TAG_BITS (1) +#define RESERVED_BITS 1 + #ifdef Py_GIL_DISABLED static const _PyStackRef PyStackRef_NULL = { .bits = 0 | Py_TAG_DEFERRED}; #else static const _PyStackRef PyStackRef_NULL = { .bits = 0 }; #endif -#define PyStackRef_IsNull(stackref) ((stackref).bits == PyStackRef_NULL.bits) - #ifdef Py_GIL_DISABLED -# define PyStackRef_True ((_PyStackRef){.bits = ((uintptr_t)&_Py_TrueStruct) | Py_TAG_DEFERRED }) +# ifdef Py_STACKREF_DEBUG + // From pycore_init_stackrefs in pylifecycle.c + static const _PyStackRef PyStackRef_True = { .bits = 1 << RESERVED_BITS }; +# else +# define PyStackRef_True ((_PyStackRef){.bits = ((uintptr_t)&_Py_TrueStruct) | Py_TAG_DEFERRED }) +# endif #else # define PyStackRef_True ((_PyStackRef){.bits = ((uintptr_t)&_Py_TrueStruct) }) #endif #ifdef Py_GIL_DISABLED -# define PyStackRef_False ((_PyStackRef){.bits = ((uintptr_t)&_Py_FalseStruct) | Py_TAG_DEFERRED }) +# ifdef Py_STACKREF_DEBUG + // From pycore_init_stackrefs in pylifecycle.c + static const _PyStackRef PyStackRef_False = { .bits = 2 << RESERVED_BITS }; +# else +# define PyStackRef_False ((_PyStackRef){.bits = ((uintptr_t)&_Py_FalseStruct) | Py_TAG_DEFERRED }) +# endif #else # define PyStackRef_False ((_PyStackRef){.bits = ((uintptr_t)&_Py_FalseStruct) }) #endif #ifdef Py_GIL_DISABLED +# ifdef Py_STACKREF_DEBUG + // From pycore_init_stackrefs in pylifecycle.c + static const _PyStackRef PyStackRef_None = { .bits = 3 << RESERVED_BITS }; +# else # define PyStackRef_None ((_PyStackRef){.bits = ((uintptr_t)&_Py_NoneStruct) | Py_TAG_DEFERRED }) +# endif #else # define PyStackRef_None ((_PyStackRef){.bits = ((uintptr_t)&_Py_NoneStruct) }) #endif // Note: the following are all macros because MSVC (Windows) has trouble inlining them. -#define PyStackRef_Is(a, b) ((a).bits == (b).bits) +#ifdef Py_GIL_DISABLED +static inline int +PyStackRef_Is(_PyStackRef a, _PyStackRef b) { +# ifdef Py_STACKREF_DEBUG + // Note: stackrefs are unique. So even immortal objects will produce different stackrefs. + return _Py_stackref_to_object_transition(a, BORROW) == _Py_stackref_to_object_transition(b, BORROW); +# else + return a.bits == b.bits; +# endif +} +#else +# define PyStackRef_Is(a, b) ((a).bits == (b).bits) +#endif + +static inline int +PyStackRef_IsNull(_PyStackRef stackref) +{ + return PyStackRef_Is(stackref, PyStackRef_NULL); +} + #define PyStackRef_IsDeferred(ref) (((ref).bits & Py_TAG_BITS) == Py_TAG_DEFERRED) @@ -97,8 +149,12 @@ typedef union _PyStackRef { static inline PyObject * PyStackRef_AsPyObjectBorrow(_PyStackRef stackref) { +# ifdef Py_STACKREF_DEBUG + return _Py_stackref_to_object_transition(stackref, BORROW); +# else PyObject *cleared = ((PyObject *)((stackref).bits & (~Py_TAG_BITS))); return cleared; +# endif } #else # define PyStackRef_AsPyObjectBorrow(stackref) ((PyObject *)(stackref).bits) @@ -110,10 +166,14 @@ PyStackRef_AsPyObjectBorrow(_PyStackRef stackref) static inline PyObject * PyStackRef_AsPyObjectSteal(_PyStackRef stackref) { +# ifdef Py_STACKREF_DEBUG + return _Py_stackref_to_object_transition(stackref, STEAL); +# else if (!PyStackRef_IsNull(stackref) && PyStackRef_IsDeferred(stackref)) { return Py_NewRef(PyStackRef_AsPyObjectBorrow(stackref)); } return PyStackRef_AsPyObjectBorrow(stackref); +# endif } #else # define PyStackRef_AsPyObjectSteal(stackref) PyStackRef_AsPyObjectBorrow(stackref) @@ -133,7 +193,11 @@ _PyStackRef_FromPyObjectSteal(PyObject *obj) // Make sure we don't take an already tagged value. assert(((uintptr_t)obj & Py_TAG_BITS) == 0); int tag = (obj == NULL || _Py_IsImmortal(obj)) ? (Py_TAG_DEFERRED) : Py_TAG_PTR; +# ifdef Py_STACKREF_DEBUG + return _Py_object_to_stackref_transition(obj, tag, STEAL); +# else return ((_PyStackRef){.bits = ((uintptr_t)(obj)) | tag}); +# endif } # define PyStackRef_FromPyObjectSteal(obj) _PyStackRef_FromPyObjectSteal(_PyObject_CAST(obj)) #else @@ -151,10 +215,18 @@ PyStackRef_FromPyObjectNew(PyObject *obj) assert(obj != NULL); // TODO (gh-117139): Add deferred objects later. if (_Py_IsImmortal(obj)) { +# ifdef Py_STACKREF_DEBUG + return _Py_object_to_stackref_transition(obj, Py_TAG_DEFERRED, NEW); +# else return (_PyStackRef){ .bits = (uintptr_t)obj | Py_TAG_DEFERRED }; +# endif } else { +# ifdef Py_STACKREF_DEBUG + return _Py_object_to_stackref_transition(Py_NewRef(obj), Py_TAG_PTR, NEW); +# else return (_PyStackRef){ .bits = (uintptr_t)(Py_NewRef(obj)) | Py_TAG_PTR }; +# endif } } # define PyStackRef_FromPyObjectNew(obj) PyStackRef_FromPyObjectNew(_PyObject_CAST(obj)) @@ -171,7 +243,11 @@ PyStackRef_FromPyObjectImmortal(PyObject *obj) assert(((uintptr_t)obj & Py_TAG_BITS) == 0); assert(obj != NULL); assert(_Py_IsImmortal(obj)); +# ifdef Py_STACKREF_DEBUG + return _Py_object_to_stackref_transition(obj, Py_TAG_DEFERRED, NEW); +# else return (_PyStackRef){ .bits = (uintptr_t)obj | Py_TAG_DEFERRED }; +# endif } # define PyStackRef_FromPyObjectImmortal(obj) PyStackRef_FromPyObjectImmortal(_PyObject_CAST(obj)) #else @@ -199,6 +275,9 @@ PyStackRef_CLOSE(_PyStackRef stackref) return; } Py_DECREF(PyStackRef_AsPyObjectBorrow(stackref)); +# ifdef Py_STACKREF_DEBUG + _PyStackRef_Close(stackref); +# endif } #else # define PyStackRef_CLOSE(stackref) Py_DECREF(PyStackRef_AsPyObjectBorrow(stackref)); @@ -217,6 +296,9 @@ PyStackRef_CLOSE(_PyStackRef stackref) static inline _PyStackRef PyStackRef_DUP(_PyStackRef stackref) { +# ifdef Py_STACKREF_DEBUG + stackref = _PyStackRef_Dup(stackref); +# endif if (PyStackRef_IsDeferred(stackref)) { assert(PyStackRef_IsNull(stackref) || _Py_IsImmortal(PyStackRef_AsPyObjectBorrow(stackref))); diff --git a/Makefile.pre.in b/Makefile.pre.in index 3b2e35d22a6d95..665bbfee12d0ec 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -474,6 +474,7 @@ PYTHON_OBJS= \ Python/qsbr.o \ Python/bootstrap_hash.o \ Python/specialize.o \ + Python/stackref.o \ Python/structmember.o \ Python/symtable.o \ Python/sysmodule.o \ diff --git a/Misc/NEWS.d/next/Build/2024-06-29-00-02-42.gh-issue-117139.-lKXPB.rst b/Misc/NEWS.d/next/Build/2024-06-29-00-02-42.gh-issue-117139.-lKXPB.rst new file mode 100644 index 00000000000000..26714016643274 --- /dev/null +++ b/Misc/NEWS.d/next/Build/2024-06-29-00-02-42.gh-issue-117139.-lKXPB.rst @@ -0,0 +1,4 @@ +Added a new ``--with-pystackrefs`` configure build option for CPython. This +option is fully experimental and may be modified or removed without prior +notice. Enabling the option turns on internal handle-like debugging for +CPython's main interpreter loop. diff --git a/Python/ceval.c b/Python/ceval.c index 026e018676caad..f39825b6c498ea 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -726,6 +726,18 @@ _PyObjectArray_Free(PyObject **array, PyObject **scratch) } } +#if Py_STACKREF_DEBUG +static int +_PyEval_StackIsAllLive(_PyStackRef *stack_base, _PyStackRef *stack_pointer) +{ + while (stack_pointer > stack_base) { + assert(_PyStackRef_IsLive(stack_pointer[-1])); + stack_pointer--; + } + return 1; +} +#endif + /* _PyEval_EvalFrameDefault() is a *big* function, * so consume 3 units of C stack */ #define PY_EVAL_C_STACK_UNITS 2 diff --git a/Python/ceval_macros.h b/Python/ceval_macros.h index 595b72bfaf9613..6744e2ae248e56 100644 --- a/Python/ceval_macros.h +++ b/Python/ceval_macros.h @@ -86,6 +86,17 @@ #define PRE_DISPATCH_GOTO() ((void)0) #endif +#ifdef Py_STACKREF_DEBUG +#define STACKREF_CHECK() \ + do { \ + if (frame->f_executable && PyCode_Check(frame->f_executable)) { \ + _PyEval_StackIsAllLive(_PyFrame_Stackbase(frame), stack_pointer); \ + }; \ + } while (0); +#else +#define STACKREF_CHECK() ((void)(0)) +#endif + #if LLTRACE #define LLTRACE_RESUME_FRAME() \ do { \ @@ -110,13 +121,15 @@ do { \ { \ NEXTOPARG(); \ PRE_DISPATCH_GOTO(); \ + STACKREF_CHECK(); \ DISPATCH_GOTO(); \ } #define DISPATCH_SAME_OPARG() \ { \ opcode = next_instr->op.code; \ - PRE_DISPATCH_GOTO(); \ + PRE_DISPATCH_GOTO(); \ + STACKREF_CHECK(); \ DISPATCH_GOTO(); \ } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 39eaa86098b2e5..f789afbae221d0 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -837,6 +837,35 @@ pycore_init_builtins(PyThreadState *tstate) return _PyStatus_ERR("can't initialize builtins module"); } +#if defined(Py_STACKREF_DEBUG) && defined(Py_GIL_DISABLED) +static PyStatus +pycore_init_stackrefs(PyInterpreterState *interp) +{ + _Py_hashtable_allocator_t alloc = { + .malloc = PyMem_Malloc, + .free = PyMem_Free, + }; + interp->stackref_state.entries = _Py_hashtable_new_full( + _Py_hashtable_hash_ptr, + _Py_hashtable_compare_direct, + NULL, + NULL, + &alloc + ); + if (interp->stackref_state.entries == NULL) { + goto error; + } + interp->stackref_state.next_ref = 1; + // Reserve True, False, None + _Py_object_to_stackref_transition(Py_True, Py_TAG_DEFERRED, STEAL); + _Py_object_to_stackref_transition(Py_False, Py_TAG_DEFERRED, STEAL); + _Py_object_to_stackref_transition(Py_None, Py_TAG_DEFERRED, STEAL); + return _PyStatus_OK(); + +error: + return _PyStatus_ERR("can't initialize stackrefs to pyobject mapping"); +} +#endif static PyStatus pycore_interp_init(PyThreadState *tstate) @@ -845,6 +874,13 @@ pycore_interp_init(PyThreadState *tstate) PyStatus status; PyObject *sysmod = NULL; +#if defined(Py_STACKREF_DEBUG) && defined(Py_GIL_DISABLED) + status = pycore_init_stackrefs(interp); + if (_PyStatus_EXCEPTION(status)) { + return status; + } +#endif + // Create singletons before the first PyType_Ready() call, since // PyType_Ready() uses singletons like the Unicode empty string (tp_doc) // and the empty tuple singletons (tp_bases). diff --git a/Python/pystate.c b/Python/pystate.c index f77a2cc54e867a..406cf3c64c414e 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -903,6 +903,10 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) interp->code_watchers[i] = NULL; } interp->active_code_watchers = 0; + +#if defined(Py_STACKREF_DEBUG) && defined(Py_GIL_DISABLED) + PyMem_Free(interp->stackref_state.entries); +#endif // XXX Once we have one allocator per interpreter (i.e. // per-interpreter GC) we must ensure that all of the interpreter's // objects have been cleaned up at the point. diff --git a/Python/stackref.c b/Python/stackref.c new file mode 100644 index 00000000000000..63d7127b0bfd77 --- /dev/null +++ b/Python/stackref.c @@ -0,0 +1,103 @@ +#include "Python.h" + +#include "pycore_stackref.h" +#include "pycore_interp.h" +#include "pycore_pyatomic_ft_wrappers.h" +#include "pycore_critical_section.h" + +#if defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG) +PyObject * +_Py_stackref_to_object_transition(_PyStackRef stackref, _PyStackRef_OpKind op) +{ + PyObject *res = NULL; + PyInterpreterState *interp = PyInterpreterState_Get(); + assert(interp != NULL); + int is_null = 0; + void *bits = (void *)(stackref.bits >> RESERVED_BITS); + switch (op) { + case STEAL: { + // We need to distinguish NULL here because _Py_hashtable_get + // returns NULL if it cannot find a value. + if (bits == 0) { + is_null = 1; + break; + } + PyMutex_Lock(&interp->stackref_state.lock); + res = (PyObject *)_Py_hashtable_get(interp->stackref_state.entries, bits); + if (res != NULL && !_Py_IsImmortal(res)) { + res = (PyObject *)_Py_hashtable_steal(interp->stackref_state.entries, bits); + } + PyMutex_Unlock(&interp->stackref_state.lock); + break; + } + case NEW: + case BORROW: { + if (bits == 0) { + is_null = 1; + break; + } + PyMutex_Lock(&interp->stackref_state.lock); + res = (PyObject *)_Py_hashtable_get(interp->stackref_state.entries, bits); + PyMutex_Unlock(&interp->stackref_state.lock); + break; + } + } + assert((res != NULL) ^ is_null); + return res; +} + +_PyStackRef +_Py_object_to_stackref_transition(PyObject *obj, char tag, _PyStackRef_OpKind op) +{ + _PyStackRef res; + PyInterpreterState *interp = PyInterpreterState_Get(); + assert(interp != NULL); + switch (op) { + case STEAL: + case NEW: + { + if (obj == NULL) { + return PyStackRef_NULL; + } + PyMutex_Lock(&interp->stackref_state.lock); + uintptr_t key = interp->stackref_state.next_ref; + res.bits = (key << RESERVED_BITS) | tag; + int err = _Py_hashtable_set(interp->stackref_state.entries, (void *)key, obj); + if (err < 0) { + Py_FatalError("Stackref handle allocation failed.\n"); + } + interp->stackref_state.next_ref++; + PyMutex_Unlock(&interp->stackref_state.lock); + break; + } + case BORROW: + Py_UNREACHABLE(); + } + return res; +} + +void +_PyStackRef_Close(_PyStackRef stackref) +{ + _Py_stackref_to_object_transition(stackref, STEAL); +} + +_PyStackRef +_PyStackRef_Dup(_PyStackRef stackref) +{ + PyInterpreterState *interp = PyInterpreterState_Get(); + assert(interp != NULL); + char tag = stackref.bits & RESERVED_BITS; + PyObject *val = _Py_stackref_to_object_transition(stackref, BORROW); + return _Py_object_to_stackref_transition(val, tag, NEW); +} +#endif + +int +_PyStackRef_IsLive(_PyStackRef stackref) +{ +#if Py_STACKREF_DEBUG + _Py_stackref_to_object_transition(stackref, BORROW); +#endif + return 1; +} diff --git a/configure b/configure index 73d3bda9ddcdaa..4bd0ff955df292 100755 --- a/configure +++ b/configure @@ -1083,6 +1083,7 @@ enable_shared with_static_libpython enable_profiling enable_gil +with_pystackrefdebug with_pydebug with_trace_refs enable_pystats @@ -1861,6 +1862,7 @@ Optional Packages: --without-static-libpython do not build libpythonMAJOR.MINOR.a and do not install python.o (default is yes) + --with-pystackrefdebug build with Py_STACKREF_DEBUG defined (default is no) --with-pydebug build with Py_DEBUG defined (default is no) --with-trace-refs enable tracing references for debugging purpose (default is no) @@ -8127,6 +8129,32 @@ printf "%s\n" "#define Py_GIL_DISABLED 1" >>confdefs.h ABI_THREAD="t" fi +# Check for --with-pystackrefdebug +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for --with-pystackrefdebug" >&5 +printf %s "checking for --with-pystackrefdebug... " >&6; } + +# Check whether --with-pystackrefdebug was given. +if test ${with_pystackrefdebug+y} +then : + withval=$with_pystackrefdebug; +if test "$withval" != no +then + +printf "%s\n" "#define Py_STACKREF_DEBUG 1" >>confdefs.h + + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: yes" >&5 +printf "%s\n" "yes" >&6; }; + Py_STACKREF_DEBUG='true' + ABIFLAGS="${ABIFLAGS}d" +else { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: no" >&5 +printf "%s\n" "no" >&6; }; Py_STACKREF_DEBUG='false' +fi +else $as_nop + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: no" >&5 +printf "%s\n" "no" >&6; } +fi + + # Check for --with-pydebug { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for --with-pydebug" >&5 printf %s "checking for --with-pydebug... " >&6; } diff --git a/configure.ac b/configure.ac index 00246a12100863..dabc8965ebb2c8 100644 --- a/configure.ac +++ b/configure.ac @@ -1705,6 +1705,22 @@ then ABI_THREAD="t" fi +# Check for --with-pystackrefdebug +AC_MSG_CHECKING([for --with-pystackrefdebug]) +AC_ARG_WITH([pystackrefdebug], + [AS_HELP_STRING([--with-pystackrefdebug], [build with Py_STACKREF_DEBUG defined (default is no)]) ], +[ +if test "$withval" != no +then + AC_DEFINE([Py_STACKREF_DEBUG], [1], + [Define if you want to build an interpreter with stack reference checks on (WARNING: EXPERIMENTAL).]) + AC_MSG_RESULT([yes]); + Py_STACKREF_DEBUG='true' + ABIFLAGS="${ABIFLAGS}d" +else AC_MSG_RESULT([no]); Py_STACKREF_DEBUG='false' +fi], +[AC_MSG_RESULT([no])]) + # Check for --with-pydebug AC_MSG_CHECKING([for --with-pydebug]) AC_ARG_WITH([pydebug], diff --git a/pyconfig.h.in b/pyconfig.h.in index 8fbba7ed3b949e..bbf305fc660d25 100644 --- a/pyconfig.h.in +++ b/pyconfig.h.in @@ -1695,6 +1695,10 @@ /* Define if rl_startup_hook takes arguments */ #undef Py_RL_STARTUP_HOOK_TAKES_ARGS +/* Define if you want to build an interpreter with stack reference checks on + (WARNING: EXPERIMENTAL). */ +#undef Py_STACKREF_DEBUG + /* Define if you want to enable internal statistics gathering. */ #undef Py_STATS