Fix float to int conversion in base/numerics

base/numerics/safe_conversions_impl.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_NUMERICS_SAFE_CONVERSIONS_IMPL_H_
 #define BASE_NUMERICS_SAFE_CONVERSIONS_IMPL_H_
 
 #include <limits>
 
 #include "base/template_util.h"
@@ skipping 90 matching lines @@
 
 // This function creates a RangeConstraint from an upper and lower bound
 // check by taking advantage of the fact that only NaN can be out of range in
 // both directions at once.
 inline RangeConstraint GetRangeConstraint(bool is_in_upper_bound,
                                    bool is_in_lower_bound) {
   return GetRangeConstraint((is_in_upper_bound ? 0 : RANGE_OVERFLOW) |
                             (is_in_lower_bound ? 0 : RANGE_UNDERFLOW));
 }
 
+// The following helper template addresses a corner case in range checks for
+// conversion from a floating-point type to an integral type of smaller range
+// but larger precision (e.g. float -> int32_t). The problem is as follows:
+//   1. Integral maximum is always one less than a power of two, so it must be
+//      truncated to fit the mantissa of the floating point. The direction of
+//      rounding is implementation defined, but in practice it's always IEEE
+//      floats, which round to nearest and thus result in a value of larger

[brucedawson, 2015/09/11 23:23:27]

The FPU can be set to various rounding modes, some of which would then cause the
original code to work correctly. I believe that the new code works regardless of
rounding mode (because the calculated integer constant converts with no
rounding) but it might be good to state that.

[jschuh, 2015/09/12 00:43:12]

Yes, this should be reliable and completely portable since it truncates the
value to match the resolution of the target ieee floating point.

+//      magnitude than the integral value (maximum + 1).
+//   2. If the floating point value is equal to the promoted integral maximum
+//      value, a range check will erroneously pass.
+//   3. When the floating point value is then converted to an integer, the
+//      resulting value is out of range for the target integral type and
+//      thus is implementation defined.
+// To fix this bug we manually truncate the maximum value when the destination
+// type is an integral of larger precision than a source floating-point type.

[brucedawson, 2015/09/11 23:23:27]

Could clarify what it is truncated to, perhaps with an example for float to int.

[jschuh, 2015/09/12 00:43:12]

Done.

+template <typename Dst, typename Src>
+struct NarrowingRange {
+  typedef typename std::numeric_limits<Src> SrcLimits;
+  typedef typename std::numeric_limits<Dst> DstLimits;
+
+  static Dst max() {
+    // The following logic avoids warnings where the max function is
+    // instantiated with invalid values for a bit shift (even though
+    // the such a function can never be called).
+    static const int shift =
+        (MaxExponent<Src>::value > MaxExponent<Dst>::value &&
+         SrcLimits::digits < DstLimits::digits && SrcLimits::is_iec559 &&
+         !DstLimits::is_iec559)
+            ? (DstLimits::digits - SrcLimits::digits)
+            : 0;
+    return DstLimits::max() - static_cast<Dst>((UINTMAX_C(1) << shift) - 1);
+  }
+
+  static Dst min() {
+    return std::numeric_limits<Dst>::is_iec559 ? -DstLimits::max()
+                                               : DstLimits::min();
+  }
+};
+
 template <
     typename Dst,
     typename Src,
     IntegerRepresentation DstSign = std::numeric_limits<Dst>::is_signed
                                             ? INTEGER_REPRESENTATION_SIGNED
                                             : INTEGER_REPRESENTATION_UNSIGNED,
     IntegerRepresentation SrcSign = std::numeric_limits<Src>::is_signed
                                             ? INTEGER_REPRESENTATION_SIGNED
                                             : INTEGER_REPRESENTATION_UNSIGNED,
     NumericRangeRepresentation DstRange =
@@ skipping 19 matching lines @@
 
 // Signed to signed narrowing: Both the upper and lower boundaries may be
 // exceeded.
 template <typename Dst, typename Src>
 struct DstRangeRelationToSrcRangeImpl<Dst,
                                       Src,
                                       INTEGER_REPRESENTATION_SIGNED,
                                       INTEGER_REPRESENTATION_SIGNED,
                                       NUMERIC_RANGE_NOT_CONTAINED> {
   static RangeConstraint Check(Src value) {
-    return std::numeric_limits<Dst>::is_iec559
-               ? GetRangeConstraint((value < std::numeric_limits<Dst>::max()),
-                                    (value > -std::numeric_limits<Dst>::max()))
-               : GetRangeConstraint((value < std::numeric_limits<Dst>::max()),
-                                    (value > std::numeric_limits<Dst>::min()));
+    return GetRangeConstraint((value <= NarrowingRange<Dst, Src>::max()),
+                              (value >= NarrowingRange<Dst, Src>::min()));
   }
 };
 
 // Unsigned to unsigned narrowing: Only the upper boundary can be exceeded.
 template <typename Dst, typename Src>
 struct DstRangeRelationToSrcRangeImpl<Dst,
                                       Src,
                                       INTEGER_REPRESENTATION_UNSIGNED,
                                       INTEGER_REPRESENTATION_UNSIGNED,
                                       NUMERIC_RANGE_NOT_CONTAINED> {
   static RangeConstraint Check(Src value) {
-    return GetRangeConstraint(value < std::numeric_limits<Dst>::max(), true);
+    return GetRangeConstraint(value <= NarrowingRange<Dst, Src>::max(), true);
   }
 };
 
 // Unsigned to signed: The upper boundary may be exceeded.
 template <typename Dst, typename Src>
 struct DstRangeRelationToSrcRangeImpl<Dst,
                                       Src,
                                       INTEGER_REPRESENTATION_SIGNED,
                                       INTEGER_REPRESENTATION_UNSIGNED,
                                       NUMERIC_RANGE_NOT_CONTAINED> {
   static RangeConstraint Check(Src value) {
     return sizeof(Dst) > sizeof(Src)
                ? RANGE_VALID
                : GetRangeConstraint(
-                     value < static_cast<Src>(std::numeric_limits<Dst>::max()),
+                     value <= static_cast<Src>(NarrowingRange<Dst, Src>::max()),
                      true);
   }
 };
 
 // Signed to unsigned: The upper boundary may be exceeded for a narrower Dst,
 // and any negative value exceeds the lower boundary.
 template <typename Dst, typename Src>
 struct DstRangeRelationToSrcRangeImpl<Dst,
                                       Src,
                                       INTEGER_REPRESENTATION_UNSIGNED,
                                       INTEGER_REPRESENTATION_SIGNED,
                                       NUMERIC_RANGE_NOT_CONTAINED> {
   static RangeConstraint Check(Src value) {
     return (MaxExponent<Dst>::value >= MaxExponent<Src>::value)
                ? GetRangeConstraint(true, value >= static_cast<Src>(0))
                : GetRangeConstraint(
-                     value < static_cast<Src>(std::numeric_limits<Dst>::max()),
+                     value <= static_cast<Src>(NarrowingRange<Dst, Src>::max()),
                      value >= static_cast<Src>(0));
   }
 };
 
 template <typename Dst, typename Src>
 inline RangeConstraint DstRangeRelationToSrcRange(Src value) {
   static_assert(std::numeric_limits<Src>::is_specialized,
                 "Argument must be numeric.");
   static_assert(std::numeric_limits<Dst>::is_specialized,
                 "Result must be numeric.");
   return DstRangeRelationToSrcRangeImpl<Dst, Src>::Check(value);
 }
 
 }  // namespace internal
 }  // namespace base
 
 #endif  // BASE_NUMERICS_SAFE_CONVERSIONS_IMPL_H_