From 7879e9278a48b4bf7727b640ea1485bf3fb01eb8 Mon Sep 17 00:00:00 2001 From: Benjamin Barenblat Date: Tue, 9 Feb 2021 14:41:13 -0500 Subject: Correct string formatting on POWER Prevent assertion failures when formatting small doubles on double- double systems like POWER. --- debian/changelog | 6 + debian/patches/ppc-float-conversion.diff | 268 +++++++++++++++++++++++++++++++ debian/patches/series | 1 + 3 files changed, 275 insertions(+) create mode 100644 debian/patches/ppc-float-conversion.diff diff --git a/debian/changelog b/debian/changelog index 5f980167..ce59cc14 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +abseil (0~20200923.3-2) unstable; urgency=medium + + * Correct string formatting on POWER. + + -- Benjamin Barenblat Tue, 09 Feb 2021 14:41:06 -0500 + abseil (0~20200923.3-1) unstable; urgency=medium * New upstream release. diff --git a/debian/patches/ppc-float-conversion.diff b/debian/patches/ppc-float-conversion.diff new file mode 100644 index 00000000..f0ff9595 --- /dev/null +++ b/debian/patches/ppc-float-conversion.diff @@ -0,0 +1,268 @@ +From: Samuel Benzaquen +Subject: Fix float conversion for PPC. +Origin: backport, https://github.com/abseil/abseil-cpp/commit/c36d825d9a5443f81d2656685ae021d6326da90c + +In PPC `long double` is a double-double representation which behaves weirdly +wrt numeric_limits. Don't take `long double` into account when we are not +handling `long double` natively anyway. + +Fix the convert test to always run the conversion even if we are not going to +compare against libc's printf result. This allows exercising the code itself to +make sure we don't trigger assertions or UB found by sanitizers. + +The author works at Google. Upstream applied this patch as Piper revision +355857729 and exported it to GitHub; the Applied-Upstream URL above points to +the exported commit. + +--- a/absl/strings/internal/str_format/convert_test.cc ++++ b/absl/strings/internal/str_format/convert_test.cc +@@ -540,7 +540,8 @@ + } + + template +-void TestWithMultipleFormatsHelper(const std::vector &floats) { ++void TestWithMultipleFormatsHelper(const std::vector &floats, ++ const std::set &skip_verify) { + const NativePrintfTraits &native_traits = VerifyNativeImplementation(); + // Reserve the space to ensure we don't allocate memory in the output itself. + std::string str_format_result; +@@ -588,7 +589,16 @@ + AppendPack(&str_format_result, format, absl::MakeSpan(args)); + } + +- if (string_printf_result != str_format_result) { ++#ifdef _MSC_VER ++ // MSVC has a different rounding policy than us so we can't test our ++ // implementation against the native one there. ++ continue; ++#elif defined(__APPLE__) ++ // Apple formats NaN differently (+nan) vs. (nan) ++ if (std::isnan(d)) continue; ++#endif ++ if (string_printf_result != str_format_result && ++ skip_verify.find(d) == skip_verify.end()) { + // We use ASSERT_EQ here because failures are usually correlated and a + // bug would print way too many failed expectations causing the test + // to time out. +@@ -602,12 +612,6 @@ + } + + TEST_F(FormatConvertTest, Float) { +-#ifdef _MSC_VER +- // MSVC has a different rounding policy than us so we can't test our +- // implementation against the native one there. +- return; +-#endif // _MSC_VER +- + std::vector floats = {0.0f, + -0.0f, + .9999999f, +@@ -621,7 +625,8 @@ + std::numeric_limits::epsilon(), + std::numeric_limits::epsilon() + 1.0f, + std::numeric_limits::infinity(), +- -std::numeric_limits::infinity()}; ++ -std::numeric_limits::infinity(), ++ std::nanf("")}; + + // Some regression tests. + floats.push_back(0.999999989f); +@@ -650,21 +655,14 @@ + std::sort(floats.begin(), floats.end()); + floats.erase(std::unique(floats.begin(), floats.end()), floats.end()); + +-#ifndef __APPLE__ +- // Apple formats NaN differently (+nan) vs. (nan) +- floats.push_back(std::nan("")); +-#endif +- +- TestWithMultipleFormatsHelper(floats); ++ TestWithMultipleFormatsHelper(floats, {}); + } + + TEST_F(FormatConvertTest, Double) { +-#ifdef _MSC_VER +- // MSVC has a different rounding policy than us so we can't test our +- // implementation against the native one there. +- return; +-#endif // _MSC_VER +- ++ // For values that we know won't match the standard library implementation we ++ // skip verification, but still run the algorithm to catch asserts/sanitizer ++ // bugs. ++ std::set skip_verify; + std::vector doubles = {0.0, + -0.0, + .99999999999999, +@@ -678,7 +676,8 @@ + std::numeric_limits::epsilon(), + std::numeric_limits::epsilon() + 1, + std::numeric_limits::infinity(), +- -std::numeric_limits::infinity()}; ++ -std::numeric_limits::infinity(), ++ std::nan("")}; + + // Some regression tests. + doubles.push_back(0.99999999999999989); +@@ -708,33 +707,29 @@ + "5084551339423045832369032229481658085593321233482747978262041447231" + "68738177180919299881250404026184124858368.000000"; + +- if (!gcc_bug_22142) { +- for (int exp = -300; exp <= 300; ++exp) { +- const double all_ones_mantissa = 0x1fffffffffffff; +- doubles.push_back(std::ldexp(all_ones_mantissa, exp)); ++ for (int exp = -300; exp <= 300; ++exp) { ++ const double all_ones_mantissa = 0x1fffffffffffff; ++ doubles.push_back(std::ldexp(all_ones_mantissa, exp)); ++ if (gcc_bug_22142) { ++ skip_verify.insert(doubles.back()); + } + } + + if (gcc_bug_22142) { +- for (auto &d : doubles) { +- using L = std::numeric_limits; +- double d2 = std::abs(d); +- if (d2 == L::max() || d2 == L::min() || d2 == L::denorm_min()) { +- d = 0; +- } +- } ++ using L = std::numeric_limits; ++ skip_verify.insert(L::max()); ++ skip_verify.insert(L::min()); // NOLINT ++ skip_verify.insert(L::denorm_min()); ++ skip_verify.insert(-L::max()); ++ skip_verify.insert(-L::min()); // NOLINT ++ skip_verify.insert(-L::denorm_min()); + } + + // Remove duplicates to speed up the logic below. + std::sort(doubles.begin(), doubles.end()); + doubles.erase(std::unique(doubles.begin(), doubles.end()), doubles.end()); + +-#ifndef __APPLE__ +- // Apple formats NaN differently (+nan) vs. (nan) +- doubles.push_back(std::nan("")); +-#endif +- +- TestWithMultipleFormatsHelper(doubles); ++ TestWithMultipleFormatsHelper(doubles, skip_verify); + } + + TEST_F(FormatConvertTest, DoubleRound) { +@@ -1055,11 +1050,6 @@ + } + + TEST_F(FormatConvertTest, LongDouble) { +-#ifdef _MSC_VER +- // MSVC has a different rounding policy than us so we can't test our +- // implementation against the native one there. +- return; +-#endif // _MSC_VER + const NativePrintfTraits &native_traits = VerifyNativeImplementation(); + const char *const kFormats[] = {"%", "%.3", "%8.5", "%9", "%.5000", + "%.60", "%+", "% ", "%-10"}; +@@ -1120,10 +1110,18 @@ + for (auto d : doubles) { + FormatArgImpl arg(d); + UntypedFormatSpecImpl format(fmt_str); ++ std::string result = FormatPack(format, {&arg, 1}); ++ ++#ifdef _MSC_VER ++ // MSVC has a different rounding policy than us so we can't test our ++ // implementation against the native one there. ++ continue; ++#endif // _MSC_VER ++ + // We use ASSERT_EQ here because failures are usually correlated and a + // bug would print way too many failed expectations causing the test to + // time out. +- ASSERT_EQ(StrPrint(fmt_str.c_str(), d), FormatPack(format, {&arg, 1})) ++ ASSERT_EQ(StrPrint(fmt_str.c_str(), d), result) + << fmt_str << " " << StrPrint("%.18Lg", d) << " " + << StrPrint("%La", d) << " " << StrPrint("%.1080Lf", d); + } +--- a/absl/strings/internal/str_format/float_conversion.cc ++++ b/absl/strings/internal/str_format/float_conversion.cc +@@ -98,12 +98,22 @@ + return next_carry % divisor; + } + ++constexpr bool IsDoubleDouble() { ++ // This is the `double-double` representation of `long double`. ++ // We do not handle it natively. Fallback to snprintf. ++ return std::numeric_limits::digits == ++ 2 * std::numeric_limits::digits; ++} ++ ++using MaxFloatType = ++ typename std::conditional::type; ++ + // Generates the decimal representation for an integer of the form `v * 2^exp`, + // where `v` and `exp` are both positive integers. + // It generates the digits from the left (ie the most significant digit first) + // to allow for direct printing into the sink. + // +-// Requires `0 <= exp` and `exp <= numeric_limits::max_exponent`. ++// Requires `0 <= exp` and `exp <= numeric_limits::max_exponent`. + class BinaryToDecimal { + static constexpr int ChunksNeeded(int exp) { + // We will left shift a uint128 by `exp` bits, so we need `128+exp` total +@@ -118,10 +128,10 @@ + static void RunConversion(uint128 v, int exp, + absl::FunctionRef f) { + assert(exp > 0); +- assert(exp <= std::numeric_limits::max_exponent); ++ assert(exp <= std::numeric_limits::max_exponent); + static_assert( + StackArray::kMaxCapacity >= +- ChunksNeeded(std::numeric_limits::max_exponent), ++ ChunksNeeded(std::numeric_limits::max_exponent), + ""); + + StackArray::RunWithCapacity( +@@ -218,14 +228,14 @@ + + // Converts a value of the form `x * 2^-exp` into a sequence of decimal digits. + // Requires `-exp < 0` and +-// `-exp >= limits::min_exponent - limits::digits`. ++// `-exp >= limits::min_exponent - limits::digits`. + class FractionalDigitGenerator { + public: + // Run the conversion for `v * 2^exp` and call `f(generator)`. + // This function will allocate enough stack space to perform the conversion. + static void RunConversion( + uint128 v, int exp, absl::FunctionRef f) { +- using Limits = std::numeric_limits; ++ using Limits = std::numeric_limits; + assert(-exp < 0); + assert(-exp >= Limits::min_exponent - 128); + static_assert(StackArray::kMaxCapacity >= +@@ -858,10 +868,10 @@ + // This buffer holds the "0x1.ab1de3" portion of "0x1.ab1de3pe+2". Compute the + // size with long double which is the largest of the floats. + constexpr size_t kBufSizeForHexFloatRepr = +- 2 // 0x +- + std::numeric_limits::digits / 4 // number of hex digits +- + 1 // round up +- + 1; // "." (dot) ++ 2 // 0x ++ + std::numeric_limits::digits / 4 // number of hex digits ++ + 1 // round up ++ + 1; // "." (dot) + char digits_buffer[kBufSizeForHexFloatRepr]; + char *digits_iter = digits_buffer; + const char *const digits = +@@ -1380,10 +1390,7 @@ + + bool ConvertFloatImpl(long double v, const FormatConversionSpecImpl &conv, + FormatSinkImpl *sink) { +- if (std::numeric_limits::digits == +- 2 * std::numeric_limits::digits) { +- // This is the `double-double` representation of `long double`. +- // We do not handle it natively. Fallback to snprintf. ++ if (IsDoubleDouble()) { + return FallbackToSnprintf(v, conv, sink); + } + diff --git a/debian/patches/series b/debian/patches/series index 6330dc6c..ceceb694 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -6,3 +6,4 @@ cpu-frequency.diff nan-narrowing.diff endian-hash.diff endian-random.diff +ppc-float-conversion.diff -- cgit v1.2.3