https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110860

--- Comment #12 from Paul Dreik <gcc at pauldreik dot se> ---
The last fix is unfortunately not sufficient either, because for abs(__v)<1
log10 becomes negative and that wont convert gracefully to size_t.

I implemented the following fix, which avoids log10 and uses frexp instead
since we
only need an approximation anyway. That also removes the need for handling the
sign.

The magic constant 4004U / 13301U is obtained from Gnu Octave rats(log10(2),11)
which I chose because it is slightly conservative. I think integer math is good
here, to avoid conversions.

I experimented with bitwise fiddling to get the exponent but I think that is
less portable and readable than frexp. It does however avoid a function call to
frexp and is twice as fast (I benchmarked it).

What do you think of the following patch?

commit a7b133fb073ebd7f6ba686f31530ed20d656bd57
Author: Paul Dreik <git...@pauldreik.se>
Date:   Sat Aug 12 13:16: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.

diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
index 23da6b008..8d147abe9 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;
                }

Reply via email to