Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 1f7cb5c

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. 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.
1 parent 644a0a6 commit 1f7cb5c

File tree

9 files changed

+233
-83
lines changed

9 files changed

+233
-83
lines changed

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

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

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

698686
/*
@@ -709,7 +697,8 @@ int4mul(PG_FUNCTION_ARGS)
709697
if (!(arg1 >= (int32) SHRT_MIN && arg1 <= (int32) SHRT_MAX &&
710698
arg2 >= (int32) SHRT_MIN && arg2 <= (int32) SHRT_MAX) &&
711699
arg2 != 0 &&
712-
(result / arg2 != arg1 || (arg2 == -1 && arg1 < 0 && result < 0)))
700+
((arg2 == -1 && arg1 < 0 && result < 0) ||
701+
result / arg2 != arg1))
713702
ereport(ERROR,
714703
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
715704
errmsg("integer out of range")));
@@ -732,30 +721,27 @@ int4div(PG_FUNCTION_ARGS)
732721
PG_RETURN_NULL();
733722
}
734723

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

747743
result = arg1 / arg2;
748744

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

@@ -877,18 +863,27 @@ int2div(PG_FUNCTION_ARGS)
877863
PG_RETURN_NULL();
878864
}
879865

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

@@ -1065,18 +1060,27 @@ int42div(PG_FUNCTION_ARGS)
10651060
PG_RETURN_NULL();
10661061
}
10671062

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

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

Lines changed: 59 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,8 @@ int8mul(PG_FUNCTION_ARGS)
574574
if (arg1 != (int64) ((int32) arg1) || arg2 != (int64) ((int32) arg2))
575575
{
576576
if (arg2 != 0 &&
577-
(result / arg2 != arg1 || (arg2 == -1 && arg1 < 0 && result < 0)))
577+
((arg2 == -1 && arg1 < 0 && result < 0) ||
578+
result / arg2 != arg1))
578579
ereport(ERROR,
579580
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
580581
errmsg("bigint out of range")));
@@ -598,18 +599,27 @@ int8div(PG_FUNCTION_ARGS)
598599
PG_RETURN_NULL();
599600
}
600601

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

@@ -838,18 +848,27 @@ int84div(PG_FUNCTION_ARGS)
838848
PG_RETURN_NULL();
839849
}
840850

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

@@ -1026,18 +1045,27 @@ int82div(PG_FUNCTION_ARGS)
10261045
PG_RETURN_NULL();
10271046
}
10281047

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

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,3 +255,14 @@ SELECT ((-1::int2<<15)+1::int2)::text;
255255
-32767
256256
(1 row)
257257

258+
-- check sane handling of INT16_MIN overflow cases
259+
SELECT (-32768)::int2 * (-1)::int2;
260+
ERROR: smallint out of range
261+
SELECT (-32768)::int2 / (-1)::int2;
262+
ERROR: smallint out of range
263+
SELECT (-32768)::int2 % (-1)::int2;
264+
?column?
265+
----------
266+
0
267+
(1 row)
268+

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,3 +342,24 @@ SELECT ((-1::int4<<31)+1)::text;
342342
-2147483647
343343
(1 row)
344344

345+
-- check sane handling of INT_MIN overflow cases
346+
SELECT (-2147483648)::int4 * (-1)::int4;
347+
ERROR: integer out of range
348+
SELECT (-2147483648)::int4 / (-1)::int4;
349+
ERROR: integer out of range
350+
SELECT (-2147483648)::int4 % (-1)::int4;
351+
?column?
352+
----------
353+
0
354+
(1 row)
355+
356+
SELECT (-2147483648)::int4 * (-1)::int2;
357+
ERROR: integer out of range
358+
SELECT (-2147483648)::int4 / (-1)::int2;
359+
ERROR: integer out of range
360+
SELECT (-2147483648)::int4 % (-1)::int2;
361+
?column?
362+
----------
363+
0
364+
(1 row)
365+

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,3 +815,34 @@ SELECT ((-1::int8<<63)+1)::text;
815815
-9223372036854775807
816816
(1 row)
817817

818+
-- check sane handling of INT64_MIN overflow cases
819+
SELECT (-9223372036854775808)::int8 * (-1)::int8;
820+
ERROR: bigint out of range
821+
SELECT (-9223372036854775808)::int8 / (-1)::int8;
822+
ERROR: bigint out of range
823+
SELECT (-9223372036854775808)::int8 % (-1)::int8;
824+
?column?
825+
----------
826+
0
827+
(1 row)
828+
829+
SELECT (-9223372036854775808)::int8 * (-1)::int4;
830+
ERROR: bigint out of range
831+
SELECT (-9223372036854775808)::int8 / (-1)::int4;
832+
ERROR: bigint out of range
833+
SELECT (-9223372036854775808)::int8 % (-1)::int4;
834+
?column?
835+
----------
836+
0
837+
(1 row)
838+
839+
SELECT (-9223372036854775808)::int8 * (-1)::int2;
840+
ERROR: bigint out of range
841+
SELECT (-9223372036854775808)::int8 / (-1)::int2;
842+
ERROR: bigint out of range
843+
SELECT (-9223372036854775808)::int8 % (-1)::int2;
844+
?column?
845+
----------
846+
0
847+
(1 row)
848+

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,3 +815,34 @@ SELECT ((-1::int8<<63)+1)::text;
815815
-9223372036854775807
816816
(1 row)
817817

818+
-- check sane handling of INT64_MIN overflow cases
819+
SELECT (-9223372036854775808)::int8 * (-1)::int8;
820+
ERROR: bigint out of range
821+
SELECT (-9223372036854775808)::int8 / (-1)::int8;
822+
ERROR: bigint out of range
823+
SELECT (-9223372036854775808)::int8 % (-1)::int8;
824+
?column?
825+
----------
826+
0
827+
(1 row)
828+
829+
SELECT (-9223372036854775808)::int8 * (-1)::int4;
830+
ERROR: bigint out of range
831+
SELECT (-9223372036854775808)::int8 / (-1)::int4;
832+
ERROR: bigint out of range
833+
SELECT (-9223372036854775808)::int8 % (-1)::int4;
834+
?column?
835+
----------
836+
0
837+
(1 row)
838+
839+
SELECT (-9223372036854775808)::int8 * (-1)::int2;
840+
ERROR: bigint out of range
841+
SELECT (-9223372036854775808)::int8 / (-1)::int2;
842+
ERROR: bigint out of range
843+
SELECT (-9223372036854775808)::int8 % (-1)::int2;
844+
?column?
845+
----------
846+
0
847+
(1 row)
848+

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,8 @@ SELECT '' AS five, i.f1, i.f1 / int4 '2' AS x FROM INT2_TBL i;
8787
-- corner cases
8888
SELECT (-1::int2<<15)::text;
8989
SELECT ((-1::int2<<15)+1::int2)::text;
90+
91+
-- check sane handling of INT16_MIN overflow cases
92+
SELECT (-32768)::int2 * (-1)::int2;
93+
SELECT (-32768)::int2 / (-1)::int2;
94+
SELECT (-32768)::int2 % (-1)::int2;

0 commit comments

Comments
 (0)