Обсуждение: 64-bit integer subtraction bug on some platforms

One of the new tests in the infinite interval patch has revealed a bug
in our 64-bit integer subtraction code. Consider the following:

select 0::int8 - '-9223372036854775808'::int8;

This should overflow, since the correct result (+9223372036854775808)
is out of range. However, on platforms without integer overflow
builtins or 128-bit integers, pg_sub_s64_overflow() does the
following:

    if ((a < 0 && b > 0 && a < PG_INT64_MIN + b) ||
        (a > 0 && b < 0 && a > PG_INT64_MAX + b))
    {
        *result = 0x5EED;        /* to avoid spurious warnings */
        return true;
    }
    *result = a - b;
    return false;

which fails to spot the fact that overflow is also possible when a ==
0. So on such platforms, it returns the wrong result.

Patch attached.

Regards,
Dean

On Wed, 2023-11-08 at 11:58 +0000, Dean Rasheed wrote:
> One of the new tests in the infinite interval patch has revealed a bug
> in our 64-bit integer subtraction code. Consider the following:
>
> select 0::int8 - '-9223372036854775808'::int8;
>
> This should overflow, since the correct result (+9223372036854775808)
> is out of range. However, on platforms without integer overflow
> builtins or 128-bit integers, pg_sub_s64_overflow() does the
> following:
>
>     if ((a < 0 && b > 0 && a < PG_INT64_MIN + b) ||
>         (a > 0 && b < 0 && a > PG_INT64_MAX + b))
>     {
>         *result = 0x5EED;        /* to avoid spurious warnings */
>         return true;
>     }
>     *result = a - b;
>     return false;
>
> which fails to spot the fact that overflow is also possible when a ==
> 0. So on such platforms, it returns the wrong result.
>
> Patch attached.

The patch looks good to me.

Yours,
Laurenz Albe

Laurenz Albe <laurenz.albe@cybertec.at> writes:
> On Wed, 2023-11-08 at 11:58 +0000, Dean Rasheed wrote:
>> This should overflow, since the correct result (+9223372036854775808)
>> is out of range. However, on platforms without integer overflow
>> builtins or 128-bit integers, pg_sub_s64_overflow() does the
>> following:
>> ...
>> which fails to spot the fact that overflow is also possible when a ==
>> 0. So on such platforms, it returns the wrong result.
>> 
>> Patch attached.

> The patch looks good to me.

+1: good catch, fix looks correct.

            regards, tom lane