gh-148871: make `LOAD_COMMON_CONSTANT` use immortal stackref borrows (GH-149625)

diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h
index f13bc2178b1e7eb527c8959308b01be011b9abfc..4c8ad11447aa5918318324d1e53c28bcbf55cf78 100644 (file)
--- a/Include/internal/pycore_interp_structs.h
+++ b/Include/internal/pycore_interp_structs.h
@@ -1001,7 +1001,7 @@ struct _is {
     struct ast_state ast;
     struct types_state types;
     struct callable_cache callable_cache;
-    PyObject *common_consts[NUM_COMMON_CONSTANTS];
+    _PyStackRef common_consts[NUM_COMMON_CONSTANTS];
     bool jit;
     bool compiling;
 

diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h
index ca4a7c216eda532ac6da8667a011acca8d569de3..9495ccc8ac38896e19262c6a80c3bf8683cad45c 100644 (file)
--- a/Include/internal/pycore_stackref.h
+++ b/Include/internal/pycore_stackref.h
@@ -263,6 +263,18 @@ _PyStackRef_DUP(_PyStackRef ref, const char *filename, int linenumber)
 }
 #define PyStackRef_DUP(REF) _PyStackRef_DUP(REF, __FILE__, __LINE__)
 
+static inline _PyStackRef
+_PyStackRef_DupImmortal(_PyStackRef ref, const char *filename, int linenumber)
+{
+    assert(!PyStackRef_IsError(ref));
+    assert(!PyStackRef_IsTaggedInt(ref));
+    assert(!PyStackRef_RefcountOnObject(ref));
+    PyObject *obj = _Py_stackref_get_object(ref);
+    assert(_Py_IsImmortal(obj));
+    return _Py_stackref_create(obj, Py_TAG_REFCNT, filename, linenumber);
+}
+#define PyStackRef_DupImmortal(REF) _PyStackRef_DupImmortal((REF), __FILE__, __LINE__)
+
 static inline void
 _PyStackRef_CLOSE_SPECIALIZED(_PyStackRef ref, destructor destruct, const char *filename, int linenumber)
 {
@@ -633,6 +645,15 @@ PyStackRef_DUP(_PyStackRef ref)
 }
 #endif
 
+static inline _PyStackRef
+PyStackRef_DupImmortal(_PyStackRef ref)
+{
+    assert(!PyStackRef_IsNull(ref));
+    assert(!PyStackRef_RefcountOnObject(ref));
+    assert(_Py_IsImmortal(BITS_TO_PTR_MASKED(ref)));
+    return ref;
+}
+
 static inline bool
 PyStackRef_IsHeapSafe(_PyStackRef ref)
 {

diff --git a/Modules/_testinternalcapi/test_cases.c.h b/Modules/_testinternalcapi/test_cases.c.h
index b463bb18b160564245031128a7f8bc95c8cac885..11dfcc68eb2dacdfd743d34d4922417cfc62b665 100644 (file)
--- a/Modules/_testinternalcapi/test_cases.c.h
+++ b/Modules/_testinternalcapi/test_cases.c.h
@@ -9314,7 +9314,7 @@
             INSTRUCTION_STATS(LOAD_COMMON_CONSTANT);
             _PyStackRef value;
             assert(oparg < NUM_COMMON_CONSTANTS);
-            value = PyStackRef_FromPyObjectNew(tstate->interp->common_consts[oparg]);
+            value = PyStackRef_DupImmortal(tstate->interp->common_consts[oparg]);
             stack_pointer[0] = value;
             stack_pointer += 1;
             ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__);

diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index 300b7da753c2baf9e119b6af6dfd206febf77255..993d231751409ba9db22b10f4fb519b0bb7de27e 100644 (file)
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -1974,7 +1974,7 @@ dummy_func(
         inst(LOAD_COMMON_CONSTANT, ( -- value)) {
             // Keep in sync with _common_constants in opcode.py
             assert(oparg < NUM_COMMON_CONSTANTS);
-            value = PyStackRef_FromPyObjectNew(tstate->interp->common_consts[oparg]);
+            value = PyStackRef_DupImmortal(tstate->interp->common_consts[oparg]);
         }
 
         inst(LOAD_BUILD_CLASS, ( -- bc)) {

diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h
index 952860f01b8a6826eb0d3d2cce965a0e6d72e913..9aaf9639b9b90155330a036d5c2089462b9e6d4b 100644 (file)
--- a/Python/executor_cases.c.h
+++ b/Python/executor_cases.c.h
@@ -9396,7 +9396,7 @@
             _PyStackRef value;
             oparg = CURRENT_OPARG();
             assert(oparg < NUM_COMMON_CONSTANTS);
-            value = PyStackRef_FromPyObjectNew(tstate->interp->common_consts[oparg]);
+            value = PyStackRef_DupImmortal(tstate->interp->common_consts[oparg]);
             _tos_cache0 = value;
             SET_CURRENT_CACHED_VALUES(1);
             assert(WITHIN_STACK_BOUNDS_IGNORING_CACHE());
@@ -9410,7 +9410,7 @@
             _PyStackRef _stack_item_0 = _tos_cache0;
             oparg = CURRENT_OPARG();
             assert(oparg < NUM_COMMON_CONSTANTS);
-            value = PyStackRef_FromPyObjectNew(tstate->interp->common_consts[oparg]);
+            value = PyStackRef_DupImmortal(tstate->interp->common_consts[oparg]);
             _tos_cache1 = value;
             _tos_cache0 = _stack_item_0;
             SET_CURRENT_CACHED_VALUES(2);
@@ -9426,7 +9426,7 @@
             _PyStackRef _stack_item_1 = _tos_cache1;
             oparg = CURRENT_OPARG();
             assert(oparg < NUM_COMMON_CONSTANTS);
-            value = PyStackRef_FromPyObjectNew(tstate->interp->common_consts[oparg]);
+            value = PyStackRef_DupImmortal(tstate->interp->common_consts[oparg]);
             _tos_cache2 = value;
             _tos_cache1 = _stack_item_1;
             _tos_cache0 = _stack_item_0;

diff --git a/Python/flowgraph.c b/Python/flowgraph.c
index 6e3e5378cfbf155b8925cdc5b00cc77799003da5..eb0faf8cd183885cc83349f0c3e1f22ce1b73807 100644 (file)
--- a/Python/flowgraph.c
+++ b/Python/flowgraph.c
@@ -11,6 +11,7 @@
 #include "pycore_opcode_utils.h"
 #include "pycore_opcode_metadata.h" // OPCODE_HAS_ARG, etc
 #include "pycore_pystate.h"         // _PyInterpreterState_GET()
+#include "pycore_stackref.h"        // PyStackRef_AsPyObjectBorrow()
 
 #include <stdbool.h>
 
@@ -1330,7 +1331,8 @@ get_const_value(int opcode, int oparg, PyObject *co_consts)
     }
     if (opcode == LOAD_COMMON_CONSTANT) {
         assert(oparg < NUM_COMMON_CONSTANTS);
-        return Py_NewRef(_PyInterpreterState_GET()->common_consts[oparg]);
+        return PyStackRef_AsPyObjectBorrow(
+            _PyInterpreterState_GET()->common_consts[oparg]);
     }
 
     if (constant == NULL) {

diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index 83051cf41cc043b780fa6df389fd2947c4bb505b..94384d5db3c107f9fe74f5228595af6aae84d029 100644 (file)
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -9313,7 +9313,7 @@
             INSTRUCTION_STATS(LOAD_COMMON_CONSTANT);
             _PyStackRef value;
             assert(oparg < NUM_COMMON_CONSTANTS);
-            value = PyStackRef_FromPyObjectNew(tstate->interp->common_consts[oparg]);
+            value = PyStackRef_DupImmortal(tstate->interp->common_consts[oparg]);
             stack_pointer[0] = value;
             stack_pointer += 1;
             ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__);

diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c
index c968185d77c3317f07d93ad2dce1974626b55afa..edb4c644bccbf6f3e44bae4579f11a20c0cd6a2b 100644 (file)
--- a/Python/optimizer_bytecodes.c
+++ b/Python/optimizer_bytecodes.c
@@ -2,6 +2,7 @@
 #include "pycore_long.h"
 #include "pycore_opcode_utils.h"
 #include "pycore_optimizer.h"
+#include "pycore_stackref.h"
 #include "pycore_typeobject.h"
 #include "pycore_uops.h"
 #include "pycore_uop_ids.h"
@@ -870,15 +871,11 @@ dummy_func(void) {
 
     op(_LOAD_COMMON_CONSTANT, (-- value)) {
         assert(oparg < NUM_COMMON_CONSTANTS);
-        PyObject *val = _PyInterpreterState_GET()->common_consts[oparg];
-        if (_Py_IsImmortal(val)) {
-            ADD_OP(_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)val);
-            value = PyJitRef_Borrow(sym_new_const(ctx, val));
-        }
-        else {
-            ADD_OP(_LOAD_CONST_INLINE, 0, (uintptr_t)val);
-            value = sym_new_const(ctx, val);
-        }
+        PyObject *val = PyStackRef_AsPyObjectBorrow(
+            _PyInterpreterState_GET()->common_consts[oparg]);
+        assert(_Py_IsImmortal(val));
+        ADD_OP(_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)val);
+        value = PyJitRef_Borrow(sym_new_const(ctx, val));
     }
 
     op(_LOAD_SMALL_INT, (-- value)) {

diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h
index d52ebb9804197da993d0da57b05af2041f53f565..8895e02d47b169330ab060c232219fc17dc71686 100644 (file)
--- a/Python/optimizer_cases.c.h
+++ b/Python/optimizer_cases.c.h
@@ -1991,15 +1991,11 @@
         case _LOAD_COMMON_CONSTANT: {
             JitOptRef value;
             assert(oparg < NUM_COMMON_CONSTANTS);
-            PyObject *val = _PyInterpreterState_GET()->common_consts[oparg];
-            if (_Py_IsImmortal(val)) {
-                ADD_OP(_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)val);
-                value = PyJitRef_Borrow(sym_new_const(ctx, val));
-            }
-            else {
-                ADD_OP(_LOAD_CONST_INLINE, 0, (uintptr_t)val);
-                value = sym_new_const(ctx, val);
-            }
+            PyObject *val = PyStackRef_AsPyObjectBorrow(
+                _PyInterpreterState_GET()->common_consts[oparg]);
+            assert(_Py_IsImmortal(val));
+            ADD_OP(_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)val);
+            value = PyJitRef_Borrow(sym_new_const(ctx, val));
             CHECK_STACK_BOUNDS(1);
             stack_pointer[0] = value;
             stack_pointer += 1;

diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index a17c3baca3d006c248246303fa4f0d8fa7ae3bd7..18223332949807611daa8218c6f936ae01a8e0a5 100644 (file)
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -28,6 +28,7 @@
 #include "pycore_runtime.h"       // _Py_ID()
 #include "pycore_runtime_init.h"  // _PyRuntimeState_INIT
 #include "pycore_setobject.h"     // _PySet_NextEntry()
+#include "pycore_stackref.h"      // PyStackRef_FromPyObjectBorrow()
 #include "pycore_stats.h"         // _PyStats_InterpInit()
 #include "pycore_sysmodule.h"     // _PySys_ClearAttrString()
 #include "pycore_traceback.h"     // PyUnstable_TracebackThreads()
@@ -878,25 +879,28 @@ pycore_init_builtins(PyThreadState *tstate)
         goto error;
     }
 
-    interp->common_consts[CONSTANT_ASSERTIONERROR] = PyExc_AssertionError;
-    interp->common_consts[CONSTANT_NOTIMPLEMENTEDERROR] = PyExc_NotImplementedError;
-    interp->common_consts[CONSTANT_BUILTIN_TUPLE] = (PyObject *)&PyTuple_Type;
-    interp->common_consts[CONSTANT_BUILTIN_ALL] = all;
-    interp->common_consts[CONSTANT_BUILTIN_ANY] = any;
-    interp->common_consts[CONSTANT_BUILTIN_LIST] = (PyObject *)&PyList_Type;
-    interp->common_consts[CONSTANT_BUILTIN_SET] = (PyObject *)&PySet_Type;
-    interp->common_consts[CONSTANT_NONE] = Py_None;
-    interp->common_consts[CONSTANT_EMPTY_STR] =
+    PyObject *common_objs[NUM_COMMON_CONSTANTS] = {NULL};
+    common_objs[CONSTANT_ASSERTIONERROR] = PyExc_AssertionError;
+    common_objs[CONSTANT_NOTIMPLEMENTEDERROR] = PyExc_NotImplementedError;
+    common_objs[CONSTANT_BUILTIN_TUPLE] = (PyObject *)&PyTuple_Type;
+    common_objs[CONSTANT_BUILTIN_ALL] = all;
+    common_objs[CONSTANT_BUILTIN_ANY] = any;
+    common_objs[CONSTANT_BUILTIN_LIST] = (PyObject *)&PyList_Type;
+    common_objs[CONSTANT_BUILTIN_SET] = (PyObject *)&PySet_Type;
+    common_objs[CONSTANT_NONE] = Py_None;
+    common_objs[CONSTANT_EMPTY_STR] =
         Py_GetConstantBorrowed(Py_CONSTANT_EMPTY_STR);
-    interp->common_consts[CONSTANT_TRUE] = Py_True;
-    interp->common_consts[CONSTANT_FALSE] = Py_False;
-    interp->common_consts[CONSTANT_MINUS_ONE] =
+    common_objs[CONSTANT_TRUE] = Py_True;
+    common_objs[CONSTANT_FALSE] = Py_False;
+    common_objs[CONSTANT_MINUS_ONE] =
         (PyObject *)&_PyLong_SMALL_INTS[_PY_NSMALLNEGINTS - 1];
