From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id BD6CE388C002 for ; Tue, 23 Feb 2021 16:30:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BD6CE388C002 Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-482-X5-NWAC-N8yFp9N2liHaPQ-1; Tue, 23 Feb 2021 11:30:17 -0500 X-MC-Unique: X5-NWAC-N8yFp9N2liHaPQ-1 Received: by mail-qk1-f200.google.com with SMTP id h21so4978272qkl.12 for ; Tue, 23 Feb 2021 08:30:17 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:mime-version; bh=Zdy0xHwZT9/6z6LLJQiL5OMsuI4pE6EVBall5r1JIyQ=; b=dvCnOTmsIX82KToP1iH8iztySE5fgxir2zlMZkRwOpWhAD3J3T6zbybfP0i3rDrVJ0 bKNzEfp1a0270fK8KBqmCD1szlQ0yIPo/VkOIdX6tjroz8lFym2lAzT7M9B6biu0m19a rsGWJToRj6GsyLfD/aJgsTyt78DXVllzrYXE+vWLb8/yqAIr02vvfwNSKvTnKfcVRVaX +D9etUCiDZrtdSVUFXubJRDZfyavzU/7soS1ml7cQGhPqjLgnIVX9ZA8x5kr4YJ24gan OwrL8o6ASa2QWYL6yo87BSW6/af0xJT6A7fyck8W1AlcE6xhkwTxj+rJe6wRUFSwGNcN MACA== X-Gm-Message-State: AOAM532oFdswYx9i54b1zJApeAXyVUbBXPLeEsAKG3zcdxbj/sqlLi+d Nze23HtnErLzuB74gg63FkATBE0dKMK0SxmfL866P/Y9UpiYesmqvMEWn5aPCLt8y9gOYRKRukY ZBsTNKzdGoUqErLU= X-Received: by 2002:ac8:768b:: with SMTP id g11mr26515413qtr.384.1614097816264; Tue, 23 Feb 2021 08:30:16 -0800 (PST) X-Google-Smtp-Source: ABdhPJyU6SNDZtknDlzTs3QBDmbl3x1v2ehTO6k4Dao0udXVT9aOsHicXQrkbCNKmE1ybq72IJ8H7w== X-Received: by 2002:ac8:768b:: with SMTP id g11mr26515389qtr.384.1614097815972; Tue, 23 Feb 2021 08:30:15 -0800 (PST) Received: from [192.168.1.130] (ool-457d493a.dyn.optonline.net. [69.125.73.58]) by smtp.gmail.com with ESMTPSA id i7sm15278774qke.5.2021.02.23.08.30.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Feb 2021 08:30:15 -0800 (PST) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Tue, 23 Feb 2021 11:30:14 -0500 (EST) To: Patrick Palka cc: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org Subject: Re: [PATCH 1/2] libstdc++: Robustify long double std::to_chars testcase [PR98384] In-Reply-To: <20210222215559.1834935-1-ppalka@redhat.com> Message-ID: References: <20210222215559.1834935-1-ppalka@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-16.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Feb 2021 16:30:23 -0000 On Mon, 22 Feb 2021, Patrick Palka wrote: > This makes the hexadecimal section of the long double std::to_chars > testcase more robust by avoiding false-negative FAILs due to printf > using a different leading hex digit than us, and by additionally > verifying the correctness of the hexadecimal form via round-tripping > through std::from_chars. > > Tested on x86, x86_64, powerpc64be, powerpc64le and aarch64. Does this > look OK for trunk? The commit message could explain the issue better, so here's v2 with a more detailed commit message. -- >8 -- Subject: [PATCH] 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 so 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 yield the original value. Tested on x86, x86_64, powerpc64be, powerpc64le and aarch64. Does this look OK for trunk? libstdc++-v3/ChangeLog: PR libstdc++/98384 * testsuite/20_util/to_chars/long_double.cc: Include . (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 with the output of printf. Also verify the output by round-tripping it through from_chars. --- .../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 #include #include +#include #include #include @@ -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 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::max()), detail::nextupl(numeric_limits::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); } } } -- 2.30.1.602.g966e671106