On Tue, 11 Jan 2022 at 20:03, Patrick Palka via Libstdc++ <
libstd...@gcc.gnu.org> wrote:
> We currently crash when the floating-point to_chars overloads are passed
> a precision value near INT_MAX, ultimately due to overflow in the bounds
> checks that verify the output range is large enough.
>
> The most portable fix seems to be to replace bounds checks of the form
> A >= B + C (where B + C may overflow) with the otherwise equivalent check
> A >= B && A - B >= C, which is what this patch implements.
>
> Before we could do this in __floating_to_chars_hex, there we first need
> to track the unbounded "excess" precision (i.e. the number of trailing
> digits in the output that are guaranteed to be '0') separately from the
> bounded "effective" precision (i.e. the number of significant fractional
> digits in the output), like we do in __floating_to_chars_precision.
>
> Tested on x86_64 and i686, does this look OK for trunk/11?
>
OK for both, thanks.
> PR libstdc++/103955
>
> libstdc++-v3/ChangeLog:
>
> * src/c++17/floating_to_chars.cc (__floating_to_chars_hex):
> Track the excess precision separately from the effective
> precision. Avoid overflow in bounds check by splitting it into
> two checks.
> (__floating_to_chars_precision): Avoid overlflow in bounds
> checks similarly.
> * testsuite/20_util/to_chars/103955.cc: New test.
> ---
> libstdc++-v3/src/c++17/floating_to_chars.cc | 46 +++++++++++++------
> .../testsuite/20_util/to_chars/103955.cc | 31 +++++++++++++
> 2 files changed, 64 insertions(+), 13 deletions(-)
> create mode 100644 libstdc++-v3/testsuite/20_util/to_chars/103955.cc
>
> diff --git a/libstdc++-v3/src/c++17/floating_to_chars.cc
> b/libstdc++-v3/src/c++17/floating_to_chars.cc
> index c825a3c8b24..6cd92553d05 100644
> --- a/libstdc++-v3/src/c++17/floating_to_chars.cc
> +++ b/libstdc++-v3/src/c++17/floating_to_chars.cc
> @@ -747,7 +747,8 @@ template<typename T>
> __glibcxx_assert(shortest_full_precision >= 0);
>
> int written_exponent = unbiased_exponent;
> - const int effective_precision =
> precision.value_or(shortest_full_precision);
> + int effective_precision = precision.value_or(shortest_full_precision);
> + int excess_precision = 0;
> if (effective_precision < shortest_full_precision)
> {
> // When limiting the precision, we need to determine how to round
> the
> @@ -794,6 +795,11 @@ template<typename T>
> }
> }
> }
> + else
> + {
> + excess_precision = effective_precision - shortest_full_precision;
> + effective_precision = shortest_full_precision;
> + }
>
> // Compute the leading hexit and mask it out from the mantissa.
> char leading_hexit;
> @@ -816,26 +822,30 @@ template<typename T>
> // Now before we start writing the string, determine the total length
> of
> // the output string and perform a single bounds check.
> int expected_output_length = sign + 1;
> - if (effective_precision != 0)
> - expected_output_length += strlen(".") + effective_precision;
> + if (effective_precision + excess_precision > 0)
> + expected_output_length += strlen(".");
> + expected_output_length += effective_precision;
> const int abs_written_exponent = abs(written_exponent);
> expected_output_length += (abs_written_exponent >= 10000 ?
> strlen("p+ddddd")
> : abs_written_exponent >= 1000 ?
> strlen("p+dddd")
> : abs_written_exponent >= 100 ?
> strlen("p+ddd")
> : abs_written_exponent >= 10 ?
> strlen("p+dd")
> : strlen("p+d"));
> - if (last - first < expected_output_length)
> + if (last - first < expected_output_length
> + || last - first - expected_output_length < excess_precision)
> return {last, errc::value_too_large};
> + char* const expected_last = first + expected_output_length +
> excess_precision;
>
> - const auto saved_first = first;
> // Write the negative sign and the leading hexit.
> if (sign)
> *first++ = '-';
> *first++ = leading_hexit;
>
> + if (effective_precision + excess_precision > 0)
> + *first++ = '.';
> +
> if (effective_precision > 0)
> {
> - *first++ = '.';
> int written_hexits = 0;
> // Extract and mask out the leading nibble after the decimal point,
> // write its corresponding hexit, and repeat until the mantissa is
> @@ -863,13 +873,18 @@ template<typename T>
> }
> }
>
> + if (excess_precision > 0)
> + {
> + memset(first, '0', excess_precision);
> + first += excess_precision;
> + }
> +
> // Finally, write the exponent.
> *first++ = 'p';
> if (written_exponent >= 0)
> *first++ = '+';
> const to_chars_result result = to_chars(first, last,
> written_exponent);
> - __glibcxx_assert(result.ec == errc{}
> - && result.ptr == saved_first +
> expected_output_length);
> + __glibcxx_assert(result.ec == errc{} && result.ptr == expected_last);
> return result;
> }
>
> @@ -1250,7 +1265,8 @@ template<typename T>
> }
>
> // Copy the string from the buffer over to the output range.
> - if (last - first < output_length + excess_precision)
> + if (last - first < output_length
> + || last - first - output_length < excess_precision)
> return {last, errc::value_too_large};
> memcpy(first, buffer, output_length);
> first += output_length;
> @@ -1304,7 +1320,8 @@ template<typename T>
> output_length_upper_bound += strlen("e+dd");
>
> int output_length;
> - if (last - first >= output_length_upper_bound + excess_precision)
> + if (last - first >= output_length_upper_bound
> + && last - first - output_length_upper_bound >=
> excess_precision)
> {
> // The result will definitely fit into the output range, so we
> can
> // write directly into it.
> @@ -1325,7 +1342,8 @@ template<typename T>
> buffer, nullptr);
> __glibcxx_assert(output_length == output_length_upper_bound - 1
> || output_length ==
> output_length_upper_bound);
> - if (last - first < output_length + excess_precision)
> + if (last - first < output_length
> + || last - first - output_length < excess_precision)
> return {last, errc::value_too_large};
> memcpy(first, buffer, output_length);
> }
> @@ -1365,7 +1383,8 @@ template<typename T>
> output_length_upper_bound += strlen(".") + effective_precision;
>
> int output_length;
> - if (last - first >= output_length_upper_bound + excess_precision)
> + if (last - first >= output_length_upper_bound
> + && last - first - output_length_upper_bound >=
> excess_precision)
> {
> // The result will definitely fit into the output range, so we
> can
> // write directly into it.
> @@ -1382,7 +1401,8 @@ template<typename T>
> output_length = ryu::d2fixed_buffered_n(value,
> effective_precision,
> buffer);
> __glibcxx_assert(output_length <= output_length_upper_bound);
> - if (last - first < output_length + excess_precision)
> + if (last - first < output_length
> + || last - first - output_length < excess_precision)
> return {last, errc::value_too_large};
> memcpy(first, buffer, output_length);
> }
> diff --git a/libstdc++-v3/testsuite/20_util/to_chars/103955.cc
> b/libstdc++-v3/testsuite/20_util/to_chars/103955.cc
> new file mode 100644
> index 00000000000..a47a0a5be8a
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/20_util/to_chars/103955.cc
> @@ -0,0 +1,31 @@
> +// PR libstdc++/103955
> +// Verify we don't crash when the floating-point to_chars overloads are
> passed
> +// a large precision argument.
> +
> +#include <charconv>
> +
> +#include <climits>
> +#include <initializer_list>
> +#include <testsuite_hooks.h>
> +
> +void
> +test01()
> +{
> + constexpr int size = 12;
> + char result[size];
> +
> + std::to_chars_result tcr;
> + for (auto fmt : { std::chars_format::fixed, std::chars_format::general,
> + std::chars_format::hex })
> + for (int precision : { INT_MAX, INT_MAX-1 })
> + {
> + tcr = std::to_chars(result, result+size, 1.337, fmt, precision);
> + VERIFY( tcr.ptr == result+size && tcr.ec ==
> std::errc::value_too_large );
> + }
> +}
> +
> +int
> +main()
> +{
> + test01();
> +}
> --
> 2.35.0.rc0
>
>
