[libc] Use MPFR for strtofloat fuzzing

[michaelrj]

Summary

The previous string to float tests didn't check correctness, but due to
the atof differential test proving unreliable the strtofloat fuzz test
has been changed to use MPFR for correctness checking. Some minor bugs
have been found and fixed as well.

[lntue]

libc/src/__support/str_to_float.h

276

Does replacing UInt128(1) << 64 with UInt128({0, 1}) work?

282

Does replacing static_cast<UInt128>(low64(approx_middle)) << 64 with UInt128({0, low64(approx_middle)}) work?

[michaelrj]

libc/src/__support/str_to_float.h

276

this UInt128 isn't necessarily the UInt class, it's a raw uint128_t when that's available, so we can't do any sort of constructor on it.

[michaelrj]

changed how MPFR is included to support downstream configs in the same way as MPFRUtils

libc/cmake/modules/LLVMLibCTestRules.cmake

 #      <target name>
 #      SRCS  <list of .cpp files for the test>
 #      HDRS  <list of .h files for the test>
 #      DEPENDS <list of dependencies>
 #    )
 function(add_libc_fuzzer target_name)
   cmake_parse_arguments(
     "LIBC_FUZZER"
-    "" # No optional arguments
+    "NEED_MPFR" # Optional arguments
     "" # Single value arguments
     "SRCS;HDRS;DEPENDS;COMPILE_OPTIONS" # Multi-value arguments
     ${ARGN}
   )
   if(NOT LIBC_FUZZER_SRCS)
     message(FATAL_ERROR "'add_libc_fuzzer' target requires a SRCS list of .cpp "
                         "files.")
   endif()
   if(NOT LIBC_FUZZER_DEPENDS)
     message(FATAL_ERROR "'add_libc_fuzzer' target requires a DEPENDS list of "
                         "'add_entrypoint_object' targets.")
   endif()
 
+  list(APPEND LIBC_FUZZER_LINK_LIBRARIES "")
+  if(LIBC_FUZZER_NEED_MPFR)
+    if(NOT LIBC_TESTS_CAN_USE_MPFR)
+      message(VERBOSE "Fuzz test ${name} will be skipped as MPFR library is not available.")
+      return()
+    endif()
+    list(APPEND LIBC_FUZZER_LINK_LIBRARIES mpfr gmp)
+  endif()
+
+
   get_fq_target_name(${target_name} fq_target_name)
   get_fq_deps_list(fq_deps_list ${LIBC_FUZZER_DEPENDS})
   get_object_files_for_test(
       link_object_files skipped_entrypoints_list ${fq_deps_list})
   if(skipped_entrypoints_list)
     if(LIBC_CMAKE_VERBOSE_LOGGING)
       set(msg "Skipping fuzzer target ${fq_target_name} as it has missing deps: "
               "${skipped_entrypoints_list}.")
function(add_libc_fuzzer target_name)
   target_include_directories(
     ${fq_target_name}
     PRIVATE
       ${LIBC_SOURCE_DIR}
       ${LIBC_BUILD_DIR}
       ${LIBC_BUILD_DIR}/include
   )
 
-  target_link_libraries(${fq_target_name} PRIVATE ${link_object_files})
+  target_link_libraries(${fq_target_name} PRIVATE 
+    ${link_object_files} 
+    ${LIBC_FUZZER_LINK_LIBRARIES}
+  )
 
   set_target_properties(${fq_target_name}
       PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
 
   add_dependencies(
     ${fq_target_name}
     ${fq_deps_list}
   )

libc/fuzzing/stdlib/CMakeLists.txt

 add_libc_fuzzer(
   qsort_fuzz
   SRCS
     qsort_fuzz.cpp
   DEPENDS
     libc.src.stdlib.qsort
 )
 
 add_libc_fuzzer(
   atof_differential_fuzz
   SRCS
     atof_differential_fuzz.cpp
   HDRS
     StringParserOutputDiff.h
   DEPENDS
     libc.src.stdlib.atof
-  COMPILE_OPTIONS
-    -DLLVM_LIBC_ATOF_DIF_FUZZ_SKIP_GLIBC_HEX_SUBNORMAL_ERR
 )
 
 add_libc_fuzzer(
   strtofloat_fuzz
+  NEED_MPFR
   SRCS
     strtofloat_fuzz.cpp
   DEPENDS
     libc.src.stdlib.atof
     libc.src.stdlib.strtof
     libc.src.stdlib.strtod
     libc.src.stdlib.strtold
 )

libc/fuzzing/stdlib/atof_differential_fuzz.cpp

 //===----------------------------------------------------------------------===//
 #include "src/stdlib/atof.h"
 #include <stddef.h>
 #include <stdint.h>
 #include <stdlib.h>
 
 #include "fuzzing/stdlib/StringParserOutputDiff.h"
 
-// TODO: Remove this once glibc fixes hex subnormal rounding. See
-// https://sourceware.org/bugzilla/show_bug.cgi?id=30220
-#ifdef LLVM_LIBC_ATOF_DIF_FUZZ_SKIP_GLIBC_HEX_SUBNORMAL_ERR
-#include <ctype.h>
-constexpr double MIN_NORMAL = 0x1p-1022;
-
-bool has_hex_prefix(const uint8_t *str) {
-  size_t index = 0;
-
-  // Skip over leading whitespace
-  while (isspace(str[index])) {
-    ++index;
-  }
-  // Skip over sign
-  if (str[index] == '-' || str[index] == '+') {
-    ++index;
-  }
-  return str[index] == '0' && (tolower(str[index + 1])) == 'x';
-}
-
-bool should_be_skipped(const uint8_t *str) {
-  double init_result = ::atof(reinterpret_cast<const char *>(str));
-  if (init_result < 0) {
-    init_result = -init_result;
-  }
-  if (init_result < MIN_NORMAL && init_result != 0) {
-    return has_hex_prefix(str);
-  }
-  return false;
-}
-#else
-bool should_be_skipped(const uint8_t *) { return false; }
-#endif // LLVM_LIBC_ATOF_DIF_FUZZ_SKIP_GLIBC_HEX_SUBNORMAL_ERR
-
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
   uint8_t *container = new uint8_t[size + 1];
   if (!container)
     __builtin_trap();
   size_t i;
 
   for (i = 0; i < size; ++i)
     container[i] = data[i];
   container[size] = '\0'; // Add null terminator to container.
 
-  if (!should_be_skipped(container)) {
-    StringParserOutputDiff<double>(&__llvm_libc::atof, &::atof, container,
-                                   size);
-  }
+  StringParserOutputDiff<double>(&__llvm_libc::atof, &::atof, container, size);
   delete[] container;
   return 0;
 }

libc/fuzzing/stdlib/strtofloat_fuzz.cpp

 //===-- strtofloat_fuzz.cpp -----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// Fuzzing test for llvm-libc atof implementation.
 ///
 //===----------------------------------------------------------------------===//
 #include "src/stdlib/atof.h"
 #include "src/stdlib/strtod.h"
 #include "src/stdlib/strtof.h"
 #include "src/stdlib/strtold.h"
+
 #include <math.h>
 #include <stddef.h>
 #include <stdint.h>
 
+#include "utils/MPFRWrapper/mpfr_inc.h"
+
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
   uint8_t *container = new uint8_t[size + 1];
   if (!container)
     __builtin_trap();
   size_t i;
 
-  for (i = 0; i < size; ++i)
+  for (i = 0; i < size; ++i) {
+    // MPFR's strtofr uses "@" as a base-independent exponent symbol
+    if (data[i] != '@')
       container[i] = data[i];
+    else {
+      container[i] = '#';
+    }
+  }
   container[size] = '\0'; // Add null terminator to container.
 
   const char *str_ptr = reinterpret_cast<const char *>(container);
 
   char *out_ptr = nullptr;
 
-  // This fuzzer only checks that the algorithms didn't read beyond the end of
-  // the string in container. Combined with sanitizers, this will check that the
-  // code is not reading memory beyond what's expected. This test does not
-  // effectively check the correctness of the result.
+  mpfr_t result;
+  mpfr_init2(result, 256);
+  mpfr_t bin_result;
+  mpfr_init2(bin_result, 256);
+  mpfr_strtofr(result, str_ptr, &out_ptr, 0 /* base */, MPFR_RNDN);
+  ptrdiff_t result_strlen = out_ptr - str_ptr;
+  mpfr_strtofr(bin_result, str_ptr, &out_ptr, 2 /* base */, MPFR_RNDN);
+  ptrdiff_t bin_result_strlen = out_ptr - str_ptr;
+
+  long double bin_result_ld = mpfr_get_ld(bin_result, MPFR_RNDN);
+  long double result_ld = mpfr_get_ld(result, MPFR_RNDN);
+
+  // This detects if mpfr's strtofr selected a base of 2, which libc does not
+  // support. If a base 2 decoding is detected, it is replaced by a base 10
+  // decoding.
+  if ((bin_result_ld != 0.0 || bin_result_strlen == result_strlen) &&
+      bin_result_ld == result_ld) {
+    mpfr_strtofr(result, str_ptr, &out_ptr, 10 /* base */, MPFR_RNDN);
+    result_strlen = out_ptr - str_ptr;
+  }
+
   auto volatile atof_output = __llvm_libc::atof(str_ptr);
   auto volatile strtof_output = __llvm_libc::strtof(str_ptr, &out_ptr);
-  if (str_ptr + size < out_ptr)
+  ptrdiff_t strtof_strlen = out_ptr - str_ptr;
+  if (result_strlen != strtof_strlen)
     __builtin_trap();
   auto volatile strtod_output = __llvm_libc::strtod(str_ptr, &out_ptr);
-  if (str_ptr + size < out_ptr)
+  ptrdiff_t strtod_strlen = out_ptr - str_ptr;
+  if (result_strlen != strtod_strlen)
     __builtin_trap();
   auto volatile strtold_output = __llvm_libc::strtold(str_ptr, &out_ptr);
-  if (str_ptr + size < out_ptr)
+  ptrdiff_t strtold_strlen = out_ptr - str_ptr;
+  if (result_strlen != strtold_strlen)
     __builtin_trap();
 
-  // If any of the outputs are NaN
-  if (isnan(atof_output) || isnan(strtof_output) || isnan(strtod_output) ||
-      isnan(strtold_output)) {
-    // Then all the outputs should be NaN.
-    // This is a trivial check meant to silence the "unused variable" warnings.
-    if (!isnan(atof_output) || !isnan(strtof_output) || !isnan(strtod_output) ||
-        !isnan(strtold_output)) {
+  // If any result is NaN, all of them should be NaN. We can't use the usual
+  // comparisons because NaN != NaN.
+  if (isnan(result_ld)) {
+    if (!(isnan(atof_output) && isnan(strtof_output) && isnan(strtod_output) &&
+          isnan(strtold_output)))
+      __builtin_trap();
+  } else {
+    if (mpfr_get_d(result, MPFR_RNDN) != atof_output)
+      __builtin_trap();
+    if (mpfr_get_flt(result, MPFR_RNDN) != strtof_output)
+      __builtin_trap();
+    if (mpfr_get_d(result, MPFR_RNDN) != strtod_output)
+      __builtin_trap();
+    if (mpfr_get_ld(result, MPFR_RNDN) != strtold_output)
       __builtin_trap();
-    }
   }
 
+  mpfr_clear(result);
+  mpfr_clear(bin_result);
   delete[] container;
   return 0;
 }

libc/src/__support/str_to_float.h

