[libc-commits] [libc] 63f8922 - [libc] Fix printf g conversion with high precision

[Michael Jones via libc-commits]

Author: Michael Jones
Date: 2023-07-28T14:07:00-07:00
New Revision: 63f8922f49b2fea982856b9e696c6fe220fdfede

URL: https://github.com/llvm/llvm-project/commit/63f8922f49b2fea982856b9e696c6fe220fdfede
DIFF: https://github.com/llvm/llvm-project/commit/63f8922f49b2fea982856b9e696c6fe220fdfede.diff

LOG: [libc] Fix printf g conversion with high precision

The number of trailing zeroes was being calculated incorrectly. It was
assuming that it could add all of the implicit leading zeroes in the
final block, not accounting for the number of digits actually reqested
by the precision.

Reviewed By: lntue

Differential Revision: https://reviews.llvm.org/D156489

Added: 
    

Modified: 
    libc/src/stdio/printf_core/float_dec_converter.h
    libc/test/src/stdio/sprintf_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/src/stdio/printf_core/float_dec_converter.h b/libc/src/stdio/printf_core/float_dec_converter.h
index 65fefdb1130448..35bbb823e83834 100644
--- a/libc/src/stdio/printf_core/float_dec_converter.h
+++ b/libc/src/stdio/printf_core/float_dec_converter.h
@@ -965,15 +965,6 @@ LIBC_INLINE int convert_float_dec_auto_typed(Writer *writer,
   if (digits_checked == 0) {
     last_block_size = int_to_str.size();
     implicit_leading_zeroes = 0;
-  } else {
-    // If the block is not the maximum size, that means it has leading
-    // zeroes, and zeroes are not nines.
-    if (implicit_leading_zeroes > 0) {
-      trailing_nines = 0;
-    }
-
-    // But leading zeroes are zeroes (that could be trailing).
-    trailing_zeroes += implicit_leading_zeroes;
   }
 
   int digits_requested = (exp_precision + 1) - digits_checked;
@@ -983,6 +974,19 @@ LIBC_INLINE int convert_float_dec_auto_typed(Writer *writer,
     digits_to_check = 0;
   }
 
+  // If the block is not the maximum size, that means it has leading
+  // zeroes, and zeroes are not nines.
+  if (implicit_leading_zeroes > 0) {
+    trailing_nines = 0;
+  }
+
+  // But leading zeroes are zeroes (that could be trailing). We take the
+  // minimum of the leading zeroes and digits requested because if there are
+  // more requested digits than leading zeroes we shouldn't count those.
+  trailing_zeroes +=
+      (implicit_leading_zeroes > digits_requested ? digits_requested
+                                                  : implicit_leading_zeroes);
+
   // Check the upper digits of this block.
   for (int i = 0; i < digits_to_check; ++i) {
     if (int_to_str[i] == '9') {
@@ -1103,6 +1107,24 @@ LIBC_INLINE int convert_float_dec_auto_typed(Writer *writer,
     } else {
       // If alt form isn't set, then we need to determine the number of trailing
       // zeroes and set the precision such that they are removed.
+
+      /*
+      Here's a diagram of an example:
+
+      printf("%.15g", 22.25);
+
+                            +--- init_precision = 15
+                            |
+                            +-------------------+
+                            |                   |
+                            |  ++--- trimmed_precision = 2
+                            |  ||               |
+                            22.250000000000000000
+                            ||   |              |
+                            ++   +--------------+
+                             |   |
+       base_10_exp + 1 = 2 --+   +--- trailing_zeroes = 11
+      */
       int trimmed_precision =
           digits_checked - (base_10_exp + 1) - trailing_zeroes;
       if (trimmed_precision < 0) {

diff  --git a/libc/test/src/stdio/sprintf_test.cpp b/libc/test/src/stdio/sprintf_test.cpp
index 7214233220a1fb..8fc4997b0268b0 100644
--- a/libc/test/src/stdio/sprintf_test.cpp
+++ b/libc/test/src/stdio/sprintf_test.cpp
@@ -2437,6 +2437,10 @@ TEST_F(LlvmLibcSPrintfTest, FloatAutoConv) {
   written = __llvm_libc::sprintf(buff, "%.3g", 1256.0);
   ASSERT_STREQ_LEN(written, buff, "1.26e+03");
 
+  // Found through large scale testing.
+  written = __llvm_libc::sprintf(buff, "%.15g", 22.25);
+  ASSERT_STREQ_LEN(written, buff, "22.25");
+
   // Subnormal Precision Tests
 
   written = __llvm_libc::sprintf(buff, "%.310g", 0x1.0p-1022);