diff --git a/CHANGES.rst b/CHANGES.rst index 0f88250f..0bbb0793 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,7 +5,15 @@ 3.5.6 (unreleased) ================== -- Nothing changed yet. +- Fix a crash (segfault) on the first greenlet switch on free-threaded + builds of Python 3.14.0 through 3.14.3. Python 3.14.4 changed the + layout of an internal structure that greenlet reads, so wheels built + for one 3.14.x looked in the wrong place on another. greenlet now + locates that field in the running interpreter rather than assuming the + layout it was compiled against, and refuses to import with a clear + error if it cannot. Reported by Federico Caselli in `issue 515 + `_. See `issue + 527 `_. 3.5.5 (2026-08-10) diff --git a/src/greenlet/PyModule.cpp b/src/greenlet/PyModule.cpp index f6190206..8b5dd8ba 100644 --- a/src/greenlet/PyModule.cpp +++ b/src/greenlet/PyModule.cpp @@ -210,7 +210,36 @@ mod_get_tstate_trash_delete_nesting(PyObject* UNUSED(module)) +#if GREENLET_PY314 && defined(Py_GIL_DISABLED) +PyDoc_STRVAR(mod_probe_c_stack_refs_offset_doc, + "_probe_c_stack_refs_offset(start) -> int\n" + "\n" + "Testing only. Locate _PyThreadStateImpl.c_stack_refs by searching\n" + "``start`` and the words around it. Returns 0 if it was not found.\n"); +static PyObject* +mod_probe_c_stack_refs_offset(PyObject* UNUSED(module), PyObject* arg) +{ + const Py_ssize_t start = PyLong_AsSsize_t(arg); + if (start < 0) { + if (!PyErr_Occurred()) { + PyErr_SetString(PyExc_ValueError, "start must be non-negative"); + } + return NULL; + } + return PyLong_FromSize_t( + greenlet::probe_c_stack_refs_offset((size_t)start)); +} +#endif + static PyMethodDef GreenMethods[] = { +#if GREENLET_PY314 && defined(Py_GIL_DISABLED) + { + .ml_name="_probe_c_stack_refs_offset", + .ml_meth=(PyCFunction)mod_probe_c_stack_refs_offset, + .ml_flags=METH_O, + .ml_doc=mod_probe_c_stack_refs_offset_doc + }, +#endif { .ml_name="getcurrent", .ml_meth=(PyCFunction)mod_getcurrent, diff --git a/src/greenlet/TGreenlet.hpp b/src/greenlet/TGreenlet.hpp index 7838ab92..501003af 100644 --- a/src/greenlet/TGreenlet.hpp +++ b/src/greenlet/TGreenlet.hpp @@ -113,6 +113,30 @@ namespace greenlet tstate->context_ver++; } }; +#if GREENLET_PY314 && defined(Py_GIL_DISABLED) + // Byte offset of _PyThreadStateImpl::c_stack_refs in the interpreter we are + // running on. Our compile-time offsetof() is only right if that interpreter + // has the same PyThreadState layout as the headers we were built against, + // and 3.14.4 broke that inside a released series by appending + // PyThreadState::datastack_cached_chunk: one cp314t wheel has to serve every + // 3.14.x. See https://github.com/python-greenlet/greenlet/issues/527. + extern size_t c_stack_refs_offset; + + // Get CPython to tell us where it keeps c_stack_refs, searching ``start`` + // and the words around it. Returns 0 if it could not be pinned down. + size_t probe_c_stack_refs_offset(size_t start) noexcept; + + // Set c_stack_refs_offset. Returns -1 with an exception set if the probe + // failed and we are not running on the interpreter we were built against. + int resolve_c_stack_refs_offset() noexcept; + + static inline _PyCStackRef** c_stack_refs_of(const PyThreadState* tstate) noexcept + { + char* const base = const_cast(reinterpret_cast(tstate)); + return reinterpret_cast<_PyCStackRef**>(base + c_stack_refs_offset); + } +#endif + class SwitchingArgs; class PythonState : public PythonStateContext { diff --git a/src/greenlet/TPythonState.cpp b/src/greenlet/TPythonState.cpp index bdedebe5..2f4e9af0 100644 --- a/src/greenlet/TPythonState.cpp +++ b/src/greenlet/TPythonState.cpp @@ -2,6 +2,8 @@ #define GREENLET_PYTHON_STATE_CPP #include +#include +#include #include "TGreenlet.hpp" namespace greenlet { @@ -93,20 +95,167 @@ PythonState::PythonState() } #if GREENLET_PY314 && defined(Py_GIL_DISABLED) + +size_t c_stack_refs_offset = offsetof(_PyThreadStateImpl, c_stack_refs); + +namespace { + +// Shared with probe_descr_get() for the duration of one probe. Probing happens +// at import, and from one test, so it does not need to be re-entrant. +uintptr_t probe_stack_top = 0; +size_t probe_start = 0; +size_t probe_found = 0; +int probe_hits = 0; + +// How far either side of probe_start to look, in pointer-sized steps. +const int PROBE_STEPS = 8; + +// And how far probe_start itself may sit from where we were compiled to expect +// the field. There are over 14000 bytes of _PyThreadStateImpl past c_stack_refs +// in every layout we know of, so this keeps every read inside the allocation. +const size_t PROBE_MAX_DRIFT = 256; + +PyObject* +probe_descr_get(PyObject* self, PyObject* UNUSED(obj), PyObject* UNUSED(type)) +{ + // _PyObject_GenericGetAttrWithDict resolved us through a _PyCStackRef + // holding ``self``, and that node is on its frame, between us and + // probe_stack_top. Whichever word of the thread state points at it is + // c_stack_refs. Comparing against a private object we just built means a + // near miss cannot pass for a hit. + char here; + const char* const base = reinterpret_cast(PyThreadState_GET()); + const uintptr_t low = reinterpret_cast(&here); + + for (int step = -PROBE_STEPS; step <= PROBE_STEPS; step++) { + const size_t offset = static_cast( + static_cast(probe_start) + step * (ptrdiff_t)sizeof(void*)); + uintptr_t value; + memcpy(&value, base + offset, sizeof(value)); + // Everything from &here up to probe_stack_top is our own live stack, so + // this bound is what makes the dereference below safe. + if (value <= low || value >= probe_stack_top || value % sizeof(void*)) { + continue; + } + const _PyCStackRef* const node = reinterpret_cast(value); + if (PyStackRef_IsNullOrInt(node->ref) + || PyStackRef_AsPyObjectBorrow(node->ref) != self) { + continue; + } + probe_found = offset; + probe_hits++; + } + Py_RETURN_NONE; +} + +int +probe_descr_set(PyObject* UNUSED(self), PyObject* UNUSED(obj), PyObject* UNUSED(value)) +{ + // Never called. It exists so PyDescr_IsData() is true and generic getattr + // takes its first branch, which calls us with the _PyCStackRef still held. + return 0; +} + +PyType_Slot probe_slots[] = { + {Py_tp_descr_get, (void*)probe_descr_get}, + {Py_tp_descr_set, (void*)probe_descr_set}, + {0, nullptr}, +}; + +PyType_Spec probe_spec = { + "greenlet._greenlet._c_stack_refs_probe", + sizeof(PyObject), + 0, + Py_TPFLAGS_DEFAULT, + probe_slots, +}; + +} // namespace + +size_t +probe_c_stack_refs_offset(size_t start) noexcept +{ + char outer; + const size_t expected = offsetof(_PyThreadStateImpl, c_stack_refs); + if (start < PROBE_STEPS * sizeof(void*) + || start + PROBE_MAX_DRIFT < expected + || start > expected + PROBE_MAX_DRIFT) { + return 0; + } + + // descr on a throwaway class, then read it back: type(o).attr.__get__ runs + // inside the lookup that holds the _PyCStackRef we are hunting for. + const OwnedObject descr_type = OwnedObject::consuming(PyType_FromSpec(&probe_spec)); + const OwnedObject descr = descr_type + ? OwnedObject::consuming(PyObject_CallNoArgs(descr_type.borrow())) + : OwnedObject(); + const OwnedObject attrs = OwnedObject::consuming(PyDict_New()); + if (!descr || !attrs + || PyDict_SetItemString(attrs.borrow(), "attr", descr.borrow()) < 0) { + PyErr_Clear(); + return 0; + } + const OwnedObject holder_type = OwnedObject::consuming( + PyObject_CallFunction((PyObject*)&PyType_Type, "s()O", + "greenlet_probe", attrs.borrow())); + const OwnedObject holder = holder_type + ? OwnedObject::consuming(PyObject_CallNoArgs(holder_type.borrow())) + : OwnedObject(); + if (!holder) { + PyErr_Clear(); + return 0; + } + + probe_stack_top = reinterpret_cast(&outer); + probe_start = start; + probe_found = 0; + probe_hits = 0; + const OwnedObject got = OwnedObject::consuming( + PyObject_GetAttrString(holder.borrow(), "attr")); + if (!got) { + PyErr_Clear(); + return 0; + } + // More than one candidate word means we cannot tell which is real. + return probe_hits == 1 ? probe_found : 0; +} + +int +resolve_c_stack_refs_offset() noexcept +{ + const size_t found = probe_c_stack_refs_offset(c_stack_refs_offset); + if (found) { + c_stack_refs_offset = found; + return 0; + } + if (Py_Version == PY_VERSION_HEX) { + // Built against exactly this interpreter, so offsetof() holds. + return 0; + } + PyErr_Format(PyExc_ImportError, + "greenlet was built for Python %d.%d.%d but is running on " + "%d.%d.%d, and could not locate c_stack_refs. Rebuild greenlet " + "for this interpreter.", + PY_MAJOR_VERSION, PY_MINOR_VERSION, PY_MICRO_VERSION, + (int)((Py_Version >> 24) & 0xFF), + (int)((Py_Version >> 16) & 0xFF), + (int)((Py_Version >> 8) & 0xFF)); + return -1; +} + void PythonState::capture_c_stack_refs(const PyThreadState* tstate) noexcept { - // Runs from operator<< while our C stack is still live and coherent, so we - // can walk tstate's _PyCStackRef list and take a strong reference to every - // object it holds. tp_traverse visits these once we're suspended, because - // by then the nodes themselves have been relocated into the heap stack copy - // and the saved list head no longer points at them. Strong references (not - // _Py_VISIT_STACKREF, whose _PyGC_VisitStackRef isn't exported before 3.15); - // a std::vector rather than a Python list/tuple because operator<< must not - // allocate a GC-tracked object mid-switch. Rebuilt from scratch each time; - // the list is empty at a typical switch, so this is usually just an empty - // loop. + // Runs from operator<< while our C stack is still live, so we can walk + // tstate's _PyCStackRef list and take a strong reference to everything it + // holds. tp_traverse visits those once we're suspended, by which point the + // nodes have moved into the heap stack copy and the saved head no longer + // points at them. Strong references rather than _Py_VISIT_STACKREF because + // _PyGC_VisitStackRef is not exported before 3.15, and a std::vector rather + // than a Python container because operator<< must not allocate a GC-tracked + // object mid-switch. Usually an empty loop; the list is empty at a typical + // switch. this->c_stack_ref_snapshot.clear(); - for (const _PyCStackRef* node = ((_PyThreadStateImpl*)tstate)->c_stack_refs; + for (const _PyCStackRef* node = *c_stack_refs_of(tstate); node != nullptr; node = node->next) { if (!PyStackRef_IsNullOrInt(node->ref)) { this->c_stack_ref_snapshot.push_back( @@ -168,7 +317,7 @@ void PythonState::operator<<(const PyThreadState *const tstate) noexcept this->py_recursion_depth = tstate->py_recursion_limit - tstate->py_recursion_remaining; this->current_executor = tstate->current_executor; #ifdef Py_GIL_DISABLED - this->c_stack_refs = ((_PyThreadStateImpl*)tstate)->c_stack_refs; + this->c_stack_refs = *c_stack_refs_of(tstate); // Capture the deferred references now, while our C stack is still live, so // tp_traverse can keep them from being collected while we're suspended. this->capture_c_stack_refs(tstate); @@ -291,7 +440,7 @@ void PythonState::operator>>(PyThreadState *const tstate) noexcept tstate->py_recursion_remaining = tstate->py_recursion_limit - this->py_recursion_depth; tstate->current_executor = this->current_executor; #ifdef Py_GIL_DISABLED - ((_PyThreadStateImpl*)tstate)->c_stack_refs = this->c_stack_refs; + *c_stack_refs_of(tstate) = this->c_stack_refs; // We're the running greenlet again: our C-stack refs live in the thread // state now and gc_visit_thread_stacks() covers them, so drop the strong // references tp_traverse held on our behalf while we were suspended. diff --git a/src/greenlet/greenlet.cpp b/src/greenlet/greenlet.cpp index 02dfa946..aa8834b7 100644 --- a/src/greenlet/greenlet.cpp +++ b/src/greenlet/greenlet.cpp @@ -219,6 +219,13 @@ greenlet_internal_mod_init() noexcept mod_globs = new greenlet::GreenletGlobals; ThreadState::init(); +#if GREENLET_PY314 && defined(Py_GIL_DISABLED) + // Before any switch can read it. + Require(greenlet::resolve_c_stack_refs_offset()); + m.PyAddObject("_C_STACK_REFS_OFFSET", + (long)greenlet::c_stack_refs_offset); +#endif + m.PyAddObject("greenlet", PyGreenlet_Type); m.PyAddObject("UnswitchableGreenlet", PyGreenletUnswitchable_Type); m.PyAddObject("error", mod_globs->PyExc_GreenletError); diff --git a/src/greenlet/tests/test_gc.py b/src/greenlet/tests/test_gc.py index fe075baa..15355970 100644 --- a/src/greenlet/tests/test_gc.py +++ b/src/greenlet/tests/test_gc.py @@ -1,5 +1,6 @@ import gc +import struct import weakref import sys import greenlet @@ -106,6 +107,38 @@ def test_c_stack_refs_suspended_gc(self): output = self.run_script('fail_c_stack_refs_suspended_gc.py') self.assertIn('C STACK REFS GC OK', output) + def _c_stack_refs_probe(self): + if not RUNNING_ON_FREETHREAD_BUILD or sys.version_info < (3, 14): + self.skipTest("Only free-threaded 3.14+ resolves the offset") + mod = greenlet._greenlet + return mod._probe_c_stack_refs_offset, mod._C_STACK_REFS_OFFSET + + def test_c_stack_refs_offset_resolved(self): + # Issue #527: 3.14.4 appended a PyThreadState field, which moved + # _PyThreadStateImpl.c_stack_refs by 8 bytes inside a released series. A + # wheel built against one 3.14.x read the wrong word on another and + # segfaulted on the first switch, so we ask the interpreter where the + # field is rather than trusting offsetof(). + _, offset = self._c_stack_refs_probe() + self.assertGreater(offset, 0) + self.assertEqual(offset % struct.calcsize('P'), 0) + + def test_c_stack_refs_offset_survives_a_wrong_start(self): + # The regression this guards: a build whose compile-time offsetof() is + # off by a pointer or two still finds the real field. + probe, offset = self._c_stack_refs_probe() + for bias in (-24, -16, -8, 0, 8, 16, 24): + self.assertEqual(probe(offset + bias), offset, bias) + + def test_c_stack_refs_offset_admits_defeat(self): + # Out of range it reports 0 instead of guessing, which is what turns an + # unrecognized layout into an ImportError rather than a crash. + probe, offset = self._c_stack_refs_probe() + self.assertEqual(probe(offset + 200), 0) + self.assertEqual(probe(0), 0) + with self.assertRaises(ValueError): + probe(-1) + def test_crashing_deferred_object(self): if sys.version_info < (3, 15): self.skipTest("Test is 3.15+ only")