Work around UB signed-integer-overflow in Histogram

Summary: [Folly] Work around UB `signed-integer-overflow` in `Histogram`.

Reviewed By: igorsugak

Differential Revision: D6922381

fbshipit-source-id: 96ab6ffae9509ce8138f1741ea976226dec597c8

folly/stats/Histogram.h

@@ -25,6 +25,7 @@
 #include <vector>
 
 #include <folly/CPortability.h>
+#include <folly/Traits.h>
 #include <folly/stats/detail/Bucket.h>
 
 namespace folly {
@@ -248,27 +249,24 @@ class Histogram {
       : buckets_(bucketSize, min, max, Bucket()) {}
 
   /* Add a data point to the histogram */
-  void addValue(ValueType value) FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER(
-      "signed-integer-overflow",
-      "unsigned-integer-overflow") {
+  void addValue(ValueType value) {
     Bucket& bucket = buckets_.getByValue(value);
     // NOTE: Overflow is handled elsewhere and tests check this
     // behavior (see HistogramTest.cpp TestOverflow* tests).
     // TODO: It would be nice to handle overflow here and redesign this class.
-    bucket.sum += value;
+    auto const addend = to_unsigned(value);
+    bucket.sum = static_cast<ValueType>(to_unsigned(bucket.sum) + addend);
     bucket.count += 1;
   }
 
   /* Add multiple same data points to the histogram */
-  void addRepeatedValue(ValueType value, uint64_t nSamples)
-      FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER(
-          "signed-integer-overflow",
-          "unsigned-integer-overflow") {
+  void addRepeatedValue(ValueType value, uint64_t nSamples) {
     Bucket& bucket = buckets_.getByValue(value);
     // NOTE: Overflow is handled elsewhere and tests check this
     // behavior (see HistogramTest.cpp TestOverflow* tests).
     // TODO: It would be nice to handle overflow here and redesign this class.
-    bucket.sum += value * nSamples;
+    auto const addend = to_unsigned(value) * nSamples;
+    bucket.sum = static_cast<ValueType>(to_unsigned(bucket.sum) + addend);
     bucket.count += nSamples;
   }
 
@@ -279,15 +277,14 @@ class Histogram {
    * had previously been added to the histogram; it merely subtracts the
    * requested value from the appropriate bucket's sum.
    */
-  void removeValue(ValueType value) FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER(
-      "signed-integer-overflow",
-      "unsigned-integer-overflow") {
+  void removeValue(ValueType value) {
     Bucket& bucket = buckets_.getByValue(value);
     // NOTE: Overflow is handled elsewhere and tests check this
     // behavior (see HistogramTest.cpp TestOverflow* tests).
     // TODO: It would be nice to handle overflow here and redesign this class.
     if (bucket.count > 0) {
-      bucket.sum -= value;
+      auto const subtrahend = to_unsigned(value);
+      bucket.sum = static_cast<ValueType>(to_unsigned(bucket.sum) - subtrahend);
       bucket.count -= 1;
     } else {
       bucket.sum = ValueType();
@@ -475,6 +472,19 @@ class Histogram {
   };
 
  private:
+  template <
+      typename S,
+      typename = _t<std::enable_if<std::is_integral<S>::value>>>
+  static constexpr _t<std::make_unsigned<S>> to_unsigned(S s) {
+    return static_cast<_t<std::make_unsigned<S>>>(s);
+  }
+  template <
+      typename S,
+      typename = _t<std::enable_if<!std::is_integral<S>::value>>>
+  static constexpr S to_unsigned(S s) {
+    return s;
+  }
+
   detail::HistogramBuckets<ValueType, Bucket> buckets_;
 };