[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Mark Dickinson]

New submission from Mark Dickinson :

Lines 3884 and 3890 of Objects/typeobject.c (trunk, r80782), which check for a 
subclass overriding __eq__ (or __cmp__) but not __hash__, call PyErr_WarnPy3k 
but don't check the return value.  [This was reported when compiling Python 
with clang.] This can lead to an "XXX undetected error", if the warning 
actually raised an exception:

newton:trunk dickinsm$ ./python.exe -3
Python 2.7b1+ (trunk:80783, May  5 2010, 15:25:42) 
[GCC 4.2.1 (Apple Inc. build 5659)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import warnings
[35022 refs]
>>> warnings.filterwarnings("error")
[35046 refs]
>>> class A(object):
... def __eq__(self, other):
... return False
... 
XXX undetected error
Traceback (most recent call last):
  File "", line 1, in 
DeprecationWarning: Overriding __eq__ blocks inheritance of __hash__ in 3.x
[35139 refs]

Nick, I think this came from a checkin of yours: r65642, related to issue 2235. 
 Any suggestions about how to fix it?  It's a bit tricky, since the function 
the check occurs in (inherit_slots) has return type 'void'.  Is it okay if I 
change inherit_slots to return an int here?  If so, I'll produce a patch.

Also, can we remove the check related to __hash__ and __cmp__?  With __cmp__ 
gone in 3.1, and 3.0 considered defunct, I don't think it's relevant any more.

--
messages: 105041
nosy: mark.dickinson, ncoghlan
priority: normal
severity: normal
status: open
title: Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c
versions: Python 2.6, Python 2.7, Python 3.1, Python 3.2

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Mark Dickinson]

Mark Dickinson  added the comment:

Hmm.  Fixing this isn't easy:  a simple 'if (inherit_slots(...) < 0) goto 
error;' produces a huge reference leak in PyType_Ready.

--

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Brett Cannon]

Brett Cannon  added the comment:

I just came across the warning myself (after ignoring all the PyType_INIT() 
warnings; what to do about those?) and came to the same conclusion: it's a pain 
to fix.

One option is to do a PyErr_Occurred() check at inherit_slots() call sites. 
Would that mitigate the leak? If I remember correctly inherit_slots() is not 
called constantly (only when a type is being created?) so having the expensive 
PyErr_Occurred() call should not hurt performance.

--
nosy: +brett.cannon

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Nick Coghlan]

Nick Coghlan  added the comment:

I haven't looked at this in a while, but I do remember it was hard to get 
working at all without a ridiculous number of false alarms - type 
initialisation isn't the most straightforward thing in the world.

Agreed the warning for __cmp__ should just go away - I think we nuked the 
actual check in 3.1 when __cmp__ was removed, but must have missed this at the 
time.

For the refleak, I'm wondering if you may be seeing a problem with statically 
allocated type objects. type_new does a DECREF on the allocated object if 
PyType_Ready fails, so everything should be getting cleaned up at that point, 
but that won't happen for a statically allocated type.

Looking at type_clear, I suggest sticking a "Py_CLEAR(type->tp_mro);" in before 
the goto error line and see if that eliminates the leak. If that actually 
works, then I'll have my doubts about the correctness of the other "goto error" 
lines in PyType_Ready that happen after the call to mro_internal().

--

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Nick Coghlan]

Nick Coghlan  added the comment:

(Not sure how relevant my second last paragraph is - I meant to take that out 
after noticing the MRO details).

--

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Mark Dickinson]

Mark Dickinson  added the comment:

I also have my doubts about the other 'goto error' lines in PyType_Ready, but I 
haven't figured out how to trigger those gotos in normal code.

Trying to figure out how to clean up properly on error in PyType_Ready is 
making my head hurt.

As a quick fix, we could simply do a PyErr_Clear() if the PyErr_WarnPy3k 
returns -1;  this clearly isn't right, but at least it gets rid of the "XXX 
undetected error".

--

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Mark Dickinson]

Mark Dickinson  added the comment:

