]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
[3.13] gh-113993: For string interning, do not rely on (or assert) _Py_IsImmortal...
authorMiss Islington (bot) <31488909+miss-islington@users.noreply.github.com>
Tue, 16 Jul 2024 13:42:49 +0000 (15:42 +0200)
committerGitHub <noreply@github.com>
Tue, 16 Jul 2024 13:42:49 +0000 (13:42 +0000)
gh-113993: For string interning, do not rely on (or assert) _Py_IsImmortal (GH-121358)

Older stable ABI extensions are allowed to make immortal objects mortal.
Instead, use `_PyUnicode_STATE` (`interned` and `statically_allocated`).
(cherry picked from commit 956270d08d5c23f59937e2f29f8e0b7f63d68afd)

Co-authored-by: Petr Viktorin <encukou@gmail.com>
Misc/NEWS.d/next/C API/2024-07-04-13-23-27.gh-issue-113601.K3RLqp.rst [new file with mode: 0644]
Objects/unicodeobject.c

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 (file)
index 0000000..009cc2b
--- /dev/null
@@ -0,0 +1,2 @@
+Removed debug build assertions related to interning strings, which were
+falsely triggered by stable ABI extensions.
index b21886e22c178b51e20e8b91a83645cc61b52d29..c7bd474c1e3a9e942263a6f8e17f37334601b298 100644 (file)
@@ -247,7 +247,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++;
        }
     }
@@ -683,10 +684,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
@@ -702,11 +707,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();
@@ -1899,7 +1902,6 @@ static PyObject*
 get_latin1_char(Py_UCS1 ch)
 {
     PyObject *o = LATIN1(ch);
-    assert(_Py_IsImmortal(o));
     return o;
 }
 
@@ -15049,7 +15051,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:
@@ -15190,7 +15191,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);