public inbox for libstdc++-cvs@sourceware.org
help / color / mirror / Atom feed
From: Patrick Palka <ppalka@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org
Subject: [gcc r11-7367] libstdc++: Robustify long double std::to_chars testcase [PR98384]
Date: Wed, 24 Feb 2021 17:26:57 +0000 (GMT)	[thread overview]
Message-ID: <20210224172657.9348F38708D9@sourceware.org> (raw)

https://gcc.gnu.org/g:70aa0e6eef9d65744f37adc2a3cffef1a8217dc1

commit r11-7367-g70aa0e6eef9d65744f37adc2a3cffef1a8217dc1
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Feb 24 12:24:43 2021 -0500

    libstdc++: Robustify long double std::to_chars testcase [PR98384]
    
    The long double std::to_chars testcase currently verifies the
    correctness of its output by comparing it to that of printf, so if
    there's a mismatch between to_chars and printf, the test FAILs.  This
    works well for the scientific, fixed and general formatting modes,
    because the corresponding printf conversion specifiers (%e, %f and %g)
    are rigidly specified.
    
    But this doesn't work well for the hex formatting mode because the
    corresponding printf conversion specifier %a is more flexibly specified.
    For instance, the hexadecimal forms 0x1p+0, 0x2p-1, 0x4p-2 and 0x8p-3
    are all equivalent and valid outputs of the %a specifier for the number 1.
    The apparent freedom here is the choice of leading hex digit -- the
    standard just requires that the leading hex digit is nonzero for
    normalized numbers.
    
    Currently, our hexadecimal formatting implementation uses 0/1/2 as the
    leading hex digit for floating point types that have an implicit leading
    mantissa bit which in practice means all supported floating point types
    except x86 long double.  The latter type has a 64 bit mantissa with an
    explicit leading mantissa bit, and for this type our implementation uses
    the most significant four bits of the mantissa as leading hex digit.
    This seems to be consistent with most printf implementations, but not
    all, as PR98384 illustrates.
    
    In order to avoid false-positive FAILs due to arbitrary disagreement
    between to_chars and printf about the choice of leading hex digit, this
    patch makes the testcase's verification via printf conditional on the
    leading hex digits first agreeing.  An additional verification step is
    also added: round-tripping the output of to_chars through from_chars
    should recover the value exactly.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/98384
            * testsuite/20_util/to_chars/long_double.cc: Include <optional>.
            (test01): Simplify verifying the nearby values by using a
            2-iteration loop and a dedicated output buffer to check that the
            nearby values are different.  Factor out the printf-based
            verification into a local function, and check that the leading
            hex digits agree before comparing to the output of printf.  Also
            verify the output by round-tripping it through from_chars.

Diff:
---
 .../testsuite/20_util/to_chars/long_double.cc      | 73 ++++++++++++++--------
 1 file changed, 47 insertions(+), 26 deletions(-)

diff --git a/libstdc++-v3/testsuite/20_util/to_chars/long_double.cc b/libstdc++-v3/testsuite/20_util/to_chars/long_double.cc
index 4f72cb65400..da847ae5401 100644
--- a/libstdc++-v3/testsuite/20_util/to_chars/long_double.cc
+++ b/libstdc++-v3/testsuite/20_util/to_chars/long_double.cc
@@ -26,6 +26,7 @@
 #include <cmath>
 #include <cstring>
 #include <iterator>
+#include <optional>
 #include <limits>
 
 #include <testsuite_hooks.h>
