From 18396d96084f7e846d50d77f99c5bfe1fd36747a Mon Sep 17 00:00:00 2001 From: skdas20 Date: Thu, 27 Aug 2026 15:05:05 +0000 Subject: [PATCH] gh-156399: Don't specialize LOAD_ATTR_MODULE for module subclasses Attribute loads on a types.ModuleType subclass were specialized to LOAD_ATTR_MODULE, which reads the module dictionary directly. Both the specialization site and the runtime guard tested Py_TYPE(owner)->tp_getattro == PyModule_Type.tp_getattro which a subclass inherits unless it overrides __getattr__, so subclasses took the path. A data descriptor defined on the subclass was then silently bypassed once the instruction had specialized, and the value from the module dictionary was returned instead. Before 3.14 the guard was PyModule_CheckExact, so subclasses never specialized. Require the exact module type in both places. The runtime guard matters independently of the specialization site, because one instruction can be specialized on an exact module and later executed against a subclass instance. --- Lib/test/test_opcache.py | 29 +++++++++++++++++++ ...-08-27-00-00-00.gh-issue-156399.aB3xYz.rst | 4 +++ Modules/_testinternalcapi/test_cases.c.h | 2 +- Python/bytecodes.c | 2 +- Python/executor_cases.c.h | 2 +- Python/generated_cases.c.h | 2 +- Python/specialize.c | 2 +- 7 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-08-27-00-00-00.gh-issue-156399.aB3xYz.rst diff --git a/Lib/test/test_opcache.py b/Lib/test/test_opcache.py index 7946550ec0db637..7589c93c712ebdb 100644 --- a/Lib/test/test_opcache.py +++ b/Lib/test/test_opcache.py @@ -2209,6 +2209,35 @@ def load_module_attr_missing(): finally: sys.modules.pop("test_module_with_getattr", None) + @cpython_only + @requires_specialization + def test_load_attr_module_subclass_with_data_descriptor(self): + # gh-156399: a types.ModuleType subclass inherits + # PyModule_Type.tp_getattro, so guarding specialization on tp_getattro + # alone let subclasses take LOAD_ATTR_MODULE. That reads the module + # dict directly, silently bypassing a data descriptor defined on the + # subclass once the instruction had specialized. + class Descriptor: + def __get__(self, instance, owner=None): + return "from descriptor" + + def __set__(self, instance, value): + instance.__dict__["attr"] = value + + class Module(types.ModuleType): + attr = Descriptor() + + module = Module("test_module_subclass") + module.__dict__["attr"] = "from dict" + + def load_module_attr(): + return module.attr + + for _ in range(_testinternalcapi.SPECIALIZATION_THRESHOLD): + self.assertEqual(load_module_attr(), "from descriptor") + + self.assert_no_opcode(load_module_attr, "LOAD_ATTR_MODULE") + @cpython_only @requires_specialization def test_specialized_iter_doesnt_skip_send_check(self): diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-08-27-00-00-00.gh-issue-156399.aB3xYz.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-27-00-00-00.gh-issue-156399.aB3xYz.rst new file mode 100644 index 000000000000000..7680a16bfe6fd7f --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-27-00-00-00.gh-issue-156399.aB3xYz.rst @@ -0,0 +1,4 @@ +Fix a regression where attribute access on a :class:`types.ModuleType` +subclass could bypass a data descriptor defined on the subclass. The +``LOAD_ATTR_MODULE`` specialization guarded on ``tp_getattro``, which +subclasses inherit, and then read the module dictionary directly. diff --git a/Modules/_testinternalcapi/test_cases.c.h b/Modules/_testinternalcapi/test_cases.c.h index 7a75e80298fcd82..8d1cf11d8e3c8d1 100644 --- a/Modules/_testinternalcapi/test_cases.c.h +++ b/Modules/_testinternalcapi/test_cases.c.h @@ -9207,7 +9207,7 @@ uint32_t dict_version = read_u32(&this_instr[2].cache); uint16_t index = read_u16(&this_instr[4].cache); PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); - if (Py_TYPE(owner_o)->tp_getattro != PyModule_Type.tp_getattro) { + if (!PyModule_CheckExact(owner_o)) { UPDATE_MISS_STATS(LOAD_ATTR); assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR)); JUMP_TO_PREDICTED(LOAD_ATTR); diff --git a/Python/bytecodes.c b/Python/bytecodes.c index fb0cdf4d65e060d..daf826c04e2ed4b 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2928,7 +2928,7 @@ dummy_func( op(_LOAD_ATTR_MODULE, (dict_version/2, index/1, owner -- attr, o)) { PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); - EXIT_IF(Py_TYPE(owner_o)->tp_getattro != PyModule_Type.tp_getattro); + EXIT_IF(!PyModule_CheckExact(owner_o)); PyDictObject *dict = (PyDictObject *)((PyModuleObject *)owner_o)->md_dict; assert(dict != NULL); PyDictKeysObject *keys = FT_ATOMIC_LOAD_PTR_ACQUIRE(dict->ma_keys); diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 9aad9e003765cf8..2c0ce2308515638 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -12324,7 +12324,7 @@ uint32_t dict_version = (uint32_t)CURRENT_OPERAND0_32(); uint16_t index = (uint16_t)CURRENT_OPERAND1_16(); PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); - if (Py_TYPE(owner_o)->tp_getattro != PyModule_Type.tp_getattro) { + if (!PyModule_CheckExact(owner_o)) { UOP_STAT_INC(uopcode, miss); _tos_cache0 = owner; SET_CURRENT_CACHED_VALUES(1); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 77c18b3d61fefc7..4fe319e351a308e 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -9206,7 +9206,7 @@ uint32_t dict_version = read_u32(&this_instr[2].cache); uint16_t index = read_u16(&this_instr[4].cache); PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); - if (Py_TYPE(owner_o)->tp_getattro != PyModule_Type.tp_getattro) { + if (!PyModule_CheckExact(owner_o)) { UPDATE_MISS_STATS(LOAD_ATTR); assert(_PyOpcode_Deopt[opcode] == (LOAD_ATTR)); JUMP_TO_PREDICTED(LOAD_ATTR); diff --git a/Python/specialize.c b/Python/specialize.c index 05cb76ff015ff40..590819448b11e80 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -1010,7 +1010,7 @@ _Py_Specialize_LoadAttr(_PyStackRef owner_st, _Py_CODEUNIT *instr, PyObject *nam SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OTHER); fail = true; } - else if (Py_TYPE(owner)->tp_getattro == PyModule_Type.tp_getattro) { + else if (PyModule_CheckExact(owner)) { fail = specialize_module_load_attr(owner, instr, name); } else if (PyType_Check(owner)) {