Skip to content

Commit

Permalink
GH-119462: Enforce invariants of type versioning (GH-120731)
Browse files Browse the repository at this point in the history
* Remove uses of Py_TPFLAGS_VALID_VERSION_TAG
  • Loading branch information
markshannon authored Jun 19, 2024
1 parent f385d99 commit 00257c7
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 103 deletions.
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 @@ -566,7 +566,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

0 comments on commit 00257c7

Please sign in to comment.