diff --git a/include/boost/rational.hpp b/include/boost/rational.hpp index 4b66aae..3bdf85f 100644 --- a/include/boost/rational.hpp +++ b/include/boost/rational.hpp @@ -516,6 +516,10 @@ inline rational operator- (const rational& r) template BOOST_CXX14_CONSTEXPR rational& rational::operator+= (const rational& r) { + // Prevent division by zero below if *this == 0, r == 0 and num_g == 0. + if (r.num == IntType(0)) + return *this; // r == 0 => *this need not be changed. + // This calculation avoids overflow, and minimises the number of expensive // calculations. Thanks to Nickolay Mladenov for this algorithm. // @@ -534,15 +538,22 @@ BOOST_CXX14_CONSTEXPR rational& rational::operator+= (const ra // Which proves that instead of normalizing the result, it is better to // divide num and den by gcd((a*d1 + c*b1), g) + // First dividing, then multiplying by num_g avoids overflow even further, + // at the cost of a branch, an extra GCD calculation, two integer divisions + // and one integer multiplication. + // Protect against self-modification IntType r_num = r.num; IntType r_den = r.den; + const IntType num_g = integer::gcd(num, r_num); IntType g = integer::gcd(den, r_den); den /= g; // = b1 from the calculations above - num = num * (r_den / g) + r_num * den; + num = (num / num_g) * (r_den / g) + (r_num / num_g) * den; + g = integer::gcd(num, g); num /= g; + num *= num_g; den *= r_den/g; return *this; diff --git a/test/rational_test.cpp b/test/rational_test.cpp index ccd58ea..d55ff54 100644 --- a/test/rational_test.cpp +++ b/test/rational_test.cpp @@ -1478,6 +1478,39 @@ BOOST_AUTO_TEST_SUITE_END() // The bugs, patches, and requests checking suite BOOST_AUTO_TEST_SUITE( bug_patch_request_suite ) +BOOST_AUTO_TEST_CASE( operator_plus_overflow_test ) +{ + MyOverflowingUnsigned const base = 10; + MyOverflowingUnsigned num1 = 1; + while ( num1 < UINT_MAX / base ) num1 *= base; + if ( UINT_MAX / num1 < 4 ) // e.g. 64-bit unsigned + { + num1 /= base; + num1 *= 7; + } + else // e.g. 32-bit unsigned + { + num1 *= 2; + } + MyOverflowingUnsigned const num2 = num1 * 2; + + // Other unsigned types are not covered by this test + // (but it should still pass on all platforms). + if ( sizeof( MyOverflowingUnsigned ) == 4 + || sizeof( MyOverflowingUnsigned ) == 8 ) + { + BOOST_TEST( UINT_MAX - num1 < num2 ); + } + + MyOverflowingUnsigned const denom = 3; + boost::rational const r1( num1, denom ), + r2( num2, denom ); + // This test succeeds when MyOverflowingUnsigned is replaced with a built-in + // unsigned type due to unsigned integer overflow wraparound. But it guards + // against the undefined behavior of signed integer overflow. + BOOST_TEST( r1 + r2 == num1 ); +} + // "rational operator< can overflow" BOOST_AUTO_TEST_CASE( bug_798357_test ) {