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
Attachments:
fix-64-bit-integer-subtraction.patchtext/x-patch; charset=US-ASCII; name=fix-64-bit-integer-subtraction.patchDownload+8-1
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