Skip to content

Raise OverflowError when use_single_float cannot represent a value - #728

Merged
methane merged 1 commit into
msgpack:mainfrom
dylanpulver:single-float-overflow
Aug 26, 2026
Merged

Raise OverflowError when use_single_float cannot represent a value#728
methane merged 1 commit into
msgpack:mainfrom
dylanpulver:single-float-overflow

Conversation

@dylanpulver

Copy link
Copy Markdown

Fuzzing the Cython packer against the pure Python fallback over 4000 random objects, packb disagreed on 38 of them. Every disagreement was the same case: use_single_float=True with a value above FLT_MAX.

# C extension
>>> msgpack.packb(1e300, use_single_float=True).hex()
'ca7f800000'          # +inf

# MSGPACK_PUREPYTHON=1
>>> msgpack.packb(1e300, use_single_float=True)
OverflowError: float too large to pack with f format

Two things kept the C path quiet. _packer.pyx cast the value with <float>o before the call, so msgpack_pack_float only ever saw a narrowed float. And msgpack_pack_float discarded the return value of PyFloat_Pack4, which is what reports the overflow.

Passing the double through and checking that return value covers both. PyFloat_Pack4 backs struct.pack('>f', ...), so the exception type and message now match the fallback exactly.

Out-of-range integers are handled this way today: packb(2**64) raises PackOverflowError from either packer, pinned by test_limits.py::test_integer. The new test sits next to it.

Reverting the source makes the new test fail on the C extension. It passes either way under MSGPACK_PUREPYTHON=1, where it pins existing behaviour. After the change the same fuzz run reports 0 of 4000 disagreements on packb.

Infinity is representable in single precision and packs unchanged, which the test also covers.

The C extension narrowed the double to a C float before calling
PyFloat_Pack4, so values above FLT_MAX were silently packed as
infinity. PyFloat_Pack4's return value was also discarded, so the
overflow it detects could not surface. The pure Python packer uses
struct.pack('>f') and has always raised OverflowError here.

Pass the double through and check the return value, matching the
fallback and the existing out-of-range integer behaviour.
@dylanpulver

Copy link
Copy Markdown
Author

The one red job (3.15t on windows-latest) does not come from this change.

Its step Test (C extension) in parallel under free-threading passed, and that is the path this PR touches. The job then spent 46 minutes inside Test (pure Python fallback) in parallel under free-threading and was killed there.

That stall reproduces on e94e1dc with no patch applied. MSGPACK_PUREPYTHON=1 pytest --parallel-threads=auto --iterations=20 test parks on test_case.py::test_array16 on an 18 core machine. At --parallel-threads=4 the same suite finishes in about 25 seconds on both e94e1dc and this branch. auto scales with runner CPU count, so the cost of test_array16 and test_array32 under the pure Python packer grows with the size of the runner.

A re-run should come back green.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Aligns C-extension single-float packing with the Python fallback by raising OverflowError for unrepresentable finite values.

Changes:

  • Preserves double precision until PyFloat_Pack4.
  • Propagates packing overflow errors.
  • Tests float boundaries and infinities.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
msgpack/_packer.pyx Passes doubles to single-float packing.
msgpack/pack_template.h Checks and propagates PyFloat_Pack4 errors.
test/test_limits.py Covers FLT_MAX, overflow, and infinities.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@methane
methane merged commit 57d28a2 into msgpack:main Aug 26, 2026
50 of 51 checks passed
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.

3 participants