On Thu, Aug 29, 2019 at 08:14:54AM +0200, Fabien COELHO wrote: > Attached v7 does create uint64 overflow inline functions. The stuff yet is > not moved to "common/int.c", a file which does not exists, even if > "common/int.h" does.
Thanks for the new version. I have begun reviewing your patch, and attached is a first cut that I would like to commit separately which adds all the compatibility overflow routines to int.h for uint16, uint32 and uint64 with all the fallback implementations (int128-based method added as well if available). I have also grouped at the top of the file the comments about each routine's return policy to avoid duplication. For the fallback implementations of uint64 using int128, I think that it is cleaner to use uint128 so as there is no need to check after negative results for multiplications, and I have made the various expressions consistent for each size. Attached is a small module called "overflow" with various regression tests that I used to check each implementation. I don't propose that for commit as I am not sure if that's worth the extra CPU, so let's consider it as a toy for now. What do you think? -- Michael
overflow.tar.gz
Description: application/gzip
diff --git a/src/include/common/int.h b/src/include/common/int.h
index d754798543..ccef1b4cc9 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -20,11 +20,26 @@
#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
+
+/*---------
+ * The following guidelines apply to all the routines:
+ * - 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.
+ * - 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.
+ * - 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.
+ *---------
*/
+
+/*------------------------------------------------------------------------
+ * Overflow routines for signed integers
+ *------------------------------------------------------------------------
+ */
+
static inline bool
pg_add_s16_overflow(int16 a, int16 b, int16 *result)
{
@@ -43,11 +58,6 @@ pg_add_s16_overflow(int16 a, int16 b, int16 *result)
#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_sub_s16_overflow(int16 a, int16 b, int16 *result)
{
@@ -66,11 +76,6 @@ pg_sub_s16_overflow(int16 a, int16 b, int16 *result)
#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_mul_s16_overflow(int16 a, int16 b, int16 *result)
{
@@ -89,11 +94,6 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
#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_add_s32_overflow(int32 a, int32 b, int32 *result)
{
@@ -112,11 +112,6 @@ pg_add_s32_overflow(int32 a, int32 b, int32 *result)
#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_sub_s32_overflow(int32 a, int32 b, int32 *result)
{
@@ -135,11 +130,6 @@ pg_sub_s32_overflow(int32 a, int32 b, int32 *result)
#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_mul_s32_overflow(int32 a, int32 b, int32 *result)
{
@@ -158,11 +148,6 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
#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_add_s64_overflow(int64 a, int64 b, int64 *result)
{
@@ -190,11 +175,6 @@ pg_add_s64_overflow(int64 a, int64 b, int64 *result)
#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_sub_s64_overflow(int64 a, int64 b, int64 *result)
{
@@ -222,11 +202,6 @@ pg_sub_s64_overflow(int64 a, int64 b, int64 *result)
#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_mul_s64_overflow(int64 a, int64 b, int64 *result)
{
@@ -270,4 +245,195 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
#endif
}
+/*------------------------------------------------------------------------
+ * Overflow routines for unsigned integers
+ *------------------------------------------------------------------------
+ */
+
+static inline bool
+pg_add_u16_overflow(uint16 a, uint16 b, uint16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_add_overflow(a, b, result);
+#else
+ uint16 res = a + b;
+
+ if (res < a)
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = res;
+ return false;
+#endif
+}
+
+static inline bool
+pg_sub_u16_overflow(uint16 a, uint16 b, uint16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(a, b, result);
+#else
+ if (b > a)
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = a - b;
+ return false;
+#endif
+}
+
+static inline bool
+pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_mul_overflow(a, b, result);
+#else
+ uint16 res = a * b;
+
+ if (a != 0 && b != res / a)
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = res;
+ return false;
+#endif
+}
+
+static inline bool
+pg_add_u32_overflow(uint32 a, uint32 b, uint32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_add_overflow(a, b, result);
+#else
+ uint32 res = a + b;
+
+ if (res < a)
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = res;
+ return false;
+#endif
+}
+
+static inline bool
+pg_sub_u32_overflow(uint32 a, uint32 b, uint32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(a, b, result);
+#else
+ if (b > a)
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = a - b;
+ return false;
+#endif
+}
+
+static inline bool
+pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_mul_overflow(a, b, result);
+#else
+ uint32 res = a * b;
+
+ if (a != 0 && b != res / a)
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = res;
+ return false;
+#endif
+}
+
+static inline bool
+pg_add_u64_overflow(uint64 a, uint64 b, uint64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_add_overflow(a, b, result);
+#elif defined(HAVE_INT128)
+ uint128 res = (uint128) a + (uint128) b;
+
+ if (res > PG_UINT64_MAX)
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = (uint64) res;
+ return false;
+#else
+ uint64 res = a + b;
+
+ if (res < a)
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = res;
+ return false;
+#endif
+}
+
+static inline bool
+pg_sub_u64_overflow(uint64 a, uint64 b, uint64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(a, b, result);
+#elif defined(HAVE_INT128)
+ uint128 res = (uint128) a - (uint128) b;
+
+ if (res > PG_UINT64_MAX)
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = (uint64) res;
+ return false;
+#else
+ if (b > a)
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = a - b;
+ return false;
+#endif
+}
+
+static inline bool
+pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_mul_overflow(a, b, result);
+#elif defined(HAVE_INT128)
+ uint128 res = (uint128) a * (uint128) b;
+
+ if (res > PG_UINT64_MAX)
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = (uint64) res;
+ return false;
+#else
+ uint64 res = a * b;
+
+ if (a != 0 && b != res / a)
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = res;
+ return false;
+#endif
+}
+
#endif /* COMMON_INT_H */
signature.asc
Description: PGP signature
