| Message ID | 3311a355-15c8-4cdb-1644-b52d8aecbd63@pauldreik.se |
|---|---|
| State | New |
| Headers | show |
| Series | Fix for bug libstdc++/110860 | expand |
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; > } >
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;
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> }