-    interp->common_consts[CONSTANT_BUILTIN_FROZENSET] = (PyObject *)&PyFrozenSet_Type;
-    interp->common_consts[CONSTANT_EMPTY_TUPLE] =
+    common_objs[CONSTANT_BUILTIN_FROZENSET] = (PyObject *)&PyFrozenSet_Type;
+    common_objs[CONSTANT_EMPTY_TUPLE] =
         Py_GetConstantBorrowed(Py_CONSTANT_EMPTY_TUPLE);
     for (int i = 0; i < NUM_COMMON_CONSTANTS; i++) {
-        assert(interp->common_consts[i] != NULL);
+        assert(common_objs[i] != NULL);
+        _Py_SetImmortal(common_objs[i]);
+        interp->common_consts[i] = PyStackRef_FromPyObjectBorrow(common_objs[i]);
     }
 
     PyObject *list_append = _PyType_Lookup(&PyList_Type, &_Py_ID(append));

diff --git a/Python/pystate.c b/Python/pystate.c
index 530bd567b770be3afc0da47692c47375e84fdeaa..fed1df0173bacf11526cf673d0d8f7a67f7e0d3f 100644 (file)
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -12,8 +12,9 @@
 #include "pycore_freelist.h"      // _PyObject_ClearFreeLists()
 #include "pycore_initconfig.h"    // _PyStatus_OK()
 #include "pycore_interpframe.h"   // _PyThreadState_HasStackSpace()
-#include "pycore_object.h"        // _PyType_InitCache()
+#include "pycore_object.h"        // _PyType_InitCache(), _Py_ClearImmortal()
 #include "pycore_obmalloc.h"      // _PyMem_obmalloc_state_on_heap()
+#include "pycore_opcode_utils.h"  // NUM_COMMON_CONSTANTS
 #include "pycore_optimizer.h"     // JIT_CLEANUP_THRESHOLD
 #include "pycore_parking_lot.h"   // _PyParkingLot_AfterFork()
 #include "pycore_pyerrors.h"      // _PyErr_Clear()
@@ -21,7 +22,7 @@
 #include "pycore_pymem.h"         // _PyMem_DebugEnabled()
 #include "pycore_runtime.h"       // _PyRuntime
 #include "pycore_runtime_init.h"  // _PyRuntimeState_INIT
-#include "pycore_stackref.h"      // Py_STACKREF_DEBUG
+#include "pycore_stackref.h"      // PyStackRef_AsPyObjectBorrow()
 #include "pycore_stats.h"         // FT_STAT_WORLD_STOP_INC()
 #include "pycore_time.h"          // _PyTime_Init()
 #include "pycore_uniqueid.h"      // _PyObject_FinalizePerThreadRefcounts()
@@ -778,6 +779,36 @@ extern void
 _Py_stackref_report_leaks(PyInterpreterState *interp);
 #endif
 
+static int
+common_const_is_initialized(_PyStackRef ref)
+{
+#if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG)
+    return !PyStackRef_IsNull(ref);
+#else
+    return ref.bits != 0 && !PyStackRef_IsNull(ref);
+#endif
+}
+
+
+static void
+common_constants_clear(PyInterpreterState *interp)
+{
+    for (int i = 0; i < NUM_COMMON_CONSTANTS; i++) {
+        _PyStackRef ref = interp->common_consts[i];
+        if (!common_const_is_initialized(ref)) {
+            continue;
+        }
+        PyObject *obj = PyStackRef_AsPyObjectBorrow(ref);
+        PyStackRef_XCLOSE(ref);
+        interp->common_consts[i] = PyStackRef_NULL;
+        // Refcount reclamation skips heap immortals; release manually.
+        if (_Py_IsImmortal(obj) && !_Py_IsStaticImmortal(obj)) {
+            _Py_ClearImmortal(obj);
+        }
+    }
+}
+
+
 static void
 interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)
 {
@@ -904,6 +935,7 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)
     PyDict_Clear(interp->builtins);
     Py_CLEAR(interp->sysdict);
     Py_CLEAR(interp->builtins);
+    common_constants_clear(interp);
 
 #if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG)
 #  ifdef Py_STACKREF_CLOSE_DEBUG

diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py
index 42459eedad6b1d7ef17f6103f2d8c43b70452d31..22a321b4953de7dd27c8d501cb21cc6a0f48f3d3 100644 (file)
--- a/Tools/cases_generator/analyzer.py
+++ b/Tools/cases_generator/analyzer.py
@@ -609,6 +609,7 @@ NON_ESCAPING_FUNCTIONS = (
     "PyStackRef_CLEAR",
     "PyStackRef_CLOSE_SPECIALIZED",
     "PyStackRef_DUP",
+    "PyStackRef_DupImmortal",
     "PyStackRef_False",
     "PyStackRef_FromPyObjectBorrow",
     "PyStackRef_FromPyObjectNew",