Commit 8973236c authored by Elliott Clark's avatar Elliott Clark Committed by Facebook Github Bot

Don't overflow/underflow in folly::stats::detail::Bucket

Summary: Underflow/overflow of signed integers is fraught with dangers. So this diff removes the potential for overflow while adding or subtracting buckets in folly stats.

Reviewed By: yfeldblum

Differential Revision: D16898755

fbshipit-source-id: 4d3df1024b4c01ea0b276278105071e62e27fa27
parent 1dd79077
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#pragma once #pragma once
#include <folly/ConstexprMath.h>
#include <chrono> #include <chrono>
#include <cstdint> #include <cstdint>
#include <type_traits> #include <type_traits>
...@@ -54,6 +55,40 @@ avgHelper(ValueType sum, uint64_t count) { ...@@ -54,6 +55,40 @@ avgHelper(ValueType sum, uint64_t count) {
return static_cast<ReturnType>(sumf / countf); return static_cast<ReturnType>(sumf / countf);
} }
// Helpers to add bucket counts and values without
// ever causing undefined behavior
//
// For non-integral vlaues everyhing is easy
template <
typename ValueType,
typename std::enable_if<!std::is_integral<ValueType>::value, int>::type = 0>
void addHelper(ValueType& a, const ValueType& b) {
a += b;
}
template <
typename ValueType,
typename std::enable_if<!std::is_integral<ValueType>::value, int>::type = 0>
void subtractHelper(ValueType& a, const ValueType& b) {
a -= b;
}
// For integral values we use folly/ConstexprMath.h to make
// the math safe and predictable
template <
typename ValueType,
typename std::enable_if<std::is_integral<ValueType>::value, int>::type = 0>
void addHelper(ValueType& a, const ValueType& b) {
a = constexpr_add_overflow_clamped(a, b);
}
template <
typename ValueType,
typename std::enable_if<std::is_integral<ValueType>::value, int>::type = 0>
void subtractHelper(ValueType& a, const ValueType& b) {
a = constexpr_sub_overflow_clamped(a, b);
}
/* /*
* Helper function to compute the rate per Interval, * Helper function to compute the rate per Interval,
* given the specified count recorded over the elapsed time period. * given the specified count recorded over the elapsed time period.
...@@ -100,9 +135,8 @@ struct Bucket { ...@@ -100,9 +135,8 @@ struct Bucket {
} }
void add(const ValueType& s, uint64_t c) { void add(const ValueType& s, uint64_t c) {
// TODO: It would be nice to handle overflow here. addHelper(sum, s);
sum += s; addHelper(count, c);
count += c;
} }
Bucket& operator+=(const Bucket& o) { Bucket& operator+=(const Bucket& o) {
...@@ -111,9 +145,8 @@ struct Bucket { ...@@ -111,9 +145,8 @@ struct Bucket {
} }
Bucket& operator-=(const Bucket& o) { Bucket& operator-=(const Bucket& o) {
// TODO: It would be nice to handle overflow here. subtractHelper(sum, o.sum);
sum -= o.sum; subtractHelper(count, o.count);
count -= o.count;
return *this; return *this;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment