Skip to content

Commit eb865db

Browse files
tglsfdctglsfdc
committed
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 1f7cb5c. Per discussion with Xi Wang, though this is different from the patch he submitted.
1 parent 15939a7 commit eb865db

File tree

9 files changed

+232
-83
lines changed

9 files changed

+232
-83
lines changed

‎src/backend/utils/adt/int.c

Lines changed: 56 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -680,18 +680,6 @@ int4mul(PG_FUNCTION_ARGS)
680680
int32 arg2 = PG_GETARG_INT32(1);
681681
int32 result;
682682

683-
#ifdef WIN32
684-
685-
/*
686-
* Win32 doesn't throw a catchable exception for SELECT -2147483648 *
687-
* (-1); -- INT_MIN
688-
*/
689-
if (arg2 == -1 && arg1 == INT_MIN)
690-
ereport(ERROR,
691-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
692-
errmsg("integer out of range")));
693-
#endif
694-
695683
result = arg1 * arg2;
696684

697685
/*
@@ -708,7 +696,8 @@ int4mul(PG_FUNCTION_ARGS)
708696
if (!(arg1 >= (int32) SHRT_MIN && arg1 <= (int32) SHRT_MAX &&
709697
arg2 >= (int32) SHRT_MIN && arg2 <= (int32) SHRT_MAX) &&
710698
arg2 != 0 &&
711-
(result / arg2 != arg1 || (arg2 == -1 && arg1 < 0 && result < 0)))
699+
((arg2 == -1 && arg1 < 0 && result < 0) ||
700+
result / arg2 != arg1))
712701
ereport(ERROR,
713702
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
714703
errmsg("integer out of range")));
@@ -731,30 +720,27 @@ int4div(PG_FUNCTION_ARGS)
731720
PG_RETURN_NULL();
732721
}
733722

734-
#ifdef WIN32
735-
736723
/*
737-
* Win32 doesn't throw a catchable exception for SELECT -2147483648 /
738-
* (-1); -- INT_MIN
724+
* INT_MIN / -1 is problematic, since the result can't be represented on a
725+
* two's-complement machine. Some machines produce INT_MIN, some produce
726+
* zero, some throw an exception. We can dodge the problem by recognizing
727+
* that division by -1 is the same as negation.
739728
*/
740-
if (arg2 == -1 && arg1 == INT_MIN)
741-
ereport(ERROR,
742-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
743-
errmsg("integer out of range")));
744-
#endif
729+
if (arg2 == -1)
730+
{
731+
result = -arg1;
732+
/* overflow check (needed for INT_MIN) */
733+
if (arg1 != 0 && SAMESIGN(result, arg1))
734+
ereport(ERROR,
735+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
736+
errmsg("integer out of range")));
737+
PG_RETURN_INT32(result);
738+
}
739+
740+
/* No overflow is possible */
745741

746742
result = arg1 / arg2;
747743

748-
/*
749-
* Overflow check. The only possible overflow case is for arg1 = INT_MIN,
750-
* arg2 = -1, where the correct result is -INT_MIN, which can't be
751-
* represented on a two's-complement machine. Most machines produce
752-
* INT_MIN but it seems some produce zero.
753-
*/
754-
if (arg2 == -1 && arg1 < 0 && result <= 0)
755-
ereport(ERROR,
756-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
757-
errmsg("integer out of range")));
758744
PG_RETURN_INT32(result);
759745
}
760746

@@ -876,18 +862,27 @@ int2div(PG_FUNCTION_ARGS)
876862
PG_RETURN_NULL();
877863
}
878864

