Skip to content

Commit 5f794f7

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 127ad57 commit 5f794f7

File tree

7 files changed

+46
-57
lines changed

7 files changed

+46
-57
lines changed

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

Lines changed: 8 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1227,15 +1227,8 @@ dtoi4(PG_FUNCTION_ARGS)
12271227
*/
12281228
num = rint(num);
12291229

1230-
/*
1231-
* Range check. We must be careful here that the boundary values are
1232-
* expressed exactly in the float domain. We expect PG_INT32_MIN to be an
1233-
* exact power of 2, so it will be represented exactly; but PG_INT32_MAX
1234-
* isn't, and might get rounded off, so avoid using it.
1235-
*/
1236-
if (num < (float8) PG_INT32_MIN ||
1237-
num >= -((float8) PG_INT32_MIN) ||
1238-
isnan(num))
1230+
/* Range check */
1231+
if (isnan(num) || !FLOAT8_FITS_IN_INT32(num))
12391232
ereport(ERROR,
12401233
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
12411234
errmsg("integer out of range")));
@@ -1259,15 +1252,8 @@ dtoi2(PG_FUNCTION_ARGS)
12591252
*/
12601253
num = rint(num);
12611254

1262-
/*
1263-
* Range check. We must be careful here that the boundary values are
1264-
* expressed exactly in the float domain. We expect PG_INT16_MIN to be an
1265-
* exact power of 2, so it will be represented exactly; but PG_INT16_MAX
1266-
* isn't, and might get rounded off, so avoid using it.
1267-
*/
1268-
if (num < (float8) PG_INT16_MIN ||
1269-
num >= -((float8) PG_INT16_MIN) ||
1270-
isnan(num))
1255+
/* Range check */
1256+
if (isnan(num) || !FLOAT8_FITS_IN_INT16(num))
12711257
ereport(ERROR,
12721258
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
12731259
errmsg("smallint out of range")));
@@ -1315,15 +1301,8 @@ ftoi4(PG_FUNCTION_ARGS)
13151301
*/
13161302
num = rint(num);
13171303

1318-
/*
1319-
* Range check. We must be careful here that the boundary values are
1320-
* expressed exactly in the float domain. We expect PG_INT32_MIN to be an
1321-
* exact power of 2, so it will be represented exactly; but PG_INT32_MAX
1322-
* isn't, and might get rounded off, so avoid using it.
1323-
*/
1324-
if (num < (float4) PG_INT32_MIN ||
1325-
num >= -((float4) PG_INT32_MIN) ||
1326-
isnan(num))
1304+
/* Range check */
1305+
if (isnan(num) || !FLOAT4_FITS_IN_INT32(num))
13271306
ereport(ERROR,
13281307
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
13291308
errmsg("integer out of range")));
@@ -1347,15 +1326,8 @@ ftoi2(PG_FUNCTION_ARGS)
13471326
*/
13481327
num = rint(num);
13491328

1350-
/*
1351-
* Range check. We must be careful here that the boundary values are
1352-
* expressed exactly in the float domain. We expect PG_INT16_MIN to be an
1353-
* exact power of 2, so it will be represented exactly; but PG_INT16_MAX
1354-
* isn't, and might get rounded off, so avoid using it.
1355-
*/
1356-
if (num < (float4) PG_INT16_MIN ||
1357-
num >= -((float4) PG_INT16_MIN) ||
1358-
isnan(num))
1329+
/* Range check */
1330+
if (isnan(num) || !FLOAT4_FITS_IN_INT16(num))
13591331
ereport(ERROR,
13601332
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
13611333
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
@@ -1351,15 +1351,8 @@ dtoi8(PG_FUNCTION_ARGS)
13511351
*/
13521352
num = rint(num);
13531353

1354-
/*
1355-
* Range check. We must be careful here that the boundary values are
1356-
* expressed exactly in the float domain. We expect PG_INT64_MIN to be an
1357-
* exact power of 2, so it will be represented exactly; but PG_INT64_MAX
1358-
* isn't, and might get rounded off, so avoid using it.
1359-
*/
1360-
if (num < (float8) PG_INT64_MIN ||
1361-
num >= -((float8) PG_INT64_MIN) ||
1362-
isnan(num))
1354+
/* Range check */
1355+
if (isnan(num) || !FLOAT8_FITS_IN_INT64(num))
13631356
ereport(ERROR,
13641357
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
13651358
errmsg("bigint out of range")));
@@ -1393,15 +1386,8 @@ ftoi8(PG_FUNCTION_ARGS)
13931386
*/
13941387
num = rint(num);
13951388

