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 issue15989.
|
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 issue15989.
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.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:45 | admin | set | github: 62119 |
2013-12-16 13:35:56 | serhiy.storchaka | set | messages:
+ msg206308 |
2013-12-16 12:28:59 | python-dev | set | messages:
+ msg206295 |
2013-12-16 09:23:39 | vstinner | set | messages:
+ msg206286 |
2013-12-15 22:58:44 | christian.heimes | set | nosy:
+ christian.heimes messages:
+ msg206265
|
2013-12-15 22:55:55 | skrah | set | nosy:
+ skrah messages:
+ msg206264
|
2013-12-14 20:08:37 | serhiy.storchaka | set | messages:
+ msg206201 |
2013-12-14 20:08:00 | serhiy.storchaka | set | status: open -> closed resolution: fixed stage: resolved |
2013-12-14 17:19:16 | python-dev | set | messages:
+ msg206194 |
2013-12-14 16:38:27 | serhiy.storchaka | set | messages:
+ msg206191 |
2013-12-13 11:04:18 | vstinner | set | messages:
+ msg206056 |
2013-12-13 11:00:32 | vstinner | set | messages:
+ msg206054 |
2013-12-13 10:56:30 | serhiy.storchaka | set | files:
+ poll_events_mask_overflow.patch
messages:
+ msg206052 versions:
+ Python 3.3 |
2013-12-13 10:21:23 | vstinner | set | messages:
+ msg206046 |
2013-12-13 10:13:02 | vstinner | set | nosy:
+ vstinner messages:
+ msg206040
|
2013-12-13 10:12:08 | python-dev | set | nosy:
+ python-dev messages:
+ msg206039
|
2013-12-13 10:00:33 | serhiy.storchaka | set | assignee: serhiy.storchaka messages:
+ msg206038 versions:
- Python 3.2, Python 3.3, Python 3.5 |
2013-11-12 14:53:57 | vstinner | set | nosy:
+ serhiy.storchaka
|
2013-11-12 14:50:36 | haubi | set | versions:
+ Python 3.2, Python 3.3, Python 3.4, Python 3.5 |
2013-11-12 14:50:12 | haubi | set | files:
+ python-tip-unsigned-pollfd-events.patch nosy:
+ haubi messages:
+ msg202692
|
2013-05-07 00:25:13 | David.Edelsohn | set | messages:
+ msg188612 |
2013-05-06 20:26:13 | pitrou | set | nosy:
+ David.Edelsohn
|
2013-05-06 15:06:57 | delhallt | create | |