Bug 759208 - CheckedInt.h depends on undefined value of signed arithmetic. r=bjacob.
authorRafael Ávila de Espíndola <respindola@mozilla.com>
Tue, 29 May 2012 12:48:26 -0400
changeset 95186 98410387837c1b689a719d1655ad6e868ee78be6
parent 95185 4c8dfa5b92ea8b46edc1ef989fe9bf8faf9381e1
child 95187 ccee89d6d9e0c6bc8003305dc3d49baebf1c1f5a
push idunknown
push userunknown
push dateunknown
reviewersbjacob
bugs759208
milestone15.0a1
Bug 759208 - CheckedInt.h depends on undefined value of signed arithmetic. r=bjacob.
mfbt/CheckedInt.h
--- a/mfbt/CheckedInt.h
+++ b/mfbt/CheckedInt.h
@@ -314,32 +314,43 @@ template<typename T, typename U>
 inline bool
 IsInRange(U x)
 {
   return IsInRangeImpl<T, U>::run(x);
 }
 
 template<typename T>
 inline bool
-IsAddValid(T x, T y, T result)
+IsAddValid(T x, T y)
 {
   // Addition is valid if the sign of x+y is equal to either that of x or that
-  // of y. Beware! These bitwise operations can return a larger integer type,
+  // of y. Since the value of x+y is undefined if we have a signed type, we
+  // compute it using the unsigned type of the same size.
+  // Beware! These bitwise operations can return a larger integer type,
   // if T was a small type like int8_t, so we explicitly cast to T.
+
+  typename UnsignedType<T>::Type ux = x;
+  typename UnsignedType<T>::Type uy = y;
+  typename UnsignedType<T>::Type result = ux + uy;
   return IsSigned<T>::value
          ? HasSignBit(BinaryComplement(T((result ^ x) & (result ^ y))))
          : BinaryComplement(x) >= y;
 }
 
 template<typename T>
 inline bool
-IsSubValid(T x, T y, T result)
+IsSubValid(T x, T y)
 {
   // Subtraction is valid if either x and y have same sign, or x-y and x have
-  // same sign.
+  // same sign. Since the value of x-y is undefined if we have a signed type,
+  // we compute it using the unsigned type of the same size.
+  typename UnsignedType<T>::Type ux = x;
+  typename UnsignedType<T>::Type uy = y;
+  typename UnsignedType<T>::Type result = ux - uy;
+
   return IsSigned<T>::value
          ? HasSignBit(BinaryComplement(T((result ^ x) & (x ^ y))))
          : x >= y;
 }
 
 template<typename T,
          bool IsSigned = IsSigned<T>::value,
          bool TwiceBiggerTypeIsSupported =
@@ -387,17 +398,17 @@ struct IsMulValidImpl<T, false, false>
     static bool run(T x, T y)
     {
       return y == 0 ||  x <= MaxValue<T>::value / y;
     }
 };
 
 template<typename T>
 inline bool
-IsMulValid(T x, T y, T /* result not used */)
+IsMulValid(T x, T y)
 {
   return IsMulValidImpl<T>::run(x, y);
 }
 
 template<typename T>
 inline bool
 IsDivValid(T x, T y)
 {
@@ -582,18 +593,17 @@ class CheckedInt
     {
       // Circumvent msvc warning about - applied to unsigned int.
       // if we're unsigned, the only valid case anyway is 0
       // in which case - is a no-op.
       T result = detail::OppositeIfSigned(mValue);
       /* Help the compiler perform RVO (return value optimization). */
       return CheckedInt(result,
                         mIsValid && detail::IsSubValid(T(0),
-                                                       mValue,
-                                                       result));
+                                                       mValue));
     }
 
     /**
      * @returns true if the left and right hand sides are valid
      * and have the same value.
      *
      * Note that these semantics are the reason why we don't offer
      * a operator!=. Indeed, we'd want to have a!=b be equivalent to !(a==b)
@@ -664,18 +674,17 @@ class CheckedInt
 #define MOZ_CHECKEDINT_BASIC_BINARY_OPERATOR(NAME, OP)                \
 template<typename T>                                                  \
 inline CheckedInt<T> operator OP(const CheckedInt<T> &lhs,            \
                                  const CheckedInt<T> &rhs)            \
 {                                                                     \
   T x = lhs.mValue;                                                   \
   T y = rhs.mValue;                                                   \
   T result = x OP y;                                                  \
-  T isOpValid                                                         \
-      = detail::Is##NAME##Valid(x, y, result);                        \
+  T isOpValid = detail::Is##NAME##Valid(x, y);                        \
   /* Help the compiler perform RVO (return value optimization). */    \
   return CheckedInt<T>(result,                                        \
                        lhs.mIsValid && rhs.mIsValid && isOpValid);    \
 }
 
 MOZ_CHECKEDINT_BASIC_BINARY_OPERATOR(Add, +)
 MOZ_CHECKEDINT_BASIC_BINARY_OPERATOR(Sub, -)
 MOZ_CHECKEDINT_BASIC_BINARY_OPERATOR(Mul, *)