Fix integer overflow failure in spark SumAggregationTest (facebookinc…

…ubator#9964)

Summary:
Pull Request resolved: facebookincubator#9964

When summing up integers, the spark_sum() function intentionally
handles integer overflow by letting the integer value wrap around
(relying on the compiler implementation). However, signed integer
overflow is an undefined behavior, so the undefined behavior sanitizer
catches this issue and fails the test. This diff turns off the undefined
behavior sanitizer in the spark_sum() function to avoid unit test
failures.

Reviewed By: bikramSingh91

Differential Revision: D57893466

fbshipit-source-id: 412b42245a4d0d7b19b88cd4290384d2383496f0

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 <typename TValue, bool Overflow>
+#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<TValue, int64_t> && Overflow) ||

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 <typename TData>
+#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<TData, int64_t> && Overflow) ||
@@ -162,7 +173,12 @@ class SumAggregateBase
     }
   }
 
+  // Disable undefined behavior sanitizer to not fail on signed integer
+  // overflow.
   template <typename TData>
+#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<TData, int64_t> && Overflow) ||

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 <typename InputType, typename ResultType, bool Overflow = false>
+#if defined(FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER)
+FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER("signed-integer-overflow")
+#endif
 void testHookLimits(bool expectOverflow = false) {
   // Pair of <limit, value to overflow>.
   std::vector<std::pair<InputType, InputType>> limits = {
@@ -103,10 +108,8 @@ void verifyAggregates(
 }
 
 template <typename InputType, typename ResultType, typename IntermediateType>
-#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,