[libc-commits] [libc] [libc] Fix typo in long double negative block (PR #68243)

[via libc-commits]

https://github.com/michaelrj-google updated https://github.com/llvm/llvm-project/pull/68243

>From c9f6955507bf7e38caaeec5230a12b445691131e Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Wed, 4 Oct 2023 11:01:06 -0700
Subject: [PATCH 1/2] [libc] Fix typo in long double negative block

The long double version of float to string's get_negative_block had a
bug in table mode. In table mode, one of the tables is named
"MIN_BLOCK_2" and it stores the number of blocks that are all zeroes
before the digits start for a given index. The check for long doubles
was incorrectly "block_index <= MIN_BLOCK_2[idx]" when it should be
"block_index < MIN_BLOCK_2[idx]" (without the equal sign). This bug
caused an off-by-one error for some long double values. This patch fixes
the bug and adds tests to ensure it doesn't regress.
---
 libc/src/__support/float_to_string.h |  2 +-
 libc/test/src/stdio/sprintf_test.cpp | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/libc/src/__support/float_to_string.h b/libc/src/__support/float_to_string.h
index d3fcec7a652faba..eb06cd9c08af287 100644
--- a/libc/src/__support/float_to_string.h
+++ b/libc/src/__support/float_to_string.h
@@ -708,7 +708,7 @@ FloatToString<long double>::get_negative_block(int block_index) {
     const int32_t SHIFT_CONST = TABLE_SHIFT_CONST;
 
     // if the requested block is zero
-    if (block_index <= MIN_BLOCK_2[idx]) {
+    if (block_index < MIN_BLOCK_2[idx]) {
       return 0;
     }
     const uint32_t p = POW10_OFFSET_2[idx] + block_index - MIN_BLOCK_2[idx];
diff --git a/libc/test/src/stdio/sprintf_test.cpp b/libc/test/src/stdio/sprintf_test.cpp
index 8b9c919bed203d4..63de6f94dc1ab5e 100644
--- a/libc/test/src/stdio/sprintf_test.cpp
+++ b/libc/test/src/stdio/sprintf_test.cpp
@@ -2757,6 +2757,17 @@ TEST_F(LlvmLibcSPrintfTest, FloatAutoConv) {
   written = LIBC_NAMESPACE::sprintf(buff, "%.10g", 0x1.0p-1074);
   ASSERT_STREQ_LEN(written, buff, "4.940656458e-324");
 
+  written = LIBC_NAMESPACE::sprintf(buff, "%g", 0xa.aaaaaaaaaaaaaabp-7);
+  ASSERT_STREQ_LEN(written, buff, "0.0833333");
+
+  written = LIBC_NAMESPACE::sprintf(buff, "%Lg", 0xa.aaaaaaaaaaaaaabp-7L);
+  ASSERT_STREQ_LEN(written, buff, "0.0833333");
+
+  written = LIBC_NAMESPACE::sprintf(buff, "%.60Lg", 0xa.aaaaaaaaaaaaaabp-7L);
+  ASSERT_STREQ_LEN(
+      written, buff,
+      "0.0833333333333333333355920878593448009041821933351457118988037");
+
   // Long double precision tests.
   // These are currently commented out because they require long double support
   // that isn't ready yet.

>From 0ab8d4274e83444be39926e7675db5bc9dbe6e72 Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Wed, 4 Oct 2023 11:28:09 -0700
Subject: [PATCH 2/2] Reorganize tests

---
 libc/test/src/stdio/sprintf_test.cpp | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/libc/test/src/stdio/sprintf_test.cpp b/libc/test/src/stdio/sprintf_test.cpp
index 63de6f94dc1ab5e..b7e8b7548588107 100644
--- a/libc/test/src/stdio/sprintf_test.cpp
+++ b/libc/test/src/stdio/sprintf_test.cpp
@@ -2433,6 +2433,9 @@ TEST_F(LlvmLibcSPrintfTest, FloatAutoConv) {
   written = LIBC_NAMESPACE::sprintf(buff, "%g", 9999999000000.00);
   ASSERT_STREQ_LEN(written, buff, "1e+13");
 
+  written = LIBC_NAMESPACE::sprintf(buff, "%g", 0xa.aaaaaaaaaaaaaabp-7);
+  ASSERT_STREQ_LEN(written, buff, "0.0833333");
+
   // Simple Subnormal Tests.
 
   written = LIBC_NAMESPACE::sprintf(buff, "%g", 0x1.0p-1027);
@@ -2457,9 +2460,16 @@ TEST_F(LlvmLibcSPrintfTest, FloatAutoConv) {
 
   // Length Modifier Tests.
 
+#if defined(SPECIAL_X86_LONG_DOUBLE)
+
   written = LIBC_NAMESPACE::sprintf(buff, "%Lg", 0xf.fffffffffffffffp+16380L);
   ASSERT_STREQ_LEN(written, buff, "1.18973e+4932");
 
+  written = LIBC_NAMESPACE::sprintf(buff, "%Lg", 0xa.aaaaaaaaaaaaaabp-7L);
+  ASSERT_STREQ_LEN(written, buff, "0.0833333");
+
+#endif // SPECIAL_X86_LONG_DOUBLE
+
   // TODO: Uncomment the below tests after long double support is added
   /*
   written = LIBC_NAMESPACE::sprintf(buff, "%Lf", 1e100L);
@@ -2757,17 +2767,15 @@ TEST_F(LlvmLibcSPrintfTest, FloatAutoConv) {
   written = LIBC_NAMESPACE::sprintf(buff, "%.10g", 0x1.0p-1074);
   ASSERT_STREQ_LEN(written, buff, "4.940656458e-324");
 
-  written = LIBC_NAMESPACE::sprintf(buff, "%g", 0xa.aaaaaaaaaaaaaabp-7);
-  ASSERT_STREQ_LEN(written, buff, "0.0833333");
-
-  written = LIBC_NAMESPACE::sprintf(buff, "%Lg", 0xa.aaaaaaaaaaaaaabp-7L);
-  ASSERT_STREQ_LEN(written, buff, "0.0833333");
+#if defined(SPECIAL_X86_LONG_DOUBLE)
 
   written = LIBC_NAMESPACE::sprintf(buff, "%.60Lg", 0xa.aaaaaaaaaaaaaabp-7L);
   ASSERT_STREQ_LEN(
       written, buff,
       "0.0833333333333333333355920878593448009041821933351457118988037");
 
+#endif // SPECIAL_X86_LONG_DOUBLE
+
   // Long double precision tests.
   // These are currently commented out because they require long double support
   // that isn't ready yet.