Обсуждение: Checking for overflow of integer arithmetic

Поиск
Список
Период
Сортировка

Checking for overflow of integer arithmetic

От
Tom Lane
Дата:
I'm working on a patch to detect overflow in the integer-arithmetic
operators.  The first stage, covering the basic int4 operators, is
attached if anyone wants to comment on details.  A couple of general
questions though:

1. Does anyone object to applying this for 8.0?  I think we already had
consensus that it's a good idea, but if not now's the time to speak up.
(There are a couple of regression tests that fail and will need to be
adjusted, if that influences anyone's thinking.)

2. For the int2 and int8 operators, should we stick to a one-size-fits-all
message "integer out of range", or be more specific: "smallint out of
range" and "bigint out of range"?  The existing messages are not
completely consistent about this.  I'm inclined to go with mentioning
the specific type but I'm not dead set on it.
        regards, tom lane


*** src/backend/utils/adt/int.c.orig    Sun Aug 29 09:55:24 2004
--- src/backend/utils/adt/int.c    Sun Oct  3 15:20:13 2004
***************
*** 45,50 ****
--- 45,52 ---- #define SHRT_MIN (-0x8000) #endif 
+ #define SAMESIGN(a,b)    (((a) < 0) == ((b) < 0))
+  typedef struct {     int32        current;
***************
*** 601,608 **** int4um(PG_FUNCTION_ARGS) {     int32        arg = PG_GETARG_INT32(0); 
!     PG_RETURN_INT32(-arg); }  Datum
--- 604,618 ---- 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))
!         ereport(ERROR,
!                 (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
!                  errmsg("integer out of range")));
!     PG_RETURN_INT32(result); }  Datum
***************
*** 618,625 **** {     int32        arg1 = PG_GETARG_INT32(0);     int32        arg2 = PG_GETARG_INT32(1); 
!     PG_RETURN_INT32(arg1 + arg2); }  Datum
--- 628,646 ---- {     int32        arg1 = PG_GETARG_INT32(0);     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))
!         ereport(ERROR,
!                 (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
!                  errmsg("integer out of range")));
!     PG_RETURN_INT32(result); }  Datum
***************
*** 627,634 **** {     int32        arg1 = PG_GETARG_INT32(0);     int32        arg2 = PG_GETARG_INT32(1); 
!     PG_RETURN_INT32(arg1 - arg2); }  Datum
--- 648,666 ---- {     int32        arg1 = PG_GETARG_INT32(0);     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))
!         ereport(ERROR,
!                 (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
!                  errmsg("integer out of range")));
!     PG_RETURN_INT32(result); }  Datum
***************
*** 636,643 **** {     int32        arg1 = PG_GETARG_INT32(0);     int32        arg2 = PG_GETARG_INT32(1); 
!     PG_RETURN_INT32(arg1 * arg2); }  Datum
--- 668,695 ---- {     int32        arg1 = PG_GETARG_INT32(0);     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 &&
!         (result/arg2 != arg1 || (arg2 == -1 && arg1 < 0 && result < 0)))
!         ereport(ERROR,
!                 (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
!                  errmsg("integer out of range")));
!     PG_RETURN_INT32(result); }  Datum
***************
*** 645,665 **** {     int32        arg1 = PG_GETARG_INT32(0);     int32        arg2 = PG_GETARG_INT32(1);      if
(arg2== 0)         ereport(ERROR,                 (errcode(ERRCODE_DIVISION_BY_ZERO),                  errmsg("division
byzero"))); 
 
!     PG_RETURN_INT32(arg1 / arg2); }  Datum int4inc(PG_FUNCTION_ARGS) {     int32        arg = PG_GETARG_INT32(0); 
!     PG_RETURN_INT32(arg + 1); }  Datum
--- 697,736 ---- {     int32        arg1 = PG_GETARG_INT32(0);     int32        arg2 = PG_GETARG_INT32(1);
+     int32        result;      if (arg2 == 0)         ereport(ERROR,
(errcode(ERRCODE_DIVISION_BY_ZERO),                 errmsg("division by zero"))); 
 
!     result = arg1 / arg2;
!     /*
!      * Overflow check.  The only possible overflow case is for
!      * arg1 = INT_MIN, arg2 = -1, where the correct result is -INT_MIN,
!      * which can't be represented on a two's-complement machine.
!      */
!     if (arg2 == -1 && arg1 < 0 && result < 0)
!         ereport(ERROR,
!                 (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
!                  errmsg("integer out of range")));
!     PG_RETURN_INT32(result); }  Datum int4inc(PG_FUNCTION_ARGS) {     int32        arg = PG_GETARG_INT32(0);
+     int32        result;
+ 
+     result = arg + 1;
+     /* Overflow check */
+     if (arg > 0 && result < 0)
+         ereport(ERROR,
+                 (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                  errmsg("integer out of range"))); 
!     PG_RETURN_INT32(result); }  Datum


Re: Checking for overflow of integer arithmetic

От
"Dave Page"
Дата:

> -----Original Message-----
> From: pgsql-hackers-owner@postgresql.org
> [mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Tom Lane
> Sent: 03 October 2004 20:39
> To: pgsql-hackers@postgresql.org
> Subject: [HACKERS] Checking for overflow of integer arithmetic
>
> 2. For the int2 and int8 operators, should we stick to a
> one-size-fits-all message "integer out of range", or be more
> specific: "smallint out of range" and "bigint out of range"?
> The existing messages are not completely consistent about
> this.  I'm inclined to go with mentioning the specific type
> but I'm not dead set on it.

I vote for being more specific. A little extra info can sometimes ease
debugging problems no matter how trivial it seems.

Regards, Dave.


Re: Checking for overflow of integer arithmetic

От
Bruno Wolff III
Дата:
On Sun, Oct 03, 2004 at 15:38:52 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 1. Does anyone object to applying this for 8.0?  I think we already had
> consensus that it's a good idea, but if not now's the time to speak up.
> (There are a couple of regression tests that fail and will need to be
> adjusted, if that influences anyone's thinking.)

I think this should go in. I think not detecting overflow is really a bug.

> 2. For the int2 and int8 operators, should we stick to a one-size-fits-all
> message "integer out of range", or be more specific: "smallint out of
> range" and "bigint out of range"?  The existing messages are not
> completely consistent about this.  I'm inclined to go with mentioning
> the specific type but I'm not dead set on it.

I think giving the type info will be helpful for debugging.


Re: Checking for overflow of integer arithmetic

От
Gaetano Mendola
Дата:
Tom Lane wrote:

>   {
>       int32        arg1 = PG_GETARG_INT32(0);
>       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 &&
> !         (result/arg2 != arg1 || (arg2 == -1 && arg1 < 0 && result < 0)))
> !         ereport(ERROR,
> !                 (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
> !                  errmsg("integer out of range")));
> !     PG_RETURN_INT32(result);
>   }

May be is a good idea postpone the division as second "OR" argument in order to hope to avoid
it.


Regards
Gaetano Mendola