879-
result = arg1 / arg2;
880-
881865
/*
882-
* Overflow check. The only possible overflow case is for arg1 =
883-
* SHRT_MIN, arg2 = -1, where the correct result is -SHRT_MIN, which can't
884-
* be represented on a two's-complement machine. Most machines produce
885-
* SHRT_MIN but it seems some produce zero.
866+
* SHRT_MIN / -1 is problematic, since the result can't be represented on
867+
* a two's-complement machine. Some machines produce SHRT_MIN, some
868+
* produce zero, some throw an exception. We can dodge the problem by
869+
* recognizing that division by -1 is the same as negation.
886870
*/
887-
if (arg2 == -1 && arg1 < 0 && result <= 0)
888-
ereport(ERROR,
889-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
890-
errmsg("smallint out of range")));
871+
if (arg2 == -1)
872+
{
873+
result = -arg1;
874+
/* overflow check (needed for SHRT_MIN) */
875+
if (arg1 != 0 && SAMESIGN(result, arg1))
876+
ereport(ERROR,
877+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
878+
errmsg("smallint out of range")));
879+
PG_RETURN_INT16(result);
880+
}
881+
882+
/* No overflow is possible */
883+
884+
result = arg1 / arg2;
885+
891886
PG_RETURN_INT16(result);
892887
}
893888

@@ -1064,18 +1059,27 @@ int42div(PG_FUNCTION_ARGS)
10641059
PG_RETURN_NULL();
10651060
}
10661061

1067-
result = arg1 / arg2;
1068-
10691062
/*
1070-
* Overflow check. The only possible overflow case is for arg1 = INT_MIN,
1071-
* arg2 = -1, where the correct result is -INT_MIN, which can't be
1072-
* represented on a two's-complement machine. Most machines produce
1073-
* INT_MIN but it seems some produce zero.
1063+
* INT_MIN / -1 is problematic, since the result can't be represented on a
1064+
* two's-complement machine. Some machines produce INT_MIN, some produce
1065+
* zero, some throw an exception. We can dodge the problem by recognizing
1066+
* that division by -1 is the same as negation.
10741067
*/
1075-
if (arg2 == -1 && arg1 < 0 && result <= 0)
1076-
ereport(ERROR,
1077-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
1078-
errmsg("integer out of range")));
1068+
if (arg2 == -1)
1069+
{
1070+
result = -arg1;
1071+
/* overflow check (needed for INT_MIN) */
1072+
if (arg1 != 0 && SAMESIGN(result, arg1))
1073+
ereport(ERROR,
1074+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
1075+
errmsg("integer out of range")));
1076+
PG_RETURN_INT32(result);
1077+
}
1078+
1079+
/* No overflow is possible */
1080+
1081+
result = arg1 / arg2;
1082+
10791083
PG_RETURN_INT32(result);
10801084
}
10811085

‎src/backend/utils/adt/int8.c

Lines changed: 59 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,8 @@ int8mul(PG_FUNCTION_ARGS)
576576
if (arg1 != (int64) ((int32) arg1) || arg2 != (int64) ((int32) arg2))
577577
{
578578
if (arg2 != 0 &&
579-
(result / arg2 != arg1 || (arg2 == -1 && arg1 < 0 && result < 0)))
579+
((arg2 == -1 && arg1 < 0 && result < 0) ||
580+
result / arg2 != arg1))
580581
ereport(ERROR,
581582
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
582583
errmsg("bigint out of range")));
@@ -600,18 +601,27 @@ int8div(PG_FUNCTION_ARGS)
600601
PG_RETURN_NULL();
601602
}
602603

603-
result = arg1 / arg2;
604-
605604
/*
606-
* Overflow check. The only possible overflow case is for arg1 =
607-
* INT64_MIN, arg2 = -1, where the correct result is -INT64_MIN, which
608-
* can't be represented on a two's-complement machine. Most machines
609-
* produce INT64_MIN but it seems some produce zero.
605+
* INT64_MIN / -1 is problematic, since the result can't be represented on
606+
* a two's-complement machine. Some machines produce INT64_MIN, some
607+
* produce zero, some throw an exception. We can dodge the problem by
608+
* recognizing that division by -1 is the same as negation.
610609
*/
611-
if (arg2 == -1 && arg1 < 0 && result <= 0)
612-
ereport(ERROR,
613-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
614-
errmsg("bigint out of range")));
610+
if (arg2 == -1)
611+
{
612+
result = -arg1;
613+
/* overflow check (needed for INT64_MIN) */
614+
if (arg1 != 0 && SAMESIGN(result, arg1))
615+
ereport(ERROR,
616+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
617+
errmsg("bigint out of range")));
618+
PG_RETURN_INT64(result);
619+
}
620+
621+
/* No overflow is possible */
622+
623+
result = arg1 / arg2;
624+
615625
PG_RETURN_INT64(result);
616626
}
617627

@@ -840,18 +850,27 @@ int84div(PG_FUNCTION_ARGS)
840850
PG_RETURN_NULL();
841851
}
842852

843-
result = arg1 / arg2;
844-
845853
/*
846-
* Overflow check. The only possible overflow case is for arg1 =
847-
* INT64_MIN, arg2 = -1, where the correct result is -INT64_MIN, which
848-
* can't be represented on a two's-complement machine. Most machines
849-
* produce INT64_MIN but it seems some produce zero.
854+
* INT64_MIN / -1 is problematic, since the result can't be represented on
855+
* a two's-complement machine. Some machines produce INT64_MIN, some
856+
* produce zero, some throw an exception. We can dodge the problem by
857+
* recognizing that division by -1 is the same as negation.
850858
*/
851-
if (arg2 == -1 && arg1 < 0 && result <= 0)
852-
ereport(ERROR,
853-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
854-
errmsg("bigint out of range")));
859+
if (arg2 == -1)
860+
{
861+
result = -arg1;
862+
/* overflow check (needed for INT64_MIN) */
863+
if (arg1 != 0 && SAMESIGN(result, arg1))
864+
ereport(ERROR,
865+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
866+
errmsg("bigint out of range")));
867+
PG_RETURN_INT64(result);
868+
}
869+
870+
/* No overflow is possible */
871+
872+
result = arg1 / arg2;
873+
855874
PG_RETURN_INT64(result);
856875
}
857876

@@ -1028,18 +1047,27 @@ int82div(PG_FUNCTION_ARGS)
10281047
PG_RETURN_NULL();
10291048
}
10301049

1031-
result = arg1 / arg2;
1032-
10331050
/*
1034-
* Overflow check. The only possible overflow case is for arg1 =
1035-
* INT64_MIN, arg2 = -1, where the correct result is -INT64_MIN, which
1036-
* can't be represented on a two's-complement machine. Most machines
1037-
* produce INT64_MIN but it seems some produce zero.
1051+
* INT64_MIN / -1 is problematic, since the result can't be represented on
1052+
* a two's-complement machine. Some machines produce INT64_MIN, some
1053+
* produce zero, some throw an exception. We can dodge the problem by
1054+
* recognizing that division by -1 is the same as negation.
10381055
*/
1039-
if (arg2 == -1 && arg1 < 0 && result <= 0)
1040-
ereport(ERROR,
1041-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
1042-
errmsg("bigint out of range")));
1056+
if (arg2 == -1)
1057+
{
1058+
result = -arg1;
1059+
/* overflow check (needed for INT64_MIN) */
1060+
if (arg1 != 0 && SAMESIGN(result, arg1))
1061+
ereport(ERROR,
1062+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
1063+
errmsg("bigint out of range")));
1064+
PG_RETURN_INT64(result);
1065+
}
1066+
1067+
/* No overflow is possible */
1068+
1069+
result = arg1 / arg2;
1070+
10431071
PG_RETURN_INT64(result);
10441072
}
10451073

‎src/test/regress/expected/int2.out

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,3 +244,14 @@ SELECT '' AS five, i.f1, i.f1 / int4 '2' AS x FROM INT2_TBL i;
244244
| -32767 | -16383
245245
(5 rows)
246246

247+
-- check sane handling of INT16_MIN overflow cases
248+
SELECT (-32768)::int2 * (-1)::int2;
249+
ERROR: smallint out of range
250+
SELECT (-32768)::int2 / (-1)::int2;
251+
ERROR: smallint out of range
252+
SELECT (-32768)::int2 % (-1)::int2;
253+
?column?
254+
----------
255+
0
256+
(1 row)
257+

‎src/test/regress/expected/int4.out

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,3 +331,24 @@ SELECT (2 + 2) / 2 AS two;
331331
2
332332
(1 row)
333333

334+
-- check sane handling of INT_MIN overflow cases
335+
SELECT (-2147483648)::int4 * (-1)::int4;
336+
ERROR: integer out of range
337+
SELECT (-2147483648)::int4 / (-1)::int4;
338+
ERROR: integer out of range
339+
SELECT (-2147483648)::int4 % (-1)::int4;
340+
?column?
341+
----------
342+
0
343+
(1 row)
344+
345+
SELECT (-2147483648)::int4 * (-1)::int2;
346+
ERROR: integer out of range
347+
SELECT (-2147483648)::int4 / (-1)::int2;
348+
ERROR: integer out of range
349+
SELECT (-2147483648)::int4 % (-1)::int2;
350+
?column?
351+
----------
352+
0
353+
(1 row)
354+

‎src/test/regress/expected/int8-exp-three-digits.out

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,3 +802,34 @@ SELECT * FROM generate_series('+4567890123456789'::int8, '+4567890123456799'::in
802802
4567890123456799
803803
(6 rows)
804804

805+
-- check sane handling of INT64_MIN overflow cases
806+
SELECT (-9223372036854775808)::int8 * (-1)::int8;
807+
ERROR: bigint out of range
808+
SELECT (-9223372036854775808)::int8 / (-1)::int8;
809+
ERROR: bigint out of range
810+
SELECT (-9223372036854775808)::int8 % (-1)::int8;
811+
?column?
812+
----------
813+
0
814+
(1 row)
815+
816+
SELECT (-9223372036854775808)::int8 * (-1)::int4;
817+
ERROR: bigint out of range
818+
SELECT (-9223372036854775808)::int8 / (-1)::int4;
819+
ERROR: bigint out of range
820+
SELECT (-9223372036854775808)::int8 % (-1)::int4;
821+
?column?
822+
----------
823+
0
824+
(1 row)
825+
826+
SELECT (-9223372036854775808)::int8 * (-1)::int2;
827+
ERROR: bigint out of range
828+
SELECT (-9223372036854775808)::int8 / (-1)::int2;
829+
ERROR: bigint out of range
830+
SELECT (-9223372036854775808)::int8 % (-1)::int2;
831+
?column?
832+
----------
833+
0
834+
(1 row)
835+

‎src/test/regress/expected/int8.out

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,3 +802,34 @@ SELECT * FROM generate_series('+4567890123456789'::int8, '+4567890123456799'::in
802802
4567890123456799
803803
(6 rows)
804804

805+
-- check sane handling of INT64_MIN overflow cases
806+
SELECT (-9223372036854775808)::int8 * (-1)::int8;
807+
ERROR: bigint out of range
808+
SELECT (-9223372036854775808)::int8 / (-1)::int8;
809+
ERROR: bigint out of range
810+
SELECT (-9223372036854775808)::int8 % (-1)::int8;
811+
?column?
812+
----------
813+
0
814+
(1 row)
815+
816+
SELECT (-9223372036854775808)::int8 * (-1)::int4;
817+
ERROR: bigint out of range
818+
SELECT (-9223372036854775808)::int8 / (-1)::int4;
819+
ERROR: bigint out of range
820+
SELECT (-9223372036854775808)::int8 % (-1)::int4;
821+
?column?
822+
----------
823+
0
824+
(1 row)
825+
826+
SELECT (-9223372036854775808)::int8 * (-1)::int2;
827+
ERROR: bigint out of range
828+
SELECT (-9223372036854775808)::int8 / (-1)::int2;
829+
ERROR: bigint out of range
830+
SELECT (-9223372036854775808)::int8 % (-1)::int2;
831+
?column?
832+
----------
833+
0
834+
(1 row)
835+

‎src/test/regress/sql/int2.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,7 @@ SELECT '' AS five, i.f1, i.f1 / int2 '2' AS x FROM INT2_TBL i;
8686

8787
SELECT '' AS five, i.f1, i.f1 / int4 '2' AS x FROM INT2_TBL i;
8888

89+
-- check sane handling of INT16_MIN overflow cases
90+
SELECT (-32768)::int2 * (-1)::int2;
91+
SELECT (-32768)::int2 / (-1)::int2;
92+
SELECT (-32768)::int2 % (-1)::int2;

0 commit comments

Comments
 (0)