Skip to content

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

Merged
trendvidia merged 2 commits into
mainfrom
pxf-bigfloat-below-range-125
Nov 5, 2026
Merged

trendvidia merged 2 commits into
mainfrom
pxf-bigfloat-below-range-125

Conversation

@bfrancojr

Copy link
Copy Markdown
Collaborator

Fixes #125.

A BigFloat literal like 0.125p2147483647 can't be encoded. The wire stores the exponent minus prec as an int32 (pxf/bignum.proto). Here that is -2 - (2^31 - 1), below INT32_MIN. This port accepted the literal and its static_cast<int32_t> wrapped the exponent. The wrap is by reading the code; the acceptance is measured below. protowire-go refuses it (protowire-go#231), and this port now does too, in Go's words:

big float literal 0.125 is below the wire's exponent range

Why the restructure

The check needs the exponent of the value after rounding to prec. The parser only had that after widening the value to prec bits, which is 256 MiB at the prec that reaches this case. The issue's open question was whether to accept a heavy test or restructure. The done-when asks for a test whose cost suits CI, so this restructures.

Both exponent checks now run before the widening. They use an exponent worked out from the literal's numerator and denominator (RoundedBinaryExponent in src/pxf/bigfloat.cc):

  • Exact exponent. The two bit lengths give it to within one, and one comparison settles it.
  • Carry from rounding. Rounding to p bits raises the exponent by one exactly when the value is at least 2^e (1 - 2^-(p+1)). That point is the tie between 2^e and the largest p-bit value below it. That value's mantissa is all ones, so it's odd, and ties to even rounds up.
  • Cost. With A and B the integers for 2^e and the value, and g = A - B, the carry test is g * 2^(p+1) <= A. Since g >= 1, it can't hold once p + 1 reaches A's bit length. So the shift is never wider than A, and A's size is bounded by the literal. A wide prec costs nothing here.

After the division, the parser checks it got the same exponent, and fails loudly if not. The "below range" check runs before the exponent bound, the order in which Go asks them.

Evidence

  • Cheap test. PxfBigNum.BigFloatBelowTheWiresExponentRangeIsRefused takes 0.10 s. It checks four literals and that nothing gets stored. With the check removed, the same test fails: the literal is accepted, and the run takes 9.65 s and 1.09 GB peak memory in a Debug build.
  • Exponent arithmetic. PxfBigNum.BigFloatRoundedExponentIsFoundBeforeDividing reads 1606 literals built around rounding ties (exactly at the tie, just below, just above, and powers of two). Their expected exponents come from how they are built, not from the code under test. Each one also probes max_big_float_exponent at its exponent, where the early arithmetic decides. With the carry removed from RoundedBinaryExponent, it fails: internal error: rounded binary exponent -172, expected -173.
  • Against Go. Both ports' check_decode --format pxf-pb --limit MaxNumericLiteralDigits=2000000000, protowire-go at 207fdfc:
    • 0.125p2147483647: both refuse with the line above.
    • 0.25p2147483647 (wire exponent exactly INT32_MIN): both accept and write identical bytes, 268,435,485 of them, exponent 80808080f8ffffffff01. That's 10 s here and 1 s in Go, so it was measured once and is not in CI.
  • Suite. ctest passes 658 of 658 in a fresh build dir. clang-format 18 applied.
  • Go differential against protowire-go 207fdfc and spec 0317811: red on main, and this branch adds nothing. Run past the stream section, its C++ output is byte-identical to main's (a632c6e) over all 362 documents. The remaining differences come from the SBE schema-id refusal (protowire-go#270), pxf: Marshal leaves out an unset (pxf.default) or (pxf.required) field, so it doesn't read back #129, pxf: FormatDocument drops every directive, so a @dataset document formats to nothing #130 and pxf: reject null on a (pxf.required) or @required field #131.

Not changed here

This is separate from #139 (#128, the rounding check on the write side). The two touch different functions in bigfloat.cc and are not stacked.

For a literal far past the bound, such as 1e-100000p64, an older fast check still refuses with this port's own text, is past MaxBigFloatExponent=16384. Go says has binary exponent -332192, and MaxBigFloatExponent=16384 bounds its magnitude. That check exists to avoid building a huge power of ten, so matching Go's exact exponent there needs its own design. Measured; I'll file it separately.

@bfrancojr

Copy link
Copy Markdown
Collaborator Author

Rebased onto main e1c1b72 (#136 merged); head is now c561e02. The PR's own diff is unchanged, checked by comparing its added and removed lines before and after the rebase. Local ctest at this head: 665 of 665.

@trendvidia
trendvidia force-pushed the pxf-bigfloat-below-range-125 branch from c561e02 to 81db6a1 Compare November 4, 2026 22:52
@bfrancojr

Copy link
Copy Markdown
Collaborator Author

Rebased onto main df6b102; head is now 81db6a1. The diff is unchanged. Local ctest: 691 of 691.

The wire carries a BigFloat's exponent less its prec as an int32
(pxf/bignum.proto). A small value at a wide prec, such as
0.125p2147483647, has a binary exponent of -2 and a wire exponent of
-2 - (2^31 - 1), below INT32_MIN, which SetBigFloatFromLiteral wrapped.
protowire-go refuses it as "big float literal <s> is below the wire's
exponent range" (protowire-go#231), and now so does this port.

The check needs the exponent of the value rounded to prec, which the
parser had only after widening the value to prec bits: 256 MiB at the
prec that reaches this, and 9.7 s and 1.1 GB in a debug test. So both
exponent checks now run first, on an exponent found from the literal's
bit lengths and one comparison. Rounding carries it one higher exactly
when the value is at least 2^e (1 - 2^-(prec+1)), which is a comparison
on numbers no wider than the literal's own, since past that width the
gap below 2^e is too large to carry. The division afterwards asserts it
found the same exponent.

The below-range check comes before the bound, as in Go. The test of it
costs 0.1 s; a differential of 1606 literals built around rounding ties,
whose expected exponents come from their construction, pins the
arithmetic.
@trendvidia
trendvidia force-pushed the pxf-bigfloat-below-range-125 branch from 81db6a1 to f17f0fc Compare November 5, 2026 04:19
@bfrancojr

Copy link
Copy Markdown
Collaborator Author

Rebased onto main 9b9860a: 81db6a1 → f17f0fc. The earlier red "Differential against protowire-go" was Stream codecs differ: (run log). This branch was based on df6b102, from before #137 fixed the SBE stream difference. The rebase picks up #137, so the differential should now pass. The diff is unchanged. Locally: build OK, ctest 100% tests passed out of 707. The remote head was read back with git ls-remote: f17f0fc.

@trendvidia
trendvidia merged commit 48d1e01 into main Nov 5, 2026
11 checks passed
@trendvidia
trendvidia deleted the pxf-bigfloat-below-range-125 branch November 5, 2026 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pxf: a BigFloat literal below the wire's int32 exponent range wraps instead of being refused (protowire-go#231)

2 participants