From: Ken Jin Date: Sun, 12 Apr 2026 18:23:47 +0000 (+0800) Subject: gh-146261: JIT: protect against function version changes (#146300) X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6f7bb297db661c23eeec31fd3b9836a8c5adea14;p=thirdparty%2FPython%2Fcpython.git gh-146261: JIT: protect against function version changes (#146300) Co-authored-by: Kumar Aditya --- diff --git a/Include/internal/pycore_optimizer.h b/Include/internal/pycore_optimizer.h index cf01c620476f..e7b688333d9c 100644 --- a/Include/internal/pycore_optimizer.h +++ b/Include/internal/pycore_optimizer.h @@ -424,8 +424,6 @@ extern PyCodeObject *_Py_uop_sym_get_probable_func_code(JitOptRef sym); extern PyObject *_Py_uop_sym_get_probable_value(JitOptRef sym); extern PyTypeObject *_Py_uop_sym_get_probable_type(JitOptRef sym); extern JitOptRef *_Py_uop_sym_set_stack_depth(JitOptContext *ctx, int stack_depth, JitOptRef *current_sp); -extern uint32_t _Py_uop_sym_get_func_version(JitOptRef ref); -bool _Py_uop_sym_set_func_version(JitOptContext *ctx, JitOptRef ref, uint32_t version); extern void _Py_uop_abstractcontext_init(JitOptContext *ctx, _PyBloomFilter *dependencies); extern void _Py_uop_abstractcontext_fini(JitOptContext *ctx); diff --git a/Include/internal/pycore_optimizer_types.h b/Include/internal/pycore_optimizer_types.h index 8ecfbea38746..a722652cc816 100644 --- a/Include/internal/pycore_optimizer_types.h +++ b/Include/internal/pycore_optimizer_types.h @@ -36,7 +36,6 @@ typedef enum _JitSymType { JIT_SYM_NON_NULL_TAG = 3, JIT_SYM_BOTTOM_TAG = 4, JIT_SYM_TYPE_VERSION_TAG = 5, - JIT_SYM_FUNC_VERSION_TAG = 6, JIT_SYM_KNOWN_CLASS_TAG = 7, JIT_SYM_KNOWN_VALUE_TAG = 8, JIT_SYM_TUPLE_TAG = 9, diff --git a/Lib/test/test_capi/test_opt.py b/Lib/test/test_capi/test_opt.py index 2678a620763b..bc2114c9a854 100644 --- a/Lib/test/test_capi/test_opt.py +++ b/Lib/test/test_capi/test_opt.py @@ -1622,9 +1622,8 @@ class TestUopsOptimization(unittest.TestCase): self.assertEqual(uops.count("_PUSH_FRAME"), 2) # Type version propagation: one guard covers both method lookups self.assertEqual(uops.count("_GUARD_TYPE_VERSION"), 1) - # Function checks eliminated (type info resolves the callable) - self.assertNotIn("_CHECK_FUNCTION_VERSION", uops) - self.assertNotIn("_CHECK_FUNCTION_EXACT_ARGS", uops) + # Function checks cannot be eliminated for safety reasons. + self.assertIn("_CHECK_FUNCTION_VERSION", uops) def test_method_chain_guard_elimination(self): """ @@ -1669,10 +1668,7 @@ class TestUopsOptimization(unittest.TestCase): self.assertIsNotNone(ex) uops = get_opnames(ex) self.assertIn("_PUSH_FRAME", uops) - # Both should be not present, as this is a call - # to a simple function with a known function version. - self.assertNotIn("_CHECK_FUNCTION_VERSION_INLINE", uops) - self.assertNotIn("_CHECK_FUNCTION_VERSION", uops) + self.assertIn("_CHECK_FUNCTION_VERSION", uops) # Removed guard self.assertNotIn("_CHECK_FUNCTION_EXACT_ARGS", uops) @@ -5178,6 +5174,27 @@ class TestUopsOptimization(unittest.TestCase): PYTHON_JIT="1", PYTHON_JIT_STRESS="1") self.assertEqual(result[0].rc, 0, result) + def test_func_version_guarded_on_change(self): + def testfunc(n): + for i in range(n): + # Only works on functions promoted to constants + global_identity_code_will_be_modified(i) + + testfunc(TIER2_THRESHOLD) + + ex = get_first_executor(testfunc) + self.assertIsNotNone(ex) + uops = get_opnames(ex) + self.assertIn("_PUSH_FRAME", uops) + self.assertIn("_CHECK_FUNCTION_VERSION", uops) + + global_identity_code_will_be_modified.__code__ = (lambda a: 0xdeadead).__code__ + _testinternalcapi.clear_executor_deletion_list() + ex = get_first_executor(testfunc) + self.assertIsNone(ex) + # JItted code should've deopted. + self.assertEqual(global_identity_code_will_be_modified(None), 0xdeadead) + def test_call_super(self): class A: def method1(self): @@ -5224,6 +5241,9 @@ class TestUopsOptimization(unittest.TestCase): def global_identity(x): return x +def global_identity_code_will_be_modified(x): + return x + class TestObject: def test(self, *args, **kwargs): return args[0] diff --git a/Objects/funcobject.c b/Objects/funcobject.c index d47c78b933b7..0fffd36ad462 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -8,6 +8,7 @@ #include "pycore_modsupport.h" // _PyArg_NoKeywords() #include "pycore_object.h" // _PyObject_GC_UNTRACK() #include "pycore_object_deferred.h" // _PyObject_SetDeferredRefcount() +#include "pycore_optimizer.h" // _Py_Executors_InvalidateDependency() #include "pycore_pyerrors.h" // _PyErr_Occurred() #include "pycore_setobject.h" // _PySet_NextEntry() #include "pycore_stats.h" @@ -63,6 +64,13 @@ handle_func_event(PyFunction_WatchEvent event, PyFunctionObject *func, case PyFunction_EVENT_MODIFY_DEFAULTS: case PyFunction_EVENT_MODIFY_KWDEFAULTS: case PyFunction_EVENT_MODIFY_QUALNAME: +#if _Py_TIER2 + // Note: we only invalidate JIT code if a function version changes. + // Not when the function is deallocated. + // Function deallocation occurs frequently (think: lambdas), + // so we want to minimize dependency invalidation there. + _Py_Executors_InvalidateDependency(interp, func, 1); +#endif RARE_EVENT_INTERP_INC(interp, func_modification); break; default: diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 6742488a0d06..6b48f89510ef 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -283,8 +283,6 @@ add_op(JitOptContext *ctx, _PyUOpInstruction *this_instr, #define sym_get_probable_func_code _Py_uop_sym_get_probable_func_code #define sym_get_probable_value _Py_uop_sym_get_probable_value #define sym_set_stack_depth(DEPTH, SP) _Py_uop_sym_set_stack_depth(ctx, DEPTH, SP) -#define sym_get_func_version _Py_uop_sym_get_func_version -#define sym_set_func_version _Py_uop_sym_set_func_version /* Comparison oparg masks */ #define COMPARE_LT_MASK 2 diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index 6e5af4793419..fd1dfa017de4 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -1001,12 +1001,15 @@ dummy_func(void) { } op(_CHECK_FUNCTION_VERSION, (func_version/2, callable, self_or_null, unused[oparg] -- callable, self_or_null, unused[oparg])) { - if (sym_get_func_version(callable) == func_version) { - REPLACE_OP(this_instr, _NOP, 0, 0); - } - else { - sym_set_func_version(ctx, callable, func_version); + PyObject *func = sym_get_probable_value(callable); + if (func == NULL || !PyFunction_Check(func) || ((PyFunctionObject *)func)->func_version != func_version) { + ctx->contradiction = true; + ctx->done = true; + break; } + // Guarded on this, so it can be promoted. + sym_set_const(callable, func); + _Py_BloomFilter_Add(dependencies, func); } op(_CHECK_METHOD_VERSION, (func_version/2, callable, null, unused[oparg] -- callable, null, unused[oparg])) { @@ -2229,7 +2232,8 @@ dummy_func(void) { if (co->co_version == version) { _Py_BloomFilter_Add(dependencies, co); // Functions derive their version from code objects. - if (sym_get_func_version(ctx->frame->callable) == version) { + PyFunctionObject *func = (PyFunctionObject *)sym_get_const(ctx, ctx->frame->callable); + if (func != NULL && func->func_version == version) { REPLACE_OP(this_instr, _NOP, 0, 0); } } @@ -2262,7 +2266,8 @@ dummy_func(void) { op(_GUARD_IP__PUSH_FRAME, (ip/4 --)) { (void)ip; stack_pointer = sym_set_stack_depth((int)this_instr->operand1, stack_pointer); - if (sym_get_func_version(ctx->frame->callable) != 0 && + PyFunctionObject *func = (PyFunctionObject *)sym_get_const(ctx, ctx->frame->callable); + if (func != NULL && func->func_version != 0 && // We can remove this guard for simple function call targets. (((PyCodeObject *)ctx->frame->func->func_code)->co_flags & (CO_GENERATOR | CO_COROUTINE | CO_ASYNC_GENERATOR)) == 0) { diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index d90ad285a11f..dc2d5891f9c3 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -3619,12 +3619,14 @@ JitOptRef callable; callable = stack_pointer[-2 - oparg]; uint32_t func_version = (uint32_t)this_instr->operand0; - if (sym_get_func_version(callable) == func_version) { - REPLACE_OP(this_instr, _NOP, 0, 0); - } - else { - sym_set_func_version(ctx, callable, func_version); + PyObject *func = sym_get_probable_value(callable); + if (func == NULL || !PyFunction_Check(func) || ((PyFunctionObject *)func)->func_version != func_version) { + ctx->contradiction = true; + ctx->done = true; + break; } + sym_set_const(callable, func); + _Py_BloomFilter_Add(dependencies, func); break; } @@ -5057,7 +5059,8 @@ PyCodeObject *co = get_current_code_object(ctx); if (co->co_version == version) { _Py_BloomFilter_Add(dependencies, co); - if (sym_get_func_version(ctx->frame->callable) == version) { + PyFunctionObject *func = (PyFunctionObject *)sym_get_const(ctx, ctx->frame->callable); + if (func != NULL && func->func_version == version) { REPLACE_OP(this_instr, _NOP, 0, 0); } } @@ -5098,7 +5101,8 @@ PyObject *ip = (PyObject *)this_instr->operand0; (void)ip; stack_pointer = sym_set_stack_depth((int)this_instr->operand1, stack_pointer); - if (sym_get_func_version(ctx->frame->callable) != 0 && + PyFunctionObject *func = (PyFunctionObject *)sym_get_const(ctx, ctx->frame->callable); + if (func != NULL && func->func_version != 0 && // We can remove this guard for simple function call targets. (((PyCodeObject *)ctx->frame->func->func_code)->co_flags & (CO_GENERATOR | CO_COROUTINE | CO_ASYNC_GENERATOR)) == 0) { diff --git a/Python/optimizer_symbols.c b/Python/optimizer_symbols.c index d0f33b80a570..79f81482d247 100644 --- a/Python/optimizer_symbols.c +++ b/Python/optimizer_symbols.c @@ -35,12 +35,12 @@ NULL | | RECORDED_VALUE* | TYPE_VERSION | | | | | | | | <- Anything below this level has a known type. | KNOWN_CLASS--+ | | | -| | | | | | PREDICATE RECORDED_VALUE(known type) -| | | INT* | | | | -| | | | | | | | <- Anything below this level has a known truthiness. -| | | | FUNC_VERSION | | | -| TUPLE | | | TRUTHINESS | | -| | | | | | | | <- Anything below this level is a known constant. +| | | | | PREDICATE RECORDED_VALUE(known type) +| | | INT* | | | +| | | | | | | <- Anything below this level has a known truthiness. +| | | | | | | +| TUPLE | | TRUTHINESS | | +| | | | | | | <- Anything below this level is a known constant. | KNOWN_VALUE--+-------+----+------+ | | <- Anything below this level is unreachable. BOTTOM @@ -101,9 +101,6 @@ _PyUOpSymPrint(JitOptRef ref) case JIT_SYM_TYPE_VERSION_TAG: printf("", sym->version.version, (void *)sym); break; - case JIT_SYM_FUNC_VERSION_TAG: - printf("", sym->func_version.func_version); - break; case JIT_SYM_KNOWN_CLASS_TAG: printf("<%s at %p>", sym->cls.type->tp_name, (void *)sym); break; @@ -325,11 +322,6 @@ _Py_uop_sym_set_type(JitOptContext *ctx, JitOptRef ref, PyTypeObject *typ) sym_set_bottom(ctx, sym); } return; - case JIT_SYM_FUNC_VERSION_TAG: - if (typ != &PyFunction_Type) { - sym_set_bottom(ctx, sym); - } - return; case JIT_SYM_KNOWN_VALUE_TAG: if (Py_TYPE(sym->value.value) != typ) { Py_CLEAR(sym->value.value); @@ -433,12 +425,6 @@ _Py_uop_sym_set_type_version(JitOptContext *ctx, JitOptRef ref, unsigned int ver return false; } return true; - case JIT_SYM_FUNC_VERSION_TAG: - if (version != PyFunction_Type.tp_version_tag) { - sym_set_bottom(ctx, sym); - return false; - } - return true; case JIT_SYM_BOTTOM_TAG: return false; case JIT_SYM_NON_NULL_TAG: @@ -486,87 +472,6 @@ _Py_uop_sym_set_type_version(JitOptContext *ctx, JitOptRef ref, unsigned int ver Py_UNREACHABLE(); } -bool -_Py_uop_sym_set_func_version(JitOptContext *ctx, JitOptRef ref, uint32_t version) -{ - JitOptSymbol *sym = PyJitRef_Unwrap(ref); - JitSymType tag = sym->tag; - switch(tag) { - case JIT_SYM_NULL_TAG: - sym_set_bottom(ctx, sym); - return false; - case JIT_SYM_KNOWN_CLASS_TAG: - if (sym->cls.type != &PyFunction_Type) { - sym_set_bottom(ctx, sym); - return false; - } - sym->tag = JIT_SYM_FUNC_VERSION_TAG; - sym->version.version = version; - return true; - case JIT_SYM_KNOWN_VALUE_TAG: - if (Py_TYPE(sym->value.value) != &PyFunction_Type || - ((PyFunctionObject *)sym->value.value)->func_version != version) { - Py_CLEAR(sym->value.value); - sym_set_bottom(ctx, sym); - return false; - } - return true; - case JIT_SYM_TYPE_VERSION_TAG: - if (sym->version.version != PyFunction_Type.tp_version_tag) { - sym_set_bottom(ctx, sym); - return false; - } - sym->tag = JIT_SYM_FUNC_VERSION_TAG; - sym->version.version = version; - return true; - case JIT_SYM_FUNC_VERSION_TAG: - if (sym->func_version.func_version != version) { - sym_set_bottom(ctx, sym); - return false; - } - return true; - case JIT_SYM_BOTTOM_TAG: - return false; - case JIT_SYM_NON_NULL_TAG: - case JIT_SYM_UNKNOWN_TAG: - sym->tag = JIT_SYM_FUNC_VERSION_TAG; - sym->func_version.func_version = version; - return true; - case JIT_SYM_RECORDED_GEN_FUNC_TAG: - case JIT_SYM_COMPACT_INT: - case JIT_SYM_TUPLE_TAG: - case JIT_SYM_PREDICATE_TAG: - case JIT_SYM_TRUTHINESS_TAG: - sym_set_bottom(ctx, sym); - return false; - case JIT_SYM_RECORDED_VALUE_TAG: { - PyObject *val = sym->recorded_value.value; - if (Py_TYPE(val) != &PyFunction_Type || - ((PyFunctionObject *)sym->recorded_value.value)->func_version != version) { - sym_set_bottom(ctx, sym); - return false; - } - // Promote to known value, as we have guarded/checked on it. - sym->tag = JIT_SYM_KNOWN_VALUE_TAG; - // New ownership. We need to NewRef here, as - // it's originally kept alive by the trace buffer. - sym->value.value = Py_NewRef(val); - return true; - } - case JIT_SYM_RECORDED_TYPE_TAG: - if (sym->recorded_type.type == &PyFunction_Type) { - sym->tag = JIT_SYM_FUNC_VERSION_TAG; - sym->func_version.func_version = version; - return true; - } - else { - sym_set_bottom(ctx, sym); - return false; - } - } - Py_UNREACHABLE(); -} - void _Py_uop_sym_set_const(JitOptContext *ctx, JitOptRef ref, PyObject *const_val) { @@ -612,14 +517,6 @@ _Py_uop_sym_set_const(JitOptContext *ctx, JitOptRef ref, PyObject *const_val) } make_const(sym, const_val); return; - case JIT_SYM_FUNC_VERSION_TAG: - if (Py_TYPE(const_val) != &PyFunction_Type || - ((PyFunctionObject *)const_val)->func_version != sym->func_version.func_version) { - sym_set_bottom(ctx, sym); - return; - } - make_const(sym, const_val); - return; case JIT_SYM_BOTTOM_TAG: return; case JIT_SYM_RECORDED_VALUE_TAG: @@ -796,8 +693,6 @@ _Py_uop_sym_get_type(JitOptRef ref) return Py_TYPE(sym->value.value); case JIT_SYM_TYPE_VERSION_TAG: return _PyType_LookupByVersion(sym->version.version); - case JIT_SYM_FUNC_VERSION_TAG: - return &PyFunction_Type; case JIT_SYM_TUPLE_TAG: return &PyTuple_Type; case JIT_SYM_PREDICATE_TAG: @@ -820,7 +715,6 @@ _Py_uop_sym_get_probable_type(JitOptRef ref) case JIT_SYM_NON_NULL_TAG: case JIT_SYM_UNKNOWN_TAG: case JIT_SYM_TYPE_VERSION_TAG: - case JIT_SYM_FUNC_VERSION_TAG: case JIT_SYM_TUPLE_TAG: case JIT_SYM_PREDICATE_TAG: case JIT_SYM_TRUTHINESS_TAG: @@ -853,8 +747,6 @@ _Py_uop_sym_get_type_version(JitOptRef ref) return 0; case JIT_SYM_TYPE_VERSION_TAG: return sym->version.version; - case JIT_SYM_FUNC_VERSION_TAG: - return PyFunction_Type.tp_version_tag; case JIT_SYM_KNOWN_CLASS_TAG: return sym->cls.version; case JIT_SYM_KNOWN_VALUE_TAG: @@ -872,37 +764,6 @@ _Py_uop_sym_get_type_version(JitOptRef ref) Py_UNREACHABLE(); } -uint32_t -_Py_uop_sym_get_func_version(JitOptRef ref) -{ - JitOptSymbol *sym = PyJitRef_Unwrap(ref); - JitSymType tag = sym->tag; - switch(tag) { - case JIT_SYM_NULL_TAG: - case JIT_SYM_BOTTOM_TAG: - case JIT_SYM_NON_NULL_TAG: - case JIT_SYM_UNKNOWN_TAG: - case JIT_SYM_RECORDED_VALUE_TAG: - case JIT_SYM_RECORDED_TYPE_TAG: - case JIT_SYM_TYPE_VERSION_TAG: - case JIT_SYM_KNOWN_CLASS_TAG: - case JIT_SYM_TUPLE_TAG: - case JIT_SYM_PREDICATE_TAG: - case JIT_SYM_TRUTHINESS_TAG: - case JIT_SYM_COMPACT_INT: - case JIT_SYM_RECORDED_GEN_FUNC_TAG: - return 0; - case JIT_SYM_FUNC_VERSION_TAG: - return sym->func_version.func_version; - case JIT_SYM_KNOWN_VALUE_TAG: - if (Py_TYPE(sym->value.value) == &PyFunction_Type) { - return ((PyFunctionObject *)sym->value.value)->func_version; - } - return 0; - } - Py_UNREACHABLE(); -} - bool _Py_uop_sym_has_type(JitOptRef sym) @@ -935,7 +796,6 @@ _Py_uop_sym_get_probable_value(JitOptRef ref) case JIT_SYM_UNKNOWN_TAG: case JIT_SYM_RECORDED_TYPE_TAG: case JIT_SYM_TYPE_VERSION_TAG: - case JIT_SYM_FUNC_VERSION_TAG: case JIT_SYM_TUPLE_TAG: case JIT_SYM_PREDICATE_TAG: case JIT_SYM_TRUTHINESS_TAG: @@ -1005,8 +865,6 @@ _Py_uop_sym_truthiness(JitOptContext *ctx, JitOptRef ref) return -1; case JIT_SYM_KNOWN_VALUE_TAG: break; - case JIT_SYM_FUNC_VERSION_TAG: - return 1; case JIT_SYM_TUPLE_TAG: return sym->tuple.length != 0; case JIT_SYM_TRUTHINESS_TAG: @@ -1155,7 +1013,6 @@ _Py_uop_sym_set_compact_int(JitOptContext *ctx, JitOptRef ref) sym_set_bottom(ctx, sym); } return; - case JIT_SYM_FUNC_VERSION_TAG: case JIT_SYM_TUPLE_TAG: case JIT_SYM_PREDICATE_TAG: case JIT_SYM_TRUTHINESS_TAG: @@ -1343,7 +1200,6 @@ _Py_uop_sym_set_recorded_value(JitOptContext *ctx, JitOptRef ref, PyObject *valu case JIT_SYM_PREDICATE_TAG: case JIT_SYM_TRUTHINESS_TAG: case JIT_SYM_COMPACT_INT: - case JIT_SYM_FUNC_VERSION_TAG: return; } Py_UNREACHABLE(); @@ -1367,9 +1223,6 @@ _Py_uop_sym_set_recorded_gen_func(JitOptContext *ctx, JitOptRef ref, PyFunctionO case JIT_SYM_PREDICATE_TAG: case JIT_SYM_TRUTHINESS_TAG: case JIT_SYM_COMPACT_INT: - case JIT_SYM_FUNC_VERSION_TAG: - sym_set_bottom(ctx, sym); - return; case JIT_SYM_BOTTOM_TAG: return; case JIT_SYM_NON_NULL_TAG: @@ -1464,7 +1317,6 @@ _Py_uop_sym_set_recorded_type(JitOptContext *ctx, JitOptRef ref, PyTypeObject *t case JIT_SYM_TRUTHINESS_TAG: case JIT_SYM_COMPACT_INT: case JIT_SYM_RECORDED_GEN_FUNC_TAG: - case JIT_SYM_FUNC_VERSION_TAG: return; } Py_UNREACHABLE(); @@ -2128,15 +1980,6 @@ _Py_uop_symbols_test(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(ignored)) TEST_PREDICATE(_Py_uop_sym_matches_type(ref_int, &PyLong_Type), "43 is not an int"); TEST_PREDICATE(_Py_uop_sym_get_const(ctx, ref_int) == val_43, "43 isn't 43"); - // Test func version's important transitions. - JitOptRef func_version = _Py_uop_sym_new_not_null(ctx); - TEST_PREDICATE(_Py_uop_sym_get_func_version(func_version) == 0, "func version should be unset"); - _Py_uop_sym_set_func_version(ctx, func_version, 172); - TEST_PREDICATE(_Py_uop_sym_get_func_version(func_version) == 172, "func version should be set"); - func_version = _Py_uop_sym_new_type(ctx, &PyFunction_Type); - _Py_uop_sym_set_func_version(ctx, func_version, 192); - TEST_PREDICATE(_Py_uop_sym_get_func_version(func_version) == 192, "func version should be set"); - // Test recorded values /* Test that recorded values aren't treated as known values*/