Thread: [PATCH] Fix INT_MIN % -1 overflow in int8mod().

Return 0 for INT_MIN % -1 (64-bit) instead of throwing an exception.
This patch complements commit f9ac414c that fixed int4mod().
---src/backend/utils/adt/int8.c |    4 ++++src/include/c.h              |    7 +++++++2 files changed, 11
insertions(+)

diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 0e59956..9da651b 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -649,6 +649,10 @@ int8mod(PG_FUNCTION_ARGS)        PG_RETURN_NULL();    }
+    /* SELECT ((-9223372036854775808)::int8) % (-1); causes a floating point exception */
+    if (arg1 == INT64_MIN && arg2 == -1)
+        PG_RETURN_INT64(0);
+    /* No overflow is possible */    PG_RETURN_INT64(arg1 % arg2);
diff --git a/src/include/c.h b/src/include/c.h
index a6c0e6e..d20ba8c 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -294,6 +294,13 @@ typedef unsigned long long int uint64;#define UINT64CONST(x) ((uint64) x)#endif
+#ifndef INT64_MAX
+#define INT64_MAX INT64CONST(9223372036854775807)
+#endif
+
+#ifndef INT64_MIN
+#define INT64_MIN (-INT64_MAX-1)
+#endif/* Select timestamp representation (float8 or int64) */#ifdef USE_INTEGER_DATETIMES
-- 
1.7.10.4

Xi Wang <xi.wang@gmail.com> writes:
> Return 0 for INT_MIN % -1 (64-bit) instead of throwing an exception.
> This patch complements commit f9ac414c that fixed int4mod().

Meh.  I didn't care for the explicit dependency on INT_MIN in the
previous patch, and I like introducing INT64_MIN even less.  I think
we should be able to reduce the test to just
if (arg2 == -1)    return 0;

since zero is the correct result for any value of arg1, not only
INT_MIN.
        regards, tom lane

On 11/14/12 4:46 PM, Tom Lane wrote:
> Meh.  I didn't care for the explicit dependency on INT_MIN in the
> previous patch, and I like introducing INT64_MIN even less.  I think
> we should be able to reduce the test to just
> 
>     if (arg2 == -1)
>         return 0;
> 
> since zero is the correct result for any value of arg1, not only
> INT_MIN.

I agree.  Will send a v2.  Thanks. :)

- xi

Xi Wang <xi.wang@gmail.com> writes:
> I agree.  Will send a v2.  Thanks. :)

No need, I'm already patching it.
        regards, tom lane

Return 0 for x % -1 instead of throwing an exception (e.g., when x
is INT_MIN).

Suggested by Tom Lane.
---src/backend/utils/adt/int8.c |    4 ++++1 file changed, 4 insertions(+)

diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 0e59956..a30ab36 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -649,6 +649,10 @@ int8mod(PG_FUNCTION_ARGS)        PG_RETURN_NULL();    }
+    /* SELECT ((-9223372036854775808)::int8) % (-1); causes a floating point exception */
+    if (arg2 == -1)
+        PG_RETURN_INT64(0);
+    /* No overflow is possible */    PG_RETURN_INT64(arg1 % arg2);
-- 
1.7.10.4

Since x % -1 returns 0 for any x, we don't need the check x == INT_MIN.

Suggested by Tom Lane.
---src/backend/utils/adt/int.c |    2 +-1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c
index 4be3901..3e423fe 100644
--- a/src/backend/utils/adt/int.c
+++ b/src/backend/utils/adt/int.c
@@ -1096,7 +1096,7 @@ int4mod(PG_FUNCTION_ARGS)    }    /* SELECT ((-2147483648)::int4) % (-1); causes a floating point
exception*/
 
-    if (arg1 == INT_MIN && arg2 == -1)
+    if (arg2 == -1)        PG_RETURN_INT32(0);    /* No overflow is possible */
-- 
1.7.10.4

On 11/14/12 5:03 PM, Tom Lane wrote:
> No need, I'm already patching it.

Oops.  Sorry.  Ignore my patches.

- xi