Repository navigation
undefined behaviour: signed integer overflow in threadmodule.c #77813
Description
| BPO | 33632 |
|---|---|
| Nosy | @pitrou, @vstinner, @vadmium, @pganssle, @ZackerySpytz, @hongweipeng |
| PRs | |
| Superseder |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2021-12-06.13:26:08.534>
created_at = <Date 2018-05-24.08:39:55.148>
labels = ['extension-modules', '3.8', 'type-bug', '3.7']
title = 'undefined behaviour: signed integer overflow in threadmodule.c'
updated_at = <Date 2021-12-06.13:26:08.533>
user = 'https://github.com/pitrou'bugs.python.org fields:
activity = <Date 2021-12-06.13:26:08.533>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-12-06.13:26:08.534>
closer = 'vstinner'
components = ['Extension Modules']
creation = <Date 2018-05-24.08:39:55.148>
creator = 'pitrou'
dependencies = []
files = []
hgrepos = []
issue_num = 33632
keywords = ['patch']
message_count = 9.0
messages = ['317545', '317554', '317556', '339644', '340184', '340255', '340284', '407712', '407800']
nosy_count = 6.0
nosy_names = ['pitrou', 'vstinner', 'martin.panter', 'p-ganssle', 'ZackerySpytz', 'hongweipeng']
pr_nums = ['12729']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = '41710'
type = 'behavior'
url = 'https://bugs.python.org/issue33632'
versions = ['Python 3.7', 'Python 3.8']
Activity
pitrou commentedon May 24, 2018
Modules/_threadmodule.c:52:47: runtime error: signed integer overflow: 2387971499048 + 9223372036000000000 cannot be represented in type 'long'
vadmium commentedon May 24, 2018
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.
vstinner commentedon May 24, 2018
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?
ZackerySpytz commentedon Apr 8, 2019
I've created a PR based on Martin Panter's patch.
vadmium commentedon Apr 14, 2019
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:
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.
vstinner commentedon Apr 15, 2019
In short, a+b can overflow, but a-b cannot?
pganssle commentedon Apr 15, 2019
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.hongweipeng commentedon Dec 5, 2021
I think PR #28674 has resolved this issue.
vstinner commentedon Dec 6, 2021
You're right.
_threadmodule.c now uses _PyDeadline_Init() which calls _PyTime_Add(), and _PyTime_Add() prevents integer overflows; Extract of its implementation: