Skip to content

Commit f6e72dc

Browse files
tglsfdctglsfdc
committed
Fix integer-overflow edge case detection in interval_mul and pgbench.
This patch adopts the overflow check logic introduced by commit cbdb8b4 into two more places. interval_mul() failed to notice if it computed a new microseconds value that was one more than INT64_MAX, and pgbench's double-to-int64 logic had the same sorts of edge-case problems that cbdb8b4 fixed in the core code. To make this easier to get right in future, put the guts of the checks into new macros in c.h, and add commentary about how to use the macros correctly. Back-patch to all supported branches, as we did with the previous fix. Yuya Watari Discussion: https://postgr.es/m/CAJ2pMkbkkFw2hb9Qb1Zj8d06EhWAQXFLy73St4qWv6aX=vqnjw@mail.gmail.com
1 parent e5cfb8c commit f6e72dc

File tree

7 files changed

+45
-57
lines changed

7 files changed

+45
-57
lines changed

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

Lines changed: 8 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,15 +1206,8 @@ dtoi4(PG_FUNCTION_ARGS)
12061206
*/
12071207
num = rint(num);
12081208

1209-
/*
1210-
* Range check. We must be careful here that the boundary values are
1211-
* expressed exactly in the float domain. We expect PG_INT32_MIN to be an
1212-
* exact power of 2, so it will be represented exactly; but PG_INT32_MAX
1213-
* isn't, and might get rounded off, so avoid using it.
1214-
*/
1215-
if (unlikely(num < (float8) PG_INT32_MIN ||
1216-
num >= -((float8) PG_INT32_MIN) ||
1217-
isnan(num)))
1209+
/* Range check */
1210+
if (unlikely(isnan(num) || !FLOAT8_FITS_IN_INT32(num)))
12181211
ereport(ERROR,
12191212
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
12201213
errmsg("integer out of range")));
@@ -1238,15 +1231,8 @@ dtoi2(PG_FUNCTION_ARGS)
12381231
*/
12391232
num = rint(num);
12401233

1241-
/*
1242-
* Range check. We must be careful here that the boundary values are
1243-
* expressed exactly in the float domain. We expect PG_INT16_MIN to be an
1244-
* exact power of 2, so it will be represented exactly; but PG_INT16_MAX
1245-
* isn't, and might get rounded off, so avoid using it.
1246-
*/
1247-
if (unlikely(num < (float8) PG_INT16_MIN ||
1248-
num >= -((float8) PG_INT16_MIN) ||
1249-
isnan(num)))
1234+
/* Range check */
1235+
if (unlikely(isnan(num) || !FLOAT8_FITS_IN_INT16(num)))
12501236
ereport(ERROR,
12511237
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
12521238
errmsg("smallint out of range")));
@@ -1294,15 +1280,8 @@ ftoi4(PG_FUNCTION_ARGS)
12941280
*/
12951281
num = rint(num);
12961282

1297-
/*
1298-
* Range check. We must be careful here that the boundary values are
1299-
* expressed exactly in the float domain. We expect PG_INT32_MIN to be an
1300-
* exact power of 2, so it will be represented exactly; but PG_INT32_MAX
1301-
* isn't, and might get rounded off, so avoid using it.
1302-
*/
1303-
if (unlikely(num < (float4) PG_INT32_MIN ||
1304-
num >= -((float4) PG_INT32_MIN) ||
1305-
isnan(num)))
1283+
/* Range check */
1284+
if (unlikely(isnan(num) || !FLOAT4_FITS_IN_INT32(num)))
13061285
ereport(ERROR,
13071286
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
13081287
errmsg("integer out of range")));
@@ -1326,15 +1305,8 @@ ftoi2(PG_FUNCTION_ARGS)
13261305
*/
13271306
num = rint(num);
13281307

1329-
/*
1330-
* Range check. We must be careful here that the boundary values are
1331-
* expressed exactly in the float domain. We expect PG_INT16_MIN to be an
1332-
* exact power of 2, so it will be represented exactly; but PG_INT16_MAX
1333-
* isn't, and might get rounded off, so avoid using it.
1334-
*/
1335-
if (unlikely(num < (float4) PG_INT16_MIN ||
1336-
num >= -((float4) PG_INT16_MIN) ||
1337-
isnan(num)))
1308+
/* Range check */
1309+
if (unlikely(isnan(num) || !FLOAT4_FITS_IN_INT16(num)))
13381310
ereport(ERROR,
13391311
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
13401312
errmsg("smallint out of range")));

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

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1216,15 +1216,8 @@ dtoi8(PG_FUNCTION_ARGS)
12161216
*/
12171217
num = rint(num);
12181218

1219-
/*
1220-
* Range check. We must be careful here that the boundary values are
1221-
* expressed exactly in the float domain. We expect PG_INT64_MIN to be an
1222-
* exact power of 2, so it will be represented exactly; but PG_INT64_MAX
1223-
* isn't, and might get rounded off, so avoid using it.
1224-
*/
1225-
if (unlikely(num < (float8) PG_INT64_MIN ||
1226-
num >= -((float8) PG_INT64_MIN) ||
1227-
isnan(num)))
1219+
/* Range check */
1220+
if (unlikely(isnan(num) || !FLOAT8_FITS_IN_INT64(num)))
12281221
ereport(ERROR,
12291222
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
12301223
errmsg("bigint out of range")));
@@ -1258,15 +1251,8 @@ ftoi8(PG_FUNCTION_ARGS)
12581251
*/
12591252
num = rint(num);
12601253

