Issue 27473: bytes_concat seems to check overflow using undefined behaviour

bytes_concat uses following code to check overflow:

size = va.len + vb.len;
if (size < 0): {
    PyErr_NoMemory();
    goto done;
}

This is wrong since signed ints overflow is undefined bahaviour.

But one point is that Python's Makefile defines -fwrapv with gcc and
clang. So I am not sure this needs to be changed or not. But in other
parts of Python code I don't see any overflow check like this. I only
see pre-calculated overflow checks.

This should be fixed.

Attach a patch to fix this.

The patch looks correct to me.

Issue 1621 is open about the general problem of overflows.

I changed the versions without checking first. Long story short: Objects/stringobject.c (Python 2 equivalent of Objects/bytesobject.c) is already fixed, but in all versions, Objects/bytearrayobject.c is apparently unfixed.

Python 2 was fixed as part of r65335. Python 3 was supposed to be fixed in r66009 (Issue 3627), but it looks like some of the fixes were missed.

The previous code was perfectly fine with `-fwrapv` since it makes signed overflow behaviour defined. And afaik BDFLs stance is that signed integer overflow should be defined to wrap anyhow.

----

In my opinion the `-fwrapv` itself makes one proliferate all these insane wrap-checks; indeed I'd rather have them defined in a macro, something like

    if (PYSSIZE_OVERFLOWS_ON_ADD(va.len, vb.len)) {
        PyErr_NoMemory();
        goto done;
    }

    size = va.len + vb.len;

even though `-fwrapv` is defined; that way it'd be obvious what is supposed to happen there.

Yes, the current code is valid with -fwrapv. But I am not sure if every compiler supports this feature. So maybe we can't totally rely on it. And in issue1621, some efforts seem to have worked to factor these out.

Xiang Zhang, could you please write a patch for bytearray too?

Of course. I forgot to check bytearray. :(

The new patch now also includes bytearray.

And bytearray_iconcat() please.

Sorry. v3 now includes iconcat.

LGTM.

New changeset dac248056b20 by Serhiy Storchaka in branch '3.5':
Issue #27473: Fixed possible integer overflow in bytes and bytearray
https://hg.python.org/cpython/rev/dac248056b20

New changeset de8f0e9196d8 by Serhiy Storchaka in branch 'default':
Issue #27473: Fixed possible integer overflow in bytes and bytearray
https://hg.python.org/cpython/rev/de8f0e9196d8

Here is a patch for 2.7. It fixes also concatenation and repetition of str and unicode.

Left a comment.

New changeset 130d97217e36 by Serhiy Storchaka in branch '2.7':
Issue #27473: Fixed possible integer overflow in str, unicode and bytearray
https://hg.python.org/cpython/rev/130d97217e36

messages: + msg270127
stage: commit review -> patch review

messages: + msg270094

messages: + msg270087

messages: + msg270075

stage: needs patch -> commit review