pxf: refuse a BigFloat literal below the wire's exponent range

CHANGELOG.md

@@ -97,6 +97,7 @@ number says nothing about any other port. See the Releases section of
 - **A bool default is exactly `true` or `false`** (protowire-go#90). Any other spelling silently became `false`; it now fails the decode. **A schema with `(pxf.default) = "True"` now fails `UnmarshalFull`.**
 - **`ViolationKindName` and `Violation::ToString` use protowire-go's text:** `keyed field option`, `default field option`, `required field option`, and `invalid (pxf.key) = "..."`.
 - **`AppendDatasetRow` writes an empty `(pxf.required)` repeated field or map as `[]` or `{}`** (protowire-go#219). The reader counts both as supplied. This port returned `unsupported kind`, and inside a block cell it left the field out, so the reader refused the row.
+- **A PXF `BigFloat` literal whose exponent less its `prec` is below `INT32_MIN` is refused** (#125, protowire-go#231), with Go's `big float literal <s> is below the wire's exponent range`. The wire carries that number as an `int32`, and it used to wrap, so `0.125p2147483647` was stored as a different value. Both exponent checks now run before the value is widened to `prec` bits, so such a literal is refused at once instead of after building a 256 MiB mantissa. Only a `max_numeric_literal_digits` raised past 646 million admits a `prec` that wide.
 
 ### Changed
 

src/pxf/bigfloat.cc

@@ -89,6 +89,19 @@ class Nat {
     }
   }
 
+  // Sub returns a - b. a must not be less than b.
+  static Nat Sub(const Nat& a, const Nat& b) {
+    Nat out = a;
+    int64_t borrow = 0;
+    for (size_t i = 0; i < out.w_.size(); ++i) {
+      const int64_t t = int64_t{out.w_[i]} - (i < b.w_.size() ? int64_t{b.w_[i]} : 0) - borrow;
+      out.w_[i] = static_cast<uint32_t>(t);
+      borrow = t < 0 ? 1 : 0;
+    }
+    out.Trim();
+    return out;
+  }
+
   // DivSmall divides in place and returns the remainder.
   uint32_t DivSmall(uint32_t d) {
     uint64_t rem = 0;
@@ -281,6 +294,35 @@ bool ExceedsBound(int64_t e, int limit) {
   return e > limit || e < -static_cast<int64_t>(limit);
 }
 
+// RoundedBinaryExponent is the binary exponent e, value in [2^(e-1), 2^e),
+// of num/den rounded to prec bits, ties to even, as ParseBigFloatLiteral
+// rounds it. It never builds a prec-bit number, so a wide prec costs
+// nothing here. num and den must not be zero.
+//
+// The exact value's exponent comes from the two bit lengths and one
+// comparison. Rounding raises it by one exactly when the value rounds up to
+// 2^e: when it is at least 2^e (1 - 2^-(prec+1)), the tie between 2^e and
+// the largest prec-bit value below it, whose mantissa is odd. With A the
+// integer for 2^e and B the one for the value on a common scale, and
+// g = A - B > 0, that is g 2^(prec+1) <= A. Since g >= 1, it cannot hold
+// once prec + 1 reaches A's bit length, so the shift below is never wider
+// than A, whose size the literal bounds.
+int64_t RoundedBinaryExponent(const Nat& num, const Nat& den, uint32_t prec) {
+  // num/den lies in (2^(e-1), 2^(e+1)) for e the difference of bit lengths.
+  int64_t e = static_cast<int64_t>(num.BitLen()) - static_cast<int64_t>(den.BitLen());
+  const auto scaled = [&](int64_t at) {
+    // A for 2^at, and B for the value, both scaled to integers.
+    return std::pair<Nat, Nat>{at >= 0 ? den.Shl(static_cast<uint64_t>(at)) : den,
+                               at >= 0 ? num : num.Shl(static_cast<uint64_t>(-at))};
+  };
+  auto [a, b] = scaled(e);
+  if (Nat::Cmp(b, a) >= 0) std::tie(a, b) = scaled(++e);
+  const uint64_t shift = uint64_t{prec} + 1;
+  if (shift >= a.BitLen()) return e;
+  const bool carries = Nat::Cmp(Nat::Sub(a, b).Shl(shift), a) <= 0;
+  return carries ? e + 1 : e;
+}
+
 // ParsePrecision reads a suffix's digits. Longer than any uint32 is refused
 // on its length, so `1p999…` costs nothing.
 bool ParsePrecision(std::string_view digits, uint32_t* out) {
@@ -441,6 +483,21 @@ Status ParseBigFloatLiteral(std::string_view literal,
   } else {
     den = Nat::Pow10(-lit.exp10);
   }
+  // Both limits are on the value rounded to prec, and both are asked before
+  // the value is widened to prec bits, which for a wide prec is the costly
+  // part. The wire carries the exponent less prec as an int32
+  // (pxf/bignum.proto), so a small value at a wide prec has no encoding
+  // (protowire-go#231). protowire-go asks this one first.
+  const int64_t bin = RoundedBinaryExponent(num, den, prec);
+  if (bin - static_cast<int64_t>(prec) < std::numeric_limits<int32_t>::min()) {
+    return Status::Error("big float literal " + Clip(literal) +
+                         " is below the wire's exponent range");
+  }
+  if (ExceedsBound(bin, max_exponent)) {
+    return Status::Error(
+        "big float literal " + Clip(literal) + " has binary exponent " + std::to_string(bin) +
+        ", and MaxBigFloatExponent=" + std::to_string(max_exponent) + " bounds its magnitude");
+  }
   // Scale so the integer quotient has at least prec + 2 bits; the remainder
   // then only ever acts as a sticky bit.
   const int64_t scale = static_cast<int64_t>(prec) + 2 - static_cast<int64_t>(num.BitLen()) +
@@ -456,11 +513,11 @@ Status ParseBigFloatLiteral(std::string_view literal,
     q = q.Shr(1);
     ++exponent;
   }
-  const int64_t bin = BinaryExponent(q, exponent);
-  if (ExceedsBound(bin, max_exponent)) {
+  // RoundedBinaryExponent found this exponent without the division.
+  if (BinaryExponent(q, exponent) != bin) {
     return Status::Error(
-        "big float literal " + Clip(literal) + " has binary exponent " + std::to_string(bin) +
-        ", and MaxBigFloatExponent=" + std::to_string(max_exponent) + " bounds its magnitude");
+        "big float literal " + Clip(literal) + ": internal error: rounded binary exponent " +
+        std::to_string(BinaryExponent(q, exponent)) + ", expected " + std::to_string(bin));
   }
   out->mantissa = q.ToBytes();
   out->exponent = exponent;

test/pxf_bignum_test.cc

@@ -14,6 +14,7 @@
 #include <cstdio>
 #include <cstring>
 #include <memory>
+#include <random>
 #include <string>
 
 #include <google/protobuf/compiler/importer.h>
@@ -300,4 +301,143 @@ rate = 0.5
   EXPECT_NE(out->find("rate = 0.5p256"), std::string::npos);
 }
 
+// The wire carries a BigFloat's exponent less its prec as an int32
+// (pxf/bignum.proto), so a small value at a wide prec has no encoding, and
+// protowire-go refuses it in these words (protowire-go#231). 0.125 is
+// 2^-3, binary exponent -2, and -2 - (2^31 - 1) is below INT32_MIN. The
+// refusal comes before the value is widened to prec bits, 256 MiB here, so
+// this test costs what any other does.
+TEST_F(PxfBigNum, BigFloatBelowTheWiresExponentRangeIsRefused) {
+  protowire::pxf::UnmarshalOptions opts;
+  opts.max_numeric_literal_digits = 2000000000;
+  for (const char* lit : {"0.125", "-0.125", "1e-300", "3e-4900"}) {
+    SCOPED_TRACE(lit);
+    auto msg = NewWallet();
+    auto st =
+        protowire::pxf::Unmarshal(std::string("rate = ") + lit + "p2147483647\n", msg.get(), opts);
+    ASSERT_FALSE(st.ok());
+    EXPECT_NE(st.message().find(std::string("big float literal ") + lit +
+                                " is below the wire's exponent range"),
+              std::string::npos)
+        << st.ToString();
+    // No exponent is stored, wrapped or otherwise.
+    const auto& rate = msg->GetReflection()->GetMessage(*msg, desc_->FindFieldByName("rate"));
+    EXPECT_EQ(rate.ByteSizeLong(), 0u);
+  }
+  // protowire-go asks this before MaxBigFloatExponent, so a value past both
+  // gets this refusal: 0.125 has binary exponent -2, past a bound of 1.
+  opts.max_big_float_exponent = 1;
+  auto msg = NewWallet();
+  auto st = protowire::pxf::Unmarshal("rate = 0.125p2147483647\n", msg.get(), opts);
+  EXPECT_NE(st.message().find("is below the wire's exponent range"), std::string::npos)
+      << st.ToString();
+}
+
+// DecimalTimes multiplies a decimal digit string by a small number.
+std::string DecimalTimes(const std::string& d, uint32_t m) {
+  std::string out;
+  uint64_t carry = 0;
+  for (size_t i = d.size(); i > 0; --i) {
+    const uint64_t v = uint64_t(d[i - 1] - '0') * m + carry;
+    out.push_back(static_cast<char>('0' + v % 10));
+    carry = v / 10;
+  }
+  for (; carry != 0; carry /= 10) out.push_back(static_cast<char>('0' + carry % 10));
+  while (out.size() > 1 && out.back() == '0') out.pop_back();
+  return std::string(out.rbegin(), out.rend());
+}
+
+// DyadicLiteral writes m 2^j exactly: m 5^-j e j for a negative j. A
+// non-zero nudge moves it by one unit in a decimal place past its last
+// digit, down for a negative nudge and up for a positive one. That is
+// 10^(j-1) for a negative j and 0.1 otherwise, either way less than 2^(j+1),
+// so a tie nudged this way stays nearer the tie than any other boundary.
+std::string DyadicLiteral(uint64_t m, int j, int nudge) {
+  std::string d = std::to_string(m);
+  int exp10 = 0;
+  for (int i = 0; i < (j > 0 ? j : -j); ++i) d = DecimalTimes(d, j > 0 ? 2 : 5);
+  if (j < 0) exp10 = j;
+  if (nudge != 0) {
+    // d - 0.1 or d + 0.1 in the last place: one more digit.
+    if (nudge < 0) {
+      size_t i = d.size();
+      while (d[i - 1] == '0') d[--i] = '9';
+      d[i - 1] = static_cast<char>(d[i - 1] - 1);
+      d += "9";
+    } else {
+      d += "1";
+    }
+    --exp10;
+  }
+  return d + "e" + std::to_string(exp10);
+}
+
+// The parser asks both exponent limits of the value rounded to prec before
+// it divides, from the literal's bit lengths and one comparison
+// (#125). Rounding can carry the exponent one higher, and these cases are
+// built around that carry, so their expected exponent does not come from
+// the arithmetic under test: (2^(p+1) - 1) 2^(e-p-1) is the tie between the
+// largest p-bit value below 2^e and 2^e itself, and rounds up to 2^e,
+// binary exponent e + 1; a hair below the tie it stays at e, and a hair
+// above it carries. Each is read, its exponent compared, and the bound
+// probed at that exponent, which is where the early arithmetic answers.
+TEST_F(PxfBigNum, BigFloatRoundedExponentIsFoundBeforeDividing) {
+  const auto* rate_fd = desc_->FindFieldByName("rate");
+  const auto binary_exponent = [&](const pbuf::Message& wallet) {
+    const pbuf::Message& rate = wallet.GetReflection()->GetMessage(wallet, rate_fd);
+    const pbuf::Descriptor* d = rate.GetDescriptor();
+    const std::string m = rate.GetReflection()->GetString(rate, d->FindFieldByName("mantissa"));
+    int64_t bits = 0;
+    if (!m.empty()) {
+      bits = int64_t(m.size() - 1) * 8;
+      for (auto top = static_cast<uint8_t>(m[0]); top != 0; top >>= 1) ++bits;
+    }
+    return bits + rate.GetReflection()->GetInt32(rate, d->FindFieldByName("exponent"));
+  };
+  const auto check = [&](const std::string& lit, uint32_t prec, int64_t want) {
+    SCOPED_TRACE(lit + "p" + std::to_string(prec));
+    const std::string doc = "rate = " + lit + "p" + std::to_string(prec) + "\n";
+    auto msg = NewWallet();
+    auto st = protowire::pxf::Unmarshal(doc, msg.get());
+    ASSERT_TRUE(st.ok()) << st.ToString();
+    const int64_t bin = binary_exponent(*msg);
+    ASSERT_EQ(bin, want);
+    protowire::pxf::UnmarshalOptions at;
+    at.max_big_float_exponent = static_cast<int>(bin < 0 ? -bin : bin);
+    auto ok = NewWallet();
+    EXPECT_TRUE(protowire::pxf::Unmarshal(doc, ok.get(), at).ok());
+    // A bound of 0 means the default, so the refusal is probed only above 1.
+    if (at.max_big_float_exponent <= 1) return;
+    --at.max_big_float_exponent;
+    auto past = NewWallet();
+    auto refused = protowire::pxf::Unmarshal(doc, past.get(), at);
+    EXPECT_NE(refused.message().find("has binary exponent " + std::to_string(bin) + ","),
+              std::string::npos)
+        << refused.ToString();
+  };
+
+  std::mt19937_64 rng(125);
+  int cases = 0;
+  for (int i = 0; i < 400; ++i) {
+    const uint32_t p = 1 + static_cast<uint32_t>(rng() % 62);
+    const int e = static_cast<int>(rng() % 601) - 300;
+    const uint64_t top = (uint64_t{1} << (p + 1)) - 1;  // the tie's mantissa
+    const int j = e - static_cast<int>(p) - 1;
+    check(DyadicLiteral(top, j, 0), p, e + 1);
+    check(DyadicLiteral(top, j, -1), p, e);
+    check(DyadicLiteral(top, j, +1), p, e + 1);
+    check(DyadicLiteral(1, e, 0), p, e + 1);  // 2^e itself
+    cases += 4;
+  }
+  // Hand-picked: ties to even on either side, and no carry.
+  check("15.5", 4, 5);  // 1111.1: the tie rounds up to 10000
+  check("14.5", 4, 4);  // 1110.1: the tie rounds down to 1110
+  check("0.75", 1, 1);  // 0.11: the tie rounds up to 1
+  check("1023.4", 10, 10);
+  check("1023.6", 10, 11);
+  check("-0.99999", 3, 1);  // past the tie at 0.9375: rounds to 1
+  cases += 6;
+  EXPECT_EQ(cases, 1606);
+}
+
 }  // namespace