From 75c4c0e0b180f883ea728f45ffe0a81ac2599dcc Mon Sep 17 00:00:00 2001 From: Joseph Koshakow Date: Sat, 8 Jun 2024 22:16:46 -0400 Subject: [PATCH v22 1/1] Remove dependence on integer wrapping This commit updates various parts of the code to no longer rely on integer wrapping for correctness. Not all compilers support -fwrapv, so it's best not to rely on it. --- src/backend/utils/adt/cash.c | 12 ++- src/backend/utils/adt/numeric.c | 4 +- src/backend/utils/adt/numutils.c | 34 ++++--- src/backend/utils/adt/timestamp.c | 28 +----- src/include/common/int.h | 101 +++++++++++++++++++++ src/interfaces/ecpg/pgtypeslib/timestamp.c | 11 +-- src/test/regress/expected/timestamp.out | 13 +++ src/test/regress/expected/timestamptz.out | 13 +++ src/test/regress/sql/timestamp.sql | 4 + src/test/regress/sql/timestamptz.sql | 4 + 10 files changed, 167 insertions(+), 57 deletions(-) diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c index ec3c08acfc..c1a743b2a6 100644 --- a/src/backend/utils/adt/cash.c +++ b/src/backend/utils/adt/cash.c @@ -387,6 +387,7 @@ Datum cash_out(PG_FUNCTION_ARGS) { Cash value = PG_GETARG_CASH(0); + uint64 uvalue; char *result; char buf[128]; char *bufptr; @@ -429,8 +430,6 @@ cash_out(PG_FUNCTION_ARGS) if (value < 0) { - /* make the amount positive for digit-reconstruction loop */ - value = -value; /* set up formatting data */ signsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-"; sign_posn = lconvert->n_sign_posn; @@ -445,6 +444,9 @@ cash_out(PG_FUNCTION_ARGS) sep_by_space = lconvert->p_sep_by_space; } + /* make the amount positive for digit-reconstruction loop */ + uvalue = pg_abs_s64(value); + /* we build the digits+decimal-point+sep string right-to-left in buf[] */ bufptr = buf + sizeof(buf) - 1; *bufptr = '\0'; @@ -470,10 +472,10 @@ cash_out(PG_FUNCTION_ARGS) memcpy(bufptr, ssymbol, strlen(ssymbol)); } - *(--bufptr) = ((uint64) value % 10) + '0'; - value = ((uint64) value) / 10; + *(--bufptr) = (uvalue % 10) + '0'; + uvalue = uvalue / 10; digit_pos--; - } while (value || digit_pos >= 0); + } while (uvalue || digit_pos >= 0); /*---------- * Now, attach currency symbol and sign symbol in the correct order. diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index 763a7f4be0..5ee001e382 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -8111,7 +8111,7 @@ int64_to_numericvar(int64 val, NumericVar *var) if (val < 0) { var->sign = NUMERIC_NEG; - uval = -val; + uval = pg_abs_s64(val); } else { @@ -11437,7 +11437,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale, * Now we can proceed with the multiplications. */ neg = (exp < 0); - mask = abs(exp); + mask = pg_abs_s32(exp); init_var(&base_prod); set_var_from_var(base, &base_prod); diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c index adc1e8a4cb..63c2beb6a2 100644 --- a/src/backend/utils/adt/numutils.c +++ b/src/backend/utils/adt/numutils.c @@ -18,6 +18,7 @@ #include #include +#include "common/int.h" #include "port/pg_bitutils.h" #include "utils/builtins.h" @@ -131,6 +132,7 @@ pg_strtoint16_safe(const char *s, Node *escontext) uint16 tmp = 0; bool neg = false; unsigned char digit; + int16 result; /* * The majority of cases are likely to be base-10 digits without any @@ -190,10 +192,9 @@ pg_strtoint16_safe(const char *s, Node *escontext) if (neg) { - /* check the negative equivalent will fit without overflowing */ - if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1)) + if (unlikely(pg_neg_u16_overflow(tmp, &result))) goto out_of_range; - return -((int16) tmp); + return result; } if (unlikely(tmp > PG_INT16_MAX)) @@ -333,10 +334,9 @@ slow: if (neg) { - /* check the negative equivalent will fit without overflowing */ - if (tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1) + if (unlikely(pg_neg_u16_overflow(tmp, &result))) goto out_of_range; - return -((int16) tmp); + return result; } if (tmp > PG_INT16_MAX) @@ -393,6 +393,7 @@ pg_strtoint32_safe(const char *s, Node *escontext) uint32 tmp = 0; bool neg = false; unsigned char digit; + int32 result; /* * The majority of cases are likely to be base-10 digits without any @@ -452,10 +453,9 @@ pg_strtoint32_safe(const char *s, Node *escontext) if (neg) { - /* check the negative equivalent will fit without overflowing */ - if (unlikely(tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1)) + if (unlikely(pg_neg_u32_overflow(tmp, &result))) goto out_of_range; - return -((int32) tmp); + return result; } if (unlikely(tmp > PG_INT32_MAX)) @@ -595,10 +595,9 @@ slow: if (neg) { - /* check the negative equivalent will fit without overflowing */ - if (tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1) + if (unlikely(pg_neg_u32_overflow(tmp, &result))) goto out_of_range; - return -((int32) tmp); + return result; } if (tmp > PG_INT32_MAX) @@ -655,6 +654,7 @@ pg_strtoint64_safe(const char *s, Node *escontext) uint64 tmp = 0; bool neg = false; unsigned char digit; + int64 result; /* * The majority of cases are likely to be base-10 digits without any @@ -714,10 +714,9 @@ pg_strtoint64_safe(const char *s, Node *escontext) if (neg) { - /* check the negative equivalent will fit without overflowing */ - if (unlikely(tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1)) + if (unlikely(pg_neg_u64_overflow(tmp, &result))) goto out_of_range; - return -((int64) tmp); + return result; } if (unlikely(tmp > PG_INT64_MAX)) @@ -857,10 +856,9 @@ slow: if (neg) { - /* check the negative equivalent will fit without overflowing */ - if (tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1) + if (unlikely(pg_neg_u64_overflow(tmp, &result))) goto out_of_range; - return -((int64) tmp); + return result; } if (tmp > PG_INT64_MAX) diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index 69fe7860ed..43800addf4 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -618,19 +618,8 @@ make_timestamp_internal(int year, int month, int day, time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE) * USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC); - result = date * USECS_PER_DAY + time; - /* check for major overflow */ - if ((result - time) / USECS_PER_DAY != date) - ereport(ERROR, - (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), - errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g", - year, month, day, - hour, min, sec))); - - /* check for just-barely overflow (okay except time-of-day wraps) */ - /* caution: we want to allow 1999-12-31 24:00:00 */ - if ((result < 0 && date > 0) || - (result > 0 && date < -1)) + if (unlikely(pg_mul_s64_overflow(date, USECS_PER_DAY, &result) || + pg_add_s64_overflow(result, time, &result))) ereport(ERROR, (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g", @@ -2010,17 +1999,8 @@ tm2timestamp(struct pg_tm *tm, fsec_t fsec, int *tzp, Timestamp *result) date = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - POSTGRES_EPOCH_JDATE; time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec); - *result = date * USECS_PER_DAY + time; - /* check for major overflow */ - if ((*result - time) / USECS_PER_DAY != date) - { - *result = 0; /* keep compiler quiet */ - return -1; - } - /* check for just-barely overflow (okay except time-of-day wraps) */ - /* caution: we want to allow 1999-12-31 24:00:00 */ - if ((*result < 0 && date > 0) || - (*result > 0 && date < -1)) + if (unlikely(pg_mul_s64_overflow(date, USECS_PER_DAY, result) || + pg_add_s64_overflow(*result, time, result))) { *result = 0; /* keep compiler quiet */ return -1; diff --git a/src/include/common/int.h b/src/include/common/int.h index 7fc046e78a..c76c67e226 100644 --- a/src/include/common/int.h +++ b/src/include/common/int.h @@ -32,6 +32,12 @@ * - If a * b overflows, return true, otherwise store the result of a * b * into *result. The content of *result is implementation defined in case of * overflow. + * - If -a overflows, return true, otherwise store the result of -a into + * *result. The content of *result is implementation defined in case of + * overflow. + * - If a is negative, return -a, otherwise return a. Overflow cannot occur + * because the return value is an unsigned integer with the same type as the + * argument. *--------- */ @@ -97,6 +103,17 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result) #endif } +static inline uint16 +pg_abs_s16(int16 a) +{ + /* + * This first widens the argument from int16 to int32 for use with abs(). + * The result is then narrowed from int32 to uint16. This prevents any + * possibility of overflow. + */ + return (uint16) abs((int32) a); +} + /* * INT32 */ @@ -154,6 +171,17 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result) #endif } +static inline uint32 +pg_abs_s32(int32 a) +{ + /* + * This first widens the argument from int32 to int64 for use with + * i64abs(). The result is then narrowed from int64 to uint32. This + * prevents any possibility of overflow. + */ + return (uint32) i64abs((int64) a); +} + /* * INT64 */ @@ -258,6 +286,14 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result) #endif } +static inline uint64 +pg_abs_s64(int64 a) +{ + if (unlikely(a == PG_INT64_MIN)) + return (uint64) PG_INT64_MAX + 1; + return (uint64) i64abs(a); +} + /*------------------------------------------------------------------------ * Overflow routines for unsigned integers *------------------------------------------------------------------------ @@ -318,6 +354,24 @@ pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result) #endif } +static inline bool +pg_neg_u16_overflow(uint16 a, int16 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + int32 res = -((int32) a); + + if (unlikely(res < PG_INT16_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = res; + return false; +#endif +} + /* * INT32 */ @@ -373,6 +427,24 @@ pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result) #endif } +static inline bool +pg_neg_u32_overflow(uint32 a, int32 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + int64 res = -((int64) a); + + if (unlikely(res < PG_INT32_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = res; + return false; +#endif +} + /* * UINT64 */ @@ -438,6 +510,35 @@ pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result) #endif } +static inline bool +pg_neg_u64_overflow(uint64 a, int64 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#elif defined(HAVE_INT128) + uint128 res = -((int128) a); + + if (unlikely(res < PG_INT64_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = res; + return false; +#else + if (unlikely(a > (uint64) PG_INT64_MAX + 1)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + if (unlikely(a == (uint64) PG_INT64_MAX + 1)) + *result = PG_INT64_MIN; + else + *result = -((int64) a); + return false; +#endif +} + /*------------------------------------------------------------------------ * * Comparison routines for integer types. diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c index f1b143fbd2..402fae6da6 100644 --- a/src/interfaces/ecpg/pgtypeslib/timestamp.c +++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c @@ -11,6 +11,7 @@ #error -ffast-math is known to break this code #endif +#include "common/int.h" #include "dt.h" #include "pgtypes_date.h" #include "pgtypes_timestamp.h" @@ -48,14 +49,8 @@ tm2timestamp(struct tm *tm, fsec_t fsec, int *tzp, timestamp * result) dDate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - date2j(2000, 1, 1); time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec); - *result = (dDate * USECS_PER_DAY) + time; - /* check for major overflow */ - if ((*result - time) / USECS_PER_DAY != dDate) - return -1; - /* check for just-barely overflow (okay except time-of-day wraps) */ - /* caution: we want to allow 1999-12-31 24:00:00 */ - if ((*result < 0 && dDate > 0) || - (*result > 0 && dDate < -1)) + if (unlikely(pg_mul_s64_overflow(dDate, USECS_PER_DAY, result) || + pg_add_s64_overflow(*result, time, result))) return -1; if (tzp != NULL) *result = dt2local(*result, -(*tzp)); diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out index cf337da517..e287260051 100644 --- a/src/test/regress/expected/timestamp.out +++ b/src/test/regress/expected/timestamp.out @@ -2201,3 +2201,16 @@ select age(timestamp '-infinity', timestamp 'infinity'); select age(timestamp '-infinity', timestamp '-infinity'); ERROR: interval out of range +-- test timestamp near POSTGRES_EPOCH_JDATE +select timestamp '1999-12-31 24:00:00'; + timestamp +-------------------------- + Sat Jan 01 00:00:00 2000 +(1 row) + +select make_timestamp(1999, 12, 31, 24, 0, 0); + make_timestamp +-------------------------- + Sat Jan 01 00:00:00 2000 +(1 row) + diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out index bfb3825ff6..d01d174983 100644 --- a/src/test/regress/expected/timestamptz.out +++ b/src/test/regress/expected/timestamptz.out @@ -3286,3 +3286,16 @@ SELECT age(timestamptz '-infinity', timestamptz 'infinity'); SELECT age(timestamptz '-infinity', timestamptz '-infinity'); ERROR: interval out of range +-- test timestamp near POSTGRES_EPOCH_JDATE +select timestamptz '1999-12-31 24:00:00'; + timestamptz +------------------------------ + Sat Jan 01 00:00:00 2000 PST +(1 row) + +select make_timestamptz(1999, 12, 31, 24, 0, 0); + make_timestamptz +------------------------------ + Sat Jan 01 00:00:00 2000 PST +(1 row) + diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql index 820ef7752a..748469576d 100644 --- a/src/test/regress/sql/timestamp.sql +++ b/src/test/regress/sql/timestamp.sql @@ -424,3 +424,7 @@ select age(timestamp 'infinity', timestamp 'infinity'); select age(timestamp 'infinity', timestamp '-infinity'); select age(timestamp '-infinity', timestamp 'infinity'); select age(timestamp '-infinity', timestamp '-infinity'); + +-- test timestamp near POSTGRES_EPOCH_JDATE +select timestamp '1999-12-31 24:00:00'; +select make_timestamp(1999, 12, 31, 24, 0, 0); diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql index ccfd90d646..c71d5489b4 100644 --- a/src/test/regress/sql/timestamptz.sql +++ b/src/test/regress/sql/timestamptz.sql @@ -668,3 +668,7 @@ SELECT age(timestamptz 'infinity', timestamptz 'infinity'); SELECT age(timestamptz 'infinity', timestamptz '-infinity'); SELECT age(timestamptz '-infinity', timestamptz 'infinity'); SELECT age(timestamptz '-infinity', timestamptz '-infinity'); + +-- test timestamp near POSTGRES_EPOCH_JDATE +select timestamptz '1999-12-31 24:00:00'; +select make_timestamptz(1999, 12, 31, 24, 0, 0); -- 2.39.3 (Apple Git-146)