Skip to content

Commit a7145f6

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 effa402 commit a7145f6

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
@@ -1212,15 +1212,8 @@ dtoi4(PG_FUNCTION_ARGS)
12121212
*/
12131213
num = rint(num);
12141214

1215-
/*
1216-
* Range check. We must be careful here that the boundary values are
1217-
* expressed exactly in the float domain. We expect PG_INT32_MIN to be an
1218-
* exact power of 2, so it will be represented exactly; but PG_INT32_MAX
1219-
* isn't, and might get rounded off, so avoid using it.
1220-
*/
1221-
if (unlikely(num < (float8) PG_INT32_MIN ||
1222-
num >= -((float8) PG_INT32_MIN) ||
1223-
isnan(num)))
1215+
/* Range check */
1216+
if (unlikely(isnan(num) || !FLOAT8_FITS_IN_INT32(num)))
12241217
ereport(ERROR,
12251218
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
12261219
errmsg("integer out of range")));
@@ -1244,15 +1237,8 @@ dtoi2(PG_FUNCTION_ARGS)
12441237
*/
12451238
num = rint(num);
12461239

1247-
/*
1248-
* Range check. We must be careful here that the boundary values are
1249-
* expressed exactly in the float domain. We expect PG_INT16_MIN to be an
1250-
* exact power of 2, so it will be represented exactly; but PG_INT16_MAX
1251-
* isn't, and might get rounded off, so avoid using it.
1252-
*/
1253-
if (unlikely(num < (float8) PG_INT16_MIN ||
1254-
num >= -((float8) PG_INT16_MIN) ||
1255-
isnan(num)))
1240+
/* Range check */
1241+
if (unlikely(isnan(num) || !FLOAT8_FITS_IN_INT16(num)))
12561242
ereport(ERROR,
12571243
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
12581244
errmsg("smallint out of range")));
@@ -1300,15 +1286,8 @@ ftoi4(PG_FUNCTION_ARGS)
13001286
*/
13011287
num = rint(num);
13021288

1303-
/*
1304-
* Range check. We must be careful here that the boundary values are
1305-
* expressed exactly in the float domain. We expect PG_INT32_MIN to be an
1306-
* exact power of 2, so it will be represented exactly; but PG_INT32_MAX
1307-
* isn't, and might get rounded off, so avoid using it.
1308-
*/
1309-
if (unlikely(num < (float4) PG_INT32_MIN ||
1310-
num >= -((float4) PG_INT32_MIN) ||
1311-
isnan(num)))
1289+
/* Range check */
1290+
if (unlikely(isnan(num) || !FLOAT4_FITS_IN_INT32(num)))
13121291
ereport(ERROR,
13131292
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
13141293
errmsg("integer out of range")));
@@ -1332,15 +1311,8 @@ ftoi2(PG_FUNCTION_ARGS)
13321311
*/
13331312
num = rint(num);
13341313

1335-
/*
1336-
* Range check. We must be careful here that the boundary values are
1337-
* expressed exactly in the float domain. We expect PG_INT16_MIN to be an
1338-
* exact power of 2, so it will be represented exactly; but PG_INT16_MAX
1339-
* isn't, and might get rounded off, so avoid using it.
1340-
*/
1341-
if (unlikely(num < (float4) PG_INT16_MIN ||
1342-
num >= -((float4) PG_INT16_MIN) ||
1343-
isnan(num)))
1314+
/* Range check */
1315+
if (unlikely(isnan(num) || !FLOAT4_FITS_IN_INT16(num)))
13441316
ereport(ERROR,
13451317
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
13461318
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
@@ -3233,7 +3233,7 @@ interval_mul(PG_FUNCTION_ARGS)
32333233
/* cascade units down */
32343234
result->day += (int32) month_remainder_days;
32353235
result_double = rint(span->time * factor + sec_remainder * USECS_PER_SEC);
3236-
if (result_double > PG_INT64_MAX || result_double < PG_INT64_MIN)
3236+
if (isnan(result_double) || !FLOAT8_FITS_IN_INT64(result_double))
32373237
ereport(ERROR,
32383238
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
32393239
errmsg("interval out of range")));

‎src/bin/pgbench/pgbench.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1675,9 +1675,9 @@ coerceToInt(PgBenchValue *pval, int64 *ival)
16751675
}
16761676
else if (pval->type == PGBT_DOUBLE)
16771677
{
1678-
double dval = pval->u.dval;
1678+
double dval = rint(pval->u.dval);
16791679

1680-
if (dval < PG_INT64_MIN || PG_INT64_MAX < dval)
1680+
if (isnan(dval) || !FLOAT8_FITS_IN_INT64(dval))
16811681
{
16821682
fprintf(stderr, "double to int overflow for %f\n", dval);
16831683
return false;

‎src/include/c.h

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

1033+
/*
1034+
* Macros for range-checking float values before converting to integer.
1035+
* We must be careful here that the boundary values are expressed exactly
1036+
* in the float domain. PG_INTnn_MIN is an exact power of 2, so it will
1037+
* be represented exactly; but PG_INTnn_MAX isn't, and might get rounded
1038+
* off, so avoid using that.
1039+
* The input must be rounded to an integer beforehand, typically with rint(),
1040+
* else we might draw the wrong conclusion about close-to-the-limit values.
1041+
* These macros will do the right thing for Inf, but not necessarily for NaN,
1042+
* so check isnan(num) first if that's a possibility.
1043+
*/
1044+
#define FLOAT4_FITS_IN_INT16(num) \
1045+
((num) >= (float4) PG_INT16_MIN && (num) < -((float4) PG_INT16_MIN))
1046+
#define FLOAT4_FITS_IN_INT32(num) \
1047+
((num) >= (float4) PG_INT32_MIN && (num) < -((float4) PG_INT32_MIN))
1048+
#define FLOAT4_FITS_IN_INT64(num) \
1049+
((num) >= (float4) PG_INT64_MIN && (num) < -((float4) PG_INT64_MIN))
1050+
#define FLOAT8_FITS_IN_INT16(num) \
1051+
((num) >= (float8) PG_INT16_MIN && (num) < -((float8) PG_INT16_MIN))
1052+
#define FLOAT8_FITS_IN_INT32(num) \
1053+
((num) >= (float8) PG_INT32_MIN && (num) < -((float8) PG_INT32_MIN))
1054+
#define FLOAT8_FITS_IN_INT64(num) \
1055+
((num) >= (float8) PG_INT64_MIN && (num) < -((float8) PG_INT64_MIN))
1056+
10331057

10341058
/* ----------------------------------------------------------------
10351059
* 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)