1396-
/*
1397-
* Range check. We must be careful here that the boundary values are
1398-
* expressed exactly in the float domain. We expect PG_INT64_MIN to be an
1399-
* exact power of 2, so it will be represented exactly; but PG_INT64_MAX
1400-
* isn't, and might get rounded off, so avoid using it.
1401-
*/
1402-
if (num < (float4) PG_INT64_MIN ||
1403-
num >= -((float4) PG_INT64_MIN) ||
1404-
isnan(num))
1389+
/* Range check */
1390+
if (isnan(num) || !FLOAT4_FITS_IN_INT64(num))
14051391
ereport(ERROR,
14061392
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
14071393
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
@@ -3181,7 +3181,7 @@ interval_mul(PG_FUNCTION_ARGS)
31813181
/* cascade units down */
31823182
result->day += (int32) month_remainder_days;
31833183
result_double = rint(span->time * factor + sec_remainder * USECS_PER_SEC);
3184-
if (result_double > PG_INT64_MAX || result_double < PG_INT64_MIN)
3184+
if (isnan(result_double) || !FLOAT8_FITS_IN_INT64(result_double))
31853185
ereport(ERROR,
31863186
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
31873187
errmsg("interval out of range")));

‎src/bin/pgbench/pgbench.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,10 +1236,11 @@ coerceToInt(PgBenchValue *pval, int64 *ival)
12361236
}
12371237
else
12381238
{
1239-
double dval = pval->u.dval;
1239+
double dval;
12401240

12411241
Assert(pval->type == PGBT_DOUBLE);
1242-
if (dval < PG_INT64_MIN || PG_INT64_MAX < dval)
1242+
dval = rint(pval->u.dval);
1243+
if (isnan(dval) || !FLOAT8_FITS_IN_INT64(dval))
12431244
{
12441245
fprintf(stderr, "double to int overflow for %f\n", dval);
12451246
return false;

‎src/include/c.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -963,6 +963,30 @@ typedef NameData *Name;
963963
*_start++ = 0; \
964964
} while (0)
965965

966+
/*
967+
* Macros for range-checking float values before converting to integer.
968+
* We must be careful here that the boundary values are expressed exactly
969+
* in the float domain. PG_INTnn_MIN is an exact power of 2, so it will
970+
* be represented exactly; but PG_INTnn_MAX isn't, and might get rounded
971+
* off, so avoid using that.
972+
* The input must be rounded to an integer beforehand, typically with rint(),
973+
* else we might draw the wrong conclusion about close-to-the-limit values.
974+
* These macros will do the right thing for Inf, but not necessarily for NaN,
975+
* so check isnan(num) first if that's a possibility.
976+
*/
977+
#define FLOAT4_FITS_IN_INT16(num) \
978+
((num) >= (float4) PG_INT16_MIN && (num) < -((float4) PG_INT16_MIN))
979+
#define FLOAT4_FITS_IN_INT32(num) \
980+
((num) >= (float4) PG_INT32_MIN && (num) < -((float4) PG_INT32_MIN))
981+
#define FLOAT4_FITS_IN_INT64(num) \
982+
((num) >= (float4) PG_INT64_MIN && (num) < -((float4) PG_INT64_MIN))
983+
#define FLOAT8_FITS_IN_INT16(num) \
984+
((num) >= (float8) PG_INT16_MIN && (num) < -((float8) PG_INT16_MIN))
985+
#define FLOAT8_FITS_IN_INT32(num) \
986+
((num) >= (float8) PG_INT32_MIN && (num) < -((float8) PG_INT32_MIN))
987+
#define FLOAT8_FITS_IN_INT64(num) \
988+
((num) >= (float8) PG_INT64_MIN && (num) < -((float8) PG_INT64_MIN))
989+
966990

967991
/* ----------------------------------------------------------------
968992
* 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)