Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flang][runtime] Add limit check to MOD/MODULO #80026

Merged
merged 1 commit into from
Jan 31, 2024
Merged

Conversation

klausler
Copy link
Contributor

@klausler klausler commented Jan 30, 2024

When testing the arguments to see whether they are integers, check first that they are within the maximum range of a 64-bit integer; otherwise, a value of larger magnitude will set an invalid operand exception flag.

@llvmbot llvmbot added flang:runtime flang Flang issues not falling into any other category labels Jan 30, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 30, 2024

@llvm/pr-subscribers-flang-runtime

Author: Peter Klausler (klausler)

Changes

When testing the arguments to see whether they are integers, check first that they are within the maximum range of a 64-bit integer; otherwise, a value of larger magnitude will set an overflow exception flag.


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

1 Files Affected:

  • (modified) flang/runtime/numeric.cpp (+14-10)
diff --git a/flang/runtime/numeric.cpp b/flang/runtime/numeric.cpp
index 8e512aff1ea1d..ede00d69f20e2 100644
--- a/flang/runtime/numeric.cpp
+++ b/flang/runtime/numeric.cpp
@@ -145,14 +145,19 @@ inline RT_API_ATTRS T RealMod(
   } else if (std::isinf(p)) {
     return a;
   }
-  if (auto aInt{static_cast<std::int64_t>(a)}; a == aInt) {
-    if (auto pInt{static_cast<std::int64_t>(p)}; p == pInt) {
-      // Fast exact case for integer operands
-      auto mod{aInt - (aInt / pInt) * pInt};
-      if (IS_MODULO && (aInt > 0) != (pInt > 0)) {
-        mod += pInt;
+  T aAbs{std::abs(a)};
+  T pAbs{std::abs(p)};
+  if (aAbs <= static_cast<T>(std::numeric_limits<std::int64_t>::max()) &&
+      pAbs <= static_cast<T>(std::numeric_limits<std::int64_t>::max())) {
+    if (auto aInt{static_cast<std::int64_t>(a)}; a == aInt) {
+      if (auto pInt{static_cast<std::int64_t>(p)}; p == pInt) {
+        // Fast exact case for integer operands
+        auto mod{aInt - (aInt / pInt) * pInt};
+        if (IS_MODULO && (aInt > 0) != (pInt > 0)) {
+          mod += pInt;
+        }
+        return static_cast<T>(mod);
       }
-      return static_cast<T>(mod);
     }
   }
   if constexpr (std::is_same_v<T, float> || std::is_same_v<T, double> ||
@@ -183,9 +188,8 @@ inline RT_API_ATTRS T RealMod(
     // what's left is the desired remainder.  This is basically
     // the same algorithm as arbitrary precision binary long division,
     // discarding the quotient.
-    T tmp{std::abs(a)};
-    T pAbs{std::abs(p)};
-    for (T adj{SetExponent(pAbs, Exponent<int>(tmp))}; tmp >= pAbs; adj /= 2) {
+    T tmp{aAbs};
+    for (T adj{SetExponent(pAbs, Exponent<int>(aAbs))}; tmp >= pAbs; adj /= 2) {
       if (tmp >= adj) {
         tmp -= adj;
         if (tmp == 0) {

When testing the arguments to see whether they are integers,
check first that they are within the maximum range of a
64-bit integer; otherwise, a value of larger magnitude
will set an invalid operand exception flag.
Copy link
Contributor

@vdonaldson vdonaldson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok to me; thanks

Copy link
Contributor

@vzakhari vzakhari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, Peter!

@klausler klausler merged commit dbf547f into llvm:main Jan 31, 2024
4 checks passed
@klausler klausler deleted the bug1517 branch January 31, 2024 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:runtime flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants