[issue34397] remove redundant overflow checks in tuple and list implementations

[Cheryl Sabella]

Change by Cheryl Sabella :


--
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

[issue34397] remove redundant overflow checks in tuple and list implementations

[miss-islington]

miss-islington  added the comment:


New changeset e682b26a6bc6d3db1a44d82db09d26224e82ccb5 by Sergey Fedoseev in 
branch 'master':
bpo-34397: Remove redundant overflow checks in list and tuple implementation. 
(GH-8757)
https://github.com/python/cpython/commit/e682b26a6bc6d3db1a44d82db09d26224e82ccb5


--
nosy: +miss-islington

___
Python tracker 

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

[issue34397] remove redundant overflow checks in tuple and list implementations

[Windson Yang]

Windson Yang  added the comment:

I reviewed the patch months ago, maybe we need a core developer review this 
path?

--

___
Python tracker 

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

[issue34397] remove redundant overflow checks in tuple and list implementations

[Tim Peters]

Tim Peters  added the comment:

Because the behavior of signed integer overflow isn't defined in C.  Picture a 
3-bit integer type, where the maximum value of the signed integer type is 3.  
3+3 has no defined result.  Cast them to the unsigned flavor of the integer 
type, though, and the result is defined to be 6.

--

___
Python tracker 

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

[issue34397] remove redundant overflow checks in tuple and list implementations

[Windson Yang]

Windson Yang  added the comment:

Hello, Tim Peters. I wonder why we need to add size_t here:

assert((size_t)Py_SIZE(a) + (size_t)Py_SIZE(b) <= (size_t)PY_SSIZE_T_MAX);

AFAIK, PY_SSIZE_T_MAX = ((Py_ssize_t)(((size_t)-1)>>1)) which is signed, either 
Py_SIZE(a) or Py_SIZE(b) is a positive number. Why we need to concast them to 
size_t, Thank you?

--
nosy: +Windson Yang

___
Python tracker 

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

[issue34397] remove redundant overflow checks in tuple and list implementations

[Tim Peters]

Tim Peters  added the comment:

Bah - the relevant thing to assert is really

assert((size_t)Py_SIZE(a) + (size_t)Py_SIZE(b) <= (size_t)PY_SSIZE_T_MAX);

C sucks ;-)

--

___
Python tracker 

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

[issue34397] remove redundant overflow checks in tuple and list implementations

[Tim Peters]

Tim Peters  added the comment:

I agree there's pointless code now, but don't understand why the patch replaces 
it with mysterious asserts.  For example, what's the point of this?

assert(Py_SIZE(a) <= PY_SSIZE_T_MAX / sizeof(PyObject*));
assert(Py_SIZE(b) <= PY_SSIZE_T_MAX / sizeof(PyObject*));

The invariant that needs to be maintained is that the number of elements is no 
larger than PY_SSIZE_T_MAX, so the _relevant_ thing to assert is:

assert(Py_SIZE(a) + Py_SIZE(b) <= PY_SSIZE_T_MAX);

That in turn cries out for an explanation of why it must be true.  The asserts 
actually in the patch are part of that explanation, but on their own appear to 
be coming from Mars.  They're missing the conclusion, and rely on further 
unstated subtleties.

Suggest adding a comment where the structs are defined, like:

"""
Note:  Python's memory allocators return at most PY_SSIZE_T_MAX bytes.  
Therefore a vector of PyObject* can contain no more than PY_SSIZE_T_MAX / 
sizeof(PyObject*) elements.  In particular, a PyObject* consumes at least 2 
bytes, so a vector can contain no more than PY_SSIZE_T_MAX / 2 elements.  Code 
freely relies on that.

#if SIZEOF_VOID_P < 2
#   error "size of pointer less than 2 bytes?!"
#endif
"""

--

___
Python tracker 

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

[issue34397] remove redundant overflow checks in tuple and list implementations

[Raymond Hettinger]

Change by Raymond Hettinger :


--
nosy: +mark.dickinson, serhiy.storchaka, tim.peters

___
Python tracker 

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

[issue34397] remove redundant overflow checks in tuple and list implementations

[Sergey Fedoseev]

New submission from Sergey Fedoseev :

Max size of list and tuples is limited by PY_SSIZE_T_MAX / sizeof(PyObject*), 
so the sum of any two list/tuples sizes always <= PY_SSIZE_T_MAX if 
sizeof(PyObject*) > 1, which seems true for all supported (existing?) platforms.
It means that overflow checks in app1, ins1, list_concat and tupleconcat are 
redundant and can be safely removed.

--
components: Interpreter Core
messages: 323491
nosy: sir-sigurd
priority: normal
severity: normal
status: open
title: remove redundant overflow checks in tuple and list implementations
type: enhancement
versions: Python 3.8

___
Python tracker 

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

[issue34397] remove redundant overflow checks in tuple and list implementations

[Sergey Fedoseev]

Change by Sergey Fedoseev :


--
keywords: +patch
pull_requests: +8233
stage:  -> patch review

___
Python tracker 

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