diff mbox series

libstdc++: Fix ERANGE behavior for fallback FP std::from_chars

Message ID 20221101213029.940043-1-ppalka@redhat.com
State Accepted
Headers
Series libstdc++: Fix ERANGE behavior for fallback FP std::from_chars |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Patrick Palka Nov. 1, 2022, 9:30 p.m. UTC 
  The fallback implementation of floating-point std::from_chars for e.g.
float80 just calls the C library's strtod family of functions.  In case
of overflow of the parsed result, the behavior of these functions is
rigidly specified:

  If the correct value overflows and default rounding is in effect, plus
  or minus HUGE_VAL, HUGE_VALF, or HUGE_VALL is returned (according to
  the return type and sign of the value), and the value of the macro
  ERANGE is stored in errno.

But in case of underflow, implementations are given more leeway:

  If the result underflows the functions return a value whose magnitude
  is no greater than the smallest normalized positive number in the
  return type; whether errno acquires the value ERANGE is
  implementation-defined.

Thus we can (and do) portably detect overflow, but we can't portably
detect underflow.  However, glibc (and presumably other high-quality C
library implementations) will reliably set errno to ERANGE in case of
underflow too, and it will also return the nearest denormal number to
the parsed result (including zero in case of true underflow).

Since we can't be perfect here, this patch takes the best effort
approach of assuming a high quality C library implementation that
allows us to distinguish between a denormal parsed result and true
underflow by inspecting the return value.

Tested on x86_64-pc-linux-gnu, does this look OK for trunk?  Dunno
if we should backport this too.  No test because we can't portably
test this IIUC.

libstdc++-v3/ChangeLog:

	* src/c++17/floating_from_chars.cc (from_chars_impl): In the
	ERANGE case, also check for a 0 return value before returning
	result_out_of_range, occurred, otherwise assume it's a denormal
	number.
---
 libstdc++-v3/src/c++17/floating_from_chars.cc | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Jonathan Wakely Nov. 2, 2022, 11:20 a.m. UTC | #1 
On Tue, 1 Nov 2022 at 21:30, Patrick Palka via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> The fallback implementation of floating-point std::from_chars for e.g.
> float80 just calls the C library's strtod family of functions.  In case
> of overflow of the parsed result, the behavior of these functions is
> rigidly specified:
>
>   If the correct value overflows and default rounding is in effect, plus
>   or minus HUGE_VAL, HUGE_VALF, or HUGE_VALL is returned (according to
>   the return type and sign of the value), and the value of the macro
>   ERANGE is stored in errno.
>
> But in case of underflow, implementations are given more leeway:
>
>   If the result underflows the functions return a value whose magnitude
>   is no greater than the smallest normalized positive number in the
>   return type; whether errno acquires the value ERANGE is
>   implementation-defined.
>
> Thus we can (and do) portably detect overflow, but we can't portably
> detect underflow.  However, glibc (and presumably other high-quality C
> library implementations) will reliably set errno to ERANGE in case of
> underflow too, and it will also return the nearest denormal number to
> the parsed result (including zero in case of true underflow).
>
> Since we can't be perfect here, this patch takes the best effort
> approach of assuming a high quality C library implementation that
> allows us to distinguish between a denormal parsed result and true
> underflow by inspecting the return value.
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?  Dunno

OK for trunk.

> if we should backport this too.  No test because we can't portably
> test this IIUC.

I think it's worth backporting to 11 and 12 because this is a C++17
feature and that's our default mode since GCC 11.
But give it some soak time on trunk first please.


>
> libstdc++-v3/ChangeLog:
>
>         * src/c++17/floating_from_chars.cc (from_chars_impl): In the
>         ERANGE case, also check for a 0 return value before returning
>         result_out_of_range, occurred, otherwise assume it's a denormal
>         number.
> ---
>  libstdc++-v3/src/c++17/floating_from_chars.cc | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libstdc++-v3/src/c++17/floating_from_chars.cc b/libstdc++-v3/src/c++17/floating_from_chars.cc
> index a25ac5ce3aa..939c751f861 100644
> --- a/libstdc++-v3/src/c++17/floating_from_chars.cc
> +++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
> @@ -637,8 +637,13 @@ namespace
>           {
>             if (__builtin_isinf(tmpval)) // overflow
>               ec = errc::result_out_of_range;
> -           else // underflow (LWG 3081 wants to set value = tmpval here)
> +           else if (tmpval == 0) // underflow (LWG 3081 wants to set value = tmpval here)
>               ec = errc::result_out_of_range;
> +           else // denormal value
> +             {
> +               value = tmpval;
> +               ec = errc();
> +             }
>           }
>         else if (n)
>           {
> --
> 2.38.1.381.gc03801e19c
>
diff mbox series

Patch

diff --git a/libstdc++-v3/src/c++17/floating_from_chars.cc b/libstdc++-v3/src/c++17/floating_from_chars.cc
index a25ac5ce3aa..939c751f861 100644
--- a/libstdc++-v3/src/c++17/floating_from_chars.cc
+++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
@@ -637,8 +637,13 @@  namespace
 	  {
 	    if (__builtin_isinf(tmpval)) // overflow
 	      ec = errc::result_out_of_range;
-	    else // underflow (LWG 3081 wants to set value = tmpval here)
+	    else if (tmpval == 0) // underflow (LWG 3081 wants to set value = tmpval here)
 	      ec = errc::result_out_of_range;
+	    else // denormal value
+	      {
+		value = tmpval;
+		ec = errc();
+	      }
 	  }
 	else if (n)
 	  {