Обсуждение: 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