@@ -50,6 +51,38 @@ namespace detail
 void
 test01()
 {
+  // Verifies correctness of the hexadecimal form [BEGIN,END) for VALUE by
+  // round-tripping it through from_chars (if available).
+  auto verify_via_from_chars = [] (char *begin, char *end, long double value) {
+#if __cpp_lib_to_chars >= 201611L || _GLIBCXX_HAVE_USELOCALE
+    long double roundtrip;
+    auto result = from_chars(begin, end, roundtrip, chars_format::hex);
+    VERIFY( result.ec == errc{} );
+    VERIFY( result.ptr == end );
+    VERIFY( roundtrip == value );
+#endif
+  };
+
+  // Verifies correctness of the null-terminated hexadecimal form at BEGIN
+  // for VALUE and PRECISION by comparing it with the output of printf's %La
+  // conversion specifier.
+  auto verify_via_printf = [] (char *begin, long double value,
+			       optional<int> precision = nullopt) {
+    char printf_buffer[1024] = {};
+    if (precision.has_value())
+      sprintf(printf_buffer, "%.*La", precision.value(), value);
+    else
+      sprintf(printf_buffer, "%La", value);
+
+    // Only compare with the output of printf if the leading hex digits agree.
+    // If the leading hex digit of our form doesn't agree with that of printf,
+    // then the two forms may still be equivalent (e.g. 1.1p+0 vs 8.8p-3).  But
+    // if the leading hex digits do agree, then we do expect the two forms to be
+    // the same.
+    if (printf_buffer[strlen("0x")] == begin[0])
+      VERIFY( !strcmp(begin, printf_buffer+strlen("0x")) );
+  };
+
   const long double hex_testcases[]
     = { detail::nextdownl(numeric_limits<long double>::max()),
 	detail::nextupl(numeric_limits<long double>::min()),
@@ -92,38 +125,27 @@ test01()
 	if (testcase == 0.0L || isinf(testcase))
 	  continue;
 
-	char to_chars_buffer[1024], printf_buffer[1024];
-	memset(to_chars_buffer, '\0', sizeof(to_chars_buffer));
-	memset(printf_buffer, '\0', sizeof(printf_buffer));
-
+	char to_chars_buffer[1024] = {};
 	auto result = to_chars(begin(to_chars_buffer), end(to_chars_buffer),
 			       testcase, chars_format::hex);
 	VERIFY( result.ec == errc{} );
 	*result.ptr = '\0';
-	sprintf(printf_buffer, "%La", testcase);
-	VERIFY( !strcmp(to_chars_buffer, printf_buffer+strlen("0x")) );
+	verify_via_from_chars(begin(to_chars_buffer), result.ptr, testcase);
+	verify_via_printf(to_chars_buffer, testcase);
 
+	// Verify the nearby values, and also check they have a different
+	// shortest form.
+	for (long double nearby
+	     : { detail::nextdownl(testcase), detail::nextupl(testcase) })
 	  {
-	    // Verify that the nearby values have a different shortest form.
-	    testcase = detail::nextdownl(testcase);
-	    result = to_chars(begin(to_chars_buffer), end(to_chars_buffer),
-			      testcase, chars_format::hex);
-	    VERIFY( result.ec == errc{} );
-	    *result.ptr = '\0';
-	    VERIFY( strcmp(to_chars_buffer, printf_buffer+strlen("0x")) != 0);
-	    sprintf(printf_buffer, "%La", testcase);
-	    VERIFY( !strcmp(to_chars_buffer, printf_buffer+strlen("0x")) );
-
-	    testcase = detail::nextupl(detail::nextupl(testcase));
-	    result = to_chars(begin(to_chars_buffer), end(to_chars_buffer),
-			      testcase, chars_format::hex);
+	    char nearby_buffer[1024] = {};
+	    result = to_chars(begin(nearby_buffer), end(nearby_buffer),
+			      nearby, chars_format::hex);
 	    VERIFY( result.ec == errc{} );
 	    *result.ptr = '\0';
-	    VERIFY( strcmp(to_chars_buffer, printf_buffer+strlen("0x")) != 0);
-	    sprintf(printf_buffer, "%La", testcase);
-	    VERIFY( !strcmp(to_chars_buffer, printf_buffer+strlen("0x")) );
-
-	    testcase = detail::nextdownl(testcase);
+	    VERIFY( strcmp(nearby_buffer, to_chars_buffer) != 0);
+	    verify_via_from_chars(begin(nearby_buffer), result.ptr, nearby);
+	    verify_via_printf(nearby_buffer, nearby);
 	  }
 
 	for (int precision = -1; precision < 50; precision++)
@@ -132,8 +154,7 @@ test01()
 			      testcase, chars_format::hex, precision);
 	    VERIFY( result.ec == errc{} );
 	    *result.ptr = '\0';
-	    sprintf(printf_buffer, "%.*La", precision, testcase);
-	    VERIFY( !strcmp(to_chars_buffer, printf_buffer+strlen("0x")) );
+	    verify_via_printf(to_chars_buffer, testcase, precision);
 	  }
       }
 }


                 reply	other threads:[~2021-02-24 17:26 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210224172657.9348F38708D9@sourceware.org \
    --to=ppalka@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    --cc=libstdc++-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).