[issue29145] failing overflow checks in replace_*

[Roundup Robot]

Roundup Robot added the comment:

New changeset dd2c7d497878 by Martin Panter in branch '3.5':
Issues #1621, #29145: Test for str.join() overflow
https://hg.python.org/cpython/rev/dd2c7d497878

New changeset 0c6ea411af17 by Martin Panter in branch 'default':
Issue #29145: Merge test from 3.6
https://hg.python.org/cpython/rev/0c6ea411af17

--

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Xiang Zhang]

Xiang Zhang added the comment:

Thanks you all. :-）

--
resolution:  -> fixed
stage: commit 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

[issue29145] failing overflow checks in replace_*

[Roundup Robot]

Roundup Robot added the comment:

New changeset 09b1cdac74de by Xiang Zhang in branch '3.5':
Issue #29145: Fix overflow checks in str.replace() and str.join().
https://hg.python.org/cpython/rev/09b1cdac74de

New changeset d966ccda9f17 by Xiang Zhang in branch '3.6':
Issue #29145: Merge 3.5.
https://hg.python.org/cpython/rev/d966ccda9f17

New changeset f61a0e8ec022 by Xiang Zhang in branch 'default':
Issue #29145: Merge 3.6.
https://hg.python.org/cpython/rev/f61a0e8ec022

--

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Martin Panter]

Martin Panter added the comment:

Both fixes (join and replace) look good to me. However I don’t think it is 
necessary to change the exception message in 3.5 or 3.6.

--

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Xiang Zhang]

Xiang Zhang added the comment:

> FTR I thought the consensus was not to backport these fixes unless there was 
> a demonstrated problem: , though 
> personally, I would be in favour of backporting in many cases.

Sorry not know that. :-( Another benefit of the change is it actually backports 
the codes from 3.x so it's easy for later maintainence and it doesn't add 
additional work.

> I included a test case that works on 32-bit platforms, which you may use.

Thanks. But I don't get such an environment to test so I prefer not to add it 
now. I adjust my patch according to yours. :-)

--
Added file: http://bugs.python.org/file46223/overflow-checks-3x-2.patch

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Martin Panter]

Martin Panter added the comment:

FTR I thought the consensus was not to backport these fixes unless there was a 
demonstrated problem: , though 
personally, I would be in favour of backporting in many cases.

Regarding str.join() in unicode.c, see also my unicode.patch in that bug. I 
included a test case that works on 32-bit platforms, which you may use. (From 
memory, you probably need to disable -fwrapv and maybe use -ftrapv or the 
undefined behaviour sanitizer to make the test fail.)

--
nosy: +martin.panter

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Xiang Zhang]

Xiang Zhang added the comment:

I committed the patch to 2.7. The new patch is for 3.x.

--
stage: patch review -> commit review
versions: +Python 3.5, Python 3.6, Python 3.7
Added file: http://bugs.python.org/file46221/overflow-checks-3x.patch

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Roundup Robot]

Roundup Robot added the comment:

New changeset 29e505526bdd by Xiang Zhang in branch '2.7':
Issue #29145: Fix overflow checks in string, bytearray and unicode.
https://hg.python.org/cpython/rev/29e505526bdd

--
nosy: +python-dev

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

The code in PyUnicode_Join() checks on overflow after making an addition.

sz += PyUnicode_GET_LENGTH(item);
item_maxchar = PyUnicode_MAX_CHAR_VALUE(item);
maxchar = Py_MAX(maxchar, item_maxchar);
if (i != 0)
sz += seplen;
if (sz < old_sz || sz > PY_SSIZE_T_MAX) {
PyErr_SetString(PyExc_OverflowError,
"join() result is too long for a Python string");
goto onError;
}

Maybe there are other cases.

--

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Xiang Zhang]

Xiang Zhang added the comment:

> It seems to me that unicodeobject.c should be changed in 3.x too.

I don't understand. Would you mind tell me which part? The only suspicious part 
to me is `PY_SSIZE_T_MAX >> (rkind - 1)`. It seems should be `PY_SSIZE_T_MAX / 
rkind`.

And I missed jan's patch so wrote mine. :-( But they are almost the same. :-)

--

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

It seems to me that unicodeobject.c should be changed in 3.x too.

--

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Xiang Zhang]

Xiang Zhang added the comment:

> some instances are present in unicodeobject.c too

Thanks for your notification.

v3 adds the some changes needed I could find in unicodeobject.c. Nothing in v2 
is changed.

--
Added file: http://bugs.python.org/file46158/replace-overflow-check-v3.patch

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

replace-overflow-check-v2.patch LGTM.

--

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[jan matejek]

jan matejek added the comment:

some instances are present in unicodeobject.c too

--
Added file: http://bugs.python.org/file46156/unicode-overflow.patch

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Xiang Zhang]

Xiang Zhang added the comment:

> It would be better to write the code in the same form as in 3.x. This could 
> help backporting other patches if needed.

Yes, it is. :-) I only had a fast glance at at 3.x codes and thought they 
didn't share much. :-( But apparently I was wrong. The new patch backports 3.x 
code.

--
Added file: http://bugs.python.org/file46149/replace-overflow-check-v2.patch

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

It would be better to write the code in the same form as in 3.x. This could 
help backporting other patches if needed.

--
nosy: +serhiy.storchaka

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Xiang Zhang]

Xiang Zhang added the comment:

Ohh sorry I misunderstand your intention. :-( Attach a patch. :-)

--
keywords: +patch
stage:  -> patch review
Added file: http://bugs.python.org/file46141/replace-overflow-check.patch

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[jan matejek]

jan matejek added the comment:

No, your changes from issue 27473 are OK. However functions like 
replace_interleave and replace_single_character etc. still use the broken code:

/* use the difference between current and new, hence the "-1" */
/*   result_len = self_len + count * (to_len-1)  */
product = count * (to_len-1);
if (product / (to_len-1) != count) {
PyErr_SetString(PyExc_OverflowError, "replace bytes is too long");
return NULL;
}

--

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Xiang Zhang]

Xiang Zhang added the comment:

> It does, but "-fwrapv" is not automatically added when you specify custom OPT 
> flags.

Indeed I think this is why the changes in #27473 and #1621 make sense.

> GCC 6 optimizes away broken overflow checks.

I am sorry but I don't understand this. What do you mean by broken overflow 
checks? Is `if (size > PY_SSIZE_T_MAX - vo.len)` broken?

--
nosy: +xiang.zhang

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[jan matejek]

jan matejek added the comment:

It does, but "-fwrapv" is not automatically added when you specify custom OPT 
flags. I should have clarified that in the original report.

--

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[Benjamin Peterson]

Benjamin Peterson added the comment:

Does that mean it no longer respects -fwrapv?

--
nosy: +benjamin.peterson

___
Python tracker 

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

[issue29145] failing overflow checks in replace_*

[jan matejek]

New submission from jan matejek:

Related to http://bugs.python.org/issue1621 and 
http://bugs.python.org/issue27473
GCC 6 optimizes away broken overflow checks. This leads to segfaults on 
test_replace_overflow, at least for strings and bytearrays.

--
components: Interpreter Core
messages: 284588
nosy: matejcik
priority: normal
severity: normal
status: open
title: failing overflow checks in replace_*
type: crash
versions: Python 2.7

___
Python tracker 

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