[libc-commits] [PATCH] D113790: [libc] fix strtof/d/ld NaN parsing

[Michael Jones via Phabricator via libc-commits]

michaelrj updated this revision to Diff 387322.
michaelrj marked 2 inline comments as done.
michaelrj added a comment.

clarify the code


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113790/new/

https://reviews.llvm.org/D113790

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


Index: libc/test/src/stdlib/strtof_test.cpp
===================================================================
--- libc/test/src/stdlib/strtof_test.cpp
+++ libc/test/src/stdlib/strtof_test.cpp
@@ -154,10 +154,39 @@
   runTest("-iNfInItY", 9, 0xff800000);
 }
 
-TEST_F(LlvmLibcStrToFTest, NaNTests) {
+TEST_F(LlvmLibcStrToFTest, SimpleNaNTests) {
   runTest("NaN", 3, 0x7fc00000);
   runTest("-nAn", 4, 0xffc00000);
+}
+
+// These NaNs are of the form `NaN(n-character-sequence)` where the
+// n-character-sequence is 0 or more letters or numbers. If there is anything
+// other than a letter or a number, then the valid number is just `NaN`. If
+// the sequence is valid, then the interpretation of them is implementation
+// defined, in this case it's passed to strtoll with an automatic base, and
+// the result is put into the mantissa if it takes up the whole width of the
+// parentheses.
+TEST_F(LlvmLibcStrToFTest, NaNWithParenthesesEmptyTest) {
   runTest("NaN()", 5, 0x7fc00000);
+}
+
+TEST_F(LlvmLibcStrToFTest, NaNWithParenthesesValidNumberTests) {
   runTest("NaN(1234)", 9, 0x7fc004d2);
+  runTest("NaN(0x1234)", 11, 0x7fc01234);
+  runTest("NaN(01234)", 10, 0x7fc0029c);
+}
+
+TEST_F(LlvmLibcStrToFTest, NaNWithParenthesesInvalidSequenceTests) {
   runTest("NaN( 1234)", 3, 0x7fc00000);
+  runTest("NaN(-1234)", 3, 0x7fc00000);
+  runTest("NaN(asd&f)", 3, 0x7fc00000);
+  runTest("NaN(123 )", 3, 0x7fc00000);
+  runTest("NaN(123+asdf)", 3, 0x7fc00000);
+  runTest("NaN(123", 3, 0x7fc00000);
+}
+
+TEST_F(LlvmLibcStrToFTest, NaNWithParenthesesValidSequenceInvalidNumberTests) {
+  runTest("NaN(1a)", 7, 0x7fc00000);
+  runTest("NaN(asdf)", 9, 0x7fc00000);
+  runTest("NaN(1A1)", 8, 0x7fc00000);
 }
Index: libc/src/__support/str_to_float.h
===================================================================
--- libc/src/__support/str_to_float.h
+++ libc/src/__support/str_to_float.h
@@ -782,16 +782,30 @@
       seenDigit = true;
       src += 3;
       BitsType NaNMantissa = 0;
+      // this handles the case of `NaN(n-character-sequence)`, where the
+      // n-character-sequence is made of 0 or more letters and numbers in any
+      // order.
       if (*src == '(') {
-        char *tempSrc = 0;
-        if (isdigit(*(src + 1)) || *(src + 1) == ')') {
-          NaNMantissa = strtointeger<BitsType>(src + 1, &tempSrc, 0);
-          if (*tempSrc != ')') {
-            NaNMantissa = 0;
-          } else {
-            src = tempSrc + 1;
+        const char *leftParen = src;
+        ++src;
+        while (isalnum(*src))
+          ++src;
+        if (*src == ')') {
+          ++src;
+          char *tempSrc = 0;
+          if (isdigit(*(leftParen + 1))) {
+            // This is to prevent errors when BitsType is larger than 64 bits,
+            // since strtointeger only supports up to 64 bits. This is actually
+            // more than is required by the specification, which says for the
+            // input type "NAN(n-char-sequence)" that "the meaning of
+            // the n-char sequence is implementation-defined."
+            NaNMantissa = static_cast<BitsType>(
+                strtointeger<uint64_t>(leftParen + 1, &tempSrc, 0));
+            if (*tempSrc != ')')
+              NaNMantissa = 0;
           }
-        }
+        } else
+          src = leftParen;
       }
       NaNMantissa |= fputil::FloatProperties<T>::quietNaNMask;
       if (result.getSign()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D113790.387322.patch
Type: text/x-patch
Size: 3418 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libc-commits/attachments/20211115/7503f3a2/attachment.bin>