eisel_lemire<long double>(ExpandedFloat<long double> init_num,
 
   // Since the input mantissa is more than 64 bits, we have to multiply with the
   // full 128 bits of the power of ten to get an approximation with the same
   // number of significant bits. This means that we only get the one
   // approximation, and that approximation is 256 bits long.
   UInt128 approx_upper = static_cast<UInt128>(high64(mantissa)) *
                          static_cast<UInt128>(power_of_ten[1]);
 
-  UInt128 approx_middle = static_cast<UInt128>(high64(mantissa)) *
-                              static_cast<UInt128>(power_of_ten[0]) +
-                          static_cast<UInt128>(low64(mantissa)) *
-                              static_cast<UInt128>(power_of_ten[1]);
+  UInt128 approx_middle_a = static_cast<UInt128>(high64(mantissa)) *
+                            static_cast<UInt128>(power_of_ten[0]);
+  UInt128 approx_middle_b = static_cast<UInt128>(low64(mantissa)) *
 
+  UInt128 approx_middle = approx_middle_a + approx_middle_b;
+
+  // Handle overflow in the middle
+  approx_upper += (approx_middle < approx_middle_a) ? UInt128(1) << 64 : 0;
+
   UInt128 approx_lower = static_cast<UInt128>(low64(mantissa)) *
                          static_cast<UInt128>(power_of_ten[0]);
 
   UInt128 final_approx_lower =
       approx_lower + (static_cast<UInt128>(low64(approx_middle)) << 64);
   UInt128 final_approx_upper = approx_upper + high64(approx_middle) +
                                (final_approx_lower < approx_lower ? 1 : 0);
 
   // The halfway constant is used to check if the bits that will be shifted away
   // intially are all 1. For 80 bit floats this is 128 (bitstype size) - 64
   // (final mantissa size) - 3 (we shift away the last two bits separately for
   // accuracy, and the most significant bit is ignored.) = 61 bits. Similarly,
   // it's 12 bits for 128 bit floats in this case.
while (true) {
     // The character is neither a digit nor a decimal point.
     break;
   }
 
   if (!seen_digit)
     return output;
 
   if (tolower(src[index]) == EXPONENT_MARKER) {
-    if (src[index + 1] == '+' || src[index + 1] == '-' ||
-        isdigit(src[index + 1])) {
+    bool has_sign = false;
+    if (src[index + 1] == '+' || src[index + 1] == '-') {
+      has_sign = true;
+    }
+    if (isdigit(src[index + 1 + static_cast<size_t>(has_sign)])) {
       ++index;
       auto result = strtointeger<int32_t>(src + index, 10);
       if (result.has_error())
         output.error = result.error;
       int32_t add_to_exponent = result.value;
       index += result.parsed_len;
 
       // Here we do this operation as int64 to avoid overflow.
hexadecimal_string_to_float(const char *__restrict src,
 
   if (!seen_digit)
     return output;
 
   // Convert the exponent from having a base of 16 to having a base of 2.
   exponent *= 4;
 
   if (tolower(src[index]) == EXPONENT_MARKER) {
-    if (src[index + 1] == '+' || src[index + 1] == '-' ||
-        isdigit(src[index + 1])) {
+    bool has_sign = false;
+    if (src[index + 1] == '+' || src[index + 1] == '-') {
+      has_sign = true;
+    }
+    if (isdigit(src[index + 1 + static_cast<size_t>(has_sign)])) {
       ++index;
       auto result = strtointeger<int32_t>(src + index, 10);
       if (result.has_error())
         output.error = result.error;
 
       int32_t add_to_exponent = result.value;
       index += result.parsed_len;
 

libc/test/src/stdlib/strtod_test.cpp

run_test(
       "000000000017194600000000000000000070046000000000000000000000000000000000"
       "000000000000000000000000000000000000000000000000000000000000000000000000"
       "200000000000000000E608",
       1462, uint64_t(0x7ff0000000000000), ERANGE);
 
   // Same as above but for hex.
   run_test("0x0164810157p2047", 17, uint64_t(0x7ff0000000000000), ERANGE);
 
+  // This test ensures that only the correct number of characters is accepted.
+  // An exponent symbol followed by a sign isn't a valid exponent.
+  run_test("2e+", 1, uint64_t(0x4000000000000000));
+  run_test("0x2p+", 3, uint64_t(0x4000000000000000));
+
   // 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
   // input number, and then a correspondingly large exponent. This test case has
   // 24,744 zeroes.
   run_test(
       "0x."
       "000000000000000000000000000000000000000000000000000000000000000000000000"

libc/test/src/stdlib/strtold_test.cpp

run_test("2.16656806400000023841857910156251e9", 36,
                             UInt128(0x800000000000018)));
   run_test("27949676547093071875", 20,
            SELECT_CONST(uint64_t(0x43f83e132bc608c9),
                         (UInt128(0x403fc1f099) << 40) + UInt128(0x5e30464402),
                         (UInt128(0x403f83e132bc608c) << 64) +
                             UInt128(0x8803000000000000)));
 }
 
+TEST_F(LlvmLibcStrToLDTest, Float80SpecificFailures) {
+  run_test("7777777777777777777777777777777777777777777777777777777777777777777"
+           "777777777777777777777777777777777",
+           100,
+           SELECT_CONST(uint64_t(0x54ac729b8fcaf734),
+                        (UInt128(0x414ae394dc) << 40) + UInt128(0x7e57b9a0c2),
+                        (UInt128(0x414ac729b8fcaf73) << 64) +
+                            UInt128(0x4184a3d793224129)));
+}
+
 TEST_F(LlvmLibcStrToLDTest, MaxSizeNumbers) {
   run_test("1.1897314953572317650e4932", 26,
            SELECT_CONST(uint64_t(0x7FF0000000000000),
                         (UInt128(0x7ffeffffff) << 40) + UInt128(0xffffffffff),
                         (UInt128(0x7ffeffffffffffff) << 64) +
                             UInt128(0xfffd57322e3f8675)),
            SELECT_CONST(ERANGE, 0, 0));
   run_test("1.18973149535723176508e4932", 27,

libc/utils/MPFRWrapper/CMakeLists.txt

 if(LIBC_TESTS_CAN_USE_MPFR)
   add_library(libcMPFRWrapper
     MPFRUtils.cpp
     MPFRUtils.h
+    mpfr_inc.h
   )
   add_compile_options(
     -O3
   )
   add_dependencies(
     libcMPFRWrapper 
     libc.src.__support.CPP.string_view 
     libc.src.__support.CPP.type_traits 

libc/utils/MPFRWrapper/MPFRUtils.cpp

 #include "src/__support/FPUtil/PlatformDefs.h"
 #include "test/UnitTest/FPMatcher.h"
 
 #include <cmath>
 #include <fenv.h>
 #include <memory>
 #include <stdint.h>
 
-#ifdef CUSTOM_MPFR_INCLUDER
-// Some downstream repos are monoliths carrying MPFR sources in their third
-// party directory. In such repos, including the MPFR header as
-// `#include <mpfr.h>` is either disallowed or not possible. If that is the
-// case, a file named `CustomMPFRIncluder.h` should be added through which the
-// MPFR header can be included in manner allowed in that repo.
-#include "CustomMPFRIncluder.h"
-#else
-#include <mpfr.h>
-#endif
+#include "mpfr_inc.h"
 
 template <typename T> using FPBits = __llvm_libc::fputil::FPBits<T>;
 
 namespace __llvm_libc {
 namespace testing {
 namespace mpfr {
 
 template <typename T> struct Precision;

libc/utils/MPFRWrapper/mpfr_inc.h

+//===-- MPFRUtils.h ---------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_UTILS_MPFRWRAPPER_MPFR_INC_H
+#define LLVM_LIBC_UTILS_MPFRWRAPPER_MPFR_INC_H
+
+#ifdef CUSTOM_MPFR_INCLUDER
+// Some downstream repos are monoliths carrying MPFR sources in their third
+// party directory. In such repos, including the MPFR header as
+// `#include <mpfr.h>` is either disallowed or not possible. If that is the
+// case, a file named `CustomMPFRIncluder.h` should be added through which the
+// MPFR header can be included in manner allowed in that repo.
+#include "CustomMPFRIncluder.h"
+#else
+#include <mpfr.h>
+#endif
+
+#endif // LLVM_LIBC_UTILS_MPFRWRAPPER_MPFR_INC_H

utils/bazel/llvm-project-overlay/libc/utils/MPFRWrapper/BUILD.bazel

 # This file is licensed under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 # A wrapper library over MPFR for use with LLVM libc math unittests.
 
 package(default_visibility = ["//visibility:public"])
 
 licenses(["notice"])
 
 cc_library(
     name = "mpfr_impl",
+    hdrs = ["mpfr_inc.h"],
     target_compatible_with = select({
         "//conditions:default": [],
         "//libc:mpfr_disable": ["@platforms//:incompatible"],
     }),
     deps = select(
         {
             "//conditions:default": [],
             "//libc:mpfr_external": ["@mpfr//:mpfr_external"],