Checking for overflow of integer arithmetic

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 by zero")));

! 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