[PATCH] Fix for bug libstdc++/110860

[PATCH] Fix for bug libstdc++/110860

[Paul Dreik]

[-- Attachment #1.1: Type: text/plain, Size: 2777 bytes --]

The patch below fixes an issue with the fix already committed for 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110860 which unfortunately 
was not sufficient for small magnitude floating point values.

With the patch in place, the code now survives the fuzzing I used to 
find the problem in the first place. Tested on amd64.

I prepared the patch using git show, which should include the signoff as 
instructed per the DCO.

Thanks, Paul

------------------------------------------------------------------------
commit 848b8d948787495e64ed9c55d681eccf730b74fb
Author: Paul Dreik <gccpatches@pauldreik.se>
Date:   Mon Aug 14 11:52:30 2023 +0200

     libstdc++: Avoid problematic use of log10 in std::format [PR110860]

     If abs(__v) is smaller than one, the result will be on the
     form 0.xxxxx. It is only if the magnitude is large that more digits
     are needed before the decimal dot.

     This uses frexp instead of log10 which should be less expensive
     and have sufficient precision for the desired purpose.

     It removes the problematic cases where log10 will be negative or not
     fit in an int.

     Signed-off-by: Paul Dreik <gccpatches@pauldreik.se>

diff --git a/libstdc++-v3/include/std/format 
b/libstdc++-v3/include/std/format
index f4520ff3f..729e3d4b9 100644
--- a/libstdc++-v3/include/std/format
+++ b/libstdc++-v3/include/std/format
@@ -1490,14 +1490,22 @@ namespace __format
  	      // If the buffer is too small it's probably because of a large
  	      // precision, or a very large value in fixed format.
  	      size_t __guess = 8 + __prec;
-	      if (__fmt == chars_format::fixed && __v != 0) // +ddd.prec
+	      if (__fmt == chars_format::fixed) // +ddd.prec
  		{
-		  if constexpr (is_same_v<_Fp, float>)
-		    __guess += __builtin_log10f(__v < 0.0f ? -__v : __v);
-		  else if constexpr (is_same_v<_Fp, double>)
-		    __guess += __builtin_log10(__v < 0.0 ? -__v : __v);
-		  else if constexpr (is_same_v<_Fp, long double>)
-		    __guess += __builtin_log10l(__v < 0.0l ? -__v : __v);
+		  if constexpr (is_same_v<_Fp, float> || is_same_v<_Fp, double> || 
is_same_v<_Fp, long double>)
+		    {
+		      // the number of digits to the left of the decimal point
+		      // is floor(log10(max(abs(__v),1)))+1
+		      int __exp{};
+		      if constexpr (is_same_v<_Fp, float>)
+			__builtin_frexpf(__v, &__exp);
+		      else if constexpr (is_same_v<_Fp, double>)
+			__builtin_frexp(__v, &__exp);
+		      else if constexpr (is_same_v<_Fp, long double>)
+			__builtin_frexpl(__v, &__exp);
+		      if (__exp>0)
+			__guess += 1U + __exp * 4004U / 13301U; // log10(2) approx.
+		    }
  		  else
  		    __guess += numeric_limits<_Fp>::max_exponent10;
  		}

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH] Fix for bug libstdc++/110860

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 3499 bytes --]

On Mon, 14 Aug 2023 at 10:58, Paul Dreik via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:

> The patch below fixes an issue with the fix already committed for
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110860 which unfortunately
> was not sufficient for small magnitude floating point values.
>
> With the patch in place, the code now survives the fuzzing I used to
> find the problem in the first place. Tested on amd64.
>
> I prepared the patch using git show, which should include the signoff as
> instructed per the DCO.
>

I couldn't apply the patch directly from the email, but I'm not sure where
it got mangled. I just applied it by hand instead.

Pushed to trunk, thanks for the patch!

I'll push it to gcc-13 shortly too.




>
> Thanks, Paul
>
> ------------------------------------------------------------------------
> commit 848b8d948787495e64ed9c55d681eccf730b74fb
> Author: Paul Dreik <gccpatches@pauldreik.se>
> Date:   Mon Aug 14 11:52:30 2023 +0200
>
>      libstdc++: Avoid problematic use of log10 in std::format [PR110860]
>
>      If abs(__v) is smaller than one, the result will be on the
>      form 0.xxxxx. It is only if the magnitude is large that more digits
>      are needed before the decimal dot.
>
>      This uses frexp instead of log10 which should be less expensive
>      and have sufficient precision for the desired purpose.
>
>      It removes the problematic cases where log10 will be negative or not
>      fit in an int.
>
>      Signed-off-by: Paul Dreik <gccpatches@pauldreik.se>
>
> diff --git a/libstdc++-v3/include/std/format
> b/libstdc++-v3/include/std/format
> index f4520ff3f..729e3d4b9 100644
> --- a/libstdc++-v3/include/std/format
> +++ b/libstdc++-v3/include/std/format
> @@ -1490,14 +1490,22 @@ namespace __format
>               // If the buffer is too small it's probably because of a
> large
>               // precision, or a very large value in fixed format.
>               size_t __guess = 8 + __prec;
> -             if (__fmt == chars_format::fixed && __v != 0) // +ddd.prec
> +             if (__fmt == chars_format::fixed) // +ddd.prec
>                 {
> -                 if constexpr (is_same_v<_Fp, float>)
> -                   __guess += __builtin_log10f(__v < 0.0f ? -__v : __v);
> -                 else if constexpr (is_same_v<_Fp, double>)
> -                   __guess += __builtin_log10(__v < 0.0 ? -__v : __v);
> -                 else if constexpr (is_same_v<_Fp, long double>)
> -                   __guess += __builtin_log10l(__v < 0.0l ? -__v : __v);
> +                 if constexpr (is_same_v<_Fp, float> || is_same_v<_Fp,
> double> ||
> is_same_v<_Fp, long double>)
> +                   {
> +                     // the number of digits to the left of the decimal
> point
> +                     // is floor(log10(max(abs(__v),1)))+1
> +                     int __exp{};
> +                     if constexpr (is_same_v<_Fp, float>)
> +                       __builtin_frexpf(__v, &__exp);
> +                     else if constexpr (is_same_v<_Fp, double>)
> +                       __builtin_frexp(__v, &__exp);
> +                     else if constexpr (is_same_v<_Fp, long double>)
> +                       __builtin_frexpl(__v, &__exp);
> +                     if (__exp>0)
> +                       __guess += 1U + __exp * 4004U / 13301U; //
> log10(2) approx.
> +                   }
>                   else
>                     __guess += numeric_limits<_Fp>::max_exponent10;
>                 }
>