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.

Does that mean it no longer respects -fwrapv?

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

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

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

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

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

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

some instances are present in unicodeobject.c too

replace-overflow-check-v2.patch LGTM.

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

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

> 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. :-)

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.

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

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

FTR I thought the consensus was not to backport these fixes unless there was a demonstrated problem: <https://bugs.python.org/issue1621#msg144499>, 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.)

> FTR I thought the consensus was not to backport these fixes unless there was a demonstrated problem: <https://bugs.python.org/issue1621#msg144499>, 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. :-)

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.

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

Thanks you all. :-）

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

stage: commit review -> resolved

messages: + msg285022

stage: patch review -> commit review
messages: + msg285018
versions: + Python 3.5, Python 3.6, Python 3.7

messages: + msg284731

messages: + msg284721

messages: + msg284708

stage: patch review