From: Tom Lane Date: Tue, 20 Nov 2012 02:22:00 +0000 (-0500) Subject: Improve handling of INT_MIN / -1 and related cases. X-Git-Tag: REL8_3_22~14 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3352e25e85b28ee1f21e0a759c0c49e399cb032a;p=postgresql Improve handling of INT_MIN / -1 and related cases. Some platforms throw an exception for this division, rather than returning a necessarily-overflowed result. Since we were testing for overflow after the fact, an exception isn't nice. We can avoid the problem by treating division by -1 as negation. Add some regression tests so that we'll find out if any compilers try to optimize away the overflow check conditions. Back-patch of commit 1f7cb5c30983752ff8de833de30afcaee63536d0. Per discussion with Xi Wang, though this is different from the patch he submitted. --- diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c index 134bdf5b35..91067a98d5 100644 --- a/src/backend/utils/adt/int.c +++ b/src/backend/utils/adt/int.c @@ -671,18 +671,6 @@ int4mul(PG_FUNCTION_ARGS) int32 arg2 = PG_GETARG_INT32(1); int32 result; -#ifdef WIN32 - - /* - * Win32 doesn't throw a catchable exception for SELECT -2147483648 * - * (-1); -- INT_MIN - */ - if (arg2 == -1 && arg1 == INT_MIN) - ereport(ERROR, - (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("integer out of range"))); -#endif - result = arg1 * arg2; /* @@ -699,7 +687,8 @@ int4mul(PG_FUNCTION_ARGS) if (!(arg1 >= (int32) SHRT_MIN && arg1 <= (int32) SHRT_MAX && arg2 >= (int32) SHRT_MIN && arg2 <= (int32) SHRT_MAX) && arg2 != 0 && - (result / arg2 != arg1 || (arg2 == -1 && arg1 < 0 && result < 0))) + ((arg2 == -1 && arg1 < 0 && result < 0) || + result / arg2 != arg1)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); @@ -722,29 +711,27 @@ int4div(PG_FUNCTION_ARGS) PG_RETURN_NULL(); } -#ifdef WIN32 - /* - * Win32 doesn't throw a catchable exception for SELECT -2147483648 / - * (-1); -- INT_MIN + * INT_MIN / -1 is problematic, since the result can't be represented on a + * two's-complement machine. Some machines produce INT_MIN, some produce + * zero, some throw an exception. We can dodge the problem by recognizing + * that division by -1 is the same as negation. */ - if (arg2 == -1 && arg1 == INT_MIN) - ereport(ERROR, - (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("integer out of range"))); -#endif + if (arg2 == -1) + { + result = -arg1; + /* overflow check (needed for INT_MIN) */ + if (arg1 != 0 && SAMESIGN(result, arg1)) + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("integer out of range"))); + PG_RETURN_INT32(result); + } + + /* No overflow is possible */ result = arg1 / arg2; - /* - * Overflow check. The only possible overflow case is for arg1 = INT_MIN, - * arg2 = -1, where the correct result is -INT_MIN, which can't be - * represented on a two's-complement machine. - */ - if (arg2 == -1 && arg1 < 0 && result < 0) - ereport(ERROR, - (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("integer out of range"))); PG_RETURN_INT32(result); } @@ -866,17 +853,27 @@ int2div(PG_FUNCTION_ARGS) PG_RETURN_NULL(); } - result = arg1 / arg2; - /* - * Overflow check. The only possible overflow case is for arg1 = - * SHRT_MIN, arg2 = -1, where the correct result is -SHRT_MIN, which can't - * be represented on a two's-complement machine. + * SHRT_MIN / -1 is problematic, since the result can't be represented on + * a two's-complement machine. Some machines produce SHRT_MIN, some + * produce zero, some throw an exception. We can dodge the problem by + * recognizing that division by -1 is the same as negation. */ - if (arg2 == -1 && arg1 < 0 && result < 0) - ereport(ERROR, - (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("smallint out of range"))); + if (arg2 == -1) + { + result = -arg1; + /* overflow check (needed for SHRT_MIN) */ + if (arg1 != 0 && SAMESIGN(result, arg1)) + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("smallint out of range"))); + PG_RETURN_INT16(result); + } + + /* No overflow is possible */ + + result = arg1 / arg2; + PG_RETURN_INT16(result); } @@ -1054,17 +1051,27 @@ int42div(PG_FUNCTION_ARGS) PG_RETURN_NULL(); } - result = arg1 / arg2; - /* - * Overflow check. The only possible overflow case is for arg1 = INT_MIN, - * arg2 = -1, where the correct result is -INT_MIN, which can't be - * represented on a two's-complement machine. + * INT_MIN / -1 is problematic, since the result can't be represented on a + * two's-complement machine. Some machines produce INT_MIN, some produce + * zero, some throw an exception. We can dodge the problem by recognizing + * that division by -1 is the same as negation. */ - if (arg2 == -1 && arg1 < 0 && result < 0) - ereport(ERROR, - (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("integer out of range"))); + if (arg2 == -1) + { + result = -arg1; + /* overflow check (needed for INT_MIN) */ + if (arg1 != 0 && SAMESIGN(result, arg1)) + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("integer out of range"))); + PG_RETURN_INT32(result); + } + + /* No overflow is possible */ + + result = arg1 / arg2; + PG_RETURN_INT32(result); } @@ -1160,6 +1167,14 @@ int42mod(PG_FUNCTION_ARGS) PG_RETURN_NULL(); } + /* + * Some machines throw a floating-point exception for INT_MIN % -1, which + * is a bit silly since the correct answer is perfectly well-defined, + * namely zero. + */ + if (arg2 == -1) + PG_RETURN_INT32(0); + /* No overflow is possible */ PG_RETURN_INT32(arg1 % arg2); diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c index 5657b89f5f..45e023910d 100644 --- a/src/backend/utils/adt/int8.c +++ b/src/backend/utils/adt/int8.c @@ -583,7 +583,8 @@ int8mul(PG_FUNCTION_ARGS) #endif { if (arg2 != 0 && - (result / arg2 != arg1 || (arg2 == -1 && arg1 < 0 && result < 0))) + ((arg2 == -1 && arg1 < 0 && result < 0) || + result / arg2 != arg1)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -607,17 +608,27 @@ int8div(PG_FUNCTION_ARGS) PG_RETURN_NULL(); } - result = arg1 / arg2; - /* - * Overflow check. The only possible overflow case is for arg1 = - * INT64_MIN, arg2 = -1, where the correct result is -INT64_MIN, which - * can't be represented on a two's-complement machine. + * INT64_MIN / -1 is problematic, since the result can't be represented on + * a two's-complement machine. Some machines produce INT64_MIN, some + * produce zero, some throw an exception. We can dodge the problem by + * recognizing that division by -1 is the same as negation. */ - if (arg2 == -1 && arg1 < 0 && result < 0) - ereport(ERROR, - (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("bigint out of range"))); + if (arg2 == -1) + { + result = -arg1; + /* overflow check (needed for INT64_MIN) */ + if (arg1 != 0 && SAMESIGN(result, arg1)) + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("bigint out of range"))); + PG_RETURN_INT64(result); + } + + /* No overflow is possible */ + + result = arg1 / arg2; + PG_RETURN_INT64(result); } @@ -846,17 +857,27 @@ int84div(PG_FUNCTION_ARGS) PG_RETURN_NULL(); } - result = arg1 / arg2; - /* - * Overflow check. The only possible overflow case is for arg1 = - * INT64_MIN, arg2 = -1, where the correct result is -INT64_MIN, which - * can't be represented on a two's-complement machine. + * INT64_MIN / -1 is problematic, since the result can't be represented on + * a two's-complement machine. Some machines produce INT64_MIN, some + * produce zero, some throw an exception. We can dodge the problem by + * recognizing that division by -1 is the same as negation. */ - if (arg2 == -1 && arg1 < 0 && result < 0) - ereport(ERROR, - (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("bigint out of range"))); + if (arg2 == -1) + { + result = -arg1; + /* overflow check (needed for INT64_MIN) */ + if (arg1 != 0 && SAMESIGN(result, arg1)) + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("bigint out of range"))); + PG_RETURN_INT64(result); + } + + /* No overflow is possible */ + + result = arg1 / arg2; + PG_RETURN_INT64(result); } diff --git a/src/test/regress/expected/int2.out b/src/test/regress/expected/int2.out index e34e85bc97..29e2d2faa8 100644 --- a/src/test/regress/expected/int2.out +++ b/src/test/regress/expected/int2.out @@ -228,3 +228,14 @@ SELECT '' AS five, i.f1, i.f1 / int4 '2' AS x FROM INT2_TBL i; | -32767 | -16383 (5 rows) +-- check sane handling of INT16_MIN overflow cases +SELECT (-32768)::int2 * (-1)::int2; +ERROR: smallint out of range +SELECT (-32768)::int2 / (-1)::int2; +ERROR: smallint out of range +SELECT (-32768)::int2 % (-1)::int2; + ?column? +---------- + 0 +(1 row) + diff --git a/src/test/regress/expected/int4.out b/src/test/regress/expected/int4.out index 3e0eff13a3..eeae8bc5d4 100644 --- a/src/test/regress/expected/int4.out +++ b/src/test/regress/expected/int4.out @@ -315,3 +315,24 @@ SELECT (2 + 2) / 2 AS two; 2 (1 row) +-- check sane handling of INT_MIN overflow cases +SELECT (-2147483648)::int4 * (-1)::int4; +ERROR: integer out of range +SELECT (-2147483648)::int4 / (-1)::int4; +ERROR: integer out of range +SELECT (-2147483648)::int4 % (-1)::int4; + ?column? +---------- + 0 +(1 row) + +SELECT (-2147483648)::int4 * (-1)::int2; +ERROR: integer out of range +SELECT (-2147483648)::int4 / (-1)::int2; +ERROR: integer out of range +SELECT (-2147483648)::int4 % (-1)::int2; + ?column? +---------- + 0 +(1 row) + diff --git a/src/test/regress/expected/int8-exp-three-digits.out b/src/test/regress/expected/int8-exp-three-digits.out index e3697f8344..187bf8e433 100644 --- a/src/test/regress/expected/int8-exp-three-digits.out +++ b/src/test/regress/expected/int8-exp-three-digits.out @@ -317,3 +317,24 @@ select '9223372036854775807'::int8; select '9223372036854775808'::int8; ERROR: value "9223372036854775808" is out of range for type bigint +-- check sane handling of INT64_MIN overflow cases +SELECT (-9223372036854775808)::int8 * (-1)::int8; +ERROR: bigint out of range +SELECT (-9223372036854775808)::int8 / (-1)::int8; +ERROR: bigint out of range +SELECT (-9223372036854775808)::int8 % (-1)::int8; + ?column? +---------- + 0 +(1 row) + +SELECT (-9223372036854775808)::int8 * (-1)::int4; +ERROR: bigint out of range +SELECT (-9223372036854775808)::int8 / (-1)::int4; +ERROR: bigint out of range +SELECT (-9223372036854775808)::int8 % (-1)::int4; + ?column? +---------- + 0 +(1 row) + diff --git a/src/test/regress/expected/int8.out b/src/test/regress/expected/int8.out index 52cdabab74..467778c833 100644 --- a/src/test/regress/expected/int8.out +++ b/src/test/regress/expected/int8.out @@ -317,3 +317,24 @@ select '9223372036854775807'::int8; select '9223372036854775808'::int8; ERROR: value "9223372036854775808" is out of range for type bigint +-- check sane handling of INT64_MIN overflow cases +SELECT (-9223372036854775808)::int8 * (-1)::int8; +ERROR: bigint out of range +SELECT (-9223372036854775808)::int8 / (-1)::int8; +ERROR: bigint out of range +SELECT (-9223372036854775808)::int8 % (-1)::int8; + ?column? +---------- + 0 +(1 row) + +SELECT (-9223372036854775808)::int8 * (-1)::int4; +ERROR: bigint out of range +SELECT (-9223372036854775808)::int8 / (-1)::int4; +ERROR: bigint out of range +SELECT (-9223372036854775808)::int8 % (-1)::int4; + ?column? +---------- + 0 +(1 row) + diff --git a/src/test/regress/sql/int2.sql b/src/test/regress/sql/int2.sql index 65c89e4abd..b185f16c0a 100644 --- a/src/test/regress/sql/int2.sql +++ b/src/test/regress/sql/int2.sql @@ -86,3 +86,7 @@ SELECT '' AS five, i.f1, i.f1 / int2 '2' AS x FROM INT2_TBL i; SELECT '' AS five, i.f1, i.f1 / int4 '2' AS x FROM INT2_TBL i; +-- check sane handling of INT16_MIN overflow cases +SELECT (-32768)::int2 * (-1)::int2; +SELECT (-32768)::int2 / (-1)::int2; +SELECT (-32768)::int2 % (-1)::int2; diff --git a/src/test/regress/sql/int4.sql b/src/test/regress/sql/int4.sql index 5212c68795..9c24188dc0 100644 --- a/src/test/regress/sql/int4.sql +++ b/src/test/regress/sql/int4.sql @@ -125,3 +125,11 @@ SELECT 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 AS ten; SELECT 2 + 2 / 2 AS three; SELECT (2 + 2) / 2 AS two; + +-- check sane handling of INT_MIN overflow cases +SELECT (-2147483648)::int4 * (-1)::int4; +SELECT (-2147483648)::int4 / (-1)::int4; +SELECT (-2147483648)::int4 % (-1)::int4; +SELECT (-2147483648)::int4 * (-1)::int2; +SELECT (-2147483648)::int4 / (-1)::int2; +SELECT (-2147483648)::int4 % (-1)::int2; diff --git a/src/test/regress/sql/int8.sql b/src/test/regress/sql/int8.sql index 8ef92ba3f3..d540b4643f 100644 --- a/src/test/regress/sql/int8.sql +++ b/src/test/regress/sql/int8.sql @@ -69,3 +69,11 @@ select '-9223372036854775808'::int8; select '-9223372036854775809'::int8; select '9223372036854775807'::int8; select '9223372036854775808'::int8; + +-- check sane handling of INT64_MIN overflow cases +SELECT (-9223372036854775808)::int8 * (-1)::int8; +SELECT (-9223372036854775808)::int8 / (-1)::int8; +SELECT (-9223372036854775808)::int8 % (-1)::int8; +SELECT (-9223372036854775808)::int8 * (-1)::int4; +SELECT (-9223372036854775808)::int8 / (-1)::int4; +SELECT (-9223372036854775808)::int8 % (-1)::int4;