Tracing a class that defines a custom `class` appears to remove said property under certain circumstances #98148

ionelmc · 2022-10-10T13:29:27Z

Tested on 3.7, 3.8, 3.9 and 3.10

It appears that using super() in the class makes the class behave differently when traced:

import sys

def tracer(frame, event, _):
    return tracer
sys.settrace(tracer)


class Foo:
    pass

class FancyFoo:
    def bug(self):
        super()

    @property
    def __class__(self):
        return Foo

assert isinstance(FancyFoo(), Foo)

Remove either the def bug or the settrace call and the assertion passes.

Ref nedbat/coveragepy#1382.

The text was updated successfully, but these errors were encountered:

cfbolz · 2022-12-08T18:25:21Z

I'm going to comment here, because PyPy managed to follow CPython closely enough that we have exactly the same bug. slowly getting a clue about what is happening with this bug. Some (somewhat unorganized) notes:

the trace function produces the bug if it is on during the class construction of FancyFoo. whether the tracer is on or not during the isinstance call makes no difference
the presence of super anywhere in the class body leads to __class__ being a closure variable in the FancyFoo body
however, it's a somewhat weird superposition of being a closure variable and a regular name that is stored in the locals. it is accessed both using STORE_NAME and LOAD_CLOSURE bytecodes (see bytecode of the buggy code below)
the def __class__ function definition is stored in the locals dict
if the tracer is on, that "__class__" locals dict entry containing the property is overridden by the PyFrame_FastToLocalsWithError done in call_trampoline at the next tracer call that is happening after the definition of property "__class__". The class cell is empty, so the logic in map_to_dict deletes the "__class__" key from the locals dict.
this can be observed from Python: if the tracer is one, then FancyFoo.__dict__['__class__'] is a KeyError. if the tracer is off at class creation time, then FancyFoo.__dict__['__class__'] is the property object, as one would expect.

This is my current understanding how the buggy behavior happens. I don't have a clear idea how to fix this yet though.

Some tentative ideas:

__class__ could be turned into a completely regular cell by changing the logic in symtable.c? then the STORE_NAME would become a regular STORE_DEREF
The class cell that is used for super could be given a different name in the class body (it's only important that the cell is called __class__ in the context of the methods). Maybe it could be given a name that is not a valid Python identifier so there is no risk of clashes.
Wouldn't it maybe even work to give the cell the name "__classcell__" throughout the whole class body, not just to copy it to that name at the end?

The disassembled bytecode of the example code in 3.10

1           0 LOAD_CONST               0 (0)
            2 LOAD_CONST               1 (None)
            4 IMPORT_NAME              0 (sys)
            6 STORE_NAME               0 (sys)

3           8 LOAD_CONST               2 (<code object tracer at 0x7fdf27956ef0, file "bug.py", line 3>)
           10 LOAD_CONST               3 ('tracer')
           12 MAKE_FUNCTION            0
           14 STORE_NAME               1 (tracer)

8          16 LOAD_BUILD_CLASS
           18 LOAD_CONST               4 (<code object Foo at 0x7fdf27956fa0, file "bug.py", line 8>)
           20 LOAD_CONST               5 ('Foo')
           22 MAKE_FUNCTION            0
           24 LOAD_CONST               5 ('Foo')
           26 CALL_FUNCTION            2
           28 STORE_NAME               2 (Foo)

12          30 LOAD_BUILD_CLASS
           32 LOAD_CONST               6 (<code object FancyFoo at 0x7fdf279571b0, file "bug.py", line 12>)
           34 LOAD_CONST               7 ('FancyFoo')
           36 MAKE_FUNCTION            0
           38 LOAD_CONST               7 ('FancyFoo')
           40 CALL_FUNCTION            2
           42 STORE_NAME               3 (FancyFoo)

19          44 LOAD_NAME                0 (sys)
           46 LOAD_METHOD              4 (settrace)
           48 LOAD_CONST               1 (None)
           50 CALL_METHOD              1
           52 POP_TOP

21          54 LOAD_NAME                5 (isinstance)
           56 LOAD_NAME                3 (FancyFoo)
           58 CALL_FUNCTION            0
           60 LOAD_NAME                2 (Foo)
           62 CALL_FUNCTION            2
           64 POP_JUMP_IF_TRUE        35 (to 70)
           66 LOAD_ASSERTION_ERROR
           68 RAISE_VARARGS            1
      >>   70 LOAD_CONST               1 (None)
           72 RETURN_VALUE

Disassembly of <code object tracer at 0x7fdf27956ef0, file "bug.py", line 3>:
4           0 LOAD_GLOBAL              0 (print)
            2 LOAD_FAST                0 (frame)
            4 LOAD_FAST                1 (event)
            6 LOAD_FAST                2 (_)
            8 CALL_FUNCTION            3
           10 POP_TOP

5          12 LOAD_GLOBAL              1 (tracer)
           14 RETURN_VALUE

Disassembly of <code object Foo at 0x7fdf27956fa0, file "bug.py", line 8>:
8           0 LOAD_NAME                0 (__name__)
            2 STORE_NAME               1 (__module__)
            4 LOAD_CONST               0 ('Foo')
            6 STORE_NAME               2 (__qualname__)

9           8 LOAD_CONST               1 (None)
           10 RETURN_VALUE

Disassembly of <code object FancyFoo at 0x7fdf279571b0, file "bug.py", line 12>:
12           0 LOAD_NAME                0 (__name__)
            2 STORE_NAME               1 (__module__)
            4 LOAD_CONST               0 ('FancyFoo')
            6 STORE_NAME               2 (__qualname__)

13           8 LOAD_CLOSURE             0 (__class__)
           10 BUILD_TUPLE              1
           12 LOAD_CONST               1 (<code object bug at 0x7fdf27957050, file "bug.py", line 13>)
           14 LOAD_CONST               2 ('FancyFoo.bug')
           16 MAKE_FUNCTION            8 (closure)
           18 STORE_NAME               3 (bug)

16          20 LOAD_NAME                4 (property)

17          22 LOAD_CONST               3 (<code object __class__ at 0x7fdf27957100, file "bug.py", line 16>)
           24 LOAD_CONST               4 ('FancyFoo.__class__')
           26 MAKE_FUNCTION            0
           28 CALL_FUNCTION            1
           30 STORE_NAME               5 (__class__)
           32 LOAD_CLOSURE             0 (__class__)
           34 DUP_TOP
           36 STORE_NAME               6 (__classcell__)
           38 RETURN_VALUE

Disassembly of <code object bug at 0x7fdf27957050, file "bug.py", line 13>:
14           0 LOAD_GLOBAL              0 (super)
            2 CALL_FUNCTION            0
            4 POP_TOP
            6 LOAD_CONST               0 (None)
            8 RETURN_VALUE

Disassembly of <code object __class__ at 0x7fdf27957100, file "bug.py", line 16>:
18           0 LOAD_GLOBAL              0 (Foo)
            2 RETURN_VALUE

cfbolz · 2022-12-08T18:46:51Z

ah, of course a slightly simpler way to produce the bug is to call locals() in the class body directly:

class FancyFoo2:
    def bug(self):
        __class__
    __class__ = 12
    print(locals())

print(FancyFoo2.__dict__['__class__'])

produces:

{'__module__': '__main__', '__qualname__': 'FancyFoo2', 'bug': <function FancyFoo2.bug at 0x00007f4baf8574c0>}
Traceback (most recent call last):
  File "bug.py", line 16, in <module>
    print(FancyFoo2.__dict__['__class__'])
KeyError: '__class__'

ionelmc added the type-bug An unexpected behavior, bug, or error label Oct 10, 2022

Tracing a class that defines a custom `class` appears to remove said property under certain circumstances #98148

Tracing a class that defines a custom `class` appears to remove said property under certain circumstances #98148

ionelmc commented Oct 10, 2022

cfbolz commented Dec 8, 2022

cfbolz commented Dec 8, 2022

Tracing a class that defines a custom __class__ appears to remove said property under certain circumstances #98148

Tracing a class that defines a custom __class__ appears to remove said property under certain circumstances #98148

Comments

ionelmc commented Oct 10, 2022

cfbolz commented Dec 8, 2022

cfbolz commented Dec 8, 2022

Tracing a class that defines a custom `class` appears to remove said property under certain circumstances #98148

Tracing a class that defines a custom `class` appears to remove said property under certain circumstances #98148