Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

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
59 of 61 checks passed
@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