Skip to content

Commit 3352e25

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 8728fdc commit 3352e25

File tree

9 files changed

+198
-68
lines changed

9 files changed

+198
-68
lines changed

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

Lines changed: 64 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -671,18 +671,6 @@ int4mul(PG_FUNCTION_ARGS)
671671
int32 arg2 = PG_GETARG_INT32(1);
672672
int32 result;
673673

674-
#ifdef WIN32
675-
676-
/*
677-
* Win32 doesn't throw a catchable exception for SELECT -2147483648 *
678-
* (-1); -- INT_MIN
679-
*/
680-
if (arg2 == -1 && arg1 == INT_MIN)
681-
ereport(ERROR,
682-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
683-
errmsg("integer out of range")));
684-
#endif
685-
686674
result = arg1 * arg2;
687675

688676
/*
@@ -699,7 +687,8 @@ int4mul(PG_FUNCTION_ARGS)
699687
if (!(arg1 >= (int32) SHRT_MIN && arg1 <= (int32) SHRT_MAX &&
700688
arg2 >= (int32) SHRT_MIN && arg2 <= (int32) SHRT_MAX) &&
701689
arg2 != 0 &&
702-
(result / arg2 != arg1 || (arg2 == -1 && arg1 < 0 && result < 0)))
690+
((arg2 == -1 && arg1 < 0 && result < 0) ||
691+
result / arg2 != arg1))
703692
ereport(ERROR,
704693
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
705694
errmsg("integer out of range")));
@@ -722,29 +711,27 @@ int4div(PG_FUNCTION_ARGS)
722711
PG_RETURN_NULL();
723712
}
724713

725-
#ifdef WIN32
726-
727714
/*
728-
* Win32 doesn't throw a catchable exception for SELECT -2147483648 /
729-
* (-1); -- INT_MIN
715+
* INT_MIN / -1 is problematic, since the result can't be represented on a
716+
* two's-complement machine. Some machines produce INT_MIN, some produce
717+
* zero, some throw an exception. We can dodge the problem by recognizing
718+
* that division by -1 is the same as negation.
730719
*/
731-
if (arg2 == -1 && arg1 == INT_MIN)
732-
ereport(ERROR,
733-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
734-
errmsg("integer out of range")));
735-
#endif
720+
if (arg2 == -1)
721+
{
722+
result = -arg1;
723+
/* overflow check (needed for INT_MIN) */
724+
if (arg1 != 0 && SAMESIGN(result, arg1))
725+
ereport(ERROR,
726+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
727+
errmsg("integer out of range")));
728+
PG_RETURN_INT32(result);
729+
}
730+
731+
/* No overflow is possible */
736732

737733
result = arg1 / arg2;
738734

739-
/*
740-
* Overflow check. The only possible overflow case is for arg1 = INT_MIN,
741-
* arg2 = -1, where the correct result is -INT_MIN, which can't be
742-
* represented on a two's-complement machine.
743-
*/
744-
if (arg2 == -1 && arg1 < 0 && result < 0)
745-
ereport(ERROR,
746-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
747-
errmsg("integer out of range")));
748735
PG_RETURN_INT32(result);
749736
}
750737

@@ -866,17 +853,27 @@ int2div(PG_FUNCTION_ARGS)
866853
PG_RETURN_NULL();
867854
}
868855

869-
result = arg1 / arg2;
870-
871856
/*
872-
* Overflow check. The only possible overflow case is for arg1 =
873-
* SHRT_MIN, arg2 = -1, where the correct result is -SHRT_MIN, which can't
874-
* be represented on a two's-complement machine.
857+
* SHRT_MIN / -1 is problematic, since the result can't be represented on
858+
* a two's-complement machine. Some machines produce SHRT_MIN, some
859+
* produce zero, some throw an exception. We can dodge the problem by
860+
* recognizing that division by -1 is the same as negation.
875861
*/
876-
if (arg2 == -1 && arg1 < 0 && result < 0)
877-
ereport(ERROR,
878-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
879-
errmsg("smallint out of range")));
862+
if (arg2 == -1)
863+
{
864+
result = -arg1;
865+
/* overflow check (needed for SHRT_MIN) */
866+
if (arg1 != 0 && SAMESIGN(result, arg1))
867+
ereport(ERROR,
868+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
869+
errmsg("smallint out of range")));
870+
PG_RETURN_INT16(result);
871+
}
872+
873+
/* No overflow is possible */
874+
875+
result = arg1 / arg2;
876+
880877
PG_RETURN_INT16(result);
881878
}
882879

@@ -1054,17 +1051,27 @@ int42div(PG_FUNCTION_ARGS)
10541051
PG_RETURN_NULL();
10551052
}
10561053

