[llvm] Signed integer overflow in Constraint Elimination pass issue #133154 (PR #133620)

[via llvm-commits]

=?utf-8?b?6buD5ZyL5bqt?=,=?utf-8?b?6buD5ZyL5bqt?=,=?utf-8?b?6buD5ZyL5bqt?=,
=?utf-8?b?6buD5ZyL5bqt?Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/133620 at github.com>


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-support

Author: 黃國庭 (houngkoungting)

<details>
<summary>Changes</summary>

Adjusted the limit check to use MaxNegative 

---
Full diff: https://github.com/llvm/llvm-project/pull/133620.diff


1 Files Affected:

- (modified) llvm/include/llvm/Support/MathExtras.h (+31-18) 


``````````diff
diff --git a/llvm/include/llvm/Support/MathExtras.h b/llvm/include/llvm/Support/MathExtras.h
index 519fcc8fde5d5..fe6db8fe14019 100644
--- a/llvm/include/llvm/Support/MathExtras.h
+++ b/llvm/include/llvm/Support/MathExtras.h
@@ -760,31 +760,44 @@ std::enable_if_t<std::is_signed_v<T>, T> SubOverflow(T X, T Y, T &Result) {
 /// Multiply two signed integers, computing the two's complement truncated
 /// result, returning true if an overflow occurred.
 template <typename T>
-std::enable_if_t<std::is_signed_v<T>, T> MulOverflow(T X, T Y, T &Result) {
+std::enable_if_t<std::is_signed_v<T>, bool> MulOverflow(T X, T Y, T &Result) {
 #if __has_builtin(__builtin_mul_overflow)
   return __builtin_mul_overflow(X, Y, &Result);
 #else
-  // Perform the unsigned multiplication on absolute values.
   using U = std::make_unsigned_t<T>;
-  const U UX = X < 0 ? (0 - static_cast<U>(X)) : static_cast<U>(X);
-  const U UY = Y < 0 ? (0 - static_cast<U>(Y)) : static_cast<U>(Y);
-  const U UResult = UX * UY;
 
-  // Convert to signed.
-  const bool IsNegative = (X < 0) ^ (Y < 0);
-  Result = IsNegative ? (0 - UResult) : UResult;
-
-  // If any of the args was 0, result is 0 and no overflow occurs.
-  if (UX == 0 || UY == 0)
+  // Handle zero case
+  if (X == 0 || Y == 0) {
+    Result = 0;
     return false;
+  }
 
-  // UX and UY are in [1, 2^n], where n is the number of digits.
-  // Check how the max allowed absolute value (2^n for negative, 2^(n-1) for
-  // positive) divided by an argument compares to the other.
-  if (IsNegative)
-    return UX > (static_cast<U>(std::numeric_limits<T>::max()) + U(1)) / UY;
-  else
-    return UX > (static_cast<U>(std::numeric_limits<T>::max())) / UY;
+  bool IsNegative = (X < 0) ^ (Y < 0);
+
+  // Safely compute absolute values  
+  const U AbsX = X < 0 ? (0 - static_cast<U>(X)) : static_cast<U>(X);
+  const U AbsY = Y < 0 ? (0 - static_cast<U>(Y)) : static_cast<U>(Y);
+
+  // Overflow check before actual multiplication
+  constexpr U MaxPositive = static_cast<U>(std::numeric_limits<T>::max());
+  constexpr U MaxNegative = static_cast<U>(std::numeric_limits<T>::max()) + 1;
+
+  // Safe to multiply
+  U AbsResult = AbsX * AbsY;
+  Result = IsNegative ? static_cast<T>(0-AbsResult) : static_cast<T>(AbsResult);
+  
+  // Handle INT_MIN * -1 overflow case explicitly
+  if ((X == std::numeric_limits<T>::min() && Y == -1) ||
+      (Y == std::numeric_limits<T>::min() && X == -1)) {
+    return true;  // overflow
+  }
+
+  U Limit = IsNegative ? MaxNegative : MaxPositive;
+
+  if (AbsX > Limit / AbsY)
+    return true;
+
+  return false;
 #endif
 }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/133620