From 70b46c95c8bd8b70c1e420b1165e58a8a96827d7 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 4 Jul 2024 13:23:34 +0200 Subject: [PATCH 1/2] Add blurb --- .../next/C API/2024-07-04-13-23-27.gh-issue-113601.K3RLqp.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/C API/2024-07-04-13-23-27.gh-issue-113601.K3RLqp.rst diff --git a/Misc/NEWS.d/next/C API/2024-07-04-13-23-27.gh-issue-113601.K3RLqp.rst b/Misc/NEWS.d/next/C API/2024-07-04-13-23-27.gh-issue-113601.K3RLqp.rst new file mode 100644 index 00000000000000..009cc2bf017180 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2024-07-04-13-23-27.gh-issue-113601.K3RLqp.rst @@ -0,0 +1,2 @@ +Removed debug build assertions related to interning strings, which were +falsely triggered by stable ABI extensions. From 4d92f62c7ddea826aa916081eded4d234621304a Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 4 Jul 2024 13:10:26 +0200 Subject: [PATCH 2/2] gh-113993: For string interning, do not assert or rely on _Py_IsImmortal Older stable ABI extensions are allowed to make immortal objects mortal. Instead, use `_PyUnicode_STATE` (`interned` and `statically_allocated`). --- Objects/unicodeobject.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index 394ea888fc9231..c2fb135ed688fc 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -252,7 +252,8 @@ _PyUnicode_InternedSize_Immortal(void) // value, to help detect bugs in optimizations. while (PyDict_Next(dict, &pos, &key, &value)) { - if (_Py_IsImmortal(key)) { + assert(PyUnicode_CHECK_INTERNED(key) != SSTATE_INTERNED_IMMORTAL_STATIC); + if (PyUnicode_CHECK_INTERNED(key) == SSTATE_INTERNED_IMMORTAL) { count++; } } @@ -688,10 +689,14 @@ _PyUnicode_CheckConsistency(PyObject *op, int check_content) /* Check interning state */ #ifdef Py_DEBUG + // Note that we do not check `_Py_IsImmortal(op)`, since stable ABI + // extensions can make immortal strings mortal (but with a high enough + // refcount). + // The other way is extremely unlikely (worth a potential failed assertion + // in a debug build), so we do check `!_Py_IsImmortal(op)`. switch (PyUnicode_CHECK_INTERNED(op)) { case SSTATE_NOT_INTERNED: if (ascii->state.statically_allocated) { - CHECK(_Py_IsImmortal(op)); // This state is for two exceptions: // - strings are currently checked before they're interned // - the 256 one-latin1-character strings @@ -707,11 +712,9 @@ _PyUnicode_CheckConsistency(PyObject *op, int check_content) break; case SSTATE_INTERNED_IMMORTAL: CHECK(!ascii->state.statically_allocated); - CHECK(_Py_IsImmortal(op)); break; case SSTATE_INTERNED_IMMORTAL_STATIC: CHECK(ascii->state.statically_allocated); - CHECK(_Py_IsImmortal(op)); break; default: Py_UNREACHABLE(); @@ -1867,7 +1870,6 @@ static PyObject* get_latin1_char(Py_UCS1 ch) { PyObject *o = LATIN1(ch); - assert(_Py_IsImmortal(o)); return o; } @@ -15352,7 +15354,6 @@ intern_static(PyInterpreterState *interp, PyObject *s /* stolen */) assert(s != NULL); assert(_PyUnicode_CHECK(s)); assert(_PyUnicode_STATE(s).statically_allocated); - assert(_Py_IsImmortal(s)); switch (PyUnicode_CHECK_INTERNED(s)) { case SSTATE_NOT_INTERNED: @@ -15493,7 +15494,7 @@ intern_common(PyInterpreterState *interp, PyObject *s /* stolen */, { PyObject *r = (PyObject *)_Py_hashtable_get(INTERNED_STRINGS, s); if (r != NULL) { - assert(_Py_IsImmortal(r)); + assert(_PyUnicode_STATE(r).statically_allocated); assert(r != s); // r must be statically_allocated; s is not Py_DECREF(s); return Py_NewRef(r);