Skip to content

Fix exponent wrapping in Math.frexp(BigFloat) for very large values#14971

Merged
straight-shoota merged 2 commits intocrystal-lang:masterfrom
HertzDevil:bug/bigfloat-frexp-overflow
Sep 6, 2024
Merged

Fix exponent wrapping in Math.frexp(BigFloat) for very large values#14971
straight-shoota merged 2 commits intocrystal-lang:masterfrom
HertzDevil:bug/bigfloat-frexp-overflow

Conversation

@HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Sep 5, 2024

BigFloats represent their base-256 ** sizeof(LibGMP::MpLimb) exponent with a LibGMP::MpExp field, but LibGMP.mpf_get_d_2exp only returns the base-2 exponent as a LibC::Long, so values outside (2.0.to_big_f ** -0x80000001)...(2.0.to_big_f ** 0x7FFFFFFF) lead to an exponent overflow on Windows or 32-bit platforms:

require "big"

Math.frexp(2.0.to_big_f ** 0xFFFFFFF5)  # => {1.55164027193164307015e+1292913986, -10}
Math.frexp(2.0.to_big_f ** -0xFFFFFFF4) # => {1.61119819150333097422e-1292913987, 13}
Math.frexp(2.0.to_big_f ** 0x7FFFFFFF)  # raises OverflowError

This PR fixes it by computing the exponent ourselves.

@HertzDevil HertzDevil added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:numeric labels Sep 5, 2024

# :nodoc:
struct Crystal::Hasher
def self.reduce_num(value : BigFloat)
Copy link
Member

Choose a reason for hiding this comment

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

question: How is this removal related to the other changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It too uses LibGMP.mpf_get_d_2exp, so it suffers from the same issue as Math.frexp

Copy link
Member

Choose a reason for hiding this comment

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

I see. And with its removal, calls fall back to reduce_num(value : Float) which uses Math.frexp.

@straight-shoota straight-shoota added this to the 1.14.0 milestone Sep 5, 2024
@straight-shoota straight-shoota merged commit 9240f50 into crystal-lang:master Sep 6, 2024
@HertzDevil HertzDevil deleted the bug/bigfloat-frexp-overflow branch September 6, 2024 11:00
straight-shoota pushed a commit that referenced this pull request Sep 10, 2024
This is similar to #14971 where the base-10 exponent returned by `LibGMP.mpf_get_str` is not large enough to handle all values internally representable by GMP / MPIR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:numeric

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants