Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 9e9a2b7

Browse files
Remove dependence on -fwrapv semantics in a few places.
This commit attempts to update a few places, such as the money, numeric, and timestamp types, to no longer rely on signed integer wrapping for correctness. This is intended to move us closer towards removing -fwrapv, which may enable some compiler optimizations. However, there is presently no plan to actually remove that compiler option in the near future. Besides using some of the existing overflow-aware routines in int.h, this commit introduces and makes use of some new ones. Specifically, it adds functions that accept a signed integer and return its absolute value as an unsigned integer with the same width (e.g., pg_abs_s64()). It also adds functions that accept an unsigned integer, store the result of negating that integer in a signed integer with the same width, and return whether the negation overflowed (e.g., pg_neg_u64_overflow()). Finally, this commit adds a couple of tests for timestamps near POSTGRES_EPOCH_JDATE. Author: Joseph Koshakow Reviewed-by: Tom Lane, Heikki Linnakangas, Jian He Discussion: https://postgr.es/m/CAAvxfHdBPOyEGS7s%2Bxf4iaW0-cgiq25jpYdWBqQqvLtLe_t6tw%40mail.gmail.com
1 parent ad89d71 commit 9e9a2b7

File tree

10 files changed

+189
-65
lines changed

10 files changed

+189
-65
lines changed

src/backend/utils/adt/cash.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,7 @@ Datum
387387
cash_out(PG_FUNCTION_ARGS)
388388
{
389389
Cash value = PG_GETARG_CASH(0);
390+
uint64 uvalue;
390391
char *result;
391392
char buf[128];
392393
char *bufptr;
@@ -429,8 +430,6 @@ cash_out(PG_FUNCTION_ARGS)
429430

430431
if (value < 0)
431432
{
432-
/* make the amount positive for digit-reconstruction loop */
433-
value = -value;
434433
/* set up formatting data */
435434
signsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-";
436435
sign_posn = lconvert->n_sign_posn;
@@ -445,6 +444,9 @@ cash_out(PG_FUNCTION_ARGS)
445444
sep_by_space = lconvert->p_sep_by_space;
446445
}
447446

447+
/* make the amount positive for digit-reconstruction loop */
448+
uvalue = pg_abs_s64(value);
449+
448450
/* we build the digits+decimal-point+sep string right-to-left in buf[] */
449451
bufptr = buf + sizeof(buf) - 1;
450452
*bufptr = '\0';
@@ -470,10 +472,10 @@ cash_out(PG_FUNCTION_ARGS)
470472
memcpy(bufptr, ssymbol, strlen(ssymbol));
471473
}
472474

473-
*(--bufptr) = ((uint64) value % 10) + '0';
474-
value = ((uint64) value) / 10;
475+
*(--bufptr) = (uvalue % 10) + '0';
476+
uvalue = uvalue / 10;
475477
digit_pos--;
476-
} while (value || digit_pos >= 0);
478+
} while (uvalue || digit_pos >= 0);
477479

478480
/*----------
479481
* Now, attach currency symbol and sign symbol in the correct order.

src/backend/utils/adt/numeric.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8113,7 +8113,7 @@ int64_to_numericvar(int64 val, NumericVar *var)
81138113
if (val < 0)
81148114
{
81158115
var->sign = NUMERIC_NEG;
8116-
uval = -val;
8116+
uval = pg_abs_s64(val);
81178117
}
81188118
else
81198119
{
@@ -11584,7 +11584,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale,
1158411584
* Now we can proceed with the multiplications.
1158511585
*/
1158611586
neg = (exp < 0);
11587-
mask = abs(exp);
11587+
mask = pg_abs_s32(exp);
1158811588

1158911589
init_var(&base_prod);
1159011590
set_var_from_var(base, &base_prod);

src/backend/utils/adt/numutils.c

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <limits.h>
1919
#include <ctype.h>
2020

21+
#include "common/int.h"
2122
#include "port/pg_bitutils.h"
2223
#include "utils/builtins.h"
2324

@@ -131,6 +132,7 @@ pg_strtoint16_safe(const char *s, Node *escontext)
131132
uint16 tmp = 0;
132133
bool neg = false;
133134
unsigned char digit;
135+
int16 result;
134136

135137
/*
136138
* 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)
190192

191193
if (neg)
192194
{
193-
/* check the negative equivalent will fit without overflowing */
194-
if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1))
195+
if (unlikely(pg_neg_u16_overflow(tmp, &result)))
195196
goto out_of_range;
196-
return -((int16) tmp);
197+
return result;
197198
}
198199

199200
if (unlikely(tmp > PG_INT16_MAX))
@@ -333,10 +334,9 @@ pg_strtoint16_safe(const char *s, Node *escontext)
333334

334335
if (neg)
335336
{
336-
/* check the negative equivalent will fit without overflowing */
337-
if (tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1)
337+
if (unlikely(pg_neg_u16_overflow(tmp, &result)))
338338
goto out_of_range;
339-
return -((int16) tmp);
339+
return result;
340340
}
341341

342342
if (tmp > PG_INT16_MAX)
@@ -393,6 +393,7 @@ pg_strtoint32_safe(const char *s, Node *escontext)
393393
uint32 tmp = 0;
394394
bool neg = false;
395395
unsigned char digit;
396+
int32 result;
396397

397398
/*
398399
* 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)
452453

453454
if (neg)
454455
{
455-
/* check the negative equivalent will fit without overflowing */
456-
if (unlikely(tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1))
456+
if (unlikely(pg_neg_u32_overflow(tmp, &result)))
457457
goto out_of_range;
458-
return -((int32) tmp);
458+
return result;
459459
}
460460

461461
if (unlikely(tmp > PG_INT32_MAX))
@@ -595,10 +595,9 @@ pg_strtoint32_safe(const char *s, Node *escontext)
595595

596596
if (neg)
597597
{
598-
/* check the negative equivalent will fit without overflowing */
599-
if (tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1)
598+
if (unlikely(pg_neg_u32_overflow(tmp, &result)))
600599
goto out_of_range;
601-
return -((int32) tmp);
600+
return result;
602601
}
603602

604603
if (tmp > PG_INT32_MAX)
@@ -655,6 +654,7 @@ pg_strtoint64_safe(const char *s, Node *escontext)
655654
uint64 tmp = 0;
656655
bool neg = false;
657656
unsigned char digit;
657+
int64 result;
658658

659659
/*
660660
* 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)
714714

715715
if (neg)
716716
{
717-
/* check the negative equivalent will fit without overflowing */
718-
if (unlikely(tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1))
717+
if (unlikely(pg_neg_u64_overflow(tmp, &result)))
719718
goto out_of_range;
720-
return -((int64) tmp);
719+
return result;
721720
}
722721

723722
if (unlikely(tmp > PG_INT64_MAX))
@@ -857,10 +856,9 @@ pg_strtoint64_safe(const char *s, Node *escontext)
857856

858857
if (neg)
859858
{
860-
/* check the negative equivalent will fit without overflowing */
861-
if (tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1)
859+
if (unlikely(pg_neg_u64_overflow(tmp, &result)))
862860
goto out_of_range;
863-
return -((int64) tmp);
861+
return result;
864862
}
865863

866864
if (tmp > PG_INT64_MAX)

src/backend/utils/adt/timestamp.c

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -618,19 +618,8 @@ make_timestamp_internal(int year, int month, int day,
618618
time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
619619
* USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
620620

621-
result = date * USECS_PER_DAY + time;
622-
/* check for major overflow */
623-
if ((result - time) / USECS_PER_DAY != date)
624-
ereport(ERROR,
625-
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
626-
errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
627-
year, month, day,
628-
hour, min, sec)));
629-
630-
/* check for just-barely overflow (okay except time-of-day wraps) */
631-
/* caution: we want to allow 1999-12-31 24:00:00 */
632-
if ((result < 0 && date > 0) ||
633-
(result > 0 && date < -1))
621+
if (unlikely(pg_mul_s64_overflow(date, USECS_PER_DAY, &result) ||
622+
pg_add_s64_overflow(result, time, &result)))
634623
ereport(ERROR,
635624
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
636625
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)
20101999
date = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - POSTGRES_EPOCH_JDATE;
20112000
time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
20122001

2013-
*result = date * USECS_PER_DAY + time;
2014-
/* check for major overflow */
2015-
if ((*result - time) / USECS_PER_DAY != date)
2016-
{
2017-
*result = 0; /* keep compiler quiet */
2018-
return -1;
2019-
}
2020-
/* check for just-barely overflow (okay except time-of-day wraps) */
2021-
/* caution: we want to allow 1999-12-31 24:00:00 */
2022-
if ((*result < 0 && date > 0) ||
2023-
(*result > 0 && date < -1))
2002+
if (unlikely(pg_mul_s64_overflow(date, USECS_PER_DAY, result) ||
2003+
pg_add_s64_overflow(*result, time, result)))
20242004
{
20252005
*result = 0; /* keep compiler quiet */
20262006
return -1;

src/include/common/int.h

Lines changed: 123 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,35 @@
2323

2424
/*---------
2525
* The following guidelines apply to all the overflow routines:
26-
* - If a + b overflows, return true, otherwise store the result of a + b
27-
* into *result. The content of *result is implementation defined in case of
28-
* overflow.
29-
* - If a - b overflows, return true, otherwise store the result of a - b
30-
* into *result. The content of *result is implementation defined in case of
31-
* overflow.
32-
* - If a * b overflows, return true, otherwise store the result of a * b
33-
* into *result. The content of *result is implementation defined in case of
26+
*
27+
* If the result overflows, return true, otherwise store the result into
28+
* *result. The content of *result is implementation defined in case of
3429
* overflow.
30+
*
31+
* bool pg_add_*_overflow(a, b, *result)
32+
*
33+
* Calculate a + b
34+
*
35+
* bool pg_sub_*_overflow(a, b, *result)
36+
*
37+
* Calculate a - b
38+
*
39+
* bool pg_mul_*_overflow(a, b, *result)
40+
*
41+
* Calculate a * b
42+
*
43+
* bool pg_neg_*_overflow(a, *result)
44+
*
45+
* Calculate -a
46+
*
47+
*
48+
* In addition, this file contains:
49+
*
50+
* <unsigned int type> pg_abs_*(<signed int type> a)
51+
*
52+
* Calculate absolute value of a. Unlike the standard library abs()
53+
* and labs() functions, the return type is unsigned, so the operation
54+
* cannot overflow.
3555
*---------
3656
*/
3757

@@ -97,6 +117,17 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
97117
#endif
98118
}
99119

120+
static inline uint16
121+
pg_abs_s16(int16 a)
122+
{
123+
/*
124+
* This first widens the argument from int16 to int32 for use with abs().
125+
* The result is then narrowed from int32 to uint16. This prevents any
126+
* possibility of overflow.
127+
*/
128+
return (uint16) abs((int32) a);
129+
}
130+
100131
/*
101132
* INT32
102133
*/
@@ -154,6 +185,17 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
154185
#endif
155186
}
156187

188+
static inline uint32
189+
pg_abs_s32(int32 a)
190+
{
191+
/*
192+
* This first widens the argument from int32 to int64 for use with
193+
* i64abs(). The result is then narrowed from int64 to uint32. This
194+
* prevents any possibility of overflow.
195+
*/
196+
return (uint32) i64abs((int64) a);
197+
}
198+
157199
/*
158200
* INT64
159201
*/
@@ -258,6 +300,14 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
258300
#endif
259301
}
260302

303+
static inline uint64
304+
pg_abs_s64(int64 a)
305+
{
306+
if (unlikely(a == PG_INT64_MIN))
307+
return (uint64) PG_INT64_MAX + 1;
308+
return (uint64) i64abs(a);
309+
}
310+
261311
/*------------------------------------------------------------------------
262312
* Overflow routines for unsigned integers
263313
*------------------------------------------------------------------------
@@ -318,6 +368,24 @@ pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result)
318368
#endif
319369
}
320370

371+
static inline bool
372+
pg_neg_u16_overflow(uint16 a, int16 *result)
373+
{
374+
#if defined(HAVE__BUILTIN_OP_OVERFLOW)
375+
return __builtin_sub_overflow(0, a, result);
376+
#else
377+
int32 res = -((int32) a);
378+
379+
if (unlikely(res < PG_INT16_MIN))
380+
{
381+
*result = 0x5EED; /* to avoid spurious warnings */
382+
return true;
383+
}
384+
*result = res;
385+
return false;
386+
#endif
387+
}
388+
321389
/*
322390
* INT32
323391
*/
@@ -373,6 +441,24 @@ pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result)
373441
#endif
374442
}
375443

444+
static inline bool
445+
pg_neg_u32_overflow(uint32 a, int32 *result)
446+
{
447+
#if defined(HAVE__BUILTIN_OP_OVERFLOW)
448+
return __builtin_sub_overflow(0, a, result);
449+
#else
450+
int64 res = -((int64) a);
451+
452+
if (unlikely(res < PG_INT32_MIN))
453+
{
454+
*result = 0x5EED; /* to avoid spurious warnings */
455+
return true;
456+
}
457+
*result = res;
458+
return false;
459+
#endif
460+
}
461+
376462
/*
377463
* UINT64
378464
*/
@@ -438,6 +524,35 @@ pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result)
438524
#endif
439525
}
440526

527+
static inline bool
528+
pg_neg_u64_overflow(uint64 a, int64 *result)
529+
{
530+
#if defined(HAVE__BUILTIN_OP_OVERFLOW)
531+
return __builtin_sub_overflow(0, a, result);
532+
#elif defined(HAVE_INT128)
533+
int128 res = -((int128) a);
534+
535+
if (unlikely(res < PG_INT64_MIN))
536+
{
537+
*result = 0x5EED; /* to avoid spurious warnings */
538+
return true;
539+
}
540+
*result = res;
541+
return false;
542+
#else
543+
if (unlikely(a > (uint64) PG_INT64_MAX + 1))
544+
{
545+
*result = 0x5EED; /* to avoid spurious warnings */
546+
return true;
547+
}
548+
if (unlikely(a == (uint64) PG_INT64_MAX + 1))
549+
*result = PG_INT64_MIN;
550+
else
551+
*result = -((int64) a);
552+
return false;
553+
#endif
554+
}
555+
441556
/*------------------------------------------------------------------------
442557
*
443558
* Comparison routines for integer types.

0 commit comments

Comments
 (0)