| msg188558 - (view) |
Author: Delhallt (delhallt) |
Date: 2013-05-06 15:06 |
| I encounted exactly the same issue http://bugs.python.org/issue923315 with test_asyncore, test_asynchat and test_poll. On AIX6.1, POLLNVAL=0x8000=SHRT_MIN=SHRT_MAX+1 (on 2 bytes) and parsing events with PyArg_ParseTuple as a signed short 'h' do not work, i.e "OverflowError: signed short integer is greater than maximum" occurs. I changed 'h' to 'H' in the attached patch, and delete associated Overflow test. Perhaps, they're a better way to handle that ? |
|
|
| msg188612 - (view) |
Author: David Edelsohn (David.Edelsohn) * |
Date: 2013-05-07 00:25 |
| That's the way AIX allocated the bits. It's nice to check for overflow, but I think the test is imposing more semantics on the mask bits than POSIX requires. It just happens that the GLibc allocation of bits works out okay. |
|
|
| msg202692 - (view) |
Author: Michael Haubenwallner (haubi) * |
Date: 2013-11-12 14:50 |
| This is a regression since 2.7.4 because of http://bugs.python.org/issue15989. As the 'events' and 'revents' members of 'struct pollfd' both are bitfields, the question actually is why they need to be signed at all. Additionally: I'm wondering why poll_modify() and internal_devpoll_register() haven't been updated along issue#15989, as both still have 'i' for the 'events' arg. Attached patch (for hg-tip) changes the 'events' for 'struct pollfd' to be generally treated as unsigned short bitfield. Not that I know of one, but the &0xffff hack may break on platforms where 'short' is not 16bits - casting to (unsigned short) instead feels more save. Additional question: Would it make sense to have the 'BHIkK' types check for overflow? Thank you! |
|
|
| msg206038 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2013-12-13 10:00 |
| The disadvantage of 'H' is that it never raises OverflowError. The simplest solution is just revert a part of the patch for . |
|
|
| msg206039 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-12-13 10:12 |
| New changeset 08c95dd68cfc by Serhiy Storchaka in branch '3.3': Issue #17919: select.poll.poll() again works with poll.POLLNVAL on AIX. http://hg.python.org/cpython/rev/08c95dd68cfc New changeset e78743e03c8f by Serhiy Storchaka in branch 'default': Issue #17919: select.poll.poll() again works with poll.POLLNVAL on AIX. http://hg.python.org/cpython/rev/e78743e03c8f New changeset 64f32a31cd49 by Serhiy Storchaka in branch '2.7': Issue #17919: select.poll.poll() again works with poll.POLLNVAL on AIX. http://hg.python.org/cpython/rev/64f32a31cd49 |
|
|
| msg206040 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-12-13 10:13 |
| > The disadvantage of 'H' is that it never raises OverflowError. The correct fix is to write a new parser just for this function. See for example uint_converter() in Modules/zlibmodule.c. I would prefer to add such new parser to Python/getargs.c directly, but I was not motivated when I fixed #18294 (integer overflow in the zlib module). |
|
|
| msg206046 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-12-13 10:21 |
| > The simplest solution is just revert a part of the patch for . The revert reintroduced the integer overflow: self->ufds[i].events = (short)PyLong_AsLong(value); |
|
|
| msg206052 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-12-13 10:56 |
| Here is a patch which uses special converter. |
|
|
| msg206054 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-12-13 11:00 |
| + if (uval > USHRT_MAX) { + PyErr_SetString(PyExc_OverflowError, + "Python int too large for C unsigned int"); The message should be "C unsigned short". The unit tests don't check that USHRT_MAX value is accepted. |
|
|
| msg206056 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-12-13 11:04 |
| With poll_events_mask_overflow.patch, the following hack can maybe be removed? /* The &0xffff is a workaround for AIX. 'revents' is a 16-bit short, and IBM assigned POLLNVAL to be 0x8000, so the conversion to int results in a negative number. See SF bug #923315. */ num = PyLong_FromLong(self->ufds[i].revents & 0xffff); |
|
|
| msg206191 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-12-14 16:38 |
| > The message should be "C unsigned short". Thanks. > The unit tests don't check that USHRT_MAX value is accepted. And shouldn't. Not all values make sense. Meaning of USHRT_MAX is platform depended. > With poll_events_mask_overflow.patch, the following hack can maybe be removed? No. Otherwise the result can be negative. |
|
|
| msg206194 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-12-14 17:19 |
| New changeset 9bee6cc30db7 by Serhiy Storchaka in branch '2.7': Issue #17919: Fixed integer overflow in the eventmask parameter. http://hg.python.org/cpython/rev/9bee6cc30db7 New changeset 87bbe810e4e7 by Serhiy Storchaka in branch '3.3': Issue #17919: Fixed integer overflow in the eventmask parameter. http://hg.python.org/cpython/rev/87bbe810e4e7 New changeset 2fbb3c77f157 by Serhiy Storchaka in branch 'default': Issue #17919: Fixed integer overflow in the eventmask parameter. http://hg.python.org/cpython/rev/2fbb3c77f157 |
|
|
| msg206201 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-12-14 20:08 |
| Thank you Delhallt for your report. |
|
|
| msg206264 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2013-12-15 22:55 |
| Hi, this happens on the OpenIndiana bot: http://buildbot.python.org/all/builders/x86%20OpenIndiana%203.3/builds/1259/steps/test/logs/stdio test_devpoll1 (test.test_devpoll.DevPollTests) ... ok test_events_mask_overflow (test.test_devpoll.DevPollTests) ... ERROR test_timeout_overflow (test.test_devpoll.DevPollTests) ... ok ====================================================================== ERROR: test_events_mask_overflow (test.test_devpoll.DevPollTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/export/home/buildbot/32bits/3.3.cea-indiana-x86/build/Lib/test/test_devpoll.py", line 96, in test_events_mask_overflow self.assertRaises(OverflowError, pollster.register, 0, USHRT_MAX + 1) NameError: global name 'USHRT_MAX' is not defined ---------------------------------------------------------------------- Ran 3 tests in 0.006s FAILED (errors=1) test test_devpoll failed |
|
|
| msg206265 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2013-12-15 22:58 |
| I have fixed the issue in http://hg.python.org/cpython/rev/039306b45230 |
|
|
| msg206286 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-12-16 09:23 |
| > I have fixed the issue in http://hg.python.org/cpython/rev/039306b45230 You forget 2.7 and 3.3 branches. |
|
|
| msg206295 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-12-16 12:28 |
| New changeset c42647d76bd1 by Christian Heimes in branch '3.3': Issue #17919: add missing import of USHRT_MAX http://hg.python.org/cpython/rev/c42647d76bd1 New changeset 1f3f4147c35e by Christian Heimes in branch 'default': Issue #17919: add missing import of USHRT_MAX http://hg.python.org/cpython/rev/1f3f4147c35e |
|
|
| msg206308 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-12-16 13:35 |
| Thank you Christian. |
|
|