Commit 33d55e3c authored by Elliott Clark's avatar Elliott Clark Committed by Facebook Github Bot

Make MultiLevelTimeSeries UBSAN-clean

Summary:
While running a system lots of different values will be added to lots of different `MultiLevelTimeSeries`. These values can be timestamps, lengths of time, file sizes, just about anything. There's no way for the underlying library to know before hand how large they will be. So it's always going to be possible to cause undefined behavior.

That means we could fix this one of two ways. We could make this code be ignored for ubsan, or we could make it never roll over.

I have made it so that the code won't roll over here. This seems better because it will continue to give more representative numbers. Rather than going from a very large number to a very small number, we'll remain at a very large number. That should still convey to every one that the numbers being accumulated are very large.

Reviewed By: mzlee

Differential Revision: D16970762

fbshipit-source-id: c56f6db1dbdecefd190e6ac357133886a3642571
parent 151a795b
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#pragma once #pragma once
#include <folly/ConstexprMath.h>
#include <glog/logging.h> #include <glog/logging.h>
namespace folly { namespace folly {
...@@ -86,8 +87,14 @@ void MultiLevelTimeSeries<VT, CT>::addValueAggregated( ...@@ -86,8 +87,14 @@ void MultiLevelTimeSeries<VT, CT>::addValueAggregated(
flush(); flush();
cachedTime_ = now; cachedTime_ = now;
} }
cachedSum_ += total; // We have no control over how many different values get added to a time
cachedCount_ += nsamples; // series.
// We also have no control over their value. We also want to keep some partial
// ordering; meaning large numbers should stay large, and negative numbers
// should stay negative. So use the constexpr_add_overflow_clamped so that
// this never overflows
cachedSum_ = constexpr_add_overflow_clamped(cachedSum_, total);
cachedCount_ = constexpr_add_overflow_clamped(cachedCount_, nsamples);
} }
template <typename VT, typename CT> template <typename VT, typename CT>
......
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