[libc-commits] [PATCH] D148152: [libc] Fix strtod exponent overflow bug

[Michael Jones via Phabricator via libc-commits]

michaelrj created this revision.
michaelrj added reviewers: sivachandra, lntue.
Herald added subscribers: libc-commits, ecnelises, tschuett.
Herald added projects: libc-project, All.
michaelrj requested review of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148152

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


Index: libc/test/src/stdlib/strtod_test.cpp
===================================================================
--- libc/test/src/stdlib/strtod_test.cpp
+++ libc/test/src/stdlib/strtod_test.cpp
@@ -197,6 +197,35 @@
   // 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
Index: libc/src/__support/str_to_float.h
===================================================================
--- libc/src/__support/str_to_float.h
+++ libc/src/__support/str_to_float.h
@@ -943,13 +943,12 @@
 
       // 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);
       }
     }
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148152.512914.patch
Type: text/x-patch
Size: 3689 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libc-commits/attachments/20230412/fd3c97c4/attachment-0001.bin>