Skip to content

GH-119462: Enforce invariants of type versioning #120731

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Doc/c-api/typeobj.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1328,8 +1328,8 @@ and :c:data:`PyType_Type` effectively act as defaults.)
To indicate that a class has changed call :c:func:`PyType_Modified`

.. warning::
This flag is present in header files, but is an internal feature and should
not be used. It will be removed in a future version of CPython
This flag is present in header files, but is not be used.
It will be removed in a future version of CPython


.. c:member:: const char* PyTypeObject.tp_doc
Expand Down
2 changes: 1 addition & 1 deletion Include/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ given type object has a specified feature.
/* Objects behave like an unbound method */
#define Py_TPFLAGS_METHOD_DESCRIPTOR (1UL << 17)

/* Object has up-to-date type attribute cache */
/* Unused. Legacy flag */
#define Py_TPFLAGS_VALID_VERSION_TAG (1UL << 19)

/* Type is abstract and cannot be instantiated */
Expand Down
29 changes: 23 additions & 6 deletions Lib/test/test_type_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,21 @@ class C:
new_version = type_get_version(C)
self.assertEqual(new_version, 0)

def test_119462(self):

class Holder:
value = None

@classmethod
def set_value(cls):
cls.value = object()

class HolderSub(Holder):
pass

for _ in range(1050):
Holder.set_value()
HolderSub.value

@support.cpython_only
@requires_specialization
Expand All @@ -106,8 +121,10 @@ def _assign_valid_version_or_skip(self, type_):
if type_get_version(type_) == 0:
self.skipTest("Could not assign valid type version")

def _assign_and_check_version_0(self, user_type):
def _no_more_versions(self, user_type):
type_modified(user_type)
for _ in range(1001):
type_assign_specific_version_unsafe(user_type, 1000_000_000)
type_assign_specific_version_unsafe(user_type, 0)
self.assertEqual(type_get_version(user_type), 0)

Expand Down Expand Up @@ -136,7 +153,7 @@ def load_foo_1(type_):
self._check_specialization(load_foo_1, A, "LOAD_ATTR", should_specialize=True)
del load_foo_1

self._assign_and_check_version_0(A)
self._no_more_versions(A)

def load_foo_2(type_):
return type_.foo
Expand Down Expand Up @@ -187,7 +204,7 @@ def load_x_1(instance):
self._check_specialization(load_x_1, G(), "LOAD_ATTR", should_specialize=True)
del load_x_1

self._assign_and_check_version_0(G)
self._no_more_versions(G)

def load_x_2(instance):
instance.x
Expand All @@ -206,7 +223,7 @@ def store_bar_1(type_):
self._check_specialization(store_bar_1, B(), "STORE_ATTR", should_specialize=True)
del store_bar_1

self._assign_and_check_version_0(B)
self._no_more_versions(B)

def store_bar_2(type_):
type_.bar = 10
Expand All @@ -226,7 +243,7 @@ def call_class_1(type_):
self._check_specialization(call_class_1, F, "CALL", should_specialize=True)
del call_class_1

self._assign_and_check_version_0(F)
self._no_more_versions(F)

def call_class_2(type_):
type_()
Expand All @@ -245,7 +262,7 @@ def to_bool_1(instance):
self._check_specialization(to_bool_1, H(), "TO_BOOL", should_specialize=True)
del to_bool_1

self._assign_and_check_version_0(H)
self._no_more_versions(H)

def to_bool_2(instance):
not instance
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Make sure that invariants of type versioning are maintained:
* Superclasses always have their version number assigned before subclasses
* The version tag is always zero if the tag is not valid.
* The version tag is always non-if the tag is valid.
1 change: 0 additions & 1 deletion Modules/_testinternalcapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -2014,7 +2014,6 @@ type_assign_specific_version_unsafe(PyObject *self, PyObject *args)
}
assert(!PyType_HasFeature(type, Py_TPFLAGS_IMMUTABLETYPE));
_PyType_SetVersion(type, version);
type->tp_flags |= Py_TPFLAGS_VALID_VERSION_TAG;
Py_RETURN_NONE;
}

Expand Down
2 changes: 1 addition & 1 deletion Modules/pyexpat.c
Original file line number Diff line number Diff line change
Expand Up @@ -1651,7 +1651,7 @@ PyDoc_STRVAR(pyexpat_module_documentation,
static int init_handler_descrs(pyexpat_state *state)
{
int i;
assert(!PyType_HasFeature(state->xml_parse_type, Py_TPFLAGS_VALID_VERSION_TAG));
assert(state->xml_parse_type->tp_version_tag == 0);
for (i = 0; handler_info[i].name != NULL; i++) {
struct HandlerInfo *hi = &handler_info[i];
hi->getset.name = hi->name;
Expand Down
Loading
Loading