[issue27581] Fix overflow check in PySequence_Tuple

[Martin Panter]

Changes by Martin Panter :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27581] Fix overflow check in PySequence_Tuple

[Roundup Robot]

Roundup Robot added the comment:

New changeset ad3762227655 by Martin Panter in branch '3.5':
Issue #27581: Don’t rely on overflow wrapping in PySequence_Tuple()
https://hg.python.org/cpython/rev/ad3762227655

New changeset 8f84942a0e40 by Martin Panter in branch 'default':
Issue #27581: Merge overflow fix from 3.5
https://hg.python.org/cpython/rev/8f84942a0e40

New changeset 55b6e51b878b 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

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27581] Fix overflow check in PySequence_Tuple

[Martin Panter]

Martin Panter added the comment:

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() (revision 1590c594550e), where this 
technique of calculating in size_t and then checking for overflow is used. And 
also in your favour is the definition in Include/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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27581] Fix overflow check in PySequence_Tuple

[Xiang Zhang]

Xiang Zhang added the comment:

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 as 
https://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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27581] Fix overflow check in PySequence_Tuple

[Martin Panter]

Martin Panter added the comment:

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)

--
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27581] Fix overflow check in PySequence_Tuple

[Xiang Zhang]

New submission from Xiang Zhang:

Overflow check in PySequence_Tuple relies on undefined behaviour, fix it.

--
components: Interpreter Core
files: overflow_check_in_PySequence_Tuple.patch
keywords: patch
messages: 270909
nosy: martin.panter, serhiy.storchaka, xiang.zhang
priority: normal
severity: normal
status: open
title: Fix overflow check in PySequence_Tuple
type: enhancement
versions: Python 2.7, Python 3.5, Python 3.6
Added file: 
http://bugs.python.org/file43806/overflow_check_in_PySequence_Tuple.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com