[libc-commits] [libc] 3d95323 - [libc] Fix strtod exponent overflow bug

[Michael Jones via libc-commits]

Author: Michael Jones
Date: 2023-04-12T15:27:03-07:00
New Revision: 3d953234fede3cb0532c876fd4ba9ea4d89fbd7c

URL: https://github.com/llvm/llvm-project/commit/3d953234fede3cb0532c876fd4ba9ea4d89fbd7c
DIFF: https://github.com/llvm/llvm-project/commit/3d953234fede3cb0532c876fd4ba9ea4d89fbd7c.diff

LOG: [libc] Fix strtod exponent overflow bug

String to float has a condition to prevent overflowing the exponent with
the E notation. To do this it checks if adding that exponent to the
exponent found by parsing the number is greater than the maximum
exponent for the given size of float. The if statements had a gap on
exactly the maximum exponent value that caused it to be treated as the
minimum exponent value. This patch fixes those conditions.

Reviewed By: lntue

Differential Revision: https://reviews.llvm.org/D148152

Added: 
    

Modified: 
    libc/src/__support/str_to_float.h
    libc/test/src/stdlib/strtod_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/src/__support/str_to_float.h b/libc/src/__support/str_to_float.h
index 9ba0602eb5767..6c298d47ff87f 100644
--- a/libc/src/__support/str_to_float.h
+++ b/libc/src/__support/str_to_float.h
@@ -943,13 +943,12 @@ decimal_string_to_float(const char *__restrict src, const char DECIMAL_POINT,
 
       // If the result is in the valid range, then we use it. The valid range is
       // also within the int32 range, so this prevents overflow issues.
-      if (temp_exponent < fputil::FPBits<T>::MAX_EXPONENT &&
-          temp_exponent > -fputil::FPBits<T>::MAX_EXPONENT) {
-        exponent = static_cast<int32_t>(temp_exponent);
-      } else if (temp_exponent > fputil::FPBits<T>::MAX_EXPONENT) {
+      if (temp_exponent > fputil::FPBits<T>::MAX_EXPONENT) {
         exponent = fputil::FPBits<T>::MAX_EXPONENT;
-      } else {
+      } else if (temp_exponent < -fputil::FPBits<T>::MAX_EXPONENT) {
         exponent = -fputil::FPBits<T>::MAX_EXPONENT;
+      } else {
+        exponent = static_cast<int32_t>(temp_exponent);
       }
     }
   }

diff  --git a/libc/test/src/stdlib/strtod_test.cpp b/libc/test/src/stdlib/strtod_test.cpp
index 4eb1f49062dd0..3b1458a3a4a68 100644
--- a/libc/test/src/stdlib/strtod_test.cpp
+++ b/libc/test/src/stdlib/strtod_test.cpp
@@ -197,6 +197,35 @@ TEST_F(LlvmLibcStrToDTest, FuzzFailures) {
   // this bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30220
   run_test("0x30000002222225p-1077", 22, uint64_t(0x0006000000444445), ERANGE);
 
+  // This value triggered a bug by having an exponent exactly equal to the
+  // maximum. The overflow checks would accept a value less than the max value
+  // as valid and greater than the max value as invalid (and set it to the max),
+  // but an exponent of exactly max value hit the else condition which is
+  // intended for underflow and set the exponent to the min exponent.
+  run_test(
+      "184774460000000000000000000000000000052300000000000000000000000000000000"
+      "000000000000000000000000000000000000000000000000000000000000000000000000"
+      "000000000000000000000000000000000000009351662015430037656316837118788423"
+      "887774460000000000004300376000000000000000000000000000000000000000000000"
+      "000000000000000000000000000000000000000000000000000000000000000000000000"
+      "000000000000000000000000000000000000000000000000000000000000000000000000"
+      "000000000000000000000000000000000000000000000052385811247017194600000000"
+      "000000000171946000000000000000000700460000000000000000000000001000000000"
+      "000000000000000000000000000000000000000000000000000000000000000000000000"
+      "000000000000000000000000000000000000000000000000000000000000000000000000"
+      "000000000000000000000000000000000000000000000000000000020000000000000000"
+      "000000000000563168371187884238877744600000000000000000000000000000523858"
+      "112470171946000000000000000001719460000000000000000007004600000000000000"
+      "000000000000000000000000000000000000000000000000000000000000000000000000"
+      "000000000000000000020000000000000000000000000000000000000000000000000000"
+      "000000000000000000000000000000000000000000000000000000000000000000000000"
+      "000000000000000000000000000000000000000000000000000000000000000000000000"
+      "000000000000000000000000000000000000000000000005238581124701719460000000"
+      "000000000017194600000000000000000070046000000000000000000000000000000000"
+      "000000000000000000000000000000000000000000000000000000000000000000000000"
+      "200000000000000000E608",
+      1462, uint64_t(0x7ff0000000000000), ERANGE);
+
   // This bug was in the handling of very large exponents in the exponent
   // marker. Previously anything greater than 10,000 would be set to 10,000.
   // This caused incorrect behavior if there were more than 10,000 '0's in the