From 029b997ca0d47f2f59a845ebcdd47b03fa95781f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= Date: Thu, 8 Mar 2018 00:31:02 +0100 Subject: [PATCH 1/4] [safe_op] fixed typo in doc-comment --- src/safe_op.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/safe_op.hpp b/src/safe_op.hpp index 014b7f3a..ed3f30f4 100644 --- a/src/safe_op.hpp +++ b/src/safe_op.hpp @@ -186,7 +186,7 @@ namespace Safe * instructions & the compiler's diagnostic. * * However, as some compilers don't provide intrinsics for certain - * types, the default implementation of add is the version from falback. + * types, the default implementation of add is the version from fallback. * * The struct is explicitly specialized for each type via #ifdefs for * each compiler. From 31b96b58e759e939f852c723218e9681dfed3cb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= Date: Thu, 8 Mar 2018 00:32:14 +0100 Subject: [PATCH 2/4] [safe_op] improved signed int overflow checks via integer promotion The fallback signed integer overflow check is quite expensive, but the addition can be safely performed when saved in an int due to integer promotion rules. This makes the check a little less expensive. --- src/safe_op.hpp | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/src/safe_op.hpp b/src/safe_op.hpp index ed3f30f4..1ac8b9a8 100644 --- a/src/safe_op.hpp +++ b/src/safe_op.hpp @@ -114,10 +114,11 @@ namespace Safe struct fallback_add_overflow; /*! - * @brief Overload of fallback_add_overflow for signed integers + * @brief Overload of fallback_add_overflow for signed integer types + * larger then int or with the same size as int */ template - struct fallback_add_overflow::VALUE>::type> + struct fallback_add_overflow::VALUE && sizeof(T) >= sizeof(int)>::type> { /*! * @brief Adds the two summands only if no overflow occurs @@ -147,6 +148,42 @@ namespace Safe } }; + /*! + * @brief Overload of fallback_add_overflow for signed integers smaller + * then int. + */ + template + struct fallback_add_overflow::VALUE && sizeof(T) < sizeof(int)>::type> + { + /*! + * @brief Adds the two summands only if no overflow occurs + * + * This function adds summand_1 and summand_2 exploiting integer + * promotion rules, thereby not causing undefined behavior. The + * result is checked against the limits of T and true is returned if + * they are exceeded. Otherwise the sum is saved in result and false + * is returned. + * + * @return true on overflow, false on no overflow + * + * The value in result is only valid when the function returns + * false. + * + * Further information: + * https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules + */ + static bool add(T summand_1, T summand_2, T& result) + { + const int res = summand_1 + summand_2; + if ((res > std::numeric_limits::max()) || (res < std::numeric_limits::min())) { + return true; + } else { + result = static_cast(res); + return false; + } + } + }; + /*! * @brief Overload of fallback_add_overflow for unsigned integers */ From 684c8c89de4250197d7cbe2b36cbe1979f50f4c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= Date: Thu, 8 Mar 2018 00:34:14 +0100 Subject: [PATCH 3/4] [safe_op] Simplified unsigned int overflow check Simply check for overflows after the addition, as no undefined behavior can occur here. --- src/safe_op.hpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/safe_op.hpp b/src/safe_op.hpp index 1ac8b9a8..f7962b68 100644 --- a/src/safe_op.hpp +++ b/src/safe_op.hpp @@ -204,12 +204,8 @@ namespace Safe */ static bool add(T summand_1, T summand_2, T& result) { - if (summand_1 > std::numeric_limits::max() - summand_2) { - return true; - } else { - result = summand_1 + summand_2; - return false; - } + result = summand_1 + summand_2; + return result < summand_1; } }; From 06ec1e6984b403159f55f6f1731058c0bedb4eef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= Date: Fri, 9 Mar 2018 00:12:29 +0100 Subject: [PATCH 4/4] [safe_op] Refactored addition overflow checks using functions only - templated structs were not required, SFINAE works for functions too => use instead, removes some unneeded code - fix non-usage of builtins with clang - adjust unit tests --- src/safe_op.hpp | 253 ++++++++++++++++--------------------- unitTests/test_safe_op.cpp | 4 +- 2 files changed, 109 insertions(+), 148 deletions(-) diff --git a/src/safe_op.hpp b/src/safe_op.hpp index f7962b68..c4c89b04 100644 --- a/src/safe_op.hpp +++ b/src/safe_op.hpp @@ -42,33 +42,32 @@ namespace Safe { /*! - * @brief Helper structs for providing integer overflow checks. + * @brief Helper functions for providing integer overflow checks. * - * This namespace contains the internal helper structs fallback_add_overflow - * and builtin_add_overflow. Both have a public static member function add - * with the following interface: + * This namespace contains internal helper functions fallback_$op_overflow + * and builtin_$op_overflow (where $op is an arithmetic operation like add, + * subtract, etc.). Both provide the following interface: * - * bool add(T summand_1, T summand_2, T& result) + * bool fallback/builtin_$op_overflow(T first, T second, T& result); * - * where T is the type over which the struct is templated. + * where T is an integer type. * - * The function performs a check whether the addition summand_1 + summand_2 - * can be performed without an overflow. If the operation would overflow, - * true is returned and the addition is not performed if it would result in - * undefined behavior. If no overflow occurs, the sum is saved in result and - * false is returned. + * Each function performs checks whether first $op second can be safely + * performed without overflows. If yes, the result is saved in result and + * false is returned. Otherwise true is returned and the contents of result + * are unspecified. * - * fallback_add_overflow implements a portable but slower overflow check. - * builtin_add_overflow uses compiler builtins (when available) and should - * be considerably faster. As builtins are not available for all types, - * builtin_add_overflow falls back to fallback_add_overflow when no builtin + * fallback_$op_overflow implements a portable but slower overflow check. + * builtin_$op_overflow uses compiler builtins (when available) and should + * be faster. As builtins are not available for all types, + * builtin_$op_overflow falls back to fallback_$op_overflow when no builtin * is available. */ namespace Internal { /*! * @brief Helper struct to determine whether a type is signed or unsigned - + * * This struct is a backport of std::is_signed from C++11. It has a public * enum with the property VALUE which is true when the type is signed or * false if it is unsigned. @@ -103,145 +102,111 @@ namespace Safe }; /*! - * @brief Fallback overflow checker, specialized via SFINAE + * @brief Check the addition of two numbers for overflows for signed + * integer types larger than int or with the same size as int. * - * This struct implements a 'fallback' addition with an overflow check, - * i.e. it does not rely on compiler intrinsics. It is specialized via - * SFINAE for signed and unsigned integer types and provides a public - * static member function add. - */ - template - struct fallback_add_overflow; - - /*! - * @brief Overload of fallback_add_overflow for signed integer types - * larger then int or with the same size as int + * This function performs a check if summand_1 + summand_2 would + * overflow and returns true in that case. If no overflow occurs, + * the sum is saved in result and false is returned. + * + * @return true on overflow, false on no overflow + * + * @param[in] summand_1, summand_2 The summands with are added + * @param[out] result Result of the addition, only populated when no + * overflow occurs. + * + * Further information: + * https://wiki.sei.cmu.edu/confluence/display/c/INT32-C.+Ensure+that+operations+on+signed+integers+do+not+result+in+overflow */ template - struct fallback_add_overflow::VALUE && sizeof(T) >= sizeof(int)>::type> + typename enable_if::VALUE && sizeof(T) >= sizeof(int), bool>::type fallback_add_overflow( + T summand_1, T summand_2, T& result) { - /*! - * @brief Adds the two summands only if no overflow occurs - * - * This function performs a check if summand_1 + summand_2 would - * overflow and returns true in that case. If no overflow occurs, - * the sum is saved in result and false is returned. - * - * @return true on overflow, false on no overflow - * - * The check for an overflow is performed before the addition to - * ensure that no undefined behavior occurs. The value in result is - * only valid when the function returns false. - * - * Further information: - * https://wiki.sei.cmu.edu/confluence/display/c/INT32-C.+Ensure+that+operations+on+signed+integers+do+not+result+in+overflow - */ - static bool add(T summand_1, T summand_2, T& result) - { - if (((summand_2 >= 0) && (summand_1 > std::numeric_limits::max() - summand_2)) || - ((summand_2 < 0) && (summand_1 < std::numeric_limits::min() - summand_2))) { - return true; - } else { - result = summand_1 + summand_2; - return false; - } + if (((summand_2 >= 0) && (summand_1 > std::numeric_limits::max() - summand_2)) || + ((summand_2 < 0) && (summand_1 < std::numeric_limits::min() - summand_2))) { + return true; + } else { + result = summand_1 + summand_2; + return false; } - }; + } /*! - * @brief Overload of fallback_add_overflow for signed integers smaller - * then int. + * @brief Check the addition of two numbers for overflows for signed + * integer types smaller than int. + * + * This function adds summand_1 and summand_2 exploiting integer + * promotion rules, thereby not causing undefined behavior. The + * result is checked against the limits of T and true is returned if + * they are exceeded. Otherwise the sum is saved in result and false + * is returned. + * + * @return true on overflow, false on no overflow + * + * @param[in] summand_1, summand_2 The summands with are added + * @param[out] result Result of the addition, only populated when no + * overflow occurs. + * + * Further information: + * https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules */ template - struct fallback_add_overflow::VALUE && sizeof(T) < sizeof(int)>::type> + typename enable_if::VALUE && sizeof(T) < sizeof(int), bool>::type fallback_add_overflow( + T summand_1, T summand_2, T& result) { - /*! - * @brief Adds the two summands only if no overflow occurs - * - * This function adds summand_1 and summand_2 exploiting integer - * promotion rules, thereby not causing undefined behavior. The - * result is checked against the limits of T and true is returned if - * they are exceeded. Otherwise the sum is saved in result and false - * is returned. - * - * @return true on overflow, false on no overflow - * - * The value in result is only valid when the function returns - * false. - * - * Further information: - * https://wiki.sei.cmu.edu/confluence/display/c/INT02-C.+Understand+integer+conversion+rules - */ - static bool add(T summand_1, T summand_2, T& result) - { - const int res = summand_1 + summand_2; - if ((res > std::numeric_limits::max()) || (res < std::numeric_limits::min())) { - return true; - } else { - result = static_cast(res); - return false; - } + const int res = summand_1 + summand_2; + if ((res > std::numeric_limits::max()) || (res < std::numeric_limits::min())) { + return true; + } else { + result = static_cast(res); + return false; } - }; + } /*! - * @brief Overload of fallback_add_overflow for unsigned integers + * @brief Check the addition of two numbers for overflows for unsigned + * integer types. + * + * This function adds summand_1 and summand_2 and checks after that if + * the operation overflowed. Since these are unsigned integers, no + * undefined behavior is invoked. + * + * @return true on overflow, false on no overflow + * + * @param[in] summand_1, summand_2 The summands with are added + * @param[out] result Result of the addition + * + * Further information: + * https://wiki.sei.cmu.edu/confluence/display/c/INT30-C.+Ensure+that+unsigned+integer+operations+do+not+wrap */ template - struct fallback_add_overflow::VALUE>::type> + typename enable_if::VALUE, bool>::type fallback_add_overflow(T summand_1, T summand_2, T& result) { - /*! - * @brief Adds the two summands only if no overflow occurs - * - * This function performs a check if summand_1 + summand_2 would - * overflow and returns true in that case. If no overflow occurs, - * the sum is saved in result and false is returned. - * - * @return true on overflow, false on no overflow - * - * Further information: - * https://wiki.sei.cmu.edu/confluence/display/c/INT30-C.+Ensure+that+unsigned+integer+operations+do+not+wrap - */ - static bool add(T summand_1, T summand_2, T& result) - { - result = summand_1 + summand_2; - return result < summand_1; - } - }; + result = summand_1 + summand_2; + return result < summand_1; + } /*! - * @brief Overflow checker using compiler intrinsics + * @brief Overflow addition check using compiler intrinsics. * - * This struct provides an add function with the same interface & - * behavior as fallback_add_overload::add but it relies on compiler - * intrinsics instead. This version should be considerably faster than - * the fallback version as it can fully utilize available CPU + * This function behaves exactly like fallback_add_overflow() but it + * relies on compiler intrinsics instead. This version should be faster + * than the fallback version as it can fully utilize available CPU * instructions & the compiler's diagnostic. * * However, as some compilers don't provide intrinsics for certain - * types, the default implementation of add is the version from fallback. + * types, the default implementation is the version from fallback. * - * The struct is explicitly specialized for each type via #ifdefs for - * each compiler. + * This function is fully specialized for each compiler. */ template - struct builtin_add_overflow + bool builtin_add_overflow(T summand_1, T summand_2, T& result) { - /*! - * @brief Add summand_1 and summand_2 and check for overflows. - * - * This is the default add() function that uses - * fallback_add_overflow::add(). All specializations must have - * exactly the same interface and behave the same way. - */ - static inline bool add(T summand_1, T summand_2, T& result) - { - return fallback_add_overflow::add(summand_1, summand_2, result); - } - }; + return fallback_add_overflow(summand_1, summand_2, result); + } #if defined(__GNUC__) || defined(__clang__) -#if __GNUC__ >= 5 +#if __GNUC__ >= 5 || __clang_major__ >= 3 /*! * This macro pastes a specialization of builtin_add_overflow using gcc's & @@ -253,14 +218,13 @@ namespace Safe * The intrinsics are documented here: * https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html#Integer-Overflow-Builtins */ -#define SPECIALIZE_builtin_add_overflow(type, builtin_name) \ - template <> \ - struct builtin_add_overflow \ - { \ - static inline bool add(type summand_1, type summand_2, type& result) \ - { \ - return builtin_name(summand_1, summand_2, &result); \ - } \ +#define SPECIALIZE_builtin_add_overflow(type, builtin_name) \ + /* Full specialization of builtin_add_overflow for type using the */ \ + /* builtin_name intrinsic */ \ + template <> \ + bool builtin_add_overflow(type summand_1, type summand_2, type & result) \ + { \ + return builtin_name(summand_1, summand_2, &result); \ } SPECIALIZE_builtin_add_overflow(int, __builtin_sadd_overflow); @@ -272,7 +236,7 @@ namespace Safe SPECIALIZE_builtin_add_overflow(unsigned long long, __builtin_uaddll_overflow); #undef SPECIALIZE_builtin_add_overflow -#endif +#endif // __GNUC__ >= 5 || __clang_major >= 3 #elif defined(_MSC_VER) @@ -289,14 +253,11 @@ namespace Safe * The intrinsics are documented here: * https://msdn.microsoft.com/en-us/library/windows/desktop/ff516460(v=vs.85).aspx */ -#define SPECIALIZE_builtin_add_overflow_WIN(type, builtin_name) \ - template <> \ - struct builtin_add_overflow \ - { \ - static inline bool add(type summand_1, type summand_2, type& result) \ - { \ - return builtin_name(summand_1, summand_2, &result) != S_OK; \ - } \ +#define SPECIALIZE_builtin_add_overflow_WIN(type, builtin_name) \ + template <> \ + bool builtin_add_overflow(type summand_1, type summand_2, type& result) \ + { \ + return builtin_name(summand_1, summand_2, &result) != S_OK; \ } SPECIALIZE_builtin_add_overflow_WIN(unsigned int, UIntAdd); @@ -305,7 +266,7 @@ namespace Safe #undef SPECIALIZE_builtin_add_overflow_WIN -#endif +#endif // defined(_MSC_VER) } // namespace Internal @@ -332,7 +293,7 @@ namespace Safe T add(T summand_1, T summand_2) { T res = 0; - if (Internal::builtin_add_overflow::add(summand_1, summand_2, res)) { + if (Internal::builtin_add_overflow(summand_1, summand_2, res)) { throw std::overflow_error("Overflow in addition"); } return res; diff --git a/unitTests/test_safe_op.cpp b/unitTests/test_safe_op.cpp index 83f76038..49ab4b39 100644 --- a/unitTests/test_safe_op.cpp +++ b/unitTests/test_safe_op.cpp @@ -115,8 +115,8 @@ void test_add() } \ } - TEST_ADD(si::fallback_add_overflow::add) - TEST_ADD(si::builtin_add_overflow::add) + TEST_ADD(si::fallback_add_overflow) + TEST_ADD(si::builtin_add_overflow) #undef TEST_ADD }