Use Py_TPFLAGS_IMMUTABLETYPE in __class__ assignments check

@gvanrossum

BPO	43973
Nosy	@gvanrossum, @pitrou, @vstinner, @erlend-aasland, @shreyanavigyan
PRs	bpo-43973: Use Py_TPFLAGS_IMMUTABLETYPE to check for class assignments #25714

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-04-30.10:09:43.485>
created_at = <Date 2021-04-29.08:27:05.693>
labels = []
title = 'Use Py_TPFLAGS_IMMUTABLETYPE in __class__ assignments check'
updated_at = <Date 2021-04-30.10:58:54.503>
user = 'https://github.com/erlend-aasland'

bugs.python.org fields:

activity = <Date 2021-04-30.10:58:54.503>
actor = 'erlendaasland'
assignee = 'none'
closed = True
closed_date = <Date 2021-04-30.10:09:43.485>
closer = 'erlendaasland'
components = []
creation = <Date 2021-04-29.08:27:05.693>
creator = 'erlendaasland'
dependencies = []
files = []
hgrepos = []
issue_num = 43973
keywords = ['patch']
message_count = 7.0
messages = ['392290', '392291', '392313', '392404', '392405', '392412', '392415']
nosy_count = 5.0
nosy_names = ['gvanrossum', 'pitrou', 'vstinner', 'erlendaasland', 'shreyanavigyan']
pr_nums = ['25714']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue43973'
versions = []

Use Py_TPFLAGS_IMMUTABLETYPE iso. Py_TPFLAGS_HEAPTYPE in object_set_class().

See also:

bpo-43908 (particularly msg392286)
bpo-24912

cpython/Objects/typeobject.c

Lines 4620 to 4668 in e047239

    
               /* In versions of CPython prior to 3.5, the code in 
        
                  compatible_for_assignment was not set up to correctly check for memory 
        
                  layout / slot / etc. compatibility for non-HEAPTYPE classes, so we just 
        
                  disallowed __class__ assignment in any case that wasn't HEAPTYPE -> 
        
                  HEAPTYPE. 
        
                  During the 3.5 development cycle, we fixed the code in 
        
                  compatible_for_assignment to correctly check compatibility between 
        
                  arbitrary types, and started allowing __class__ assignment in all cases 
        
                  where the old and new types did in fact have compatible slots and 
        
                  memory layout (regardless of whether they were implemented as HEAPTYPEs 
        
                  or not). 
        
                  Just before 3.5 was released, though, we discovered that this led to 
        
                  problems with immutable types like int, where the interpreter assumes 
        
                  they are immutable and interns some values. Formerly this wasn't a 
        
                  problem, because they really were immutable -- in particular, all the 
        
                  types where the interpreter applied this interning trick happened to 
        
                  also be statically allocated, so the old HEAPTYPE rules were 
        
                  "accidentally" stopping them from allowing __class__ assignment. But 
        
                  with the changes to __class__ assignment, we started allowing code like 
        
                    class MyInt(int): 
        
                        ... 
        
                    # Modifies the type of *all* instances of 1 in the whole program, 
        
                    # including future instances (!), because the 1 object is interned. 
        
                    (1).__class__ = MyInt 
        
                  (see https://bugs.python.org/issue24912). 
        
                  In theory the proper fix would be to identify which classes rely on 
        
                  this invariant and somehow disallow __class__ assignment only for them, 
        
                  perhaps via some mechanism like a new Py_TPFLAGS_IMMUTABLE flag (a 
        
                  "denylisting" approach). But in practice, since this problem wasn't 
        
                  noticed late in the 3.5 RC cycle, we're taking the conservative 
        
                  approach and reinstating the same HEAPTYPE->HEAPTYPE check that we used 
        
                  to have, plus an "allowlist". For now, the allowlist consists only of 
        
                  ModuleType subtypes, since those are the cases that motivated the patch 
        
                  in the first place -- see https://bugs.python.org/issue22986 -- and 
        
                  since module objects are mutable we can be sure that they are 
        
                  definitely not being interned. So now we allow HEAPTYPE->HEAPTYPE *or* 
        
                  ModuleType subtype -> ModuleType subtype. 
        
                  So far as we know, all the code beyond the following 'if' statement 
        
                  will correctly handle non-HEAPTYPE classes, and the HEAPTYPE check is 
        
                  needed only to protect that subset of non-HEAPTYPE classes for which 
        
                  the interpreter has baked in the assumption that all instances are 
        
                  truly immutable. 
        
               */

IMO, we can drop most (all?) of the huge comment before the check in object_set_class(). History is preserved in the git log. Contrary to Py_TPFLAGS_HEAPTYPE, using Py_TPFLAGS_IMMUTABLETYPE speaks for itself. We might reduce it to just refer to to bpo-43908 and bpo-24912.

Something a la this:
/* Historically, only static types were immutable.

Py_TPFLAGS_IMMUTABLETYPE was introduced in Python 3.10.
See bpo-43908 and bpo-24912. */

What do you think?

IMO, we can drop most (all?) of the huge comment before the check in object_set_class().

I don't think so. I don't understand why it's allowed to override the __class__ attribute of ModuleType instances whereas it's a static type. IMO the long comment remains useful and something should be done later to make the condition more generic, rather than checking PyType_IsSubtype(newto, &PyModule_Type) specifically.

New changeset b73b5fb by Erlend Egeberg Aasland in branch 'master':
bpo-43973: object_set_class() checks Py_TPFLAGS_IMMUTABLETYPE (GH-25714)
b73b5fb

Ok! Did you see bpo-24912 regarding the ModuleType check?

Ok! Did you see bpo-24912 regarding the ModuleType check?

No, I didn't. Sorry, there are too many things to look at, and too little bit. Do you think that something should be changed? If yes, please open a new separated issue.

Do you think that something should be changed? If yes, please open a new separated issue.

No, my comment was just for information :)

Navigation Menu

Repository navigation

Use Py_TPFLAGS_IMMUTABLETYPE in class assignments check #88139

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	/* In versions of CPython prior to 3.5, the code in
	compatible_for_assignment was not set up to correctly check for memory
	layout / slot / etc. compatibility for non-HEAPTYPE classes, so we just
	disallowed __class__ assignment in any case that wasn't HEAPTYPE ->
	HEAPTYPE.

	During the 3.5 development cycle, we fixed the code in
	compatible_for_assignment to correctly check compatibility between
	arbitrary types, and started allowing __class__ assignment in all cases
	where the old and new types did in fact have compatible slots and
	memory layout (regardless of whether they were implemented as HEAPTYPEs
	or not).

	Just before 3.5 was released, though, we discovered that this led to
	problems with immutable types like int, where the interpreter assumes
	they are immutable and interns some values. Formerly this wasn't a
	problem, because they really were immutable -- in particular, all the
	types where the interpreter applied this interning trick happened to
	also be statically allocated, so the old HEAPTYPE rules were
	"accidentally" stopping them from allowing __class__ assignment. But
	with the changes to __class__ assignment, we started allowing code like

	class MyInt(int):
	...
	# Modifies the type of all instances of 1 in the whole program,
	# including future instances (!), because the 1 object is interned.
	(1).__class__ = MyInt

	(see https://bugs.python.org/issue24912).

	In theory the proper fix would be to identify which classes rely on
	this invariant and somehow disallow __class__ assignment only for them,
	perhaps via some mechanism like a new Py_TPFLAGS_IMMUTABLE flag (a
	"denylisting" approach). But in practice, since this problem wasn't
	noticed late in the 3.5 RC cycle, we're taking the conservative
	approach and reinstating the same HEAPTYPE->HEAPTYPE check that we used
	to have, plus an "allowlist". For now, the allowlist consists only of
	ModuleType subtypes, since those are the cases that motivated the patch
	in the first place -- see https://bugs.python.org/issue22986 -- and
	since module objects are mutable we can be sure that they are
	definitely not being interned. So now we allow HEAPTYPE->HEAPTYPE or
	ModuleType subtype -> ModuleType subtype.

	So far as we know, all the code beyond the following 'if' statement
	will correctly handle non-HEAPTYPE classes, and the HEAPTYPE check is
	needed only to protect that subset of non-HEAPTYPE classes for which
	the interpreter has baked in the assumption that all instances are
	truly immutable.
	*/

Use Py_TPFLAGS_IMMUTABLETYPE in __class__ assignments check #88139

Description

Activity

erlend-aasland commented on Apr 29, 2021

erlend-aasland commented on Apr 29, 2021

vstinner commented on Apr 29, 2021

vstinner commented on Apr 30, 2021

erlend-aasland commented on Apr 30, 2021

vstinner commented on Apr 30, 2021

erlend-aasland commented on Apr 30, 2021

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

Use Py_TPFLAGS_IMMUTABLETYPE in class assignments check #88139