Hi, On 2017-10-24 03:39:54 -0700, Andres Freund wrote: > Largely that's due to the overflow checks. > > For integers we currently do: > > #define SAMESIGN(a,b) (((a) < 0) == ((b) < 0)) > > /* > * Overflow check. If the inputs are of different signs then their sum > * cannot overflow. If the inputs are of the same sign, their sum had > * better be that sign too. > */ > if (SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) > ereport(ERROR, > (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), > errmsg("integer out of range"))); > > which means that we turn a single integer instruction into ~10, > including a bunch of branches. All that despite the fact that most > architectures have flag registers signalling integer overflow. It's just > that C doesn't easily make that available. > > gcc exposes more efficient overflow detection via intrinsics: > https://gcc.gnu.org/onlinedocs/gcc-7.1.0/gcc/Integer-Overflow-Builtins.html > > Using that turns the non-error path from int4pl from: > > 0x0000000000826ec0 <+0>: mov 0x20(%rdi),%rcx # arg1 > 0x0000000000826ec4 <+4>: mov 0x28(%rdi),%rdx # arg2 > 0x0000000000826ec8 <+8>: mov %ecx,%esi > 0x0000000000826eca <+10>: lea (%rdx,%rcx,1),%eax # add > # overflow check > 0x0000000000826ecd <+13>: shr $0x1f,%edx > 0x0000000000826ed0 <+16>: not %esi > 0x0000000000826ed2 <+18>: shr $0x1f,%esi > 0x0000000000826ed5 <+21>: cmp %dl,%sil > 0x0000000000826ed8 <+24>: je 0x826f30 <int4pl+112> > 0x0000000000826eda <+26>: mov %eax,%edx > 0x0000000000826edc <+28>: shr $0x1f,%ecx > 0x0000000000826edf <+31>: shr $0x1f,%edx > 0x0000000000826ee2 <+34>: cmp %cl,%dl > 0x0000000000826ee4 <+36>: je 0x826f30 <int4pl+112> > /* overflow error code */ > 0x0000000000826f30 <+112>: retq > > into > > 0x0000000000826ec0 <+0>: mov 0x28(%rdi),%rax # arg2 > 0x0000000000826ec4 <+4>: add 0x20(%rdi),%eax # arg1 + arg2 > 0x0000000000826ec7 <+7>: jo 0x826ecc <int4pl+12> # jump if > overflowed > 0x0000000000826ec9 <+9>: mov %eax,%eax # clear high bits > 0x0000000000826ecb <+11>: retq > > which, not that surprisingly, is faster. Not to speak of easier to read > ;) > > Besides the fact that the code is faster, there's also the issue that > the current way to do overflow checks is not actually correct C, and > requires compiler flags like -fwrapv.
Attached is a series of patches that: 0001) Introduces pg_{add,sub,mul}{16,32,64}_overflow(a, b, *result) These use compiler intrinsics on gcc/clang. If that's not available, they cast to a wider type and to overflow checks. For 64bit there's a fallback for the case 128bit math is not available (here I stole from an old patch of Greg's). These fallbacks are, as far as I can tell, C free of overflow related undefined behaviour. Perhaps it should rather be pg_add_s32_overflow, or a similar naming scheme? 0002) Converts int.c, int8.c and a smattering of other functions to use the new facilities. This removes a fair amount of code. It might make sense to split this up further, but right now that's the set of functions that either are affected performancewise by previous overflow checks, and/or relied on wraparound overflow. There's probably more places, but this is what I found by visual inspection and compiler warnings. 0003) Removes -fwrapv. I'm *NOT* suggesting we apply this right now, but it seems like an important test for the new facilities. Without 0002, tests would fail after this, after it all tests run successfully. Greetings, Andres Freund
>From 98fbe53be0a3046f8ace687f846f91a0043deee8 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Sun, 29 Oct 2017 22:13:54 -0700 Subject: [PATCH 1/3] Provide overflow safe integer math inline functions. Author: Andres Freund, with some code stolen from Greg Stark Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- config/c-compiler.m4 | 22 ++++ configure | 33 ++++++ configure.in | 4 + src/include/common/int.h | 229 ++++++++++++++++++++++++++++++++++++++++++ src/include/pg_config.h.in | 3 + src/include/pg_config.h.win32 | 3 + 6 files changed, 294 insertions(+) create mode 100644 src/include/common/int.h diff --git a/config/c-compiler.m4 b/config/c-compiler.m4 index 6dcc7906491..0d91e52a28f 100644 --- a/config/c-compiler.m4 +++ b/config/c-compiler.m4 @@ -296,6 +296,28 @@ fi])# PGAC_C_BUILTIN_CONSTANT_P +# PGAC_C_BUILTIN_OP_OVERFLOW +# ------------------------- +# Check if the C compiler understands __builtin_$op_overflow(), +# and define HAVE__BUILTIN_OP_OVERFLOW if so. +# +# Check for the most complicated case, 64 bit multiplication, as a +# proxy for all of the operations. +AC_DEFUN([PGAC_C_BUILTIN_OP_OVERFLOW], +[AC_CACHE_CHECK(for __builtin_mul_overflow, pgac_cv__builtin_op_overflow, +[AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], +[PG_INT64_TYPE result; +__builtin_mul_overflow((PG_INT64_TYPE) 1, (PG_INT64_TYPE) 2, &result);] +)], +[pgac_cv__builtin_op_overflow=yes], +[pgac_cv__builtin_op_overflow=no])]) +if test x"$pgac_cv__builtin_op_overflow" = xyes ; then +AC_DEFINE(HAVE__BUILTIN_OP_OVERFLOW, 1, + [Define to 1 if your compiler understands __builtin_$op_overflow.]) +fi])# PGAC_C_BUILTIN_OP_OVERFLOW + + + # PGAC_C_BUILTIN_UNREACHABLE # -------------------------- # Check if the C compiler understands __builtin_unreachable(), diff --git a/configure b/configure index 4ecd2e19224..f66899488cc 100755 --- a/configure +++ b/configure @@ -14467,6 +14467,39 @@ esac fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_mul_overflow" >&5 +$as_echo_n "checking for __builtin_mul_overflow... " >&6; } +if ${pgac_cv__builtin_op_overflow+:} false; then : + $as_echo_n "(cached) " >&6 +else + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ +PG_INT64_TYPE result; +__builtin_mul_overflow((PG_INT64_TYPE) 1, (PG_INT64_TYPE) 2, &result); + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO"; then : + pgac_cv__builtin_op_overflow=yes +else + pgac_cv__builtin_op_overflow=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__builtin_op_overflow" >&5 +$as_echo "$pgac_cv__builtin_op_overflow" >&6; } +if test x"$pgac_cv__builtin_op_overflow" = xyes ; then + +$as_echo "#define HAVE__BUILTIN_OP_OVERFLOW 1" >>confdefs.h + +fi + # Check size of void *, size_t (enables tweaks for > 32bit address space) # The cast to long int works around a bug in the HP C Compiler # version HP92453-01 B.11.11.23709.GP, which incorrectly rejects diff --git a/configure.in b/configure.in index cea7fd07553..edf1dd2e7b8 100644 --- a/configure.in +++ b/configure.in @@ -1764,6 +1764,10 @@ if test $pgac_need_repl_snprintf = yes; then AC_LIBOBJ(snprintf) fi +# has to be down here, rather than with the other builtins, because +# the test uses PG_INT64_TYPE. +PGAC_C_BUILTIN_OP_OVERFLOW + # Check size of void *, size_t (enables tweaks for > 32bit address space) AC_CHECK_SIZEOF([void *]) AC_CHECK_SIZEOF([size_t]) diff --git a/src/include/common/int.h b/src/include/common/int.h new file mode 100644 index 00000000000..648cbd49f14 --- /dev/null +++ b/src/include/common/int.h @@ -0,0 +1,229 @@ +/*------------------------------------------------------------------------- + * + * int.h + * Routines to perform integer math, while checking for overflows. + * + * The routines in this file are intended to be well defined C, without + * relying on compiler flags like -fwrapv. + * + * To reduce the overhead of these routines try to use compiler intrinsics + * where available. That's not that important for the 16, 32 bit cases, but + * the 64 bit cases can be considerably faster with intrinsics. In case no + * intrinsics are available 128 bit math is used where available. + * + * Copyright (c) 2017, PostgreSQL Global Development Group + * + * src/include/common/int.h + * + *------------------------------------------------------------------------- + */ +#ifndef COMMON_INT_H +#define COMMON_INT_H + +/* + * 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. + */ +static inline bool +pg_add16_overflow(int16 a, int16 b, int16 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_add_overflow(a, b, result); +#else + int32 res = (int32) a + (int32) b; + if (res > PG_INT16_MAX || res < PG_INT16_MIN) + return true; + *result = (int16) res; + return false; +#endif +} + +/* + * 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. + */ +static inline bool +pg_sub16_overflow(int16 a, int16 b, int16 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(a, b, result); +#else + int32 res = (int32) a - (int32) b; + if (res > PG_INT16_MAX || res < PG_INT16_MIN) + return true; + *result = (int16) res; + return false; +#endif +} + +/* + * 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. + */ +static inline bool +pg_mul16_overflow(int16 a, int16 b, int16 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_mul_overflow(a, b, result); +#else + int32 res = (int32) a * (int32) b; + if (res > PG_INT16_MAX || res < PG_INT16_MIN) + return true; + *result = (int16) res; + return false; +#endif +} + +/* + * 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. + */ +static inline bool +pg_add32_overflow(int32 a, int32 b, int32 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_add_overflow(a, b, result); +#else + int64 res = (int64) a + (int64) b; + if (res > PG_INT32_MAX || res < PG_INT32_MIN) + return true; + *result = (int32) res; + return false; +#endif +} + +/* + * 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. + */ +static inline bool +pg_sub32_overflow(int32 a, int32 b, int32 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(a, b, result); +#else + int64 res = (int64) a - (int64) b; + if (res > PG_INT32_MAX || res < PG_INT32_MIN) + return true; + *result = (int32) res; + return false; +#endif +} + +/* + * 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. + */ +static inline bool +pg_mul32_overflow(int32 a, int32 b, int32 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_mul_overflow(a, b, result); +#else + int64 res = (int64) a * (int64) b; + if (res > PG_INT32_MAX || res < PG_INT32_MIN) + return true; + *result = (int32) res; + return false; +#endif +} + +/* + * 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. + */ +static inline bool +pg_add64_overflow(int64 a, int64 b, int64 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_add_overflow(a, b, result); +#elif defined(HAVE_INT128) + int128 res = (int128) a + (int128) b; + if (res > PG_INT64_MAX || res < PG_INT64_MIN) + return true; + *result = (int64) res; + return false; +#else + if ((a > 0 && b > 0 && a > PG_INT64_MAX - b) || + (a < 0 && b < 0 && a < PG_INT64_MIN - b)) + return true; + *result = a + b; + return false; +#endif +} + +/* + * 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. + */ +static inline bool +pg_sub64_overflow(int64 a, int64 b, int64 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(a, b, result); +#elif defined(HAVE_INT128) + int128 res = (int128) a - (int128) b; + if (res > PG_INT64_MAX || res < PG_INT64_MIN) + return true; + *result = (int64) res; + return false; +#else + if ((a < 0 && b > 0 && a < PG_INT64_MIN + b) || + (a > 0 && b < 0 && a > PG_INT64_MAX + b)) + return true; + *result = a - b; + return false; +#endif +} + +/* + * 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. + */ +static inline bool +pg_mul64_overflow(int64 a, int64 b, int64 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_mul_overflow(a, b, result); +#elif defined(HAVE_INT128) + int128 res = (int128) a * (int128) b; + if (res > PG_INT64_MAX || res < PG_INT64_MIN) + return true; + *result = (int64) res; + return false; +#else + /* Overflow can only happen if at least one value is outside the range + * sqrt(min)..sqrt(max) so check that first as the division can be quite a bit + * more expensive than the multiplication. + * + * Multiplying by 0 or 1 can't overflow of course and checking for 0 + * separately avoids any risk of dividing by 0. Be careful about dividing + * INT_MIN by -1 also, note reversing the a and b to ensure we're always + * dividing it by a positive value. + * + */ + if ((a > PG_INT32_MAX || a < PG_INT32_MIN || + b > PG_INT32_MAX || b < PG_INT32_MIN) && + a != 0 && a != 1 && b != 0 && b != 1 && + ((a > 0 && b > 0 && a > PG_INT64_MAX / b) || + (a > 0 && b < 0 && b < PG_INT64_MIN / a) || + (a < 0 && b > 0 && a < PG_INT64_MIN / b) || + (a < 0 && b < 0 && a < PG_INT64_MAX / b))) + { + return true; + } + *result = a * b; + return false; +#endif +} + +#endif /* COMMON_INT_H */ diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index cfdcc5ac62f..dab6d41f5e0 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -687,6 +687,9 @@ /* Define to 1 if your compiler understands __builtin_constant_p. */ #undef HAVE__BUILTIN_CONSTANT_P +/* Define to 1 if your compiler understands __builtin_$op_overflow. */ +#undef HAVE__BUILTIN_OP_OVERFLOW + /* Define to 1 if your compiler understands __builtin_types_compatible_p. */ #undef HAVE__BUILTIN_TYPES_COMPATIBLE_P diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32 index ab9b941e89d..7dbf67ddf69 100644 --- a/src/include/pg_config.h.win32 +++ b/src/include/pg_config.h.win32 @@ -512,6 +512,9 @@ /* Define to 1 if your compiler understands __builtin_constant_p. */ /* #undef HAVE__BUILTIN_CONSTANT_P */ +/* Define to 1 if your compiler understands __builtin_$op_overflow. */ +/* #undef HAVE__BUILTIN_OP_OVERFLOW */ + /* Define to 1 if your compiler understands __builtin_types_compatible_p. */ /* #undef HAVE__BUILTIN_TYPES_COMPATIBLE_P */ -- 2.14.1.536.g6867272d5b.dirty
>From da29ed162ce5439b91c6d5e7b0aca2080e92c01f Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Mon, 30 Oct 2017 03:08:55 -0700 Subject: [PATCH 2/3] Rewrite overflow handling to be faster and not rely on UB. --- src/backend/utils/adt/array_userfuncs.c | 9 +- src/backend/utils/adt/cash.c | 43 ++-- src/backend/utils/adt/float.c | 9 +- src/backend/utils/adt/int.c | 201 ++++-------------- src/backend/utils/adt/int8.c | 347 ++++++++------------------------ src/backend/utils/adt/numeric.c | 44 ++-- src/backend/utils/adt/oracle_compat.c | 18 +- 7 files changed, 181 insertions(+), 490 deletions(-) diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c index 87d79f3f98b..06bfe50de1c 100644 --- a/src/backend/utils/adt/array_userfuncs.c +++ b/src/backend/utils/adt/array_userfuncs.c @@ -13,6 +13,7 @@ #include "postgres.h" #include "catalog/pg_type.h" +#include "common/int.h" #include "utils/array.h" #include "utils/builtins.h" #include "utils/lsyscache.h" @@ -123,10 +124,8 @@ array_append(PG_FUNCTION_ARGS) lb = eah->lbound; dimv = eah->dims; ub = dimv[0] + lb[0] - 1; - indx = ub + 1; - /* overflow? */ - if (indx < ub) + if (pg_add32_overflow(ub, 1, &indx)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); @@ -176,11 +175,9 @@ array_prepend(PG_FUNCTION_ARGS) { /* prepend newelem */ lb = eah->lbound; - indx = lb[0] - 1; lb0 = lb[0]; - /* overflow? */ - if (indx > lb[0]) + if (pg_sub32_overflow(lb0, 1, &indx)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c index 7bbc634bd2b..e0c8f8413fb 100644 --- a/src/backend/utils/adt/cash.c +++ b/src/backend/utils/adt/cash.c @@ -22,6 +22,7 @@ #include <ctype.h> #include <math.h> +#include "common/int.h" #include "libpq/pqformat.h" #include "utils/builtins.h" #include "utils/cash.h" @@ -199,20 +200,21 @@ cash_in(PG_FUNCTION_ARGS) for (; *s; s++) { - /* we look for digits as long as we have found less */ - /* than the required number of decimal places */ + /* + * We look for digits as long as we have found less than the required + * number of decimal places. + */ if (isdigit((unsigned char) *s) && (!seen_dot || dec < fpoint)) { - Cash newvalue = (value * 10) - (*s - '0'); + int8 digit = *s - '0'; - if (newvalue / 10 != value) + if (pg_mul64_overflow(value, 10, &value) || + pg_sub64_overflow(value, digit, &value)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("value \"%s\" is out of range for type %s", str, "money"))); - value = newvalue; - if (seen_dot) dec++; } @@ -230,26 +232,23 @@ cash_in(PG_FUNCTION_ARGS) /* round off if there's another digit */ if (isdigit((unsigned char) *s) && *s >= '5') - value--; /* remember we build the value in the negative */ - - if (value > 0) - ereport(ERROR, - (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("value \"%s\" is out of range for type %s", - str, "money"))); - - /* adjust for less than required decimal places */ - for (; dec < fpoint; dec++) { - Cash newvalue = value * 10; - - if (newvalue / 10 != value) + /* remember we build the value in the negative */ + if (pg_sub64_overflow(value, 1, &value)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("value \"%s\" is out of range for type %s", str, "money"))); + } - value = newvalue; + /* adjust for less than required decimal places */ + for (; dec < fpoint; dec++) + { + if (pg_mul64_overflow(value, 10, &value)) + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("value \"%s\" is out of range for type %s", + str, "money"))); } /* @@ -285,12 +284,12 @@ cash_in(PG_FUNCTION_ARGS) */ if (sgn > 0) { - result = -value; - if (result < 0) + if (value == PG_INT64_MIN) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("value \"%s\" is out of range for type %s", str, "money"))); + result = -value; } else result = value; diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c index 18b3b949acb..ecbb3d5432b 100644 --- a/src/backend/utils/adt/float.c +++ b/src/backend/utils/adt/float.c @@ -20,6 +20,7 @@ #include <limits.h> #include "catalog/pg_type.h" +#include "common/int.h" #include "libpq/pqformat.h" #include "utils/array.h" #include "utils/builtins.h" @@ -3548,9 +3549,7 @@ width_bucket_float8(PG_FUNCTION_ARGS) result = 0; else if (operand >= bound2) { - result = count + 1; - /* check for overflow */ - if (result < count) + if (pg_add32_overflow(count, 1, &result)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); @@ -3564,9 +3563,7 @@ width_bucket_float8(PG_FUNCTION_ARGS) result = 0; else if (operand <= bound2) { - result = count + 1; - /* check for overflow */ - if (result < count) + if (pg_add32_overflow(count, 1, &result)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c index 4cd8960b3fc..19102c7ba7a 100644 --- a/src/backend/utils/adt/int.c +++ b/src/backend/utils/adt/int.c @@ -32,14 +32,12 @@ #include <limits.h> #include "catalog/pg_type.h" +#include "common/int.h" #include "funcapi.h" #include "libpq/pqformat.h" #include "utils/array.h" #include "utils/builtins.h" - -#define SAMESIGN(a,b) (((a) < 0) == ((b) < 0)) - #define Int2VectorSize(n) (offsetof(int2vector, values) + (n) * sizeof(int16)) typedef struct @@ -328,7 +326,7 @@ i4toi2(PG_FUNCTION_ARGS) { int32 arg1 = PG_GETARG_INT32(0); - if (arg1 < SHRT_MIN || arg1 > SHRT_MAX) + if (unlikely(arg1 < SHRT_MIN || arg1 > SHRT_MAX)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("smallint out of range"))); @@ -598,15 +596,13 @@ Datum int4um(PG_FUNCTION_ARGS) { int32 arg = PG_GETARG_INT32(0); - int32 result; - result = -arg; - /* overflow check (needed for INT_MIN) */ - if (arg != 0 && SAMESIGN(result, arg)) + if (unlikely(arg == PG_INT32_MIN)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); - PG_RETURN_INT32(result); + + PG_RETURN_INT32(-arg); } Datum @@ -624,14 +620,7 @@ int4pl(PG_FUNCTION_ARGS) int32 arg2 = PG_GETARG_INT32(1); int32 result; - result = arg1 + arg2; - - /* - * Overflow check. If the inputs are of different signs then their sum - * cannot overflow. If the inputs are of the same sign, their sum had - * better be that sign too. - */ - if (SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_add32_overflow(arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); @@ -645,14 +634,7 @@ int4mi(PG_FUNCTION_ARGS) int32 arg2 = PG_GETARG_INT32(1); int32 result; - result = arg1 - arg2; - - /* - * Overflow check. If the inputs are of the same sign then their - * difference cannot overflow. If they are of different signs then the - * result should be of the same sign as the first input. - */ - if (!SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_sub32_overflow(arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); @@ -666,24 +648,7 @@ int4mul(PG_FUNCTION_ARGS) int32 arg2 = PG_GETARG_INT32(1); int32 result; - result = arg1 * arg2; - - /* - * Overflow check. We basically check to see if result / arg2 gives arg1 - * again. There are two cases where this fails: arg2 = 0 (which cannot - * overflow) and arg1 = INT_MIN, arg2 = -1 (where the division itself will - * overflow and thus incorrectly match). - * - * Since the division is likely much more expensive than the actual - * multiplication, we'd like to skip it where possible. The best bang for - * the buck seems to be to check whether both inputs are in the int16 - * range; if so, no overflow is possible. - */ - if (!(arg1 >= (int32) SHRT_MIN && arg1 <= (int32) SHRT_MAX && - arg2 >= (int32) SHRT_MIN && arg2 <= (int32) SHRT_MAX) && - arg2 != 0 && - ((arg2 == -1 && arg1 < 0 && result < 0) || - result / arg2 != arg1)) + if (unlikely(pg_mul32_overflow(arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); @@ -714,12 +679,11 @@ int4div(PG_FUNCTION_ARGS) */ if (arg2 == -1) { - result = -arg1; - /* overflow check (needed for INT_MIN) */ - if (arg1 != 0 && SAMESIGN(result, arg1)) + if (unlikely(arg1 == PG_INT32_MIN)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); + result = -arg1; PG_RETURN_INT32(result); } @@ -736,9 +700,7 @@ int4inc(PG_FUNCTION_ARGS) int32 arg = PG_GETARG_INT32(0); int32 result; - result = arg + 1; - /* Overflow check */ - if (arg > 0 && result < 0) + if (unlikely(pg_add32_overflow(arg, 1, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); @@ -750,15 +712,12 @@ Datum int2um(PG_FUNCTION_ARGS) { int16 arg = PG_GETARG_INT16(0); - int16 result; - result = -arg; - /* overflow check (needed for SHRT_MIN) */ - if (arg != 0 && SAMESIGN(result, arg)) + if (unlikely(arg == PG_INT16_MIN)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("smallint out of range"))); - PG_RETURN_INT16(result); + PG_RETURN_INT16(-arg); } Datum @@ -776,14 +735,7 @@ int2pl(PG_FUNCTION_ARGS) int16 arg2 = PG_GETARG_INT16(1); int16 result; - result = arg1 + arg2; - - /* - * Overflow check. If the inputs are of different signs then their sum - * cannot overflow. If the inputs are of the same sign, their sum had - * better be that sign too. - */ - if (SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_add16_overflow(arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("smallint out of range"))); @@ -797,14 +749,7 @@ int2mi(PG_FUNCTION_ARGS) int16 arg2 = PG_GETARG_INT16(1); int16 result; - result = arg1 - arg2; - - /* - * Overflow check. If the inputs are of the same sign then their - * difference cannot overflow. If they are of different signs then the - * result should be of the same sign as the first input. - */ - if (!SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_sub16_overflow(arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("smallint out of range"))); @@ -816,20 +761,14 @@ int2mul(PG_FUNCTION_ARGS) { int16 arg1 = PG_GETARG_INT16(0); int16 arg2 = PG_GETARG_INT16(1); - int32 result32; + int16 result; - /* - * The most practical way to detect overflow is to do the arithmetic in - * int32 (so that the result can't overflow) and then do a range check. - */ - result32 = (int32) arg1 * (int32) arg2; - - if (result32 < SHRT_MIN || result32 > SHRT_MAX) + if (unlikely(pg_mul16_overflow(arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("smallint out of range"))); - PG_RETURN_INT16((int16) result32); + PG_RETURN_INT16(result); } Datum @@ -856,12 +795,11 @@ int2div(PG_FUNCTION_ARGS) */ if (arg2 == -1) { - result = -arg1; - /* overflow check (needed for SHRT_MIN) */ - if (arg1 != 0 && SAMESIGN(result, arg1)) + if (unlikely(arg1 == INT16_MIN)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("smallint out of range"))); + result = -arg1; PG_RETURN_INT16(result); } @@ -879,14 +817,7 @@ int24pl(PG_FUNCTION_ARGS) int32 arg2 = PG_GETARG_INT32(1); int32 result; - result = arg1 + arg2; - - /* - * Overflow check. If the inputs are of different signs then their sum - * cannot overflow. If the inputs are of the same sign, their sum had - * better be that sign too. - */ - if (SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_add32_overflow((int32) arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); @@ -900,14 +831,7 @@ int24mi(PG_FUNCTION_ARGS) int32 arg2 = PG_GETARG_INT32(1); int32 result; - result = arg1 - arg2; - - /* - * Overflow check. If the inputs are of the same sign then their - * difference cannot overflow. If they are of different signs then the - * result should be of the same sign as the first input. - */ - if (!SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_sub32_overflow((int32) arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); @@ -921,20 +845,7 @@ int24mul(PG_FUNCTION_ARGS) int32 arg2 = PG_GETARG_INT32(1); int32 result; - result = arg1 * arg2; - - /* - * Overflow check. We basically check to see if result / arg2 gives arg1 - * again. There is one case where this fails: arg2 = 0 (which cannot - * overflow). - * - * Since the division is likely much more expensive than the actual - * multiplication, we'd like to skip it where possible. The best bang for - * the buck seems to be to check whether both inputs are in the int16 - * range; if so, no overflow is possible. - */ - if (!(arg2 >= (int32) SHRT_MIN && arg2 <= (int32) SHRT_MAX) && - result / arg2 != arg1) + if (unlikely(pg_mul32_overflow((int32) arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); @@ -947,7 +858,7 @@ int24div(PG_FUNCTION_ARGS) int16 arg1 = PG_GETARG_INT16(0); int32 arg2 = PG_GETARG_INT32(1); - if (arg2 == 0) + if (unlikely(arg2 == 0)) { ereport(ERROR, (errcode(ERRCODE_DIVISION_BY_ZERO), @@ -967,14 +878,7 @@ int42pl(PG_FUNCTION_ARGS) int16 arg2 = PG_GETARG_INT16(1); int32 result; - result = arg1 + arg2; - - /* - * Overflow check. If the inputs are of different signs then their sum - * cannot overflow. If the inputs are of the same sign, their sum had - * better be that sign too. - */ - if (SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_add32_overflow(arg1, (int32) arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); @@ -988,14 +892,7 @@ int42mi(PG_FUNCTION_ARGS) int16 arg2 = PG_GETARG_INT16(1); int32 result; - result = arg1 - arg2; - - /* - * Overflow check. If the inputs are of the same sign then their - * difference cannot overflow. If they are of different signs then the - * result should be of the same sign as the first input. - */ - if (!SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_sub32_overflow(arg1, (int32) arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); @@ -1009,20 +906,7 @@ int42mul(PG_FUNCTION_ARGS) int16 arg2 = PG_GETARG_INT16(1); int32 result; - result = arg1 * arg2; - - /* - * Overflow check. We basically check to see if result / arg1 gives arg2 - * again. There is one case where this fails: arg1 = 0 (which cannot - * overflow). - * - * Since the division is likely much more expensive than the actual - * multiplication, we'd like to skip it where possible. The best bang for - * the buck seems to be to check whether both inputs are in the int16 - * range; if so, no overflow is possible. - */ - if (!(arg1 >= (int32) SHRT_MIN && arg1 <= (int32) SHRT_MAX) && - result / arg1 != arg2) + if (unlikely(pg_mul32_overflow(arg1, (int32) arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); @@ -1036,7 +920,7 @@ int42div(PG_FUNCTION_ARGS) int16 arg2 = PG_GETARG_INT16(1); int32 result; - if (arg2 == 0) + if (unlikely(arg2 == 0)) { ereport(ERROR, (errcode(ERRCODE_DIVISION_BY_ZERO), @@ -1053,12 +937,11 @@ int42div(PG_FUNCTION_ARGS) */ if (arg2 == -1) { - result = -arg1; - /* overflow check (needed for INT_MIN) */ - if (arg1 != 0 && SAMESIGN(result, arg1)) + if (unlikely(arg1 == PG_INT32_MIN)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); + result = -arg1; PG_RETURN_INT32(result); } @@ -1075,7 +958,7 @@ int4mod(PG_FUNCTION_ARGS) int32 arg1 = PG_GETARG_INT32(0); int32 arg2 = PG_GETARG_INT32(1); - if (arg2 == 0) + if (unlikely(arg2 == 0)) { ereport(ERROR, (errcode(ERRCODE_DIVISION_BY_ZERO), @@ -1103,7 +986,7 @@ int2mod(PG_FUNCTION_ARGS) int16 arg1 = PG_GETARG_INT16(0); int16 arg2 = PG_GETARG_INT16(1); - if (arg2 == 0) + if (unlikely(arg2 == 0)) { ereport(ERROR, (errcode(ERRCODE_DIVISION_BY_ZERO), @@ -1136,12 +1019,11 @@ int4abs(PG_FUNCTION_ARGS) int32 arg1 = PG_GETARG_INT32(0); int32 result; - result = (arg1 < 0) ? -arg1 : arg1; - /* overflow check (needed for INT_MIN) */ - if (result < 0) + if (unlikely(arg1 == INT32_MIN)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); + result = (arg1 < 0) ? -arg1 : arg1; PG_RETURN_INT32(result); } @@ -1151,12 +1033,11 @@ int2abs(PG_FUNCTION_ARGS) int16 arg1 = PG_GETARG_INT16(0); int16 result; - result = (arg1 < 0) ? -arg1 : arg1; - /* overflow check (needed for SHRT_MIN) */ - if (result < 0) + if (unlikely(arg1 == INT16_MIN)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("smallint out of range"))); + result = (arg1 < 0) ? -arg1 : arg1; PG_RETURN_INT16(result); } @@ -1381,11 +1262,11 @@ generate_series_step_int4(PG_FUNCTION_ARGS) if ((fctx->step > 0 && fctx->current <= fctx->finish) || (fctx->step < 0 && fctx->current >= fctx->finish)) { - /* increment current in preparation for next iteration */ - fctx->current += fctx->step; - - /* if next-value computation overflows, this is the final result */ - if (SAMESIGN(result, fctx->step) && !SAMESIGN(result, fctx->current)) + /* + * Increment current in preparation for next iteration. If next-value + * computation overflows, this is the final result. + */ + if (pg_add32_overflow(fctx->current, fctx->step, &fctx->current)) fctx->step = 0; /* do when there is more left to send */ diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c index afa434cfee2..96136177b9c 100644 --- a/src/backend/utils/adt/int8.c +++ b/src/backend/utils/adt/int8.c @@ -17,6 +17,7 @@ #include <limits.h> #include <math.h> +#include "common/int.h" #include "funcapi.h" #include "libpq/pqformat.h" #include "utils/int8.h" @@ -25,8 +26,6 @@ #define MAXINT8LEN 25 -#define SAMESIGN(a,b) (((a) < 0) == ((b) < 0)) - typedef struct { int64 current; @@ -56,11 +55,14 @@ scanint8(const char *str, bool errorOK, int64 *result) { const char *ptr = str; int64 tmp = 0; - int sign = 1; + bool neg = 0; /* * Do our own scan, rather than relying on sscanf which might be broken * for long long. + * + * As INT64_MIN can't be stored as a positive 64 bit integer, accumulate + * value as a negative number. */ /* skip leading spaces */ @@ -71,18 +73,7 @@ scanint8(const char *str, bool errorOK, int64 *result) if (*ptr == '-') { ptr++; - - /* - * Do an explicit check for INT64_MIN. Ugly though this is, it's - * cleaner than trying to get the loop below to handle it portably. - */ - if (strncmp(ptr, "9223372036854775808", 19) == 0) - { - tmp = PG_INT64_MIN; - ptr += 19; - goto gotdigits; - } - sign = -1; + neg = true; } else if (*ptr == '+') ptr++; @@ -102,23 +93,13 @@ scanint8(const char *str, bool errorOK, int64 *result) /* process digits */ while (*ptr && isdigit((unsigned char) *ptr)) { - int64 newtmp = tmp * 10 + (*ptr++ - '0'); + int8 digit = (*ptr++ - '0'); - if ((newtmp / 10) != tmp) /* overflow? */ - { - if (errorOK) - return false; - else - ereport(ERROR, - (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("value \"%s\" is out of range for type %s", - str, "bigint"))); - } - tmp = newtmp; + if (unlikely(pg_mul64_overflow(tmp, 10, &tmp) || + pg_sub64_overflow(tmp, digit, &tmp))) + goto out_of_range; } -gotdigits: - /* allow trailing whitespace, but not other trailing chars */ while (*ptr != '\0' && isspace((unsigned char) *ptr)) ptr++; @@ -134,9 +115,24 @@ gotdigits: str))); } - *result = (sign < 0) ? -tmp : tmp; + if (!neg) + { + if (unlikely(tmp == INT64_MIN)) + goto out_of_range; + tmp = -tmp; + } + *result = tmp; return true; + +out_of_range: + if (errorOK) + return false; + else + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("value \"%s\" is out of range for type %s", + str, "bigint"))); } /* int8in() @@ -492,12 +488,11 @@ int8um(PG_FUNCTION_ARGS) int64 arg = PG_GETARG_INT64(0); int64 result; - result = -arg; - /* overflow check (needed for INT64_MIN) */ - if (arg != 0 && SAMESIGN(result, arg)) + if (unlikely(arg == PG_INT64_MIN)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); + result = -arg; PG_RETURN_INT64(result); } @@ -516,14 +511,7 @@ int8pl(PG_FUNCTION_ARGS) int64 arg2 = PG_GETARG_INT64(1); int64 result; - result = arg1 + arg2; - - /* - * Overflow check. If the inputs are of different signs then their sum - * cannot overflow. If the inputs are of the same sign, their sum had - * better be that sign too. - */ - if (SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_add64_overflow(arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -537,14 +525,7 @@ int8mi(PG_FUNCTION_ARGS) int64 arg2 = PG_GETARG_INT64(1); int64 result; - result = arg1 - arg2; - - /* - * Overflow check. If the inputs are of the same sign then their - * difference cannot overflow. If they are of different signs then the - * result should be of the same sign as the first input. - */ - if (!SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_sub64_overflow(arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -558,28 +539,10 @@ int8mul(PG_FUNCTION_ARGS) int64 arg2 = PG_GETARG_INT64(1); int64 result; - result = arg1 * arg2; - - /* - * Overflow check. We basically check to see if result / arg2 gives arg1 - * again. There are two cases where this fails: arg2 = 0 (which cannot - * overflow) and arg1 = INT64_MIN, arg2 = -1 (where the division itself - * will overflow and thus incorrectly match). - * - * Since the division is likely much more expensive than the actual - * multiplication, we'd like to skip it where possible. The best bang for - * the buck seems to be to check whether both inputs are in the int32 - * range; if so, no overflow is possible. - */ - if (arg1 != (int64) ((int32) arg1) || arg2 != (int64) ((int32) arg2)) - { - if (arg2 != 0 && - ((arg2 == -1 && arg1 < 0 && result < 0) || - result / arg2 != arg1)) - ereport(ERROR, - (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("bigint out of range"))); - } + if (unlikely(pg_mul64_overflow(arg1, arg2, &result))) + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("bigint out of range"))); PG_RETURN_INT64(result); } @@ -607,12 +570,11 @@ int8div(PG_FUNCTION_ARGS) */ if (arg2 == -1) { - result = -arg1; - /* overflow check (needed for INT64_MIN) */ - if (arg1 != 0 && SAMESIGN(result, arg1)) + if (unlikely(arg1 == INT64_MIN)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); + result = -arg1; PG_RETURN_INT64(result); } @@ -632,12 +594,11 @@ int8abs(PG_FUNCTION_ARGS) int64 arg1 = PG_GETARG_INT64(0); int64 result; - result = (arg1 < 0) ? -arg1 : arg1; - /* overflow check (needed for INT64_MIN) */ - if (result < 0) + if (unlikely(arg1 == INT64_MIN)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); + result = (arg1 < 0) ? -arg1 : arg1; PG_RETURN_INT64(result); } @@ -650,7 +611,7 @@ int8mod(PG_FUNCTION_ARGS) int64 arg1 = PG_GETARG_INT64(0); int64 arg2 = PG_GETARG_INT64(1); - if (arg2 == 0) + if (unlikely(arg2 == 0)) { ereport(ERROR, (errcode(ERRCODE_DIVISION_BY_ZERO), @@ -687,16 +648,12 @@ int8inc(PG_FUNCTION_ARGS) if (AggCheckCallContext(fcinfo, NULL)) { int64 *arg = (int64 *) PG_GETARG_POINTER(0); - int64 result; - result = *arg + 1; - /* Overflow check */ - if (result < 0 && *arg > 0) + if (unlikely(pg_add64_overflow(*arg, 1, arg))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); - *arg = result; PG_RETURN_POINTER(arg); } else @@ -706,9 +663,7 @@ int8inc(PG_FUNCTION_ARGS) int64 arg = PG_GETARG_INT64(0); int64 result; - result = arg + 1; - /* Overflow check */ - if (result < 0 && arg > 0) + if (unlikely(pg_add64_overflow(arg, 1, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -731,16 +686,11 @@ int8dec(PG_FUNCTION_ARGS) if (AggCheckCallContext(fcinfo, NULL)) { int64 *arg = (int64 *) PG_GETARG_POINTER(0); - int64 result; - result = *arg - 1; - /* Overflow check */ - if (result > 0 && *arg < 0) + if (unlikely(pg_sub64_overflow(*arg, 1, arg))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); - - *arg = result; PG_RETURN_POINTER(arg); } else @@ -750,9 +700,7 @@ int8dec(PG_FUNCTION_ARGS) int64 arg = PG_GETARG_INT64(0); int64 result; - result = arg - 1; - /* Overflow check */ - if (result > 0 && arg < 0) + if (unlikely(pg_sub64_overflow(arg, 1, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -821,14 +769,7 @@ int84pl(PG_FUNCTION_ARGS) int32 arg2 = PG_GETARG_INT32(1); int64 result; - result = arg1 + arg2; - - /* - * Overflow check. If the inputs are of different signs then their sum - * cannot overflow. If the inputs are of the same sign, their sum had - * better be that sign too. - */ - if (SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_add64_overflow(arg1, (int64) arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -842,14 +783,7 @@ int84mi(PG_FUNCTION_ARGS) int32 arg2 = PG_GETARG_INT32(1); int64 result; - result = arg1 - arg2; - - /* - * Overflow check. If the inputs are of the same sign then their - * difference cannot overflow. If they are of different signs then the - * result should be of the same sign as the first input. - */ - if (!SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_sub64_overflow(arg1, (int64) arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -863,20 +797,7 @@ int84mul(PG_FUNCTION_ARGS) int32 arg2 = PG_GETARG_INT32(1); int64 result; - result = arg1 * arg2; - - /* - * Overflow check. We basically check to see if result / arg1 gives arg2 - * again. There is one case where this fails: arg1 = 0 (which cannot - * overflow). - * - * Since the division is likely much more expensive than the actual - * multiplication, we'd like to skip it where possible. The best bang for - * the buck seems to be to check whether both inputs are in the int32 - * range; if so, no overflow is possible. - */ - if (arg1 != (int64) ((int32) arg1) && - result / arg1 != arg2) + if (unlikely(pg_mul64_overflow(arg1, (int64) arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -907,12 +828,11 @@ int84div(PG_FUNCTION_ARGS) */ if (arg2 == -1) { - result = -arg1; - /* overflow check (needed for INT64_MIN) */ - if (arg1 != 0 && SAMESIGN(result, arg1)) + if (unlikely(arg1 == INT64_MIN)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); + result = -arg1; PG_RETURN_INT64(result); } @@ -930,14 +850,7 @@ int48pl(PG_FUNCTION_ARGS) int64 arg2 = PG_GETARG_INT64(1); int64 result; - result = arg1 + arg2; - - /* - * Overflow check. If the inputs are of different signs then their sum - * cannot overflow. If the inputs are of the same sign, their sum had - * better be that sign too. - */ - if (SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_add64_overflow((int64) arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -951,14 +864,7 @@ int48mi(PG_FUNCTION_ARGS) int64 arg2 = PG_GETARG_INT64(1); int64 result; - result = arg1 - arg2; - - /* - * Overflow check. If the inputs are of the same sign then their - * difference cannot overflow. If they are of different signs then the - * result should be of the same sign as the first input. - */ - if (!SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_sub64_overflow((int64) arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -972,20 +878,7 @@ int48mul(PG_FUNCTION_ARGS) int64 arg2 = PG_GETARG_INT64(1); int64 result; - result = arg1 * arg2; - - /* - * Overflow check. We basically check to see if result / arg2 gives arg1 - * again. There is one case where this fails: arg2 = 0 (which cannot - * overflow). - * - * Since the division is likely much more expensive than the actual - * multiplication, we'd like to skip it where possible. The best bang for - * the buck seems to be to check whether both inputs are in the int32 - * range; if so, no overflow is possible. - */ - if (arg2 != (int64) ((int32) arg2) && - result / arg2 != arg1) + if (unlikely(pg_mul64_overflow((int64) arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -998,7 +891,7 @@ int48div(PG_FUNCTION_ARGS) int32 arg1 = PG_GETARG_INT32(0); int64 arg2 = PG_GETARG_INT64(1); - if (arg2 == 0) + if (unlikely(arg2 == 0)) { ereport(ERROR, (errcode(ERRCODE_DIVISION_BY_ZERO), @@ -1018,14 +911,7 @@ int82pl(PG_FUNCTION_ARGS) int16 arg2 = PG_GETARG_INT16(1); int64 result; - result = arg1 + arg2; - - /* - * Overflow check. If the inputs are of different signs then their sum - * cannot overflow. If the inputs are of the same sign, their sum had - * better be that sign too. - */ - if (SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_add64_overflow(arg1, (int64) arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -1039,14 +925,7 @@ int82mi(PG_FUNCTION_ARGS) int16 arg2 = PG_GETARG_INT16(1); int64 result; - result = arg1 - arg2; - - /* - * Overflow check. If the inputs are of the same sign then their - * difference cannot overflow. If they are of different signs then the - * result should be of the same sign as the first input. - */ - if (!SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_sub64_overflow(arg1, (int64) arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -1060,20 +939,7 @@ int82mul(PG_FUNCTION_ARGS) int16 arg2 = PG_GETARG_INT16(1); int64 result; - result = arg1 * arg2; - - /* - * Overflow check. We basically check to see if result / arg1 gives arg2 - * again. There is one case where this fails: arg1 = 0 (which cannot - * overflow). - * - * Since the division is likely much more expensive than the actual - * multiplication, we'd like to skip it where possible. The best bang for - * the buck seems to be to check whether both inputs are in the int32 - * range; if so, no overflow is possible. - */ - if (arg1 != (int64) ((int32) arg1) && - result / arg1 != arg2) + if (unlikely(pg_mul64_overflow(arg1, (int64) arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -1087,7 +953,7 @@ int82div(PG_FUNCTION_ARGS) int16 arg2 = PG_GETARG_INT16(1); int64 result; - if (arg2 == 0) + if (unlikely(arg2 == 0)) { ereport(ERROR, (errcode(ERRCODE_DIVISION_BY_ZERO), @@ -1104,12 +970,11 @@ int82div(PG_FUNCTION_ARGS) */ if (arg2 == -1) { - result = -arg1; - /* overflow check (needed for INT64_MIN) */ - if (arg1 != 0 && SAMESIGN(result, arg1)) + if (unlikely(arg1 == INT64_MIN)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); + result = -arg1; PG_RETURN_INT64(result); } @@ -1127,14 +992,7 @@ int28pl(PG_FUNCTION_ARGS) int64 arg2 = PG_GETARG_INT64(1); int64 result; - result = arg1 + arg2; - - /* - * Overflow check. If the inputs are of different signs then their sum - * cannot overflow. If the inputs are of the same sign, their sum had - * better be that sign too. - */ - if (SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_add64_overflow((int64) arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -1148,14 +1006,7 @@ int28mi(PG_FUNCTION_ARGS) int64 arg2 = PG_GETARG_INT64(1); int64 result; - result = arg1 - arg2; - - /* - * Overflow check. If the inputs are of the same sign then their - * difference cannot overflow. If they are of different signs then the - * result should be of the same sign as the first input. - */ - if (!SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1)) + if (unlikely(pg_sub64_overflow((int64) arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -1169,20 +1020,7 @@ int28mul(PG_FUNCTION_ARGS) int64 arg2 = PG_GETARG_INT64(1); int64 result; - result = arg1 * arg2; - - /* - * Overflow check. We basically check to see if result / arg2 gives arg1 - * again. There is one case where this fails: arg2 = 0 (which cannot - * overflow). - * - * Since the division is likely much more expensive than the actual - * multiplication, we'd like to skip it where possible. The best bang for - * the buck seems to be to check whether both inputs are in the int32 - * range; if so, no overflow is possible. - */ - if (arg2 != (int64) ((int32) arg2) && - result / arg2 != arg1) + if (unlikely(pg_mul64_overflow((int64) arg1, arg2, &result))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); @@ -1195,7 +1033,7 @@ int28div(PG_FUNCTION_ARGS) int16 arg1 = PG_GETARG_INT16(0); int64 arg2 = PG_GETARG_INT64(1); - if (arg2 == 0) + if (unlikely(arg2 == 0)) { ereport(ERROR, (errcode(ERRCODE_DIVISION_BY_ZERO), @@ -1287,17 +1125,13 @@ Datum int84(PG_FUNCTION_ARGS) { int64 arg = PG_GETARG_INT64(0); - int32 result; - result = (int32) arg; - - /* Test for overflow by reverse-conversion. */ - if ((int64) result != arg) + if (unlikely(arg < PG_INT32_MIN || arg > PG_INT32_MAX)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); - PG_RETURN_INT32(result); + PG_RETURN_INT32((int32) arg); } Datum @@ -1312,17 +1146,13 @@ Datum int82(PG_FUNCTION_ARGS) { int64 arg = PG_GETARG_INT64(0); - int16 result; - result = (int16) arg; - - /* Test for overflow by reverse-conversion. */ - if ((int64) result != arg) + if (unlikely(arg < PG_INT16_MIN || arg > PG_INT16_MAX)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("smallint out of range"))); - PG_RETURN_INT16(result); + PG_RETURN_INT16((int16) arg); } Datum @@ -1348,18 +1178,15 @@ dtoi8(PG_FUNCTION_ARGS) /* Round arg to nearest integer (but it's still in float form) */ arg = rint(arg); - /* - * Does it fit in an int64? Avoid assuming that we have handy constants - * defined for the range boundaries, instead test for overflow by - * reverse-conversion. - */ - result = (int64) arg; - - if ((float8) result != arg) + if (unlikely(arg < (double) PG_INT64_MIN || + arg > (double) PG_INT64_MAX || + isnan(arg))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); + result = (int64) arg; + PG_RETURN_INT64(result); } @@ -1381,42 +1208,32 @@ Datum ftoi8(PG_FUNCTION_ARGS) { float4 arg = PG_GETARG_FLOAT4(0); - int64 result; float8 darg; /* Round arg to nearest integer (but it's still in float form) */ darg = rint(arg); - /* - * Does it fit in an int64? Avoid assuming that we have handy constants - * defined for the range boundaries, instead test for overflow by - * reverse-conversion. - */ - result = (int64) darg; - - if ((float8) result != darg) + if (unlikely(arg < (float4) PG_INT64_MIN || + arg > (float4) PG_INT64_MAX || + isnan(arg))) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); - PG_RETURN_INT64(result); + PG_RETURN_INT64((int64) darg); } Datum i8tooid(PG_FUNCTION_ARGS) { int64 arg = PG_GETARG_INT64(0); - Oid result; - result = (Oid) arg; - - /* Test for overflow by reverse-conversion. */ - if ((int64) result != arg) + if (unlikely(arg < 0 || arg > PG_UINT32_MAX)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("OID out of range"))); - PG_RETURN_OID(result); + PG_RETURN_OID((Oid) arg); } Datum @@ -1494,11 +1311,11 @@ generate_series_step_int8(PG_FUNCTION_ARGS) if ((fctx->step > 0 && fctx->current <= fctx->finish) || (fctx->step < 0 && fctx->current >= fctx->finish)) { - /* increment current in preparation for next iteration */ - fctx->current += fctx->step; - - /* if next-value computation overflows, this is the final result */ - if (SAMESIGN(result, fctx->step) && !SAMESIGN(result, fctx->current)) + /* + * Increment current in preparation for next iteration. If next-value + * computation overflows, this is the final result. + */ + if (pg_add64_overflow(fctx->current, fctx->step, &fctx->current)) fctx->step = 0; /* do when there is more left to send */ diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index 2cd14f34012..59bb90df7a5 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -28,6 +28,7 @@ #include "access/hash.h" #include "catalog/pg_type.h" +#include "common/int.h" #include "funcapi.h" #include "lib/hyperloglog.h" #include "libpq/pqformat.h" @@ -6169,8 +6170,7 @@ numericvar_to_int64(const NumericVar *var, int64 *result) int ndigits; int weight; int i; - int64 val, - oldval; + int64 val; bool neg; NumericVar rounded; @@ -6196,27 +6196,25 @@ numericvar_to_int64(const NumericVar *var, int64 *result) weight = rounded.weight; Assert(weight >= 0 && ndigits <= weight + 1); - /* Construct the result */ + /* + * Construct the result. To avoid issues with converting a value + * corresponding to INT64_MIN (which can't be represented as a positive 64 + * bit twos-complement integer), accumulate value as a negative number. + */ digits = rounded.digits; neg = (rounded.sign == NUMERIC_NEG); - val = digits[0]; + val = -digits[0]; for (i = 1; i <= weight; i++) { - oldval = val; - val *= NBASE; - if (i < ndigits) - val += digits[i]; - - /* - * The overflow check is a bit tricky because we want to accept - * INT64_MIN, which will overflow the positive accumulator. We can - * detect this case easily though because INT64_MIN is the only - * nonzero value for which -val == val (on a two's complement machine, - * anyway). - */ - if ((val / NBASE) != oldval) /* possible overflow? */ + if (pg_mul64_overflow(val, NBASE, &val)) { - if (!neg || (-val) != val || val == 0 || oldval < 0) + free_var(&rounded); + return false; + } + + if (i < ndigits) + { + if (pg_sub64_overflow(val, digits[i], &val)) { free_var(&rounded); return false; @@ -6226,7 +6224,15 @@ numericvar_to_int64(const NumericVar *var, int64 *result) free_var(&rounded); - *result = neg ? -val : val; + if (!neg) + { + if (unlikely(val == INT64_MIN)) + return false; + val = -val; + } + *result = val; + + return true; } diff --git a/src/backend/utils/adt/oracle_compat.c b/src/backend/utils/adt/oracle_compat.c index b82016500b0..5bb68332912 100644 --- a/src/backend/utils/adt/oracle_compat.c +++ b/src/backend/utils/adt/oracle_compat.c @@ -15,6 +15,7 @@ */ #include "postgres.h" +#include "common/int.h" #include "utils/builtins.h" #include "utils/formatting.h" #include "mb/pg_wchar.h" @@ -1045,19 +1046,12 @@ repeat(PG_FUNCTION_ARGS) count = 0; slen = VARSIZE_ANY_EXHDR(string); - tlen = VARHDRSZ + (count * slen); - /* Check for integer overflow */ - if (slen != 0 && count != 0) - { - int check = count * slen; - int check2 = check + VARHDRSZ; - - if ((check / slen) != count || check2 <= check) - ereport(ERROR, - (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("requested length too large"))); - } + if (pg_mul32_overflow(count, slen, &tlen) || + pg_add32_overflow(tlen, VARHDRSZ, &tlen)) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("requested length too large"))); result = (text *) palloc(tlen); -- 2.14.1.536.g6867272d5b.dirty
>From 4cb5cdb67d64375cc803491476032952c03fdd43 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Mon, 30 Oct 2017 03:19:56 -0700 Subject: [PATCH 3/3] Do-Not-Apply: Remove -fwrapv. --- configure | 36 ------------------------------------ configure.in | 2 -- 2 files changed, 38 deletions(-) diff --git a/configure b/configure index f66899488cc..6040fd4ee6e 100755 --- a/configure +++ b/configure @@ -4633,42 +4633,6 @@ if test x"$pgac_cv_prog_cc_cflags__fno_strict_aliasing" = x"yes"; then CFLAGS="$CFLAGS -fno-strict-aliasing" fi - # Disable optimizations that assume no overflow; needed for gcc 4.3+ - { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CC supports -fwrapv" >&5 -$as_echo_n "checking whether $CC supports -fwrapv... " >&6; } -if ${pgac_cv_prog_cc_cflags__fwrapv+:} false; then : - $as_echo_n "(cached) " >&6 -else - pgac_save_CFLAGS=$CFLAGS -CFLAGS="$pgac_save_CFLAGS -fwrapv" -ac_save_c_werror_flag=$ac_c_werror_flag -ac_c_werror_flag=yes -cat confdefs.h - <<_ACEOF >conftest.$ac_ext -/* end confdefs.h. */ - -int -main () -{ - - ; - return 0; -} -_ACEOF -if ac_fn_c_try_compile "$LINENO"; then : - pgac_cv_prog_cc_cflags__fwrapv=yes -else - pgac_cv_prog_cc_cflags__fwrapv=no -fi -rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext -ac_c_werror_flag=$ac_save_c_werror_flag -CFLAGS="$pgac_save_CFLAGS" -fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_cc_cflags__fwrapv" >&5 -$as_echo "$pgac_cv_prog_cc_cflags__fwrapv" >&6; } -if test x"$pgac_cv_prog_cc_cflags__fwrapv" = x"yes"; then - CFLAGS="$CFLAGS -fwrapv" -fi - # Disable FP optimizations that cause various errors on gcc 4.5+ or maybe 4.6+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CC supports -fexcess-precision=standard" >&5 $as_echo_n "checking whether $CC supports -fexcess-precision=standard... " >&6; } diff --git a/configure.in b/configure.in index edf1dd2e7b8..3a3440f8550 100644 --- a/configure.in +++ b/configure.in @@ -427,8 +427,6 @@ if test "$GCC" = yes -a "$ICC" = no; then PGAC_PROG_CC_CFLAGS_OPT([-Wformat-security]) # Disable strict-aliasing rules; needed for gcc 3.3+ PGAC_PROG_CC_CFLAGS_OPT([-fno-strict-aliasing]) - # Disable optimizations that assume no overflow; needed for gcc 4.3+ - PGAC_PROG_CC_CFLAGS_OPT([-fwrapv]) # Disable FP optimizations that cause various errors on gcc 4.5+ or maybe 4.6+ PGAC_PROG_CC_CFLAGS_OPT([-fexcess-precision=standard]) # Optimization flags for specific files that benefit from vectorization -- 2.14.1.536.g6867272d5b.dirty
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers