Correct string formatting on POWER20200923.3-2

Prevent assertion failures when formatting small doubles on double-
double systems like POWER.

3 files changed, 275 insertions, 0 deletions

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 <bbaren@debian.org>  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 <sbenza@google.com>
+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 <typename Floating>
+-void TestWithMultipleFormatsHelper(const std::vector<Floating> &floats) {
++void TestWithMultipleFormatsHelper(const std::vector<Floating> &floats,
++                                   const std::set<Floating> &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<float> floats = {0.0f,
+                                -0.0f,
+                                .9999999f,
+@@ -621,7 +625,8 @@
+                                std::numeric_limits<float>::epsilon(),
+                                std::numeric_limits<float>::epsilon() + 1.0f,
+                                std::numeric_limits<float>::infinity(),
+-                               -std::numeric_limits<float>::infinity()};
++                               -std::numeric_limits<float>::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<double> skip_verify;
+   std::vector<double> doubles = {0.0,
+                                  -0.0,
+                                  .99999999999999,
+@@ -678,7 +676,8 @@
+                                  std::numeric_limits<double>::epsilon(),
+                                  std::numeric_limits<double>::epsilon() + 1,
+                                  std::numeric_limits<double>::infinity(),
+-                                 -std::numeric_limits<double>::infinity()};
++                                 -std::numeric_limits<double>::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>;
+-      double d2 = std::abs(d);
+-      if (d2 == L::max() || d2 == L::min() || d2 == L::denorm_min()) {
+-        d = 0;
+-      }
+-    }
++    using L = std::numeric_limits<double>;
++    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<long double>::digits ==
++         2 * std::numeric_limits<double>::digits;
++}
++
++using MaxFloatType =
++    typename std::conditional<IsDoubleDouble(), double, long double>::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<long double>::max_exponent`.
++// Requires `0 <= exp` and `exp <= numeric_limits<MaxFloatType>::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<void(BinaryToDecimal)> f) {
+     assert(exp > 0);
+-    assert(exp <= std::numeric_limits<long double>::max_exponent);
++    assert(exp <= std::numeric_limits<MaxFloatType>::max_exponent);
+     static_assert(
+         StackArray::kMaxCapacity >=
+-            ChunksNeeded(std::numeric_limits<long double>::max_exponent),
++            ChunksNeeded(std::numeric_limits<MaxFloatType>::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<long double>::min_exponent - limits<long double>::digits`.
++// `-exp >= limits<MaxFloatType>::min_exponent - limits<MaxFloatType>::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<void(FractionalDigitGenerator)> f) {
+-    using Limits = std::numeric_limits<long double>;
++    using Limits = std::numeric_limits<MaxFloatType>;
+     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<long double>::digits / 4  // number of hex digits
+-      + 1                                             // round up
+-      + 1;                                            // "." (dot)
++      2                                                // 0x
++      + std::numeric_limits<MaxFloatType>::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<long double>::digits ==
+-      2 * std::numeric_limits<double>::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