1261-
/*
1262-
* Range check. We must be careful here that the boundary values are
1263-
* expressed exactly in the float domain. We expect PG_INT64_MIN to be an
1264-
* exact power of 2, so it will be represented exactly; but PG_INT64_MAX
1265-
* isn't, and might get rounded off, so avoid using it.
1266-
*/
1267-
if (unlikely(num < (float4) PG_INT64_MIN ||
1268-
num >= -((float4) PG_INT64_MIN) ||
1269-
isnan(num)))
1254+
/* Range check */
1255+
if (unlikely(isnan(num) || !FLOAT4_FITS_IN_INT64(num)))
12701256
ereport(ERROR,
12711257
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
12721258
errmsg("bigint out of range")));

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3219,7 +3219,7 @@ interval_mul(PG_FUNCTION_ARGS)
32193219
/* cascade units down */
32203220
result->day += (int32) month_remainder_days;
32213221
result_double = rint(span->time * factor + sec_remainder * USECS_PER_SEC);
3222-
if (result_double > PG_INT64_MAX || result_double < PG_INT64_MIN)
3222+
if (isnan(result_double) || !FLOAT8_FITS_IN_INT64(result_double))
32233223
ereport(ERROR,
32243224
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
32253225
errmsg("interval out of range")));

‎src/bin/pgbench/pgbench.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,9 +1651,9 @@ coerceToInt(PgBenchValue *pval, int64 *ival)
16511651
}
16521652
else if (pval->type == PGBT_DOUBLE)
16531653
{
1654-
double dval = pval->u.dval;
1654+
double dval = rint(pval->u.dval);
16551655

1656-
if (dval < PG_INT64_MIN || PG_INT64_MAX < dval)
1656+
if (isnan(dval) || !FLOAT8_FITS_IN_INT64(dval))
16571657
{
16581658
fprintf(stderr, "double to int overflow for %f\n", dval);
16591659
return false;

‎src/include/c.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,6 +1016,30 @@ extern void ExceptionalCondition(const char *conditionName,
10161016
*_start++ = 0; \
10171017
} while (0)
10181018

1019+
/*
1020+
* Macros for range-checking float values before converting to integer.
1021+
* We must be careful here that the boundary values are expressed exactly
1022+
* in the float domain. PG_INTnn_MIN is an exact power of 2, so it will
1023+
* be represented exactly; but PG_INTnn_MAX isn't, and might get rounded
1024+
* off, so avoid using that.
1025+
* The input must be rounded to an integer beforehand, typically with rint(),
1026+
* else we might draw the wrong conclusion about close-to-the-limit values.
1027+
* These macros will do the right thing for Inf, but not necessarily for NaN,
1028+
* so check isnan(num) first if that's a possibility.
1029+
*/
1030+
#define FLOAT4_FITS_IN_INT16(num) \
1031+
((num) >= (float4) PG_INT16_MIN && (num) < -((float4) PG_INT16_MIN))
1032+
#define FLOAT4_FITS_IN_INT32(num) \
1033+
((num) >= (float4) PG_INT32_MIN && (num) < -((float4) PG_INT32_MIN))
1034+
#define FLOAT4_FITS_IN_INT64(num) \
1035+
((num) >= (float4) PG_INT64_MIN && (num) < -((float4) PG_INT64_MIN))
1036+
#define FLOAT8_FITS_IN_INT16(num) \
1037+
((num) >= (float8) PG_INT16_MIN && (num) < -((float8) PG_INT16_MIN))
1038+
#define FLOAT8_FITS_IN_INT32(num) \
1039+
((num) >= (float8) PG_INT32_MIN && (num) < -((float8) PG_INT32_MIN))
1040+
#define FLOAT8_FITS_IN_INT64(num) \
1041+
((num) >= (float8) PG_INT64_MIN && (num) < -((float8) PG_INT64_MIN))
1042+
10191043

10201044
/* ----------------------------------------------------------------
10211045
* Section 8: random stuff

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,9 @@ INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483648 years');
232232
ERROR: interval out of range
233233
LINE 1: INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483648 years'...
234234
^
235+
-- Test edge-case overflow detection in interval multiplication
236+
select extract(epoch from '256 microseconds'::interval * (2^55)::float8);
237+
ERROR: interval out of range
235238
SELECT r1.*, r2.*
236239
FROM INTERVAL_TBL_OF r1, INTERVAL_TBL_OF r2
237240
WHERE r1.f1 > r2.f1

‎src/test/regress/sql/interval.sql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483649 days');
7373
INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('2147483647 years');
7474
INSERT INTO INTERVAL_TBL_OF (f1) VALUES ('-2147483648 years');
7575

76+
-- Test edge-case overflow detection in interval multiplication
77+
select extract(epoch from '256 microseconds'::interval * (2^55)::float8);
78+
7679
SELECT r1.*, r2.*
7780
FROM INTERVAL_TBL_OF r1, INTERVAL_TBL_OF r2
7881
WHERE r1.f1 > r2.f1

0 commit comments

Comments
 (0)