From d28680ff53a3debf49044f2c56f9701638972994 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Fri, 31 May 2024 14:05:24 -0700 Subject: [PATCH] gh-119821: Support non-dict globals in LOAD_FROM_DICT_OR_GLOBALS (#119822) Support non-dict globals in LOAD_FROM_DICT_OR_GLOBALS The implementation basically copies LOAD_GLOBAL. Possibly it could be deduplicated, but that seems like it may get hairy since the two operations have different operands. This is important to fix in 3.14 for PEP 649, but it's a bug in earlier versions too, and we should backport to 3.13 and 3.12 if possible. (cherry picked from commit 80a4e3899420faaa012c82b4e82cdb6675a6a944) --- Include/internal/pycore_opcode_metadata.h | 1 - Include/internal/pycore_uop_metadata.h | 4 --- Lib/test/test_type_aliases.py | 20 +++++++++++ ...-05-30-23-01-00.gh-issue-119821.jPGfvt.rst | 2 ++ Python/bytecodes.c | 35 ++++++++++++++----- Python/executor_cases.c.h | 30 +--------------- Python/generated_cases.c.h | 35 ++++++++++++++----- Python/optimizer_cases.c.h | 8 +---- 8 files changed, 76 insertions(+), 59 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-05-30-23-01-00.gh-issue-119821.jPGfvt.rst diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index 665b8627dfcc52..5b0d8dfd1b0b97 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -1299,7 +1299,6 @@ _PyOpcode_macro_expansion[256] = { [LOAD_FAST_CHECK] = { .nuops = 1, .uops = { { _LOAD_FAST_CHECK, 0, 0 } } }, [LOAD_FAST_LOAD_FAST] = { .nuops = 2, .uops = { { _LOAD_FAST, 5, 0 }, { _LOAD_FAST, 6, 0 } } }, [LOAD_FROM_DICT_OR_DEREF] = { .nuops = 1, .uops = { { _LOAD_FROM_DICT_OR_DEREF, 0, 0 } } }, - [LOAD_FROM_DICT_OR_GLOBALS] = { .nuops = 1, .uops = { { _LOAD_FROM_DICT_OR_GLOBALS, 0, 0 } } }, [LOAD_GLOBAL] = { .nuops = 1, .uops = { { _LOAD_GLOBAL, 0, 0 } } }, [LOAD_GLOBAL_BUILTIN] = { .nuops = 3, .uops = { { _GUARD_GLOBALS_VERSION, 1, 1 }, { _GUARD_BUILTINS_VERSION, 1, 2 }, { _LOAD_GLOBAL_BUILTINS, 1, 3 } } }, [LOAD_GLOBAL_MODULE] = { .nuops = 2, .uops = { { _GUARD_GLOBALS_VERSION, 1, 1 }, { _LOAD_GLOBAL_MODULE, 1, 3 } } }, diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h index 470e95e2b3b041..bc0ba16e24fe98 100644 --- a/Include/internal/pycore_uop_metadata.h +++ b/Include/internal/pycore_uop_metadata.h @@ -107,7 +107,6 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = { [_STORE_GLOBAL] = HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_DELETE_GLOBAL] = HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG, [_LOAD_LOCALS] = HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, - [_LOAD_FROM_DICT_OR_GLOBALS] = HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG, [_LOAD_GLOBAL] = HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_GUARD_GLOBALS_VERSION] = HAS_DEOPT_FLAG, [_GUARD_BUILTINS_VERSION] = HAS_DEOPT_FLAG, @@ -438,7 +437,6 @@ const char *const _PyOpcode_uop_name[MAX_UOP_ID+1] = { [_LOAD_FAST_CHECK] = "_LOAD_FAST_CHECK", [_LOAD_FAST_LOAD_FAST] = "_LOAD_FAST_LOAD_FAST", [_LOAD_FROM_DICT_OR_DEREF] = "_LOAD_FROM_DICT_OR_DEREF", - [_LOAD_FROM_DICT_OR_GLOBALS] = "_LOAD_FROM_DICT_OR_GLOBALS", [_LOAD_GLOBAL] = "_LOAD_GLOBAL", [_LOAD_GLOBAL_BUILTINS] = "_LOAD_GLOBAL_BUILTINS", [_LOAD_GLOBAL_MODULE] = "_LOAD_GLOBAL_MODULE", @@ -690,8 +688,6 @@ int _PyUop_num_popped(int opcode, int oparg) return 0; case _LOAD_LOCALS: return 0; - case _LOAD_FROM_DICT_OR_GLOBALS: - return 1; case _LOAD_GLOBAL: return 0; case _GUARD_GLOBALS_VERSION: diff --git a/Lib/test/test_type_aliases.py b/Lib/test/test_type_aliases.py index 9c325bc595f585..f8b395fdc8bb1d 100644 --- a/Lib/test/test_type_aliases.py +++ b/Lib/test/test_type_aliases.py @@ -1,4 +1,5 @@ import pickle +import textwrap import types import unittest from test.support import check_syntax_error, run_code @@ -328,3 +329,22 @@ def test_pickling_local(self): with self.subTest(thing=thing, proto=proto): with self.assertRaises(pickle.PickleError): pickle.dumps(thing, protocol=proto) + + +class TypeParamsExoticGlobalsTest(unittest.TestCase): + def test_exec_with_unusual_globals(self): + class customdict(dict): + def __missing__(self, key): + return key + + code = compile("type Alias = undefined", "test", "exec") + ns = customdict() + exec(code, ns) + Alias = ns["Alias"] + self.assertEqual(Alias.__value__, "undefined") + + code = compile("class A: type Alias = undefined", "test", "exec") + ns = customdict() + exec(code, ns) + Alias = ns["A"].Alias + self.assertEqual(Alias.__value__, "undefined") diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-05-30-23-01-00.gh-issue-119821.jPGfvt.rst b/Misc/NEWS.d/next/Core and Builtins/2024-05-30-23-01-00.gh-issue-119821.jPGfvt.rst new file mode 100644 index 00000000000000..cc25eee6dd6ae4 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-05-30-23-01-00.gh-issue-119821.jPGfvt.rst @@ -0,0 +1,2 @@ +Fix execution of :ref:`annotation scopes ` within classes +when ``globals`` is set to a non-dict. Patch by Jelle Zijlstra. diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 434eb804213810..2e5e572a484029 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1375,18 +1375,35 @@ dummy_func( ERROR_NO_POP(); } if (v == NULL) { - if (PyDict_GetItemRef(GLOBALS(), name, &v) < 0) { - ERROR_NO_POP(); - } - if (v == NULL) { - if (PyMapping_GetOptionalItem(BUILTINS(), name, &v) < 0) { + if (PyDict_CheckExact(GLOBALS()) + && PyDict_CheckExact(BUILTINS())) + { + v = _PyDict_LoadGlobal((PyDictObject *)GLOBALS(), + (PyDictObject *)BUILTINS(), + name); + if (v == NULL) { + if (!_PyErr_Occurred(tstate)) { + /* _PyDict_LoadGlobal() returns NULL without raising + * an exception if the key doesn't exist */ + _PyEval_FormatExcCheckArg(tstate, PyExc_NameError, + NAME_ERROR_MSG, name); + } ERROR_NO_POP(); } + } + else { + /* Slow-path if globals or builtins is not a dict */ + /* namespace 1: globals */ + ERROR_IF(PyMapping_GetOptionalItem(GLOBALS(), name, &v) < 0, error); if (v == NULL) { - _PyEval_FormatExcCheckArg( - tstate, PyExc_NameError, - NAME_ERROR_MSG, name); - ERROR_NO_POP(); + /* namespace 2: builtins */ + ERROR_IF(PyMapping_GetOptionalItem(BUILTINS(), name, &v) < 0, error); + if (v == NULL) { + _PyEval_FormatExcCheckArg( + tstate, PyExc_NameError, + NAME_ERROR_MSG, name); + ERROR_IF(true, error); + } } } } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 347a1e677a0832..74405293c03026 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1394,35 +1394,7 @@ break; } - case _LOAD_FROM_DICT_OR_GLOBALS: { - PyObject *mod_or_class_dict; - PyObject *v; - oparg = CURRENT_OPARG(); - mod_or_class_dict = stack_pointer[-1]; - PyObject *name = GETITEM(FRAME_CO_NAMES, oparg); - if (PyMapping_GetOptionalItem(mod_or_class_dict, name, &v) < 0) { - JUMP_TO_ERROR(); - } - if (v == NULL) { - if (PyDict_GetItemRef(GLOBALS(), name, &v) < 0) { - JUMP_TO_ERROR(); - } - if (v == NULL) { - if (PyMapping_GetOptionalItem(BUILTINS(), name, &v) < 0) { - JUMP_TO_ERROR(); - } - if (v == NULL) { - _PyEval_FormatExcCheckArg( - tstate, PyExc_NameError, - NAME_ERROR_MSG, name); - JUMP_TO_ERROR(); - } - } - } - Py_DECREF(mod_or_class_dict); - stack_pointer[-1] = v; - break; - } + /* _LOAD_FROM_DICT_OR_GLOBALS is not a viable micro-op for tier 2 because it has both popping and not-popping errors */ /* _LOAD_NAME is not a viable micro-op for tier 2 because it has both popping and not-popping errors */ diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 96161c5a6586fd..dee6567ffad5e8 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -4381,18 +4381,35 @@ goto error; } if (v == NULL) { - if (PyDict_GetItemRef(GLOBALS(), name, &v) < 0) { - goto error; - } - if (v == NULL) { - if (PyMapping_GetOptionalItem(BUILTINS(), name, &v) < 0) { + if (PyDict_CheckExact(GLOBALS()) + && PyDict_CheckExact(BUILTINS())) + { + v = _PyDict_LoadGlobal((PyDictObject *)GLOBALS(), + (PyDictObject *)BUILTINS(), + name); + if (v == NULL) { + if (!_PyErr_Occurred(tstate)) { + /* _PyDict_LoadGlobal() returns NULL without raising + * an exception if the key doesn't exist */ + _PyEval_FormatExcCheckArg(tstate, PyExc_NameError, + NAME_ERROR_MSG, name); + } goto error; } + } + else { + /* Slow-path if globals or builtins is not a dict */ + /* namespace 1: globals */ + if (PyMapping_GetOptionalItem(GLOBALS(), name, &v) < 0) goto pop_1_error; if (v == NULL) { - _PyEval_FormatExcCheckArg( - tstate, PyExc_NameError, - NAME_ERROR_MSG, name); - goto error; + /* namespace 2: builtins */ + if (PyMapping_GetOptionalItem(BUILTINS(), name, &v) < 0) goto pop_1_error; + if (v == NULL) { + _PyEval_FormatExcCheckArg( + tstate, PyExc_NameError, + NAME_ERROR_MSG, name); + if (true) goto pop_1_error; + } } } } diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index 2a4efd73d794df..6c2480d4fdfe89 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -815,13 +815,7 @@ break; } - case _LOAD_FROM_DICT_OR_GLOBALS: { - _Py_UopsSymbol *v; - v = sym_new_not_null(ctx); - if (v == NULL) goto out_of_space; - stack_pointer[-1] = v; - break; - } + /* _LOAD_FROM_DICT_OR_GLOBALS is not a viable micro-op for tier 2 */ /* _LOAD_NAME is not a viable micro-op for tier 2 */