From 9dd251b3a1ced6c9745a2e54b847fd33dd458f42 Mon Sep 17 00:00:00 2001 From: Lucian Grijincu Date: Mon, 9 May 2016 23:00:19 -0700 Subject: [PATCH] folly: ubsan: disable integer overflow tests in Histogram Summary: It would be nice to fix, but it's going to be easier to do when folly support is for GCC>=5 which adds `__builtin_add_overflow`. For now disable the check as the Histgram code handles overflow correctly and has tests for these cases. Fixes need to be done for both float and integer types as Histogram is used with both. Reviewed By: meyering Differential Revision: D3280260 fbshipit-source-id: 56e524c517566a4547346859be7770eda2acee96 --- folly/stats/Histogram.h | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/folly/stats/Histogram.h b/folly/stats/Histogram.h index 0b9baba1..f6fbea71 100644 --- a/folly/stats/Histogram.h +++ b/folly/stats/Histogram.h @@ -23,6 +23,7 @@ #include #include +#include #include namespace folly { @@ -242,17 +243,24 @@ class Histogram { : buckets_(bucketSize, min, max, Bucket()) {} /* Add a data point to the histogram */ - void addValue(ValueType value) { + void addValue(ValueType value) UBSAN_DISABLE("signed-integer-overflow") + UBSAN_DISABLE("unsigned-integer-overflow") { Bucket& bucket = buckets_.getByValue(value); - // TODO: It would be nice to handle overflow here. + // 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; bucket.count += 1; } /* Add multiple same data points to the histogram */ - void addRepeatedValue(ValueType value, uint64_t nSamples) { + void addRepeatedValue(ValueType value, uint64_t nSamples) + UBSAN_DISABLE("signed-integer-overflow") + UBSAN_DISABLE("unsigned-integer-overflow") { Bucket& bucket = buckets_.getByValue(value); - // TODO: It would be nice to handle overflow here. + // 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; bucket.count += nSamples; } @@ -264,9 +272,12 @@ 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) { + void removeValue(ValueType value) UBSAN_DISABLE("signed-integer-overflow") + UBSAN_DISABLE("unsigned-integer-overflow") { Bucket& bucket = buckets_.getByValue(value); - // TODO: It would be nice to handle overflow here. + // 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; bucket.count -= 1; -- 2.34.1