Removed the __cmp__ warning in r81736, and added a PyErr_Clear() for the __eq__ 
warning in r81740.  I'll leave this open in case anyone wants to figure out how 
to propagate the warning exception properly.

--

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Mark Dickinson]

Mark Dickinson  added the comment:

Not sure why I added 3.1 and 3.2 to the versions.

--
versions:  -Python 3.1, Python 3.2

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Brett Cannon]

Brett Cannon added the comment:

A quick search through typeobject.c in 2.7 didn't show any PyErr_WarnPy3k calls 
that didn't have their return value checked or propagated.

--
resolution:  -> out of date
status: open -> closed

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Mark Dickinson]

Mark Dickinson added the comment:

There's still a case where the return value isn't properly propagated:  it's 
the one marked by an 'XXX' in the source.

--

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Mark Dickinson]

Mark Dickinson added the comment:

It's line 3912 in the current source.  The warning is always cleared, which is 
the wrong thing to do if warnings should be turned into exceptions.

--

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Brett Cannon]

Brett Cannon added the comment:

I can't find it, Mark, after searching through every XXX marker in a 2.7 
checkout of Objects/typeobject.c .

--

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Mark Dickinson]

Mark Dickinson added the comment:

To clarify, here's the bug:  the following code should raise an exception, but 
doesn't:

iwasawa:cpython mdickinson$ ./python.exe -3
Python 2.7.3+ (2.7:333fe4c4897a, Nov 17 2012, 18:01:00) 
[GCC 4.2.1 (Apple Inc. build 5664)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import warnings; warnings.filterwarnings("error")
>>> class A(object):
... def __eq__(self, other):
... return False
... 


Without "warnings.filterwarnings("error")", the warning gets issued as expected:

iwasawa:cpython mdickinson$ ./python.exe -3
Python 2.7.3+ (2.7:333fe4c4897a, Nov 17 2012, 18:01:00) 
[GCC 4.2.1 (Apple Inc. build 5664)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> class A(object):
... def __eq__(self, object):
... return False
... 
__main__:1: DeprecationWarning: Overriding __eq__ blocks inheritance of 
__hash__ in 3.x

Brett, is it okay to re-open this?  Perhaps a change of title would help?  Or I 
can open a new issue for the remaining problem, if you think that's better.

--

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Brett Cannon]

Brett Cannon added the comment:

Go ahead and reopen. I just couldn't find the lines anymore when I closed
it.
On Nov 17, 2012 3:07 PM, "Mark Dickinson"  wrote:

>
> Mark Dickinson added the comment:
>
> To clarify, here's the bug:  the following code should raise an exception,
> but doesn't:
>
> iwasawa:cpython mdickinson$ ./python.exe -3
> Python 2.7.3+ (2.7:333fe4c4897a, Nov 17 2012, 18:01:00)
> [GCC 4.2.1 (Apple Inc. build 5664)] on darwin
> Type "help", "copyright", "credits" or "license" for more information.
> >>> import warnings; warnings.filterwarnings("error")
> >>> class A(object):
> ... def __eq__(self, other):
> ... return False
> ...
>
>
> Without "warnings.filterwarnings("error")", the warning gets issued as
> expected:
>
> iwasawa:cpython mdickinson$ ./python.exe -3
> Python 2.7.3+ (2.7:333fe4c4897a, Nov 17 2012, 18:01:00)
> [GCC 4.2.1 (Apple Inc. build 5664)] on darwin
> Type "help", "copyright", "credits" or "license" for more information.
> >>> class A(object):
> ... def __eq__(self, object):
> ... return False
> ...
> __main__:1: DeprecationWarning: Overriding __eq__ blocks inheritance of
> __hash__ in 3.x
>
> Brett, is it okay to re-open this?  Perhaps a change of title would help?
>  Or I can open a new issue for the remaining problem, if you think that's
> better.
>
> --
>
> ___
> Python tracker 
> 
> ___
>

--

___
Python tracker 

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

[issue8627] Unchecked PyErr_WarnPy3k return value in Objects/typeobject.c

[Mark Dickinson]

Changes by Mark Dickinson :


--
resolution: out of date -> 
status: closed -> open

___
Python tracker 

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