From 17f994174de9211b2baaff217eeb1033343230fc Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Fri, 8 Sep 2023 19:49:20 -0700 Subject: gh-109118: Fix runtime crash when NameError happens in PEP 695 function (#109123) --- Include/internal/pycore_opcode_metadata.h | 90 +++++++++------------- Lib/test/test_type_params.py | 40 ++++++++++ .../2023-09-07-18-24-42.gh-issue-109118.yPXRAe.rst | 2 + Python/abstract_interp_cases.c.h | 10 ++- Python/bytecodes.c | 44 ++++++++--- Python/executor_cases.c.h | 42 +++++++++- Python/generated_cases.c.h | 67 +++++++--------- 7 files changed, 189 insertions(+), 106 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-09-07-18-24-42.gh-issue-109118.yPXRAe.rst diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index fb5c046..f70b75a 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -39,40 +39,38 @@ #define _BINARY_OP_ADD_UNICODE 311 #define _BINARY_OP_INPLACE_ADD_UNICODE 312 #define _POP_FRAME 313 -#define _LOAD_LOCALS 314 -#define _LOAD_FROM_DICT_OR_GLOBALS 315 -#define _GUARD_GLOBALS_VERSION 316 -#define _GUARD_BUILTINS_VERSION 317 -#define _LOAD_GLOBAL_MODULE 318 -#define _LOAD_GLOBAL_BUILTINS 319 -#define _GUARD_TYPE_VERSION 320 -#define _CHECK_MANAGED_OBJECT_HAS_VALUES 321 -#define _LOAD_ATTR_INSTANCE_VALUE 322 -#define IS_NONE 323 -#define _ITER_CHECK_LIST 324 -#define _ITER_JUMP_LIST 325 -#define _IS_ITER_EXHAUSTED_LIST 326 -#define _ITER_NEXT_LIST 327 -#define _ITER_CHECK_TUPLE 328 -#define _ITER_JUMP_TUPLE 329 -#define _IS_ITER_EXHAUSTED_TUPLE 330 -#define _ITER_NEXT_TUPLE 331 -#define _ITER_CHECK_RANGE 332 -#define _ITER_JUMP_RANGE 333 -#define _IS_ITER_EXHAUSTED_RANGE 334 -#define _ITER_NEXT_RANGE 335 -#define _CHECK_CALL_BOUND_METHOD_EXACT_ARGS 336 -#define _INIT_CALL_BOUND_METHOD_EXACT_ARGS 337 -#define _CHECK_PEP_523 338 -#define _CHECK_FUNCTION_EXACT_ARGS 339 -#define _CHECK_STACK_SPACE 340 -#define _INIT_CALL_PY_EXACT_ARGS 341 -#define _PUSH_FRAME 342 -#define _POP_JUMP_IF_FALSE 343 -#define _POP_JUMP_IF_TRUE 344 -#define JUMP_TO_TOP 345 -#define SAVE_CURRENT_IP 346 -#define INSERT 347 +#define _GUARD_GLOBALS_VERSION 314 +#define _GUARD_BUILTINS_VERSION 315 +#define _LOAD_GLOBAL_MODULE 316 +#define _LOAD_GLOBAL_BUILTINS 317 +#define _GUARD_TYPE_VERSION 318 +#define _CHECK_MANAGED_OBJECT_HAS_VALUES 319 +#define _LOAD_ATTR_INSTANCE_VALUE 320 +#define IS_NONE 321 +#define _ITER_CHECK_LIST 322 +#define _ITER_JUMP_LIST 323 +#define _IS_ITER_EXHAUSTED_LIST 324 +#define _ITER_NEXT_LIST 325 +#define _ITER_CHECK_TUPLE 326 +#define _ITER_JUMP_TUPLE 327 +#define _IS_ITER_EXHAUSTED_TUPLE 328 +#define _ITER_NEXT_TUPLE 329 +#define _ITER_CHECK_RANGE 330 +#define _ITER_JUMP_RANGE 331 +#define _IS_ITER_EXHAUSTED_RANGE 332 +#define _ITER_NEXT_RANGE 333 +#define _CHECK_CALL_BOUND_METHOD_EXACT_ARGS 334 +#define _INIT_CALL_BOUND_METHOD_EXACT_ARGS 335 +#define _CHECK_PEP_523 336 +#define _CHECK_FUNCTION_EXACT_ARGS 337 +#define _CHECK_STACK_SPACE 338 +#define _INIT_CALL_PY_EXACT_ARGS 339 +#define _PUSH_FRAME 340 +#define _POP_JUMP_IF_FALSE 341 +#define _POP_JUMP_IF_TRUE 342 +#define JUMP_TO_TOP 343 +#define SAVE_CURRENT_IP 344 +#define INSERT 345 extern int _PyOpcode_num_popped(int opcode, int oparg, bool jump); #ifdef NEED_OPCODE_METADATA @@ -268,16 +266,12 @@ int _PyOpcode_num_popped(int opcode, int oparg, bool jump) { return 1; case DELETE_GLOBAL: return 0; - case _LOAD_LOCALS: - return 0; case LOAD_LOCALS: return 0; - case _LOAD_FROM_DICT_OR_GLOBALS: + case LOAD_FROM_DICT_OR_GLOBALS: return 1; case LOAD_NAME: return 0; - case LOAD_FROM_DICT_OR_GLOBALS: - return 1; case LOAD_GLOBAL: return 0; case _GUARD_GLOBALS_VERSION: @@ -802,16 +796,12 @@ int _PyOpcode_num_pushed(int opcode, int oparg, bool jump) { return 0; case DELETE_GLOBAL: return 0; - case _LOAD_LOCALS: - return 1; case LOAD_LOCALS: return 1; - case _LOAD_FROM_DICT_OR_GLOBALS: + case LOAD_FROM_DICT_OR_GLOBALS: return 1; case LOAD_NAME: return 1; - case LOAD_FROM_DICT_OR_GLOBALS: - return 1; case LOAD_GLOBAL: return ((oparg & 1) ? 1 : 0) + 1; case _GUARD_GLOBALS_VERSION: @@ -1305,11 +1295,9 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[OPCODE_METADATA_SIZE] = { [DELETE_ATTR] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_ERROR_FLAG }, [STORE_GLOBAL] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_ERROR_FLAG }, [DELETE_GLOBAL] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_ERROR_FLAG }, - [_LOAD_LOCALS] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG }, [LOAD_LOCALS] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG }, - [_LOAD_FROM_DICT_OR_GLOBALS] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_ERROR_FLAG }, - [LOAD_NAME] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_ERROR_FLAG }, [LOAD_FROM_DICT_OR_GLOBALS] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_ERROR_FLAG }, + [LOAD_NAME] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_ERROR_FLAG }, [LOAD_GLOBAL] = { true, INSTR_FMT_IBC000, HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_ERROR_FLAG }, [_GUARD_GLOBALS_VERSION] = { true, INSTR_FMT_IXC, HAS_DEOPT_FLAG }, [_GUARD_BUILTINS_VERSION] = { true, INSTR_FMT_IXC, HAS_DEOPT_FLAG }, @@ -1546,9 +1534,9 @@ const struct opcode_macro_expansion _PyOpcode_macro_expansion[OPCODE_MACRO_EXPAN [DELETE_ATTR] = { .nuops = 1, .uops = { { DELETE_ATTR, 0, 0 } } }, [STORE_GLOBAL] = { .nuops = 1, .uops = { { STORE_GLOBAL, 0, 0 } } }, [DELETE_GLOBAL] = { .nuops = 1, .uops = { { DELETE_GLOBAL, 0, 0 } } }, - [LOAD_LOCALS] = { .nuops = 1, .uops = { { _LOAD_LOCALS, 0, 0 } } }, - [LOAD_NAME] = { .nuops = 2, .uops = { { _LOAD_LOCALS, 0, 0 }, { _LOAD_FROM_DICT_OR_GLOBALS, 0, 0 } } }, - [LOAD_FROM_DICT_OR_GLOBALS] = { .nuops = 1, .uops = { { _LOAD_FROM_DICT_OR_GLOBALS, 0, 0 } } }, + [LOAD_LOCALS] = { .nuops = 1, .uops = { { LOAD_LOCALS, 0, 0 } } }, + [LOAD_FROM_DICT_OR_GLOBALS] = { .nuops = 1, .uops = { { LOAD_FROM_DICT_OR_GLOBALS, 0, 0 } } }, + [LOAD_NAME] = { .nuops = 1, .uops = { { LOAD_NAME, 0, 0 } } }, [LOAD_GLOBAL] = { .nuops = 1, .uops = { { LOAD_GLOBAL, 0, 0 } } }, [LOAD_GLOBAL_MODULE] = { .nuops = 2, .uops = { { _GUARD_GLOBALS_VERSION, 1, 1 }, { _LOAD_GLOBAL_MODULE, 1, 3 } } }, [LOAD_GLOBAL_BUILTIN] = { .nuops = 3, .uops = { { _GUARD_GLOBALS_VERSION, 1, 1 }, { _GUARD_BUILTINS_VERSION, 1, 2 }, { _LOAD_GLOBAL_BUILTINS, 1, 3 } } }, @@ -1633,8 +1621,6 @@ const char * const _PyOpcode_uop_name[OPCODE_UOP_NAME_SIZE] = { [_BINARY_OP_ADD_UNICODE] = "_BINARY_OP_ADD_UNICODE", [_BINARY_OP_INPLACE_ADD_UNICODE] = "_BINARY_OP_INPLACE_ADD_UNICODE", [_POP_FRAME] = "_POP_FRAME", - [_LOAD_LOCALS] = "_LOAD_LOCALS", - [_LOAD_FROM_DICT_OR_GLOBALS] = "_LOAD_FROM_DICT_OR_GLOBALS", [_GUARD_GLOBALS_VERSION] = "_GUARD_GLOBALS_VERSION", [_GUARD_BUILTINS_VERSION] = "_GUARD_BUILTINS_VERSION", [_LOAD_GLOBAL_MODULE] = "_LOAD_GLOBAL_MODULE", diff --git a/Lib/test/test_type_params.py b/Lib/test/test_type_params.py index 0045057..f93d088 100644 --- a/Lib/test/test_type_params.py +++ b/Lib/test/test_type_params.py @@ -956,3 +956,43 @@ class TypeParamsWeakRefTest(unittest.TestCase): for case in cases: with self.subTest(case=case): weakref.ref(case) + + +class TypeParamsRuntimeTest(unittest.TestCase): + def test_name_error(self): + # gh-109118: This crashed the interpreter due to a refcounting bug + code = """ + class name_2[name_5]: + class name_4[name_5](name_0): + pass + """ + with self.assertRaises(NameError): + run_code(code) + + # Crashed with a slightly different stack trace + code = """ + class name_2[name_5]: + class name_4[name_5: name_5](name_0): + pass + """ + with self.assertRaises(NameError): + run_code(code) + + def test_broken_class_namespace(self): + code = """ + class WeirdMapping(dict): + def __missing__(self, key): + if key == "T": + raise RuntimeError + raise KeyError(key) + + class Meta(type): + def __prepare__(name, bases): + return WeirdMapping() + + class MyClass[V](metaclass=Meta): + class Inner[U](T): + pass + """ + with self.assertRaises(RuntimeError): + run_code(code) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-09-07-18-24-42.gh-issue-109118.yPXRAe.rst b/Misc/NEWS.d/next/Core and Builtins/2023-09-07-18-24-42.gh-issue-109118.yPXRAe.rst new file mode 100644 index 0000000..f14fce4 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-09-07-18-24-42.gh-issue-109118.yPXRAe.rst @@ -0,0 +1,2 @@ +Fix interpreter crash when a NameError is raised inside the type parameters +of a generic class. diff --git a/Python/abstract_interp_cases.c.h b/Python/abstract_interp_cases.c.h index 11fbf44..398c046 100644 --- a/Python/abstract_interp_cases.c.h +++ b/Python/abstract_interp_cases.c.h @@ -291,13 +291,19 @@ break; } - case _LOAD_LOCALS: { + case LOAD_LOCALS: { STACK_GROW(1); PARTITIONNODE_OVERWRITE((_Py_PARTITIONNODE_t *)PARTITIONNODE_NULLROOT, PEEK(-(-1)), true); break; } - case _LOAD_FROM_DICT_OR_GLOBALS: { + case LOAD_FROM_DICT_OR_GLOBALS: { + PARTITIONNODE_OVERWRITE((_Py_PARTITIONNODE_t *)PARTITIONNODE_NULLROOT, PEEK(-(-1)), true); + break; + } + + case LOAD_NAME: { + STACK_GROW(1); PARTITIONNODE_OVERWRITE((_Py_PARTITIONNODE_t *)PARTITIONNODE_NULLROOT, PEEK(-(-1)), true); break; } diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 2106920..ff95f37 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1286,7 +1286,7 @@ dummy_func( } } - op(_LOAD_LOCALS, ( -- locals)) { + inst(LOAD_LOCALS, ( -- locals)) { locals = LOCALS(); if (locals == NULL) { _PyErr_SetString(tstate, PyExc_SystemError, @@ -1296,15 +1296,11 @@ dummy_func( Py_INCREF(locals); } - macro(LOAD_LOCALS) = _LOAD_LOCALS; - - op(_LOAD_FROM_DICT_OR_GLOBALS, (mod_or_class_dict -- v)) { + inst(LOAD_FROM_DICT_OR_GLOBALS, (mod_or_class_dict -- v)) { PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); if (PyMapping_GetOptionalItem(mod_or_class_dict, name, &v) < 0) { - Py_DECREF(mod_or_class_dict); goto error; } - Py_DECREF(mod_or_class_dict); if (v == NULL) { v = PyDict_GetItemWithError(GLOBALS(), name); if (v != NULL) { @@ -1325,11 +1321,41 @@ dummy_func( } } } + DECREF_INPUTS(); } - macro(LOAD_NAME) = _LOAD_LOCALS + _LOAD_FROM_DICT_OR_GLOBALS; - - macro(LOAD_FROM_DICT_OR_GLOBALS) = _LOAD_FROM_DICT_OR_GLOBALS; + inst(LOAD_NAME, (-- v)) { + PyObject *mod_or_class_dict = LOCALS(); + if (mod_or_class_dict == NULL) { + _PyErr_SetString(tstate, PyExc_SystemError, + "no locals found"); + ERROR_IF(true, error); + } + PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); + if (PyMapping_GetOptionalItem(mod_or_class_dict, name, &v) < 0) { + goto error; + } + if (v == NULL) { + v = PyDict_GetItemWithError(GLOBALS(), name); + if (v != NULL) { + Py_INCREF(v); + } + else if (_PyErr_Occurred(tstate)) { + goto error; + } + else { + if (PyMapping_GetOptionalItem(BUILTINS(), name, &v) < 0) { + goto error; + } + if (v == NULL) { + _PyEval_FormatExcCheckArg( + tstate, PyExc_NameError, + NAME_ERROR_MSG, name); + goto error; + } + } + } + } family(LOAD_GLOBAL, INLINE_CACHE_ENTRIES_LOAD_GLOBAL) = { LOAD_GLOBAL_MODULE, diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index fa7cb88..918991d 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1046,7 +1046,7 @@ break; } - case _LOAD_LOCALS: { + case LOAD_LOCALS: { PyObject *locals; locals = LOCALS(); if (locals == NULL) { @@ -1060,16 +1060,51 @@ break; } - case _LOAD_FROM_DICT_OR_GLOBALS: { + case LOAD_FROM_DICT_OR_GLOBALS: { PyObject *mod_or_class_dict; PyObject *v; mod_or_class_dict = stack_pointer[-1]; PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); if (PyMapping_GetOptionalItem(mod_or_class_dict, name, &v) < 0) { - Py_DECREF(mod_or_class_dict); goto error; } + if (v == NULL) { + v = PyDict_GetItemWithError(GLOBALS(), name); + if (v != NULL) { + Py_INCREF(v); + } + else if (_PyErr_Occurred(tstate)) { + goto error; + } + else { + if (PyMapping_GetOptionalItem(BUILTINS(), name, &v) < 0) { + goto error; + } + if (v == NULL) { + _PyEval_FormatExcCheckArg( + tstate, PyExc_NameError, + NAME_ERROR_MSG, name); + goto error; + } + } + } Py_DECREF(mod_or_class_dict); + stack_pointer[-1] = v; + break; + } + + case LOAD_NAME: { + PyObject *v; + PyObject *mod_or_class_dict = LOCALS(); + if (mod_or_class_dict == NULL) { + _PyErr_SetString(tstate, PyExc_SystemError, + "no locals found"); + if (true) goto error; + } + PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); + if (PyMapping_GetOptionalItem(mod_or_class_dict, name, &v) < 0) { + goto error; + } if (v == NULL) { v = PyDict_GetItemWithError(GLOBALS(), name); if (v != NULL) { @@ -1090,6 +1125,7 @@ } } } + STACK_GROW(1); stack_pointer[-1] = v; break; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 136b36c..e84599d 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1675,65 +1675,51 @@ DISPATCH(); } - TARGET(LOAD_NAME) { - PyObject *locals; + TARGET(LOAD_FROM_DICT_OR_GLOBALS) { PyObject *mod_or_class_dict; PyObject *v; - // _LOAD_LOCALS - { - locals = LOCALS(); - if (locals == NULL) { - _PyErr_SetString(tstate, PyExc_SystemError, - "no locals found"); - if (true) goto error; - } - Py_INCREF(locals); + mod_or_class_dict = stack_pointer[-1]; + PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); + if (PyMapping_GetOptionalItem(mod_or_class_dict, name, &v) < 0) { + goto error; } - // _LOAD_FROM_DICT_OR_GLOBALS - mod_or_class_dict = locals; - { - PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); - if (PyMapping_GetOptionalItem(mod_or_class_dict, name, &v) < 0) { - Py_DECREF(mod_or_class_dict); + if (v == NULL) { + v = PyDict_GetItemWithError(GLOBALS(), name); + if (v != NULL) { + Py_INCREF(v); + } + else if (_PyErr_Occurred(tstate)) { goto error; } - Py_DECREF(mod_or_class_dict); - if (v == NULL) { - v = PyDict_GetItemWithError(GLOBALS(), name); - if (v != NULL) { - Py_INCREF(v); - } - else if (_PyErr_Occurred(tstate)) { + else { + if (PyMapping_GetOptionalItem(BUILTINS(), name, &v) < 0) { goto error; } - else { - if (PyMapping_GetOptionalItem(BUILTINS(), name, &v) < 0) { - goto error; - } - if (v == NULL) { - _PyEval_FormatExcCheckArg( - tstate, PyExc_NameError, - NAME_ERROR_MSG, name); - goto error; - } + if (v == NULL) { + _PyEval_FormatExcCheckArg( + tstate, PyExc_NameError, + NAME_ERROR_MSG, name); + goto error; } } } - STACK_GROW(1); + Py_DECREF(mod_or_class_dict); stack_pointer[-1] = v; DISPATCH(); } - TARGET(LOAD_FROM_DICT_OR_GLOBALS) { - PyObject *mod_or_class_dict; + TARGET(LOAD_NAME) { PyObject *v; - mod_or_class_dict = stack_pointer[-1]; + PyObject *mod_or_class_dict = LOCALS(); + if (mod_or_class_dict == NULL) { + _PyErr_SetString(tstate, PyExc_SystemError, + "no locals found"); + if (true) goto error; + } PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); if (PyMapping_GetOptionalItem(mod_or_class_dict, name, &v) < 0) { - Py_DECREF(mod_or_class_dict); goto error; } - Py_DECREF(mod_or_class_dict); if (v == NULL) { v = PyDict_GetItemWithError(GLOBALS(), name); if (v != NULL) { @@ -1754,6 +1740,7 @@ } } } + STACK_GROW(1); stack_pointer[-1] = v; DISPATCH(); } -- cgit v0.12