classification
Title: undefined behaviour: signed integer overflow in threadmodule.c
Type: behavior Stage: patch review
Components: Extension Modules Versions: Python 3.8, Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: ZackerySpytz, martin.panter, p-ganssle, pitrou, vstinner
Priority: normal Keywords: patch

Created on 2018-05-24 08:39 by pitrou, last changed 2019-04-15 15:09 by p-ganssle.

Pull Requests
URL Status Linked Edit
PR 12729 open ZackerySpytz, 2019-04-08 14:34
Messages (7)
msg317545 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2018-05-24 08:39
Modules/_threadmodule.c:52:47: runtime error: signed integer overflow: 2387971499048 + 9223372036000000000 cannot be represented in type 'long'
msg317554 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2018-05-24 11:00
Looks like this is what my thread.patch was fixing in <https://bugs.python.org/issue1621#msg271057>. You’re welcome to use my patch, but I won’t have time to work on it myself.
msg317556 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-05-24 11:18
> Modules/_threadmodule.c:52:47: runtime error: signed integer overflow: 2387971499048 + 9223372036000000000 cannot be represented in type 'long'

How do you reproduce the issue? The thread module should limit the maximum timeout to PY_TIMEOUT_MAX. Maybe PY_TIMEOUT_MAX is too big?
msg339644 - (view) Author: Zackery Spytz (ZackerySpytz) * Date: 2019-04-08 14:37
I've created a PR based on Martin Panter's patch.
msg340184 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2019-04-14 06:05
Victor, if you run the test suite, one of the test cases should trigger the overflow. I used to compile with Undefined Behaviour Sanitizer to print messages when these errors occur; see <https://bugs.python.org/issue1621#msg271118> for my setup at the time. I presume Antoine did something similar.

I do not remember, but suspect the test case might be the following lines of “BaseLockTests.test_timeout” in Lib/test/lock_tests.py, testing a fraction of a second less than PY_TIMEOUT_MAX:

# TIMEOUT_MAX is ok
lock.acquire(timeout=TIMEOUT_MAX)

Perhaps reducing PY_TIMEOUT_MAX by a few centuries would be one way to avoid the problem. In my patch I avoided the problem by rearranging the arithmetic, so that the timeout value is only compared and reduced, never added.
msg340255 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-04-15 10:28
In short, a+b can overflow, but a-b cannot?
msg340284 - (view) Author: Paul Ganssle (p-ganssle) * (Python triager) Date: 2019-04-15 15:09
> In short, a+b can overflow, but a-b cannot?

I think it's more that by always checking the elapsed time against `now() - starttime`, you never need to represent the time at which the timeout should happen - which may be so far in the future that it causes a signed overflow.
History
Date User Action Args
2019-04-15 15:09:11p-gansslesetmessages: + msg340284
2019-04-15 10:28:13vstinnersetmessages: + msg340255
2019-04-14 06:05:46martin.pantersetmessages: + msg340184
2019-04-08 15:16:23p-gansslesetnosy: + p-ganssle
2019-04-08 14:37:00ZackerySpytzsetversions: - Python 3.6
nosy: + ZackerySpytz

messages: + msg339644

components: + Extension Modules, - Library (Lib)
2019-04-08 14:34:15ZackerySpytzsetkeywords: + patch
stage: patch review
pull_requests: + pull_request12652
2018-05-24 11:18:39vstinnersetnosy: + vstinner
messages: + msg317556
2018-05-24 11:00:27martin.pantersetnosy: + martin.panter
messages: + msg317554
2018-05-24 08:39:55pitroucreate