apacheGH-45180: [C++][Fuzzing] Fix Negation bug discovered by fuzzing (…

…apache#45181)

### Rationale for this change
If the value for Decimal32 or Decimal64 is `INT32_MIN` or `INT64_MIN` respectively, then UBSAN reports an issue when calling Negate on them due to overflow. 

### What changes are included in this PR?
Have the `Negate` methods of Decimal32 and Decimal64 use `arrow::internal::SafeSignedNegate`.

### Are these changes tested?
Unit tests were added for both cases which were able to reproduce the problem when UBSAN was on without the fix.

### Are there any user-facing changes?
No.

* OSS-Fuzz issue: https://issues.oss-fuzz.com/issues/371239168

* GitHub Issue: apache#45180

Authored-by: Matt Topol <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>

cpp/src/arrow/util/basic_decimal.cc

@@ -50,6 +50,11 @@ static constexpr uint64_t kInt64Mask = 0xFFFFFFFFFFFFFFFF;
 static constexpr uint64_t kInt32Mask = 0xFFFFFFFF;
 #endif
 
+BasicDecimal32& BasicDecimal32::Negate() {
+  value_ = arrow::internal::SafeSignedNegate(value_);
+  return *this;
+}
+
 DecimalStatus BasicDecimal32::Divide(const BasicDecimal32& divisor,
                                      BasicDecimal32* result,
                                      BasicDecimal32* remainder) const {
@@ -152,6 +157,11 @@ BasicDecimal32::operator BasicDecimal64() const {
   return BasicDecimal64(static_cast<int64_t>(value()));
 }
 
+BasicDecimal64& BasicDecimal64::Negate() {
+  value_ = arrow::internal::SafeSignedNegate(value_);
+  return *this;
+}
+
 DecimalStatus BasicDecimal64::Divide(const BasicDecimal64& divisor,
                                      BasicDecimal64* result,
                                      BasicDecimal64* remainder) const {
@@ -253,12 +263,18 @@ const BasicDecimal64& BasicDecimal64::GetHalfScaleMultiplier(int32_t scale) {
 bool BasicDecimal32::FitsInPrecision(int32_t precision) const {
   DCHECK_GE(precision, 0);
   DCHECK_LE(precision, kMaxPrecision);
+  if (value_ == INT32_MIN) {
+    return false;
+  }
   return Abs(*this) < DecimalTraits<BasicDecimal32>::powers_of_ten()[precision];
 }
 
 bool BasicDecimal64::FitsInPrecision(int32_t precision) const {
   DCHECK_GE(precision, 0);
   DCHECK_LE(precision, kMaxPrecision);
+  if (value_ == INT64_MIN) {
+    return false;
+  }
   return Abs(*this) < DecimalTraits<BasicDecimal64>::powers_of_ten()[precision];
 }
 

cpp/src/arrow/util/basic_decimal.h

@@ -276,10 +276,7 @@ class ARROW_EXPORT BasicDecimal32 : public SmallBasicDecimal<int32_t> {
   using ValueType = int32_t;
 
   /// \brief Negate the current value (in-place)
-  BasicDecimal32& Negate() {
-    value_ = -value_;
-    return *this;
-  }
+  BasicDecimal32& Negate();
 
   /// \brief Absolute value (in-place)
   BasicDecimal32& Abs() { return *this < 0 ? Negate() : *this; }
@@ -429,10 +426,7 @@ class ARROW_EXPORT BasicDecimal64 : public SmallBasicDecimal<int64_t> {
   using ValueType = int64_t;
 
   /// \brief Negate the current value (in-place)
-  BasicDecimal64& Negate() {
-    value_ = -value_;
-    return *this;
-  }
+  BasicDecimal64& Negate();
 
   /// \brief Absolute value (in-place)
   BasicDecimal64& Abs() { return *this < 0 ? Negate() : *this; }

cpp/src/arrow/util/decimal_test.cc

@@ -219,6 +219,28 @@ class DecimalFromStringTest : public ::testing::Test {
   }
 };
 
+TEST(Decimal32Test, TestIntMinNegate) {
+  Decimal32 d(INT32_MIN);
+  auto neg = d.Negate();
+  ASSERT_EQ(neg, Decimal32(arrow::internal::SafeSignedNegate(INT32_MIN)));
+}
+
+TEST(Decimal32Test, TestIntMinFitsPrecision) {
+  Decimal32 d(INT32_MIN);
+  ASSERT_FALSE(d.FitsInPrecision(9));
+}
+
+TEST(Decimal64Test, TestIntMinNegate) {
+  Decimal64 d(INT64_MIN);
+  auto neg = d.Negate();
+  ASSERT_EQ(neg, Decimal64(arrow::internal::SafeSignedNegate(INT64_MIN)));
+}
+
+TEST(Decimal64Test, TestIntMinFitsPrecision) {
+  Decimal64 d(INT64_MIN);
+  ASSERT_FALSE(d.FitsInPrecision(18));
+}
+
 TYPED_TEST_SUITE(DecimalFromStringTest, DecimalTypes);
 
 TYPED_TEST(DecimalFromStringTest, Basics) { this->TestBasics(); }