1057-
result = arg1 / arg2;
1058-
10591054
/*
1060-
* Overflow check. The only possible overflow case is for arg1 = INT_MIN,
1061-
* arg2 = -1, where the correct result is -INT_MIN, which can't be
1062-
* represented on a two's-complement machine.
1055+
* INT_MIN / -1 is problematic, since the result can't be represented on a
1056+
* two's-complement machine. Some machines produce INT_MIN, some produce
1057+
* zero, some throw an exception. We can dodge the problem by recognizing
1058+
* that division by -1 is the same as negation.
10631059
*/
1064-
if (arg2 == -1 && arg1 < 0 && result < 0)
1065-
ereport(ERROR,
1066-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
1067-
errmsg("integer out of range")));
1060+
if (arg2 == -1)
1061+
{
1062+
result = -arg1;
1063+
/* overflow check (needed for INT_MIN) */
1064+
if (arg1 != 0 && SAMESIGN(result, arg1))
1065+
ereport(ERROR,
1066+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
1067+
errmsg("integer out of range")));
1068+
PG_RETURN_INT32(result);
1069+
}
1070+
1071+
/* No overflow is possible */
1072+
1073+
result = arg1 / arg2;
1074+
10681075
PG_RETURN_INT32(result);
10691076
}
10701077

@@ -1160,6 +1167,14 @@ int42mod(PG_FUNCTION_ARGS)
11601167
PG_RETURN_NULL();
11611168
}
11621169

1170+
/*
1171+
* Some machines throw a floating-point exception for INT_MIN % -1, which
1172+
* is a bit silly since the correct answer is perfectly well-defined,
1173+
* namely zero.
1174+
*/
1175+
if (arg2 == -1)
1176+
PG_RETURN_INT32(0);
1177+
11631178
/* No overflow is possible */
11641179

11651180
PG_RETURN_INT32(arg1 % arg2);

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

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,8 @@ int8mul(PG_FUNCTION_ARGS)
583583
#endif
584584
{
585585
if (arg2 != 0 &&
586-
(result / arg2 != arg1 || (arg2 == -1 && arg1 < 0 && result < 0)))
586+
((arg2 == -1 && arg1 < 0 && result < 0) ||
587+
result / arg2 != arg1))
587588
ereport(ERROR,
588589
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
589590
errmsg("bigint out of range")));
@@ -607,17 +608,27 @@ int8div(PG_FUNCTION_ARGS)
607608
PG_RETURN_NULL();
608609
}
609610

610-
result = arg1 / arg2;
611-
612611
/*
613-
* Overflow check. The only possible overflow case is for arg1 =
614-
* INT64_MIN, arg2 = -1, where the correct result is -INT64_MIN, which
615-
* can't be represented on a two's-complement machine.
612+
* INT64_MIN / -1 is problematic, since the result can't be represented on
613+
* a two's-complement machine. Some machines produce INT64_MIN, some
614+
* produce zero, some throw an exception. We can dodge the problem by
615+
* recognizing that division by -1 is the same as negation.
616616
*/
617-
if (arg2 == -1 && arg1 < 0 && result < 0)
618-
ereport(ERROR,
619-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
620-
errmsg("bigint out of range")));
617+
if (arg2 == -1)
618+
{
619+
result = -arg1;
620+
/* overflow check (needed for INT64_MIN) */
621+
if (arg1 != 0 && SAMESIGN(result, arg1))
622+
ereport(ERROR,
623+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
624+
errmsg("bigint out of range")));
625+
PG_RETURN_INT64(result);
626+
}
627+
628+
/* No overflow is possible */
629+
630+
result = arg1 / arg2;
631+
621632
PG_RETURN_INT64(result);
622633
}
623634

@@ -846,17 +857,27 @@ int84div(PG_FUNCTION_ARGS)
846857
PG_RETURN_NULL();
847858
}
848859

849-
result = arg1 / arg2;
850-
851860
/*
852-
* Overflow check. The only possible overflow case is for arg1 =
853-
* INT64_MIN, arg2 = -1, where the correct result is -INT64_MIN, which
854-
* can't be represented on a two's-complement machine.
861+
* INT64_MIN / -1 is problematic, since the result can't be represented on
862+
* a two's-complement machine. Some machines produce INT64_MIN, some
863+
* produce zero, some throw an exception. We can dodge the problem by
864+
* recognizing that division by -1 is the same as negation.
855865
*/
856-
if (arg2 == -1 && arg1 < 0 && result < 0)
857-
ereport(ERROR,
858-
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
859-
errmsg("bigint out of range")));
866+
if (arg2 == -1)
867+
{
868+
result = -arg1;
869+
/* overflow check (needed for INT64_MIN) */
870+
if (arg1 != 0 && SAMESIGN(result, arg1))
871+
ereport(ERROR,
872+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
873+
errmsg("bigint out of range")));
874+
PG_RETURN_INT64(result);
875+
}
876+
877+
/* No overflow is possible */
878+
879+
result = arg1 / arg2;
880+
860881
PG_RETURN_INT64(result);
861882
}
862883

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

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

231+
-- check sane handling of INT16_MIN overflow cases
232+
SELECT (-32768)::int2 * (-1)::int2;
233+
ERROR: smallint out of range
234+
SELECT (-32768)::int2 / (-1)::int2;
235+
ERROR: smallint out of range
236+
SELECT (-32768)::int2 % (-1)::int2;
237+
?column?
238+
----------
239+
0
240+
(1 row)
241+

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,3 +315,24 @@ SELECT (2 + 2) / 2 AS two;
315315
2
316316
(1 row)
317317

318+
-- check sane handling of INT_MIN overflow cases
319+
SELECT (-2147483648)::int4 * (-1)::int4;
320+
ERROR: integer out of range
321+
SELECT (-2147483648)::int4 / (-1)::int4;
322+
ERROR: integer out of range
323+
SELECT (-2147483648)::int4 % (-1)::int4;
324+
?column?
325+
----------
326+
0
327+
(1 row)
328+
329+
SELECT (-2147483648)::int4 * (-1)::int2;
330+
ERROR: integer out of range
331+
SELECT (-2147483648)::int4 / (-1)::int2;
332+
ERROR: integer out of range
333+
SELECT (-2147483648)::int4 % (-1)::int2;
334+
?column?
335+
----------
336+
0
337+
(1 row)
338+

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,3 +317,24 @@ select '9223372036854775807'::int8;
317317

318318
select '9223372036854775808'::int8;
319319
ERROR: value "9223372036854775808" is out of range for type bigint
320+
-- check sane handling of INT64_MIN overflow cases
321+
SELECT (-9223372036854775808)::int8 * (-1)::int8;
322+
ERROR: bigint out of range
323+
SELECT (-9223372036854775808)::int8 / (-1)::int8;
324+
ERROR: bigint out of range
325+
SELECT (-9223372036854775808)::int8 % (-1)::int8;
326+
?column?
327+
----------
328+
0
329+
(1 row)
330+
331+
SELECT (-9223372036854775808)::int8 * (-1)::int4;
332+
ERROR: bigint out of range
333+
SELECT (-9223372036854775808)::int8 / (-1)::int4;
334+
ERROR: bigint out of range
335+
SELECT (-9223372036854775808)::int8 % (-1)::int4;
336+
?column?
337+
----------
338+
0
339+
(1 row)
340+

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,3 +317,24 @@ select '9223372036854775807'::int8;
317317

318318
select '9223372036854775808'::int8;
319319
ERROR: value "9223372036854775808" is out of range for type bigint
320+
-- check sane handling of INT64_MIN overflow cases
321+
SELECT (-9223372036854775808)::int8 * (-1)::int8;
322+
ERROR: bigint out of range
323+
SELECT (-9223372036854775808)::int8 / (-1)::int8;
324+
ERROR: bigint out of range
325+
SELECT (-9223372036854775808)::int8 % (-1)::int8;
326+
?column?
327+
----------
328+
0
329+
(1 row)
330+
331+
SELECT (-9223372036854775808)::int8 * (-1)::int4;
332+
ERROR: bigint out of range
333+
SELECT (-9223372036854775808)::int8 / (-1)::int4;
334+
ERROR: bigint out of range
335+
SELECT (-9223372036854775808)::int8 % (-1)::int4;
336+
?column?
337+
----------
338+
0
339+
(1 row)
340+

‎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;

‎src/test/regress/sql/int4.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,3 +125,11 @@ SELECT 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 AS ten;
125125
SELECT 2 + 2 / 2 AS three;
126126

127127
SELECT (2 + 2) / 2 AS two;
128+
129+
-- check sane handling of INT_MIN overflow cases
130+
SELECT (-2147483648)::int4 * (-1)::int4;
131+
SELECT (-2147483648)::int4 / (-1)::int4;
132+
SELECT (-2147483648)::int4 % (-1)::int4;
133+
SELECT (-2147483648)::int4 * (-1)::int2;
134+
SELECT (-2147483648)::int4 / (-1)::int2;
135+
SELECT (-2147483648)::int4 % (-1)::int2;

‎src/test/regress/sql/int8.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,11 @@ select '-9223372036854775808'::int8;
6969
select '-9223372036854775809'::int8;
7070
select '9223372036854775807'::int8;
7171
select '9223372036854775808'::int8;
72+
73+
-- check sane handling of INT64_MIN overflow cases
74+
SELECT (-9223372036854775808)::int8 * (-1)::int8;
75+
SELECT (-9223372036854775808)::int8 / (-1)::int8;
76+
SELECT (-9223372036854775808)::int8 % (-1)::int8;
77+
SELECT (-9223372036854775808)::int8 * (-1)::int4;
78+
SELECT (-9223372036854775808)::int8 / (-1)::int4;
79+
SELECT (-9223372036854775808)::int8 % (-1)::int4;

0 commit comments

Comments
 (0)