aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2012-11-19 12:24:25 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2012-11-19 12:24:25 -0500
commit1f7cb5c30983752ff8de833de30afcaee63536d0 (patch)
tree9a26c3aab4d7475fa1217f0327f19254e9a99a95 /src
parent644a0a6379afc00803dd89ffe8416514f5dfc217 (diff)
downloadpostgresql-1f7cb5c30983752ff8de833de30afcaee63536d0.tar.gz
postgresql-1f7cb5c30983752ff8de833de30afcaee63536d0.zip
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. This ought to be back-patched, but I'm going to see what the buildfarm reports about the regression tests first. Per discussion with Xi Wang, though this is different from the patch he submitted.
Diffstat (limited to 'src')
-rw-r--r--src/backend/utils/adt/int.c108
-rw-r--r--src/backend/utils/adt/int8.c90
-rw-r--r--src/test/regress/expected/int2.out11
-rw-r--r--src/test/regress/expected/int4.out21
-rw-r--r--src/test/regress/expected/int8-exp-three-digits.out31
-rw-r--r--src/test/regress/expected/int8.out31
-rw-r--r--src/test/regress/sql/int2.sql5
-rw-r--r--src/test/regress/sql/int4.sql8
-rw-r--r--src/test/regress/sql/int8.sql11
9 files changed, 233 insertions, 83 deletions
diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c
index 9f752edc292..3a6f587d069 100644
--- a/src/backend/utils/adt/int.c
+++ b/src/backend/utils/adt/int.c
@@ -681,18 +681,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;
/*
@@ -709,7 +697,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")));
@@ -732,30 +721,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. Most machines produce
- * INT_MIN but it seems some produce zero.
- */
- 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);
}
@@ -877,18 +863,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. Most machines produce
- * SHRT_MIN but it seems some produce zero.
+ * 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);
}
@@ -1065,18 +1060,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. Most machines produce
- * INT_MIN but it seems some produce zero.
+ * 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);
}
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 59c110b0b30..c4cb1f2eff7 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -574,7 +574,8 @@ int8mul(PG_FUNCTION_ARGS)
if (arg1 != (int64) ((int32) arg1) || arg2 != (int64) ((int32) arg2))
{
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")));
@@ -598,18 +599,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. Most machines
- * produce INT64_MIN but it seems some produce zero.
+ * 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);
}
@@ -838,18 +848,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. Most machines
- * produce INT64_MIN but it seems some produce zero.
+ * 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);
}
@@ -1026,18 +1045,27 @@ int82div(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. Most machines
- * produce INT64_MIN but it seems some produce zero.
+ * 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 021d476822c..53b484f718c 100644
--- a/src/test/regress/expected/int2.out
+++ b/src/test/regress/expected/int2.out
@@ -255,3 +255,14 @@ SELECT ((-1::int2<<15)+1::int2)::text;
-32767
(1 row)
+-- 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 8f780240ae7..fcb14e3855e 100644
--- a/src/test/regress/expected/int4.out
+++ b/src/test/regress/expected/int4.out
@@ -342,3 +342,24 @@ SELECT ((-1::int4<<31)+1)::text;
-2147483647
(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 b523bfcc01b..a1c70ed3e8d 100644
--- a/src/test/regress/expected/int8-exp-three-digits.out
+++ b/src/test/regress/expected/int8-exp-three-digits.out
@@ -815,3 +815,34 @@ SELECT ((-1::int8<<63)+1)::text;
-9223372036854775807
(1 row)
+-- 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)
+
+SELECT (-9223372036854775808)::int8 * (-1)::int2;
+ERROR: bigint out of range
+SELECT (-9223372036854775808)::int8 / (-1)::int2;
+ERROR: bigint out of range
+SELECT (-9223372036854775808)::int8 % (-1)::int2;
+ ?column?
+----------
+ 0
+(1 row)
+
diff --git a/src/test/regress/expected/int8.out b/src/test/regress/expected/int8.out
index 811d6a55200..e79c3a8af95 100644
--- a/src/test/regress/expected/int8.out
+++ b/src/test/regress/expected/int8.out
@@ -815,3 +815,34 @@ SELECT ((-1::int8<<63)+1)::text;
-9223372036854775807
(1 row)
+-- 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)
+
+SELECT (-9223372036854775808)::int8 * (-1)::int2;
+ERROR: bigint out of range
+SELECT (-9223372036854775808)::int8 / (-1)::int2;
+ERROR: bigint out of range
+SELECT (-9223372036854775808)::int8 % (-1)::int2;
+ ?column?
+----------
+ 0
+(1 row)
+
diff --git a/src/test/regress/sql/int2.sql b/src/test/regress/sql/int2.sql
index f11eb283c61..bacfbb24ac2 100644
--- a/src/test/regress/sql/int2.sql
+++ b/src/test/regress/sql/int2.sql
@@ -87,3 +87,8 @@ SELECT '' AS five, i.f1, i.f1 / int4 '2' AS x FROM INT2_TBL i;
-- corner cases
SELECT (-1::int2<<15)::text;
SELECT ((-1::int2<<15)+1::int2)::text;
+
+-- 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 ffae7ce4cb4..1843a6d33bc 100644
--- a/src/test/regress/sql/int4.sql
+++ b/src/test/regress/sql/int4.sql
@@ -127,3 +127,11 @@ SELECT (2 + 2) / 2 AS two;
-- corner case
SELECT (-1::int4<<31)::text;
SELECT ((-1::int4<<31)+1)::text;
+
+-- 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 27e0696b32f..2f7f30c91d3 100644
--- a/src/test/regress/sql/int8.sql
+++ b/src/test/regress/sql/int8.sql
@@ -194,3 +194,14 @@ SELECT * FROM generate_series('+4567890123456789'::int8, '+4567890123456799'::in
-- corner case
SELECT (-1::int8<<63)::text;
SELECT ((-1::int8<<63)+1)::text;
+
+-- 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;
+SELECT (-9223372036854775808)::int8 * (-1)::int2;
+SELECT (-9223372036854775808)::int8 / (-1)::int2;
+SELECT (-9223372036854775808)::int8 % (-1)::int2;