Skip to content

Conversation

@michaelrj-google
Copy link
Contributor

Found in testing against abseil. Two bugs were found:

  1. SHIFT_AMOUNT in float_converter would sometimes be
    negative causing an underflow when passed as the amount to left shift
    by for BigInt.
  2. is_lowest_block had an off-by-one because it was adding 1 to the
    block index.

Both are fixed and there are new tests to catch regressions.

Found in testing against abseil. Two bugs were found:

1) SHIFT_AMOUNT in float_converter<long double> would sometimes be
   negative causing an underflow when passed as the amount to left shift
   by for BigInt.
2) is_lowest_block had an off-by-one because it was adding 1 to the
   block index.

Both are fixed and there are new tests to catch regressions.
@llvmbot llvmbot added the libc label Nov 5, 2025
@llvmbot
Copy link
Member

llvmbot commented Nov 5, 2025

@llvm/pr-subscribers-libc

Author: Michael Jones (michaelrj-google)

Changes

Found in testing against abseil. Two bugs were found:

  1. SHIFT_AMOUNT in float_converter<long double> would sometimes be
    negative causing an underflow when passed as the amount to left shift
    by for BigInt.
  2. is_lowest_block had an off-by-one because it was adding 1 to the
    block index.

Both are fixed and there are new tests to catch regressions.


Full diff: https://github.com/llvm/llvm-project/pull/166474.diff

2 Files Affected:

  • (modified) libc/src/__support/float_to_string.h (+6-2)
  • (modified) libc/test/src/stdio/sprintf_test.cpp (+12)
diff --git a/libc/src/__support/float_to_string.h b/libc/src/__support/float_to_string.h
index cab146a5b8698..9115ed2856881 100644
--- a/libc/src/__support/float_to_string.h
+++ b/libc/src/__support/float_to_string.h
@@ -700,7 +700,11 @@ template <> class FloatToString<long double> {
 
       const int SHIFT_AMOUNT = FLOAT_AS_INT_WIDTH + exponent;
       static_assert(EXTRA_INT_WIDTH >= sizeof(long double) * 8);
-      float_as_fixed <<= SHIFT_AMOUNT;
+      if (SHIFT_AMOUNT > 0) {
+        float_as_fixed <<= SHIFT_AMOUNT;
+      } else {
+        float_as_fixed >>= -SHIFT_AMOUNT;
+      }
 
       // If there are still digits above the decimal point, handle those.
       if (cpp::countl_zero(float_as_fixed) <
@@ -769,7 +773,7 @@ template <> class FloatToString<long double> {
     // The decimal representation of 2**(-i) will have exactly i digits after
     // the decimal point.
     const int num_requested_digits =
-        static_cast<int>((negative_block_index + 1) * BLOCK_SIZE);
+        static_cast<int>(negative_block_index * BLOCK_SIZE);
 
     return num_requested_digits > -exponent;
   }
diff --git a/libc/test/src/stdio/sprintf_test.cpp b/libc/test/src/stdio/sprintf_test.cpp
index f1b545ba546f9..42fdd59cf4d9c 100644
--- a/libc/test/src/stdio/sprintf_test.cpp
+++ b/libc/test/src/stdio/sprintf_test.cpp
@@ -1537,6 +1537,14 @@ TEST(LlvmLibcSPrintfTest, FloatDecimalLongDoubleConv) {
 #if defined(LIBC_TYPES_LONG_DOUBLE_IS_X86_FLOAT80)
 
 #ifndef LIBC_COPT_FLOAT_TO_STR_REDUCED_PRECISION
+  written = LIBC_NAMESPACE::sprintf(
+      buff, "%.75Lf",
+      0.0833333333333333333355920878593448009041821933351457118988037109375L);
+  ASSERT_STREQ_LEN(written, buff,
+                   "0."
+                   "08333333333333333333559208785934480090418219333514571189880"
+                   "3710937500000000");
+
   written = LIBC_NAMESPACE::sprintf(buff, "%Lf", 1e100L);
   ASSERT_STREQ_LEN(written, buff,
                    "99999999999999999996693535322073426194986990198284960792713"
@@ -2976,6 +2984,10 @@ TEST(LlvmLibcSPrintfTest, FloatAutoLongDoubleConv) {
   written = LIBC_NAMESPACE::sprintf(buff, "%Lg", 0xf.fffffffffffffffp+16380L);
   ASSERT_STREQ_LEN(written, buff, "1.18973e+4932");
 
+  // Minimum normal
+  written = LIBC_NAMESPACE::sprintf(buff, "%Lg", 3.36210314311209350626E-4932L);
+  ASSERT_STREQ_LEN(written, buff, "3.3621e-4932");
+
   written = LIBC_NAMESPACE::sprintf(buff, "%Lg", 0xa.aaaaaaaaaaaaaabp-7L);
   ASSERT_STREQ_LEN(written, buff, "0.0833333");
 

@michaelrj-google michaelrj-google merged commit 4209e41 into llvm:main Nov 5, 2025
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants