
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2016-07-21 05:36 byxiang.zhang, last changed2022-04-11 14:58 byadmin. This issue is nowclosed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| overflow_check_in_PySequence_Tuple.patch | xiang.zhang,2016-07-21 05:36 | review | ||
| Messages (5) | |||
|---|---|---|---|
| msg270909 -(view) | Author: Xiang Zhang (xiang.zhang)*![]() | Date: 2016-07-21 05:36 | |
Overflow check in PySequence_Tuple relies on undefined behaviour, fix it. | |||
| msg271062 -(view) | Author: Martin Panter (martin.panter)*![]() | Date: 2016-07-23 06:02 | |
Hmm maybe this patch is okay. We are assuming size_t will fit more than PY_SSIZE_T_MAX.The alternatives I can think of would be equally ugly:/* Risks loss of precision, e.g. 64 bit integer from floating point */if (n < (Py_ssize_t)(PY_SSIZE_T_MAX / 1.25) - 10))/* PY_SSIZE_T_MAX * 4/5 - 10 without loss of precision or overflowing */if (n < PY_SSIZE_T_MAX / 5 * 4 + PY_SSIZE_T_MAX % 5 * 4 / 5 - 10) | |||
| msg271076 -(view) | Author: Xiang Zhang (xiang.zhang)*![]() | Date: 2016-07-23 13:40 | |
I'd prefer the size_t method. The others seems to make the logic not clear. I've seen some codes using size_t to do overflow checking, such ashttps://hg.python.org/cpython/file/tip/Python/bltinmodule.c#l1954. There are more if you use a simple grep. So I think the logic is okay. | |||
| msg271133 -(view) | Author: Martin Panter (martin.panter)*![]() | Date: 2016-07-24 06:31 | |
I don’t accept that the bltinmodule.c code is similar to your patch. It gets a size_t from calling strlen() on a string that potentially comes from outside Python, so it is definitely valid to check for PY_SSIZE_T_MAX.However I did find PyByteArray_Resize() (revision1590c594550e), where this technique of calculating in size_t and then checking for overflow is used. And also in your favour is the definition inInclude/pyport.h which currently guarantees size_t can store up to double PY_SSIZE_T_MAX:/* Largest positive value of type Py_ssize_t. */#define PY_SSIZE_T_MAX ((Py_ssize_t)(((size_t)-1)>>1))So I am convinced there should be no real problem with your patch. | |||
| msg271222 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2016-07-25 03:44 | |
New changesetad3762227655 by Martin Panter in branch '3.5':Issue#27581: Don’t rely on overflow wrapping in PySequence_Tuple()https://hg.python.org/cpython/rev/ad3762227655New changeset8f84942a0e40 by Martin Panter in branch 'default':Issue#27581: Merge overflow fix from 3.5https://hg.python.org/cpython/rev/8f84942a0e40New changeset55b6e51b878b by Martin Panter in branch '2.7':Issue#27581: Don’t rely on overflow wrapping in PySequence_Tuple()https://hg.python.org/cpython/rev/55b6e51b878b | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:34 | admin | set | github: 71768 |
| 2016-07-26 02:12:13 | martin.panter | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2016-07-25 03:44:28 | python-dev | set | nosy: +python-dev messages: +msg271222 |
| 2016-07-24 06:31:22 | martin.panter | set | messages: +msg271133 |
| 2016-07-23 13:40:35 | xiang.zhang | set | messages: +msg271076 |
| 2016-07-23 06:02:44 | martin.panter | set | messages: +msg271062 stage: patch review |
| 2016-07-21 05:36:48 | xiang.zhang | create | |