[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Stefan Behnel  added the comment:

Closing again since GH-21528 has been merged in issue 41295.

--
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Stefan Behnel  added the comment:

I pushed PR 21528 with a new proposal. See issue 41295.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Stefan Behnel  added the comment:

> intermediate base type "object" in the hierarchy

Sorry, I meant an intermediate base type "B", which inherits its setattr from 
"object".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Stefan Behnel  added the comment:

The problem in the test added in PR 21473 is that there is an intermediate base 
type "object" in the hierarchy:

class A(type):
def __setattr__(cls, key, value):
type.__setattr__(cls, key, value)

class B:
pass

class C(B, A):
pass

>>> [c for c in C.__mro__]
[, , , , ]

>>> [c.__setattr__ for c in C.__mro__]
[, , , , ]

I think the change to the algorithm there is too broad, it disables much of 
what the check was written for (or against).

Given Guido's second (negative) test case, I think it would also not be correct 
to add "object.__setattr__" to the list of allowed (intermediate) slot methods:

class A(type):
def __setattr__(cls, key, value):
object.__setattr__(cls, key, value)   # this should fail!

class B:
pass

class C(B, A):
pass

It's difficult to turn this into an algorithm. Is the MRO really the place to 
look? For "A", we're only really interested in the C-level bases, aren't we?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Guido van Rossum]

Guido van Rossum  added the comment:

@Stefan Do you agree that the fix proposed in PR 21473 needs to be made?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Guido van Rossum]

Guido van Rossum  added the comment:

Reopening because there appears to be a problem with the fix (see PR 21473).

--
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[David Caro]

Change by David Caro :


--
nosy: +David Caro
nosy_count: 5.0 -> 6.0
pull_requests: +20618
pull_request: https://github.com/python/cpython/pull/21473

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Guido van Rossum]

Guido van Rossum  added the comment:

You're welcome. Hope this helps Cpython users.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Stefan Behnel  added the comment:

Fixed in 3.8+. Closing.
Thanks for the feedback.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Stefan Behnel  added the comment:


New changeset 8912c182455de83e27d5c120639ec91b18247913 by scoder in branch 
'3.8':
bpo-39960: Allow heap types in the "Carlo Verre" hack check that override 
"tp_setattro()" (GH-21092) (GH-21339)
https://github.com/python/cpython/commit/8912c182455de83e27d5c120639ec91b18247913


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Change by Stefan Behnel :


--
pull_requests: +20487
pull_request: https://github.com/python/cpython/pull/21339

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[miss-islington]

miss-islington  added the comment:


New changeset bfec674254ea22ef9c0c335587eb65683f4145c7 by Miss Islington (bot) 
in branch '3.9':
bpo-39960: Allow heap types in the "Carlo Verre" hack check that override 
"tp_setattro()" (GH-21092)
https://github.com/python/cpython/commit/bfec674254ea22ef9c0c335587eb65683f4145c7


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Guido van Rossum]

Guido van Rossum  added the comment:

Stefan can you do the 3.8 backport?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[miss-islington]

miss-islington  added the comment:


New changeset 148f32913573c29250dfb3f0d079eb8847633621 by scoder in branch 
'master':
bpo-39960: Allow heap types in the "Carlo Verre" hack check that override 
"tp_setattro()" (GH-21092)
https://github.com/python/cpython/commit/148f32913573c29250dfb3f0d079eb8847633621


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[miss-islington]

Change by miss-islington :


--
pull_requests: +20435
pull_request: https://github.com/python/cpython/pull/21288

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Stefan Behnel  added the comment:

I think we missed the train for fixing 3.7 (which was questionable anyway), but 
I added a test, so it's ready for merging into 3.8+ (minus merge conflicts for 
the test in 3.8, probably).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Change by Stefan Behnel :


--
versions:  -Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Change by Stefan Behnel :


--
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Stefan Behnel  added the comment:

I chose to go through the MRO, which takes multiple inheritance into account.

--
stage: patch review -> 

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Change by Stefan Behnel :


--
keywords: +patch
pull_requests: +20258
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/21092

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Petr Viktorin]

Petr Viktorin  added the comment:

> do we still only support single inheritance at the C level?

Not any more. Heap types may have multiple bases, and they can be created at 
the C level (since Python 3.2, PEP 384).

>  So, I think we should do something like walking up the hierarchy to find the 
> C function in it that is currently being called, and then check if it's the 
> one we would expect or if a subclass defines a different one. The current 
> check does not bother to search and just assumes that it knows which 
> (super)type defines the right function to call.

Should we be bold and skip the check for heap types?
That would mean that when/if `str` is converted to a heap type, you could do 
`object.__delattr__(str, "lower")`. That would break your Python, but only at 
the Python level (similar to things like `import builtins; del 
builtins.__build_class__`).
Heap types are not shared between interpreters, so that reason is gone. But 
Guido's [2003 mail] suggests there are other reasons to prevent changing 
built-in types. What are they?

[2003 mail] https://mail.python.org/pipermail/python-dev/2003-April/034535.html

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Guido van Rossum]

Guido van Rossum  added the comment:

That sounds like the right thing to do. You should be able to recognize pure 
Python __setattr__ implementations by the presence of slot_tp_setattro in the 
tp_setattro slot. (Likewise for __delattr__.)

It's been really long since I looked at this -- do we still only support single 
inheritance at the C level? Because otherwise there would be another backdoor.

There's also tp_setattr (no 'o') but I think that's basically unused, and we 
ignored that in 2003 so we should be able to ignore it now. :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Stefan Behnel  added the comment:

This SO answer by Martijn Pieters explains the background:

https://stackoverflow.com/a/44854477

and links to the original python-dev discussion:

https://mail.python.org/pipermail/python-dev/2003-April/034535.html

The current implementation checks that the function being called is the one 
defined in the first non-heap type up the hierarchy. As long as heap types are 
only Python types, this is the first builtin/native/C-implemented (super)type. 
With extension types as heap types, however, it's no longer necessarily the 
type that defines the correct slot function.

IMHO, Greg Ewing gives the right direction here:

https://mail.python.org/pipermail/python-dev/2003-April/034569.html

> Providing some way for objects to prevent superclass
> methods from being called on them when they're not looking

So, I think we should do something like walking up the hierarchy to find the C 
function in it that is currently being called, and then check if it's the one 
we would expect or if a subclass defines a different one. The current check 
does not bother to search and just assumes that it knows which (super)type 
defines the right function to call.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Guido van Rossum]

Guido van Rossum  added the comment:

See also test_carloverre() in test_descr.py. I don't recall exactly what this 
was about, but that test is definitely relevant.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Stefan Behnel]

Stefan Behnel  added the comment:

I ran into this, too. I agree that the "hackcheck" loop on heap types is 
probably wrong.

https://github.com/python/cpython/blob/04fc4f2a46b2fd083639deb872c3a3037fdb47d6/Objects/typeobject.c#L5947-L5977

It was written at a time (Py2.3?) when (practically) only Python implemented 
types were heap types, not extension types. I think what it tried to do was to 
find the builtin base type of a Python type, and check that no-one played 
tricks on the C slot functions of that C implemented type. With extension types 
implemented as heap types, having a different slot function in there is a 
perfectly valid thing.

I'll call in a couple of people since I'm also not sure how to fix the 
"hackcheck".

I'll also leave Py3.7 in the list of affected Python versions since we still 
have a short week before its final non-secfix-release. :)

--
nosy: +gvanrossum, petr.viktorin, scoder
type:  -> behavior
versions: +Python 3.10, Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39960] Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is broken (hackcheck too eager?)

[Matthias Braun]

New submission from Matthias Braun :

This is about an extension type created via `PyType_FromSpec` that overrides 
`tp_setattro` (minimal example attached). In this case cpython does not let me 
grab and use the `__setattr__` function "manually". Example:

```
>>> import demo
>>> mytype_setattr = demo.MyType.__setattr__
>>> i = demo.MyType()
>>> mytype_setattr(i, "foo", "bar")
Traceback (most recent call last):
  File "", line 1, in 
TypeError: can't apply this __setattr__ to object object
```

I suspect this is related to the loop at the beginning of typeobject.c / 
hackcheck() that skips over types with Py_TPFLAGS_HEAPOBJECT. (Though removing 
the loop breaks things like the enum module).

--
components: C API
files: demomodule.zip
messages: 364123
nosy: Matthias Braun
priority: normal
severity: normal
status: open
title: Using typename.__setattr__ in extension type with Py_TPFLAGS_HEAPTYPE is 
broken (hackcheck too eager?)
versions: Python 3.7
Added file: https://bugs.python.org/file48974/demomodule.zip

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com