classification
Title: exception-handling bug in PyString_Format
Type: behavior Stage: patch review
Components: None Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, dmalcolm, pitrou, python-dev, serhiy.storchaka, tromey
Priority: normal Keywords:

Created on 2012-07-31 19:59 by tromey, last changed 2016-04-10 12:27 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
P tromey, 2012-07-31 20:45 patch, including test case review
P tromey, 2012-12-06 17:40 review
Messages (11)
msg167043 - (view) Author: Tom Tromey (tromey) Date: 2012-07-31 19:59
In gdb we supply a class whose nb_int method can throw
an exception.

A user wrote code like this:

    return '%x' % value

... where "value" was an instance of this class.
This caused funny exception behavior later on.
You can see the original report here:

http://sourceware.org/bugzilla/show_bug.cgi?id=14320

I tracked down the odd behavior to this code in
stringobject.c:PyString_Format:

                        iobj = PyNumber_Int(v);
                        if (iobj==NULL) iobj = PyNumber_Long(v);

Here, PyNumber_Int fails and sets the exception.
I think this ought to be cleared before calling
PyNumber_Long.  In our case, PyNumber_Long succeeds,
and the program keeps executing until something happens
to call PyErr_Occurred.

I think this patch ought to fix the problem:

diff -r e0eb7dea245f Objects/stringobject.c
--- a/Objects/stringobject.c	Mon Jul 30 04:07:49 2012 -0700
+++ b/Objects/stringobject.c	Tue Jul 31 13:58:07 2012 -0600
@@ -4489,7 +4489,10 @@
                     }
                     else {
                         iobj = PyNumber_Int(v);
-                        if (iobj==NULL) iobj = PyNumber_Long(v);
+                        if (iobj==NULL) {
+			    PyErr_Clear();
+			    iobj = PyNumber_Long(v);
+			}
                     }
                     if (iobj!=NULL) {
                         if (PyInt_Check(iobj)) {

I haven't written a test case yet; David Malcolm suggested I file
a bug here first.
msg167044 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-07-31 20:03
Hi, and thank you for reporting. The diagnosis looks good, please proceed and add a test case if possible.
msg167050 - (view) Author: Tom Tromey (tromey) Date: 2012-07-31 20:45
Here is a patch that includes a test case.
The test fails before the stringobject.c patch is applied,
and passes after.
msg177019 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2012-12-06 00:52
Wouldn't it be easier to write a Python class that raises an error from __int__() but succeeds in __long__()?
msg177047 - (view) Author: Tom Tromey (tromey) Date: 2012-12-06 17:40
It sure would.
Here's a new patch.
msg177142 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2012-12-08 03:32
Hmm, well I dont't think that test will fail without the patch. You probably need to add a special function to _testcapi to check if an exception is set (PyErr_Occurred()).
msg178829 - (view) Author: Tom Tromey (tromey) Date: 2013-01-02 18:16
It does fail without the patch:

test_format
XXX undetected error
test test_format failed -- Traceback (most recent call last):
  File "/home/tromey/Space/Python/cpython/Lib/test/test_format.py", line 251, in test_format
    def test_exc(formatstr, args, exception, excmsg):
  File "/home/tromey/Space/Python/cpython/Lib/test/test_format.py", line 240, in __int__
    raise TestFailed
TestFailed
msg178832 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-01-02 18:23
New changeset 80cfd4caa726 by Benjamin Peterson in branch '2.7':
call PyErr_Clear() when ignoring error from PyNumber_Int (closes #15516)
http://hg.python.org/cpython/rev/80cfd4caa726
msg178833 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2013-01-02 18:25
Thanks for the patch. For future reference, we use spaces instead of tabs.
msg263081 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-04-09 11:55
This fixed an issue for str, but not for unicode. See issue13410.
msg263137 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-04-10 12:27
New changeset a06654ca0134 by Serhiy Storchaka in branch '2.7':
Issue #13410: Fixed a bug in PyUnicode_Format where it failed to properly
https://hg.python.org/cpython/rev/a06654ca0134
History
Date User Action Args
2016-04-10 12:27:36python-devsetmessages: + msg263137
2016-04-09 11:55:03serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg263081
2013-01-02 18:26:04benjamin.petersonsetstatus: open -> closed
resolution: fixed
2013-01-02 18:25:35benjamin.petersonsetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg178833

stage: resolved -> patch review
2013-01-02 18:23:45python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg178832

resolution: fixed
stage: patch review -> resolved
2013-01-02 18:16:14tromeysetmessages: + msg178829
2012-12-08 03:32:23benjamin.petersonsetmessages: + msg177142
2012-12-06 17:40:42tromeysetfiles: + P

messages: + msg177047
2012-12-06 00:52:08benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg177019
2012-12-05 16:42:00dmalcolmsetstage: patch review
2012-07-31 20:45:30tromeysetfiles: + P

messages: + msg167050
2012-07-31 20:03:58pitrousetnosy: + pitrou
messages: + msg167044
2012-07-31 20:01:29dmalcolmsetnosy: + dmalcolm
2012-07-31 19:59:40tromeycreate