Skip to content

Commit 18396d9

Browse files
committed
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.
1 parent eb08902 commit 18396d9

7 files changed

Lines changed: 38 additions & 5 deletions

File tree

Lib/test/test_opcache.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2209,6 +2209,35 @@ def load_module_attr_missing():
22092209
finally:
22102210
sys.modules.pop("test_module_with_getattr", None)
22112211

2212+
@cpython_only
2213+
@requires_specialization
2214+
def test_load_attr_module_subclass_with_data_descriptor(self):
2215+
# gh-156399: a types.ModuleType subclass inherits
2216+
# PyModule_Type.tp_getattro, so guarding specialization on tp_getattro
2217+
# alone let subclasses take LOAD_ATTR_MODULE. That reads the module
2218+
# dict directly, silently bypassing a data descriptor defined on the
2219+
# subclass once the instruction had specialized.
2220+
class Descriptor:
2221+
def __get__(self, instance, owner=None):
2222+
return "from descriptor"
2223+
2224+
def __set__(self, instance, value):
2225+
instance.__dict__["attr"] = value
2226+
2227+
class Module(types.ModuleType):
2228+
attr = Descriptor()
2229+
2230+
module = Module("test_module_subclass")
2231+
module.__dict__["attr"] = "from dict"
2232+
2233+
def load_module_attr():
2234+
return module.attr
2235+
2236+
for _ in range(_testinternalcapi.SPECIALIZATION_THRESHOLD):
2237+
self.assertEqual(load_module_attr(), "from descriptor")
2238+
2239+
self.assert_no_opcode(load_module_attr, "LOAD_ATTR_MODULE")
2240+
22122241
@cpython_only
22132242
@requires_specialization
22142243
def test_specialized_iter_doesnt_skip_send_check(self):
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a regression where attribute access on a :class:`types.ModuleType`
2+
subclass could bypass a data descriptor defined on the subclass. The
3+
``LOAD_ATTR_MODULE`` specialization guarded on ``tp_getattro``, which
4+
subclasses inherit, and then read the module dictionary directly.

Modules/_testinternalcapi/test_cases.c.h

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/bytecodes.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2928,7 +2928,7 @@ dummy_func(
29282928

29292929
op(_LOAD_ATTR_MODULE, (dict_version/2, index/1, owner -- attr, o)) {
29302930
PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner);
2931-
EXIT_IF(Py_TYPE(owner_o)->tp_getattro != PyModule_Type.tp_getattro);
2931+
EXIT_IF(!PyModule_CheckExact(owner_o));
29322932
PyDictObject *dict = (PyDictObject *)((PyModuleObject *)owner_o)->md_dict;
29332933
assert(dict != NULL);
29342934
PyDictKeysObject *keys = FT_ATOMIC_LOAD_PTR_ACQUIRE(dict->ma_keys);

Python/executor_cases.c.h

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/generated_cases.c.h

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/specialize.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1010,7 +1010,7 @@ _Py_Specialize_LoadAttr(_PyStackRef owner_st, _Py_CODEUNIT *instr, PyObject *nam
10101010
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OTHER);
10111011
fail = true;
10121012
}
1013-
else if (Py_TYPE(owner)->tp_getattro == PyModule_Type.tp_getattro) {
1013+
else if (PyModule_CheckExact(owner)) {
10141014
fail = specialize_module_load_attr(owner, instr, name);
10151015
}
10161016
else if (PyType_Check(owner)) {

0 commit comments

Comments
 (0)