I've been preparing 0001 for commit. I've attached what I have so far.
The main changes are the implementations of pg_abs_* and pg_neg_*. For the
former, I've used abs()/i64abs() for the short/int implementations. For
the latter, I've tried to use __builtin_sub_overflow() when possible, as
that appears to produce slightly better code. When
__builtin_sub_overflow() is not available, the values are upcasted before
negation, and we check that result before casting to the return type. That
approach more closely matches the surrounding functions. (One exception is
pg_neg_u64_overflow() when we have neither HAVE__BUILTIN_OP_OVERFLOW nor
HAVE_INT128. In that case, we have to hand-roll everything.)
Thoughts?
--
nathan
>From c4e67e1f32562037dbc83baca5bcda00e0fcd0d7 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <kosh...@gmail.com>
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH v19 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 | 92 ++++++++++++++++++++++
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, 158 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 d0f0923710..3d69e77998 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -8117,7 +8117,7 @@ int64_to_numericvar(int64 val, NumericVar *var)
if (val < 0)
{
var->sign = NUMERIC_NEG;
- uval = -val;
+ uval = pg_abs_s64(val);
}
else
{
@@ -11443,7 +11443,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..a3d7d6bf01 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -18,6 +18,7 @@
#include <limits.h>
#include <ctype.h>
+#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 (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 (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 (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 (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 (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 (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..c76793f72d 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 (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 (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..961e52e9ad 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -32,6 +32,11 @@
* - 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.
+ * - Return the absolute value of a as an unsigned integer of the same
+ * width.
*---------
*/
@@ -97,6 +102,12 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
#endif
}
+static inline uint16
+pg_abs_s16(int16 a)
+{
+ return abs(a);
+}
+
/*
* INT32
*/
@@ -154,6 +165,12 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
#endif
}
+static inline uint32
+pg_abs_s32(int32 a)
+{
+ return i64abs(a);
+}
+
/*
* INT64
*/
@@ -258,6 +275,16 @@ 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;
+ if (a < 0)
+ return -a;
+ return a;
+}
+
/*------------------------------------------------------------------------
* Overflow routines for unsigned integers
*------------------------------------------------------------------------
@@ -318,6 +345,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(a > PG_INT16_MAX))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = (int16) res;
+ return false;
+#endif
+}
+
/*
* INT32
*/
@@ -373,6 +418,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_MAX))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = (int32) res;
+ return false;
+#endif
+}
+
/*
* UINT64
*/
@@ -438,6 +501,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_MAX))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = (int64) 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..93d4cc323d 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 (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)
