[libc] fix strtof/d/ld NaN parsing

[michaelrj]

Summary

Fix the fact that previously strtof/d/ld would only accept a NaN as
having parentheses if the thing in the parentheses was a valid number,
now it will accept any combination of letters and numbers, but will only
put valid numbers in the mantissa.

[sivachandra]

There is a wide-spread violation of "no braces for single statement blocks."

libc/src/__support/str_to_float.h

790

Add a var named numberStart to avoid repeated parentStart + 1?

libc/test/src/stdlib/strtof_test.cpp

192

Add a comment explaining whats going on here and in the above line?

[michaelrj]

clarify the code

libc/src/__support/str_to_float.h

790

While that is possible, I'm not sure if it would help. Currently parenStart + 1 is only used twice, so I don't think defining a new variable would be useful. Instead I renamed the variable to be leftParen, so that it's more clear that it points to the left parenthesis.

libc/test/src/stdlib/strtof_test.cpp

192

Should be more clear now.

libc/src/__support/str_to_float.h

if (seenDigit) {
       result.setMantissa(outputMantissa);
       result.setUnbiasedExponent(outputExponent);
     }
   } else if ((*src | 32) == 'n') { // NaN
     if ((src[1] | 32) == NAN_STRING[1] && (src[2] | 32) == NAN_STRING[2]) {
       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 == '(') {
+        const char *leftParen = src;
+        ++src;
+        while (isalnum(*src))
+          ++src;
+        if (*src == ')') {
+          ++src;
           char *tempSrc = 0;
-        if (isdigit(*(src + 1)) || *(src + 1) == ')') {
-          NaNMantissa = strtointeger<BitsType>(src + 1, &tempSrc, 0);
-          if (*tempSrc != ')') {
+          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 = tempSrc + 1;
-          }
           }
+        } else
+          src = leftParen;
       }
       NaNMantissa |= fputil::FloatProperties<T>::quietNaNMask;
       if (result.getSign()) {
         result = fputil::FPBits<T>(result.buildNaN(NaNMantissa));
         result.setSign(true);
       } else {
         result.setSign(false);
         result = fputil::FPBits<T>(result.buildNaN(NaNMantissa));

libc/test/src/stdlib/strtof_test.cpp

TEST_F(LlvmLibcStrToFTest, InfTests) {
   runTest("infnity", 3, 0x7f800000);
   runTest("infinit", 3, 0x7f800000);
   runTest("infinfinit", 3, 0x7f800000);
   runTest("innf", 0, 0x0);
   runTest("-inf", 4, 0xff800000);
   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);
 }