diff --git a/velox/exec/AggregationHook.h b/velox/exec/AggregationHook.h index e0e87b7e1584..3daeed75a9b4 100644 --- a/velox/exec/AggregationHook.h +++ b/velox/exec/AggregationHook.h @@ -15,6 +15,8 @@ */ #pragma once +#include "folly/CPortability.h" + #include "velox/common/base/CheckedArithmetic.h" #include "velox/common/base/Range.h" #include "velox/vector/LazyVector.h" @@ -98,7 +100,17 @@ class AggregationHook : public ValueHook { }; namespace { +// Spark's sum function sets Overflow to true and intentionally let the result +// value be automatically wrapped around when integer overflow happens. Hence, +// disable undefined behavior sanitizer to not fail on signed integer overflow. +// The disablement of the sanitizer only affects SumHook that is used for +// pushdown of sum aggregation functions. It doesn't affect the Presto's sum +// function that sets Overflow to false because overflow is handled explicitly +// in checkedPlus. template +#if defined(FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER) +FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER("signed-integer-overflow") +#endif inline void updateSingleValue(TValue& result, TValue value) { if constexpr ( (std::is_same_v && Overflow) || diff --git a/velox/functions/lib/aggregates/SumAggregateBase.h b/velox/functions/lib/aggregates/SumAggregateBase.h index 93923aff5abe..d940a2994069 100644 --- a/velox/functions/lib/aggregates/SumAggregateBase.h +++ b/velox/functions/lib/aggregates/SumAggregateBase.h @@ -15,6 +15,8 @@ */ #pragma once +#include "folly/CPortability.h" + #include "velox/expression/FunctionSignature.h" #include "velox/functions/lib/CheckedArithmeticImpl.h" #include "velox/functions/lib/aggregates/DecimalAggregate.h" @@ -151,7 +153,16 @@ class SumAggregateBase /// Update functions that check for overflows for integer types. /// For floating points, an overflow results in +/- infinity which is a /// valid output. + // Spark's sum function sets Overflow to true and intentionally let the result + // value be automatically wrapped around when integer overflow happens. Hence, + // disable undefined behavior sanitizer to not fail on signed integer + // overflow. The disablement of the sanitizer doesn't affect the Presto's sum + // function that sets Overflow to false because overflow is handled explicitly + // in checkedPlus. template +#if defined(FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER) + FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER("signed-integer-overflow") +#endif static void updateSingleValue(TData& result, TData value) { if constexpr ( (std::is_same_v && Overflow) || @@ -162,7 +173,12 @@ class SumAggregateBase } } + // Disable undefined behavior sanitizer to not fail on signed integer + // overflow. template +#if defined(FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER) + FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER("signed-integer-overflow") +#endif static void updateDuplicateValues(TData& result, TData value, int n) { if constexpr ( (std::is_same_v && Overflow) || diff --git a/velox/functions/lib/aggregates/tests/SumTestBase.h b/velox/functions/lib/aggregates/tests/SumTestBase.h index 6d4f72cf9e81..b1dcf5e75e0e 100644 --- a/velox/functions/lib/aggregates/tests/SumTestBase.h +++ b/velox/functions/lib/aggregates/tests/SumTestBase.h @@ -14,6 +14,8 @@ * limitations under the License. */ +#include "folly/CPortability.h" + #include "velox/common/base/tests/GTestUtils.h" #include "velox/exec/AggregationHook.h" #include "velox/exec/tests/utils/AssertQueryBuilder.h" @@ -29,6 +31,9 @@ struct SumRow { }; template +#if defined(FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER) +FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER("signed-integer-overflow") +#endif void testHookLimits(bool expectOverflow = false) { // Pair of . std::vector> limits = { @@ -103,10 +108,8 @@ void verifyAggregates( } template -#if defined(__has_feature) -#if __has_feature(__address_sanitizer__) -__attribute__((no_sanitize("integer"))) -#endif +#if defined(FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER) +FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER("signed-integer-overflow") #endif void SumTestBase::testAggregateOverflow( const std::string& function,