Skip to content

Commit 85599d3

Browse files
authored
gh-156399: Don't specialize LOAD_ATTR_MODULE for module subclasses (GH-156474)
Fixes #156399
1 parent 2c24bc1 commit 85599d3

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
@@ -2326,6 +2326,35 @@ def load_module_attr_missing():
23262326
finally:
23272327
sys.modules.pop("test_module_with_getattr", None)
23282328

2329+
@cpython_only
2330+
@requires_specialization
2331+
def test_load_attr_module_subclass_with_data_descriptor(self):
2332+
# gh-156399: a types.ModuleType subclass inherits
2333+
# PyModule_Type.tp_getattro, so guarding specialization on tp_getattro
2334+
# alone let subclasses take LOAD_ATTR_MODULE. That reads the module
2335+
# dict directly, silently bypassing a data descriptor defined on the
2336+
# subclass once the instruction had specialized.
2337+
class Descriptor:
2338+
def __get__(self, instance, owner=None):
2339+
return "from descriptor"
2340+
2341+
def __set__(self, instance, value):
2342+
instance.__dict__["attr"] = value
2343+
2344+
class Module(types.ModuleType):
2345+
attr = Descriptor()
2346+
2347+
module = Module("test_module_subclass")
2348+
module.__dict__["attr"] = "from dict"
2349+
2350+
def load_module_attr():
2351+
return module.attr
2352+
2353+
for _ in range(_testinternalcapi.SPECIALIZATION_THRESHOLD):
2354+
self.assertEqual(load_module_attr(), "from descriptor")
2355+
2356+
self.assert_no_opcode(load_module_attr, "LOAD_ATTR_MODULE")
2357+
23292358
@cpython_only
23302359
@requires_specialization
23312360
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
@@ -2888,7 +2888,7 @@ dummy_func(
28882888

28892889
op(_LOAD_ATTR_MODULE, (dict_version/2, index/1, owner -- attr, o)) {
28902890
PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner);
2891-
EXIT_IF(Py_TYPE(owner_o)->tp_getattro != PyModule_Type.tp_getattro);
2891+
EXIT_IF(!PyModule_CheckExact(owner_o));
28922892
PyDictObject *dict = (PyDictObject *)((PyModuleObject *)owner_o)->md_dict;
28932893
assert(dict != NULL);
28942894
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
@@ -1011,7 +1011,7 @@ _Py_Specialize_LoadAttr(_PyStackRef owner_st, _Py_CODEUNIT *instr, PyObject *nam
10111011
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OTHER);
10121012
fail = true;
10131013
}
1014-
else if (Py_TYPE(owner)->tp_getattro == PyModule_Type.tp_getattro) {
1014+
else if (PyModule_CheckExact(owner)) {
10151015
fail = specialize_module_load_attr(owner, instr, name);
10161016
}
10171017
else if (PyType_Check(owner)) {

0 commit comments

Comments
 (0)